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

Storage rent 2022 #1798

Closed
wants to merge 26 commits into from
Closed

Storage rent 2022 #1798

wants to merge 26 commits into from

Conversation

fedejinich
Copy link
Contributor

RSKIP240 - Storage Rent

This PR implements the Storage Rent feature: RSKIP240.

Introduced changes

  • Track trie nodes used within transaction execution.
  • Trie changes: added timestamp field.
  • MutableTrie operations: support to read and write the rent timestamp.
  • Repository methods: to retrieve tracked trie-nodes and to update rent.
  • Rent payment: a mechanism to pay rent for the tracked trie-nodes.

Class diagram including all the introduced changes and relevant stuff (green classes are new classes):

image

Repository

Now the repository has the capability of updating rent timestamps, as well another method has been added to retrieve a RentedNode from a TrackedNode.

Note that are other methods that shouldn't be part of the Repository but I've included them to prevent a major refactor:

  • getStorageRentNodes()
  • getRollbackNodes()

Those methods should be part of MutableRepsotitory (or a new MutableRepositoryTracked)

TransactionExecutor

Added a StorageRentManager to pay for storage rent a the end o the transaction execution (TransactionExecutor:525). I've included it as the last step before doing the gas refund since rent payment should be the last consuming-gas operation of the processed transaction.

Another important aspect is that rent payment is enabled only if the transaction:

  • is above the hop network upgrade (or whatever NU we decide to include in) and
    • contains any data or
    • if the provided gas limit is different from a value transfer transaction (2100)
      Also, storage rent can be enabled or disabled from the config reference.

StorageRentManager

Added this class to perform the rent payment just for the payable nodes (the ones that are above the rent threshold). It first gets all the tracked trie keys from the MutableRepository then converts those keys to RentedNode to perform the calculate the cumulative rent, then subtracts the gas consumed by the process (or OOG if it doesn't have enough gas), and finally invokes the transactionTrack to update rent timestamps.

Note RentedNodes rents are fetched from the blockTrack and not from the transactionTrack, that's because we want the storage rent data before executing the block. Also, rent is updated from the transactionTrack because it should be updated if the transaction succeeds.

RepositoryLocator

Added a new method to return a MutableRepository with node-tracking capabilities.

MutableRepository

I've wrapped all the read and write operations into "internal" methods (overrided in MutableRepositoryTracked). There are just 5 methods that perform read/write in the trie:

  • mutableTrie.get
  • mutableTrie.put
  • mutableTrie.getValueLength
  • mutableTrie.getValueHash
  • mutableTrie.deleteRecursive
  • mutableTrie.getStorageKeys

Now each time we want to use any of the above methods, we need to call it by using the internal method. This can be improved, but again, that's a major refactor and it goes out of scope.

MutableRepositoryTracked

This class extends MutableRepository to add tracking capabilities. This is useful for this project and the parallel transaction processing, in both scenarios we want to track all the read/written trie keys for the executed transaction, and since the MutableRepository is the main door, I've overridden internal methods to support tracking.

Also, it's important to remark that now we're explicitly linking the repository hierarchy used for committing atomic state changes in the transaction execution. So each time we create a new child repository (startTracking()), we also provide the parent repository, and each time we commit/rollback changes, we also affect tracked nodes:

  • commit(): pass nodes to the parent repository.
  • rollback(): pass nodes as "rollback" to the parent repository.

TrackedNode

Added this class to track trie nodes involved in transaction execution.

RentedNode

Added this class to calculate

  • the rent amount
  • new timestamp
  • rollback fee (if the tx failed)

MutableTrie

Added methods to update and get rent timestamps for a given trie-key.

Trie

If we add this RSKIP, then we'll have new trie nodes (with rent timestamp), so I've added support for these new trie-nodes by:

  • modified serialize/deserialize methods.
    • now these methods will use the TrieSerializer to serialize/deserialize trie elements.
  • adding methods to get and set timestamps for a given key.

@fedejinich fedejinich requested a review from a team as a code owner June 6, 2022 19:17
@fedejinich fedejinich removed the request for review from a team June 6, 2022 19:19
@lgtm-com
Copy link

lgtm-com bot commented Jun 6, 2022

This pull request introduces 1 alert when merging 6ce0c2d into e135cc7 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Jun 9, 2022

This pull request introduces 1 alert when merging 90bc896 into e135cc7 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Jul 8, 2022

This pull request introduces 1 alert when merging 971d55f into c865587 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@sonarcloud
Copy link

sonarcloud bot commented Jul 8, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 52 Code Smells

95.7% 95.7% Coverage
0.7% 0.7% Duplication

@fedejinich fedejinich closed this Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants