Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/remove lock balance logic in billing flow #990

Closed
Tracked by #1560
sameh-farouk opened this issue Jul 16, 2024 · 2 comments
Closed
Tracked by #1560

Refactor/remove lock balance logic in billing flow #990

sameh-farouk opened this issue Jul 16, 2024 · 2 comments
Assignees
Labels
tfchain tfchain issue
Milestone

Comments

@sameh-farouk
Copy link
Member

sameh-farouk commented Jul 16, 2024

Choosing between locking and distributing less frequently, or not locking and distributing more frequently could be debatable as both actions require modifications to the account balances storage, which involves computational resources.

Locking Tokens:
Each lock is recorded in the state, and removing or updating a lock involves additional state changes.

Transferring Tokens:
Each transfer results in changes to the state (balance updates) for both the sender and the receiver. Also, distributing tokens in our logic involves multiple transfers to (foundation, sales, staking pool, and service provider) accounts.

The advantages of the current flow:

  • Locks due amounts every hour, leading to fewer state changes during the day.

  • Transfers the total amount once daily, spreading the computational load.

The disadvantages:

  • Current locking logic is flawed
  • Locking logic adds complexity to the billing code

I believe that the current approach, if works correctly (which is not the case), is generally more efficient in terms of network usage. However, I propose simplifying the logic by transferring overdue amounts directly every cycle. This is because the added complexity of locking brings many flaws that cannot be justified by the potential savings in network usage.

We can still benefit from both worlds by changing the billing cycle to occur every 24 hours instead of hourly, aligning it with the cultivation distribution schedule. This approach consolidates multiple transfers into fewer transactions, reducing the computational load on the network to even lower levels than what we have currently.

Additional context

@sameh-farouk sameh-farouk self-assigned this Jul 16, 2024
@sameh-farouk sameh-farouk added the tfchain tfchain issue label Jul 16, 2024
@sameh-farouk sameh-farouk moved this to Accepted in 3.15.x Jul 16, 2024
@sameh-farouk sameh-farouk added this to the 2.9.0 milestone Jul 16, 2024
@ashraffouda ashraffouda moved this from Accepted to In Progress in 3.15.x Jul 18, 2024
@sameh-farouk
Copy link
Member Author

We may also consider replacing locks with reserves/holds, which would simplify the flow and make it less error-prone.

@sameh-farouk
Copy link
Member Author

sameh-farouk commented Jul 30, 2024

Update:
Here is a sum of the changes so far

  • Payment state Management:
    Introducing ContractPaymentState aligns with the need to track and manage contract states accurately, especially during grace periods and when handling overdue payments.

    • Replaced ContractLock with ContractPaymentState to track the last updated timestamp for contract payments.
    • Enhanced state handling for contract billing, particularly around grace periods and contract deletions.
    • lazy migration for contracts over a billing cycle [can be replaced with a more common migration. TBD]
  • improve events and error handling:

    • Enhanced logging and error handling to improve debuggability and ensure billing-related issues are properly tracked and addressed.
    • Introduced new events and improved error handling to effectively manage grace periods and overdraft situations.
  • Reserve Mechanism: Uses reserves instead of locks to handle funds ensuring that the funds are earmarked for feature payments, preventing issues with liquidity.
    Reserves align better with handling common billing scenarios such as partial payments, fund releases, and transfers more straightforwardly and reliably.

Bug Fixes

  • Contracts in the grace period incorrectly required a higher amount to be available in the user balance to get restored while still only deducting the due amount of the last billing cycle, rather than the whole overdraft amount from the user balance.
  • Canceling a rent contract while the node is on standby may leave the contract in a deleted state without proper storage cleaning.
  • During the grace period, contracts with public IP addresses may over-calculate the costs related to NU.
  • The amount tracked in the contract lock is incorrectly divergent from the lock on the user balance, leading to liquidity issues when it's time to distribute rewards.

completed:
[x] Reserve Mechanism
[X] overdraft tracker
[x] better logging
[x] better audibility
[x] lazy migration
[X] Adjust billing tests

Still WIP:

  • adjusts integration tests
  • Add/ Update Docs
  • A lot of testing
  • Decide about migration path (lazy vs regular storage migration)

@sameh-farouk sameh-farouk moved this from In Progress to Pending Review in 3.15.x Aug 14, 2024
@github-project-automation github-project-automation bot moved this from In Verification to Done in 3.15.x Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tfchain tfchain issue
Projects
Status: Done
Development

No branches or pull requests

2 participants