chore(dot/sync): add Fragment struct for chain of blocks
#4274
+207
−167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EclesioMeloJunior
changed the title
Fragment struct for chain of blocks
dimartiro
reviewed
Oct 22, 2024
EclesioMeloJunior
requested review from
jimjbrettj,
timwu20 and
kishansagathiya
as code owners
haikoschol
approved these changes
Nov 7, 2024
| // disjoint set and return the fragment concatenated with the chain argument | ||
| func (u *unreadyBlocks) updateDisjointFragments(chain []*types.BlockData) ([]*types.BlockData, bool) { | ||
| // connects to a disjoint fragment, and returns a ne fragment | ||
| func (u *unreadyBlocks) updateDisjointFragment(chain *Fragment) (*Fragment, bool) { |
There was a problem hiding this comment.
Suggested change
| func (u *unreadyBlocks) updateDisjointFragment(chain *Fragment) (*Fragment, bool) { | |
| func (u *unreadyBlocks) updateDisjointFragments(chain *Fragment) (*Fragment, bool) { |
I would keep the plural here. Alternatively, update the doc comment as well.
Comment on lines
+203
to
+205
| // LowerThanOrEqHighestFinalized returns true if the fragment contains | ||
| // a block that has a number lower than highest finalized number | ||
| func LowerThanOrEqHighestFinalized(highestFinalizedNumber uint) func(*Fragment) bool { |
There was a problem hiding this comment.
nit:
Suggested change
| // LowerThanOrEqHighestFinalized returns true if the fragment contains | |
| // a block that has a number lower than highest finalized number | |
| func LowerThanOrEqHighestFinalized(highestFinalizedNumber uint) func(*Fragment) bool { | |
| // LowerThanOrEq returns a function that returns true if the fragment contains | |
| // a block that has a number lower than or equal to the passed in number | |
| func LowerThanOrEq(blockNumber uint) func(*Fragment) bool { |
The function is more general than the name and doc comment suggests. It could be called with something other than the highest finalized number.
dimartiro
requested changes
Nov 9, 2024
| // connects to a disjoint fragment, if so we remove the fragment from the | ||
| // disjoint set and return the fragment concatenated with the chain argument | ||
| func (u *unreadyBlocks) updateDisjointFragments(chain []*types.BlockData) ([]*types.BlockData, bool) { | ||
| // connects to a disjoint fragment, and returns a ne fragment |
There was a problem hiding this comment.
Suggested change
| // connects to a disjoint fragment, and returns a ne fragment | |
| // connects to a disjoint fragment, and returns a new fragment |
| func (f *Fragment) Iter() iter.Seq[*types.BlockData] { | ||
| return func(yield func(*types.BlockData) bool) { | ||
| for _, bd := range f.chain { | ||
| yield(bd) |
There was a problem hiding this comment.
We should check the yield function result
Suggested change
| yield(bd) | |
| if !yield(bd){ | |
| return | |
| } |
haikoschol
reviewed
Nov 13, 2024
| @@ -43,6 +44,16 @@ func (f *Fragment) Find(p func(*types.BlockData) bool) *types.BlockData { | |||
| return nil | |||
| } | |||
|
|
|||
| // Last returns the first block in the fragment or nil otherwise | |||
There was a problem hiding this comment.
Suggested change
| // Last returns the first block in the fragment or nil otherwise | |
| // First returns the first block in the fragment or nil otherwise |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
[][]*types.BlockDataTests
Issues
Fragmenttype toFullSyncstrategy #4320