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

Add: add previous block state to the block header #3045

Closed
wants to merge 3 commits into from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Dec 31, 2023

This pr focus on solving the problem of neo block state finality.

it will work along with neo-project/neo-modules#835

Copy link
Member

@cschuchardt88 cschuchardt88 left a comment

Choose a reason for hiding this comment

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

Older version nodes will get disconnected. This will cause network protocol problems. And nothing will be able to talk to each other or sync on new update without using new version.

Please fix!!!

PS:
Also please cancel the github action, its been running for 25 mins now. And there is no timeout, its eating resources.

@Jim8y
Copy link
Contributor Author

Jim8y commented Dec 31, 2023

not a problem for neo, cause without this, we will hace more serious problem.

@cschuchardt88
Copy link
Member

Does this get sent over the network? in other words does it serialize and deserialize this new object?

@shargon
Copy link
Member

shargon commented Jan 2, 2024

I think that we need first #2942 and maybe this #3037 (comment)

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 2, 2024

I think that we need first #2942 and maybe this #3037 (comment)

We can add this later, but i wish we can have it before we release the next version, such that we can solve the state issue caused by further node update.

@vncoelho
Copy link
Member

vncoelho commented Jan 2, 2024

I think that we need first #2942 and maybe this #3037 (comment)

I think that everything can be packed together into next release

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 2, 2024

I think that we need first #2942 and maybe this #3037 (comment)

I think that everything can be packed together into next release

That is what i hope, cause otherwise we might just keep asking nodes to resync, and be careful of the node state difference.

Copy link
Contributor

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

It's obviously incompatible and it needs a lot of care before we can consider merging. We also need to integrate notary before doing stateroots. That's why 3.7/3.8 plans were laid out the way they are. Trying to fix everything in a single huge release can break the thing too easily.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Jan 2, 2024

Like @roman-khimov said incompatible, Will prevent older version nodes that haven't upgraded to new version to get disconnected on sync or resync. I think we will need a fork? Or update Version number to 1 for block and blockheader.

@roman-khimov
Copy link
Contributor

Version=1 will definitely be needed.

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.

5 participants