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

Lightweight blockstate #2565

Closed
wants to merge 2 commits into from
Closed

Lightweight blockstate #2565

wants to merge 2 commits into from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Aug 10, 2021

Transactions in the block can not ensure that all nodes are in the same state, we still need a state synchronization mechanism to ensure that all nodes are at the same pace.

This is a lightweight block state, it uses the hash of every storage operation as the current block state.

@Jim8y Jim8y marked this pull request as draft August 10, 2021 08:22
@erikzhang
Copy link
Member

@Jim8y
Copy link
Contributor Author

Jim8y commented Aug 10, 2021

https://github.com/neo-project/neo-modules/tree/master/src/StateService

It is different, we need a state in the header or in the block to show the transaction execution result, the trust of that state should directly come from and be ensured by the consensus. At least it should be verified during consensus.
Unlike UTXO, Token transactions can not 100% decide the Blockchain state transfer.

@roman-khimov
Copy link
Contributor

Looks like a prologue to #1284. Or epilogue to #1526 (but hey, neo-project/neo-modules#623 is relevant too). Either way, it reminds us about the problem of state we still have. Because block finality is nice, but state finality is much more important. The discussion around this topic goes on for quite substantial time if we're to look back at past, I can also link #2374, neo-project/neo-modules#536, #1383, #901, #712, #302 because you know me, I love links and I love state. I won't link some more problematic ones, where state or difference in state suddenly was important, but this repository remembers few of those too.

I'd say that any kind of state checking between nodes would be an improvement over the situation we have now where nodes in general just don't care about their synchrony with the network. Lightweight checksums are better than no checksums. Comparing checksums just between CNs is better than not doing it at all. So it all goes into the right direction.

Yet at the same time to really nail all the related issues we need something more heavyweight. And it's not exactly "heavy" if you're to look more closely, because some may remember preview4 benchmarking and what's shown there for NeoGo includes full MPT calculation for every block. As well as application log saving. As well as NEP-17 transfer processing for future getnep17balances and getnep17transfers requests. That's just the way NeoGo works, it doesn't have plugins. And believe it or not but our MPT processing has improved a lot since then. So performance-wise for me it's almost inexistent. If this "heavyweight" state proof is that cheap, should we bother with lighter proofs? I don't think so. Even though we have experimented with lightweight checksums way back when.

So we can use MPT state root hash to check state between CNs. But ordinary nodes will still be unsynchronized. And we still have #2373 problem. It can of course be solved with #1286, but that would put some considerable load on CNs and other than that I just don't know how to solve it without in-header state root.

And we naturally get to the point where in-header MPT-based state root hash suddenly solves all the problems. So that's what I'd really like to see in N3.1.

Yet at the same time, as written above, any kind of state checking between nodes (light/heavy/CNs only/whatever) would be an improvement over current situation.

@Jim8y
Copy link
Contributor Author

Jim8y commented Aug 10, 2021

@roman-khimov thank you roman for your detailed explaination, appreciate it. if u have a hardfork in the schedule, i would love to see mpt in the header, that would be THE perfect solution. i crashed my local leveldb today~, storage isn't reliable LOL.

@Jim8y Jim8y closed this Aug 25, 2021
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.

3 participants