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 random trie exception after pruning #5471

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Mar 21, 2023

  • If full pruning is started after new head event which is triggered by blockchainproccessor where the branch have more than one block, some of the block state will be missed causing trie exception after pruning complete.
  • This is because blockchainprocessor will process the branch first before triggered new head event multiple time. Full pruning will start switching duplicating writes after the first event, and assume the writes for the other blocks will start after that, but that does not happen as it was already processed.
  • This PR add another event OnUpdateMainChain which would get triggered per processing branch instead of multiple time. Ideally the event should be in BlockchainProcessor, but turns out there is a dependency issue so it can't refer to it without a bigger refactor.
2023-03-21 17:52:14.4630|WARN|46|Process done 8692501 
2023-03-21 17:52:14.4670|WARN|46|Processing 8692502 
2023-03-21 17:52:14.5831|WARN|46|Process done 8692502 
2023-03-21 17:52:14.5879|WARN|46|New head block 8692501 
2023-03-21 17:52:14.5899|WARN|46|Buffer size for State1 20899225 8 3028682281 
2023-03-21 17:52:14.5924|INFO|46|Full Pruning Ready to start: waiting for state 8692501 to be ready. 
2023-03-21 17:52:14.5924|WARN|46|New head block 8692502 
2023-03-21 17:52:14.5924|INFO|46|Full Pruning Waiting for block: 8692511 in order to support reorganizations. 
2023-03-21 17:52:14.5924|INFO|46|Processed    8692502 |      292ms of 802,469ms, mgasps   44.41 total   44.41, tps  396.92 total  396.92, bps    0.00 total    0.00, recv queue 0, proc queue 32 
2023-03-21 17:52:14.5924|WARN|46|Processing 8692503 

Changes

  • Use OnUpdateMainChain event instead.
  • Updated unit tests to reflect.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes: well I updated the old tests.
  • No

Notes on testing

  • 6 consecutive goerli full pruning so far...

@asdacap asdacap marked this pull request as ready for review March 21, 2023 12:36
@@ -167,6 +167,7 @@ public interface IBlockTree : IBlockFinder
event EventHandler<BlockEventArgs> NewSuggestedBlock;
event EventHandler<BlockReplacementEventArgs> BlockAddedToMain;
event EventHandler<BlockEventArgs> NewHeadBlock;
event EventHandler<OnUpdateMainChainArgs> OnUpdateMainChain;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is now the difference between BlockAddedToMain and OnUpdateMainChain? Would be good to document

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@asdacap asdacap merged commit 2567bc0 into master Mar 21, 2023
@asdacap asdacap deleted the fix/trie-exception-after-full-pruning branch March 21, 2023 18:56
@benaadams benaadams added the bug label Mar 22, 2023
tanishqjasoria pushed a commit that referenced this pull request Apr 4, 2023
* Fix trie exception after pruning if pruning is started by multiple consecutive set head event

* Missed a file

* Whitespace

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

Successfully merging this pull request may close these issues.

3 participants