tfchain icon indicating copy to clipboard operation
tfchain copied to clipboard

refactor: Billing refactor

Open sameh-farouk opened this issue 7 months ago • 3 comments

Description

This PR addresses several issues and introduces significant changes to the billing in the pallet-smart-contract module. Here are the key changes and the issues they resolve:

High-Level Changes:

  • Fixing several critical bugs.
  • Billing no longer requires all validators to have off-chain workers enabled for proper functionality. However, having more than one is highly recommended for redundancy.
  • Partial billing is now possible if a user lacks sufficient funds to cover the full amount due but can pay a portion. In such cases, the contract will enter a grace period as expected, and the remaining overdue amount will be overdrawn.
  • Certified capacity is appropriately charged at a higher rate (25% more) compared to DIY capacity.
  • Fund reservations no longer use locks/freezes. It was replaced with a more reliable reservation system, solving issues related to fund availability for reward distribution. Reserved amounts are no longer part of the free balance. The total balance is equal to free + reserved.
  • Introducing new events to provide better visibility into the payment processes.

Find below more details

Key Changes and Improvements:

Refactoring Billing Logic:

The billing logic has been refactored to improve the billing feature for smart contracts. This includes more precise tracking of contract payments, handling of overdrafts, and fund reservations.

Improved off-chain worker logic

Updated off-chain worker logic to handle billing more effectively. is_next_block_author function was removed and the verification now happening at runtime, ensuring that transaction fees are reliably waived for validators and still preventing duplicate transactions. Additionally, the improved logging ensures that the off-chain worker function works correctly without silent failures.

Introduction of ContractPaymentState:

Introducing ContractPaymentState aligns with the need to track and manage contract payment states accurately, especially during grace periods and when handling overdue payments. It fixes the issue where the amount tracked in the contract lock is incorrectly divergent from the lock on the user balance, leading to illiquidity issues when it's time to distribute rewards.

Improved Error Handling:

Enhanced error-handling mechanisms have been introduced to ensure that errors during billing are logged and appropriately managed, reducing the likelihood of contracts entering a dirty state.

Enhanced Events:

New events have been added, ContractGracePeriodElapsed, ContractPaymentOverdrafted, and RewardDistributed, to provide better visibility into the billing and payment processes.

Related Issues:

Issues Addressed:

Issue #991:

The PR resolves this issue by ensuring that the certified capacity is appropriately charged at a higher rate (25% more) compared to DIY capacity.

Issue #990:

This issue is addressed by replacing the locking mechanism with a more reliable reserve/hold system, improving the overall efficiency and accuracy of the billing process. Uses reserves instead of locks to handle funds ensuring that the funds are earmarked for feature payments, preventing issues with illiquidity. Reserves align better with handling common billing scenarios such as partial payments, fund releases, and transfers more straightforwardly and reliably.

Issue #979:

The refactoring includes better handling of contracts in grace periods and ensures that proper cleanup is done for contracts that are canceled while associated with nodes in standby.

Issue #969 :

  • The PR fixes the issue where contracts in the grace period incorrectly required a higher amount to be available in the user balance to get restored while only deducting the due amount of the last billing cycle, rather than the whole overdraft amount from the user balance.

Issue #932 :

  • The PR fix bug where validators may incur fees while submitting bill_contract transactions

Issue #834:

  • By replacing the balance lock feature with a reserve mechanism, the issue of unintended locking of funds is resolved. The new approach ensures that the correct amount is always reserved, ensuring there are no errors in reward distribution.

Issue #994:

  • The PR resolves the issue where, during the grace period, contracts with public IP addresses may over-calculate the costs related to network usage (NU).

Overall, this PR enhances the billing capabilities of the pallet-smart-contract module, making it more robust and reliable.

TODO

  • [X] implement de-duplication at the TX pool level using a signedExtension.
  • [x] Harden fund distribution functions.
  • [x] Adjust the integration tests.
  • [x] Organize and reorder logs, ensuring correct severity.
  • [x] Clean up code.
  • [ ] Plan changes that need to be propagated to other tools.
  • [x] Write new tests.
  • [ ] Conduct thorough testing.
  • [x] Update Docs and ADRs

Some reconsideration might be needed, such as:

  • ~~Using the newer hold in the fungible trait instead of reserves in reservableCurrency, as the latter is marked for deprecation in future versions.~~
  • ~~Re-evaluating how to track the overdue amount in a more compatible way. This could involve monitoring the amount in a different storage map alongside the old contract lock or adding a new field to the existing structure and migrating the storage instead of lazily migrating to a new structure.~~

Checklist:

  • [x] My change requires a change to the documentation and I have updated it accordingly
  • [x] I have added tests to cover my changes.

sameh-farouk avatar Jul 18 '24 14:07 sameh-farouk