Skip to content

Conversation

@JonathanOppenheimer
Copy link
Member

@JonathanOppenheimer JonathanOppenheimer commented Oct 15, 2025

Why this should be merged

This breaks up #1679

Copy link
Contributor

@alarso16 alarso16 left a comment

Choose a reason for hiding this comment

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

I'm still concerned about new nodes receiving an old format or old nodes receiving the new format and being unable to properly sync. If this concerned is deemed unimportant/fixed, then this looks good to me

@JonathanOppenheimer
Copy link
Member Author

I'm still concerned about new nodes receiving an old format or old nodes receiving the new format and being unable to properly sync. If this concerned is deemed unimportant/fixed, then this looks good to me

The serialization changes certainly could be breaking - I will defer to @ceyonur. Additionally if they are, maybe the Granite upgrade would work well?

Start []byte `serialize:"true"`
End []byte `serialize:"true"`
Limit uint16 `serialize:"true"`
NodeType NodeType `serialize:"false"`
Copy link
Member Author

Choose a reason for hiding this comment

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

We no longer serialize NodeType as @ceyonur requested. A better fix can be implemented at some point in the future. This should assuage @alarso16's concerns

nodeType := leafsRequest.NodeType
// TODO(JonathanOppenheimer):Handle legacy requests where NodeType was not serialized (defaults to 0)
// In this interim period, we treat NodeType 0 as StateTrieNode
if nodeType == 0 {
Copy link
Member Author

Choose a reason for hiding this comment

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

here we manually set the type.

@JonathanOppenheimer JonathanOppenheimer changed the title PARTIAL sync: coreth PR #963,981,1009: nodeType and related items PARTIAL sync: coreth PR #963,981,1009: nodeType w/o serialization Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants