fix(fork-network): fix handling of shard IDs #12469
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the
init
command is run, the fork-network command writes state roots to the DB by key "FORK_TOOL_SHARD_ID:{shard_id}", and later reads them back by iterating over that prefix. But there is inconsistent treatment of these ShardIds sometimes as ShardIds and sometimes as ShardIndexes. Firstly, the name of the key itself indicates we should expect ShardIds to be there. But when we write them, we collect the state roots in a Vec in order of ShardLayout::shard_ids(), and then iter().enumerate() over that to write the (shard_id, state_root), which is really a shard index. Then when we read them back, we index into that array withshard_uid.shard_id as usize
, which doesn't work when the ShardIds are shuffled.When running this on a localnet where shard IDs are shuffled, this gives us this error:
So fix it by actually writing a real ShardId to the
FORK_TOOL_SHARD_ID:...
keys, and actually parsing the ShardId from that key when iterating (instead of an enumerate() to get the shard ID). And just be more careful about the ShardId <-> ShardIndex mapping throughout.