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

fix(forknet): fix account shard assignment for forknet v2 #12837

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

marcelo-gonzalez
Copy link
Contributor

the fork-network amend-validators command changes the accounts and keys for use with the mirror tx sending tool. This will change all the implicit accounts so that we can control them in the forked chain. But after this mapping, the account might not be in the same shard as it was before, in which case it's left in the trie for the wrong shard.

Here we fix it by keeping a list of updates for every shard in each StorageMutator, and then writing updates to the right shard's trie when commit() is called. To synchronize this across the threads that are each iterating over different shards, we put the ongoing state roots in a Mutex that we lock whenever we want to commit changes to the shard.

We also introduce a new type DelayedReceiptTracker that will remember the mapping from (source shard, receipt index) to target shard, and then after all threads have finished iterating over the whole trie in their shards, we write all delayed receipts in one thread with write_delayed_receipts()

One other small change we make here is passing the same height to MemTries::delete_until_height() as we did to the previous call to apply_memtrie_changes(), because the garbage collection condition in that function gets rid of anything below the specified height. So with the previous code, we were keeping one extra update's worth of unnecessary nodes

When iterating over a whole mainnet trie, this might actually
be pretty unnecessary
We already have the key we want to remove, so constructing the same
trie key and then serializing it is not necessary. Also, this was
incorrect for delayed receipts, where the indices in the source DB
probably did not start from zero, so we were deleting the wrong trie key
writing the index in the state record won't work if the shard layout
isn't the same as it was in the source chain
@marcelo-gonzalez marcelo-gonzalez requested a review from a team as a code owner January 29, 2025 22:08
@marcelo-gonzalez
Copy link
Contributor Author

this PR is on top of the same branch as #12798, so it's not quite as big as it looks. Will look to merge this one after that one is merged, but opening it now since I think it should be good to go

@marcelo-gonzalez
Copy link
Contributor Author

ok other one is merged, so the diff is just from this change now

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 0% with 418 lines in your changes missing coverage. Please review.

Project coverage is 70.30%. Comparing base (6cd375e) to head (3d38e5a).

Files with missing lines Patch % Lines
tools/fork-network/src/cli.rs 0.00% 157 Missing ⚠️
tools/fork-network/src/storage_mutator.rs 0.00% 152 Missing ⚠️
tools/fork-network/src/delayed_receipts.rs 0.00% 109 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12837      +/-   ##
==========================================
- Coverage   70.39%   70.30%   -0.09%     
==========================================
  Files         848      848              
  Lines      174091   174287     +196     
  Branches   174091   174287     +196     
==========================================
- Hits       122548   122536      -12     
- Misses      46296    46501     +205     
- Partials     5247     5250       +3     
Flag Coverage Δ
backward-compatibility 0.16% <0.00%> (-0.01%) ⬇️
db-migration 0.16% <0.00%> (-0.01%) ⬇️
genesis-check 1.41% <0.00%> (-0.01%) ⬇️
linux 69.98% <0.00%> (-0.08%) ⬇️
linux-nightly 69.95% <0.00%> (-0.09%) ⬇️
pytests 1.70% <0.00%> (-0.01%) ⬇️
sanity-checks 1.52% <0.00%> (-0.01%) ⬇️
unittests 70.13% <0.00%> (-0.09%) ⬇️
upgradability 0.20% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant