Skip to content

Feature/verifytrie admin rpc#7962

Merged
asdacap merged 13 commits intomasterfrom
feature/verifytrie-admin-rpc
Jan 7, 2025
Merged

Feature/verifytrie admin rpc#7962
asdacap merged 13 commits intomasterfrom
feature/verifytrie-admin-rpc

Conversation

@asdacap
Copy link
Contributor

@asdacap asdacap commented Dec 23, 2024

  • As requested. admin_verifyTrie RPC.
  • Also, move blocking mechanism to TrieStore instead of processing queue.

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
  • No

Notes on testing

  • Tested manually.
  • Tested to cancel on shutdown.
  • Tested to not run multiple verifytrie.

@asdacap asdacap requested a review from kamilchodola December 23, 2024 02:20
Comment on lines +147 to +151
public ResultWrapper<string> admin_verifyTrie()
{
if (_blockTree.Head is null)
{
return ResultWrapper<string>.Fail("Head is null. Unable to know state root to verify.");
Copy link
Member

Choose a reason for hiding this comment

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

Potentially can we pass the BlockParameter where we want to verify tries?
Also potentially should we add admin_availableStateRoots, where it would look-up where state root is available in some block range?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, why not.

_logger.Error($"Error in verify trie", e);
}

}, TaskCreationOptions.LongRunning);
Copy link
Member

Choose a reason for hiding this comment

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

I remember we were removing LongRunning because of some reason, @benaadams ?

{
if (_blockTree.Head is null)
SearchResult<BlockHeader> headerSearchResult = _blockTree.SearchForHeader(block);
if (headerSearchResult.Object is null)
Copy link
Member

Choose a reason for hiding this comment

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

I think we need a check if state root is available?

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

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 56c0d50 into master Jan 7, 2025
@asdacap asdacap deleted the feature/verifytrie-admin-rpc branch January 7, 2025 07:22
rjnrohit pushed a commit that referenced this pull request Jan 11, 2025
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.

2 participants