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

Recalculate storage usage based on the actual state #3824

Closed
evgenykuzyakov opened this issue Jan 20, 2021 · 3 comments · Fixed by #4274
Closed

Recalculate storage usage based on the actual state #3824

evgenykuzyakov opened this issue Jan 20, 2021 · 3 comments · Fixed by #4274
Assignees
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)

Comments

@evgenykuzyakov
Copy link
Collaborator

Due to a bug in the implementation of DeleteKey action, the storage usage on the Account structure was incorrectly updated. For every deleted access key, the storage usage was decreased by one extra bug.

The bug was already fixed in a protocol upgrade and all places where the storage usage integer might underflow was switched to use saturating_sub, so it can't affect the consensus.

But it still leaves some account in the invalid state, which have storage_usage value in the Account structure lower than the actual storage usage by this account.

We need to create a protocol upgrade which addresses this problem by recalculating storage usage from the actual state.

There are a few items:

  • Need to perform this action on an existing account (once).
  • It has to be done in consensus with other nodes, so they can transition at the same time.
  • We don't want to stale the network
  • Recalculation might take substantial amount of time for contracts with large state or account with a lot of access keys.

To recalculate, you can use an example from genesis recomputation:

pub fn compute_storage_usage<Record: Borrow<StateRecord>>(

We'll have to recompute somewhat similarly, except using data from the Trie.

@bowenwang1996 bowenwang1996 added A-chain Area: Chain, client & related A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) and removed A-chain Area: Chain, client & related labels Jan 21, 2021
@EgorKulikov EgorKulikov linked a pull request Mar 29, 2021 that will close this issue
@bowenwang1996
Copy link
Collaborator

bowenwang1996 commented Mar 29, 2021

@EgorKulikov I discussed with @evgenykuzyakov on this issue today and we agreed that what was originally proposed is not preferable because iterating over a large account is prohibitively expensive. We came up with the following approach:

  • Dump the state of mainnet and compute all the accounts which need to be updated and output the account_id -> storage usage delta map into a file.
  • In nearcore we hardcode the hash of this file and do verification of it upon loading.
  • Then at runtime, whenever an account is touched, we check whether it needs to be updated and update the account in O(1) if needed

Note that this works because the number of accounts created before the original bug was fixed is relatively small (I think < 50k).

@EgorKulikov
Copy link
Contributor

@bowenwang1996 good idea, only I would store delta between storage usage at dump and correct storage usage, as account can change between dump and following protocol update with this file, but delta would be the same (unless we'll underflow on storage, but it is only possible if more than 100 keys were removed from a single account during that bug period, we can check that no delta is more than 100)

@bowenwang1996
Copy link
Collaborator

@EgorKulikov yeah that is what I meant. Sorry for not being clear. I'll update the original comment

@EgorKulikov EgorKulikov linked a pull request Apr 26, 2021 that will close this issue
@EgorKulikov EgorKulikov linked a pull request May 5, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)
Projects
None yet
3 participants