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.

Sign up for GitHub

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

Jump to bottom

Fix trie reuse after Commit in atomic sync #411

Merged
merged 7 commits into from
Dec 1, 2023
Merged

Fix trie reuse after Commit in atomic sync #411

merged 7 commits into from
Dec 1, 2023

Conversation

darioush
Copy link
Collaborator

@darioush darioush commented Dec 1, 2023 •
edited
Loading

Why this should be merged

plugin/evm code should honor the invariant specified for committing tries here:

coreth/trie/trie.go

Lines 41 to 48 in 0eef20a

// Trie is a Merkle Patricia Trie. Use New to create a trie that sits on
// top of a database. Whenever trie performs a commit operation, the generated
// nodes will be gathered and returned in a set. Once the trie is committed,
// it's not usable anymore. Callers have to re-create the trie with new root
// based on the updated trie database.
//
// Trie is not safe for concurrent use.
type Trie struct {

How this works

Re-opens the trie from the database after commit.

How this was tested

CI

@darioush darioush marked this pull request as ready for review December 1, 2023 19:42
@StephenButtolph StephenButtolph changed the title fix for multiple commits at same height in atomic sync Fix trie reuse after Commit in atomic sync Dec 1, 2023
patrick-ogrady
patrick-ogrady previously approved these changes Dec 1, 2023
View reviewed changes
Base automatically changed from atomic-syncer-dup-heights to master December 1, 2023 20:39
@StephenButtolph StephenButtolph dismissed patrick-ogrady’s stale review December 1, 2023 20:39

The base branch was changed.

aaronbuchwald
aaronbuchwald previously approved these changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

optional suggestion to make comments more explicit - LGTM

@darioush @aaronbuchwald
Update plugin/evm/atomic_syncer.go
b9b25ea 
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Darioush Jalali <darioush.jalali@avalabs.org>
@darioush @aaronbuchwald
Update plugin/evm/atomic_backend.go
8f31104 
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Darioush Jalali <darioush.jalali@avalabs.org>
@darioush darioush enabled auto-merge (squash) December 1, 2023 21:23
@darioush darioush merged commit 631a5bd into master Dec 1, 2023
7 of 8 checks passed
@darioush darioush deleted the fix-trie-invariants branch December 1, 2023 21:33
joshua-kim added a commit that referenced this pull request Dec 6, 2023
@joshua-kim
Squashed commit of the following:
891ed82 
commit abde8d6
Author: Stephen Buttolph <stephen@avalabs.org>
Date:   Wed Dec 6 13:39:32 2023 -0500

    Update avalanchego dependency to v1.10.18-rc.0 (#415)

commit bed4cf7
Author: Stephen Buttolph <stephen@avalabs.org>
Date:   Fri Dec 1 16:48:28 2023 -0500

    Clarify bonus block inclusion in atomic trie (#412)

commit 631a5bd
Author: Darioush Jalali <darioush.jalali@avalabs.org>
Date:   Fri Dec 1 13:33:07 2023 -0800

    Fix trie reuse after Commit in atomic sync (#411)

    Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
    Co-authored-by: Stephen Buttolph <stephen@avalabs.org>

commit 6d7e6b8
Author: Darioush Jalali <darioush.jalali@avalabs.org>
Date:   Fri Dec 1 12:39:07 2023 -0800

    Fix overwriting of atomic trie roots (#407)

commit 0a6a7a3
Author: Darioush Jalali <darioush.jalali@avalabs.org>
Date:   Fri Dec 1 12:09:32 2023 -0800

    Skip bonus blocks applying atomic ops from trie to shared memory (#409)

commit 0eef20a
Author: Stephen Buttolph <stephen@avalabs.org>
Date:   Fri Dec 1 13:21:45 2023 -0500

    Improve logging of unexpected state sync errors (#410)

commit f6899f9
Author: Stephen Buttolph <stephen@avalabs.org>
Date:   Fri Dec 1 13:10:26 2023 -0500

    Simplify atomic root acceptance (#406)

    Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

commit 8e388ba
Author: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Date:   Wed Nov 29 20:45:05 2023 -0500

    Use `p2p.Network` (#384)

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
darioush pushed a commit that referenced this pull request Apr 25, 2024
@ceyonur
add reward manager key string (#411)
8bc67d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrick-ogrady patrick-ogrady patrick-ogrady approved these changes

@aaronbuchwald aaronbuchwald aaronbuchwald approved these changes

@ceyonur ceyonur Awaiting requested review from ceyonur ceyonur is a code owner

Assignees

@darioush darioush

Labels
None yet
Projects
Platform Engineering Group
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@darioush @patrick-ogrady @aaronbuchwald @StephenButtolph