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

feat: World State Re-orgs #9035

Merged
merged 155 commits into from
Oct 11, 2024
Merged

feat: World State Re-orgs #9035

merged 155 commits into from
Oct 11, 2024

Conversation

PhilWindle
Copy link
Collaborator

@PhilWindle PhilWindle commented Oct 5, 2024

This PR adds pruning and re-org support to the native world state.

);
};

const compareSnapshots = async (left: MerkleTreeReadOperations, right: MerkleTreeReadOperations) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this the same code as compareChains above? Either way, I'd move this to test utils, and loop over MerkleTreeId to make it more manageable.

Copy link
Collaborator 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

@alexghr alexghr left a comment

Choose a reason for hiding this comment

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

:shipit:

Comment on lines +367 to +369
if (blockNumber == 0) {
throw std::runtime_error("Invalid block number");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we ever request the sibling path for the genesis block (0) on the archive tree? Seems not, since this is not blowing up, but I'd've expected to be possible to fetch it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't believe so.

Comment on lines +758 to +760
if (blockNumber == 0) {
throw std::runtime_error("Invalid block number");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean we cannot unwind to genesis, which means initial epoch cannot be unwound?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, should we check we are not accidentally removing the finalised block, both here and when removing historical? (maybe it's checked somewhere else)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the api here is 'off by one' compared to TS. So unwinding block 1 leaves you at block 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will protect against removing the current finalised block in either direction.

uint32_t p = static_cast<uint32_t>(std::log2(highest_index + 1)) + 1;
index_t span = static_cast<uint32_t>(std::pow(2, p));
uint64_t numBatches = workers_->num_threads();
uint32_t indexPower2Ceil = static_cast<uint32_t>(std::ceil(std::log2(highest_index + 1)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this is safer than relying on floating point?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, that's much better.

@PhilWindle PhilWindle enabled auto-merge (squash) October 10, 2024 18:27
@PhilWindle PhilWindle merged commit 04f4a7b into master Oct 11, 2024
97 checks passed
@PhilWindle PhilWindle deleted the pw/world-state-pruning branch October 11, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-all CI: Enables this CI job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants