Skip to content

feat: use gossiped commit in sync#487

Merged
tzdybal merged 3 commits intomainfrom
tzdybal/use_last_commit
Sep 8, 2022
Merged

feat: use gossiped commit in sync#487
tzdybal merged 3 commits intomainfrom
tzdybal/use_last_commit

Conversation

@tzdybal
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal commented Aug 5, 2022

Changes:

  • gossip commits with headers (using SignedHeader)
  • use gossiped commit during sync

Resolves #484.

@tzdybal tzdybal self-assigned this Aug 5, 2022
@tzdybal tzdybal force-pushed the tzdybal/commit_gossiping branch from 7b3404d to cf9367a Compare August 14, 2022 21:51
Base automatically changed from tzdybal/commit_gossiping to main August 17, 2022 08:15
@tzdybal tzdybal force-pushed the tzdybal/use_last_commit branch 6 times, most recently from 3432668 to c596d41 Compare August 23, 2022 19:34
@tzdybal tzdybal marked this pull request as ready for review August 23, 2022 21:22
@tzdybal tzdybal requested review from Wondertan and jbowen93 August 23, 2022 21:22
adlerjohn
adlerjohn previously approved these changes Aug 24, 2022
Copy link
Copy Markdown
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

utACK

Comment thread node/node.go Outdated
Comment thread block/manager.go Outdated
Comment thread block/manager.go
Comment thread block/manager.go
Comment thread node/node.go Outdated
Comment thread node/node.go Outdated
Comment thread block/manager.go
@tzdybal tzdybal marked this pull request as draft August 24, 2022 19:28
@tzdybal tzdybal force-pushed the tzdybal/use_last_commit branch from c596d41 to 8623135 Compare September 6, 2022 17:21
@tzdybal tzdybal marked this pull request as ready for review September 6, 2022 17:25
@tzdybal tzdybal force-pushed the tzdybal/use_last_commit branch from 8623135 to 358bf20 Compare September 8, 2022 08:10
@tzdybal tzdybal enabled auto-merge September 8, 2022 08:45
Copy link
Copy Markdown
Contributor

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM

This all is very interesting. I will likely visit the Optimint repo much more often now!

Comment thread block/manager.go Outdated
return fmt.Errorf("failed to save block responses: %w", err)
}

if m.daHeight > newState.DAHeight {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was that a bug you accidentally caught? 😁

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes!

Comment thread block/manager.go
Comment on lines +237 to +241
// trySyncNextBlock tries to progress one step (one block) in sync process.
//
// To be able to apply block and height h, we need to have its Commit. It is contained in block at height h+1.
// If block at height h+1 is not available, value of last gossiped commit is checked.
// If commit for block h is available, we proceed with sync process, and remove synced block from sync cache.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🙏🏻

@tzdybal tzdybal merged commit 3420af7 into main Sep 8, 2022
@tzdybal tzdybal deleted the tzdybal/use_last_commit branch September 8, 2022 15:43
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.

Keep full nodes in-sync with aggregator

3 participants