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

Make intra_rebase more user friendly #66

Merged
merged 1 commit into from
Mar 4, 2025
Merged

Conversation

michaelsproul
Copy link
Member

Remove pre-conditions from intra_rebase about applying updates and tree hashing. We can just do these inline because we have a mutable reference.

This fixes a bug found by @eserilev in my first impl of intra_rebase in LH, which forgot to tree hash before calling the method 🤦‍♂️

Copy link
Contributor

@eserilev eserilev left a comment

Choose a reason for hiding this comment

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

LGTM!

@eserilev
Copy link
Contributor

eserilev commented Mar 4, 2025

I'm not sure if this is an actual optimization (or if this would introduce a bug) but you could limit the apply_changes/tree_hash_root stuff only if self.interface.backing.tree.is_zero()?

@michaelsproul
Copy link
Member Author

@eserilev I don't think it would be substantially quicker than just calling apply_updates and tree_hash_root, because both of them will check whether they have anything to do before making changes. So it should be equivalent

@michaelsproul michaelsproul merged commit 8197ffb into main Mar 4, 2025
5 checks passed
@michaelsproul michaelsproul deleted the intra-rebase-redux branch March 4, 2025 03:06
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