neo-project / neo-modules

MIT License
60 stars 100 forks source link

Consensus: finding the best way to create a history of block executions results #835

Closed Jim8y closed 8 months ago

Jim8y commented 1 year ago

Since DBFT is an execute after consensus model, we do not have a proper mechanism to ensure the state consistency cross nodes, thus i propose adding the transaction execution results to the next round consensus.

core idea here is:

if the primary has an inconsistent execution result for the last block, then other consensus nodes will reject his proporal.

Benifit of this pr is that we can ensure a cross nodes state consistency, no matter core versions, vm versions, plugin versions, storage damages.

Problem of this idea is that can not verify current consensus states.

vncoelho commented 1 year ago

no matter core versions, vm versions, plugin versions, storage damages.

If all nodes upgrade and re-sync we may have the same problem as before.

If we go in this direction, perhaps we should do a Sparse Graph or a MPT for that purpose.

Jim8y commented 1 year ago

I just want to try to fix it as much as i can without incur much change to the existing protocol. Still thinking.

Jim8y commented 1 year ago

no matter core versions, vm versions, plugin versions, storage damages.

If all nodes upgrade and re-sync we may have the same problem as before.

If we go in this direction, perhaps we should do a Sparse Graph or a MPT for that purpose.

Can we come up with a way to avoid resync? i really hate resync.

vncoelho commented 1 year ago

I see, @Liaojinghui, Let's move resync to another topic.

Then, this issue is related to: "finding the best way to create a history of block executions results", right? If so, we need to first define this best way, which kind of data structure will be used (or if it is just on the protocol layer).

vncoelho commented 1 year ago

@Liaojinghui, I am checking the code, it is in fact something that can help, in particular considering that the Consensus is made by nodes with different protocols. It is a kind of "hard" warning! We could even use the MPT hash of previous block as well (but that is more complex because of StateService being assync).

In any case, I think that, doing things correctly, we may need Tests for this PR. We used to have Akka Test Kit for Consensus, it was nice with many examples and steps to check Consensus. However, we lost these battery of tests when migrated from Neo to Neo-Modules.

Would you like to give a hand in pushing forward these tests again? With that previous toolkit for tests we can add tests for this PR by injecting different states on a specific Actor.

What we need would be just the template of the PR compiling with simple tests. Then, I can tune and all the previous cases and more.

Jim8y commented 1 year ago

I can do the test kit thing. But we can discuss more before we carry out the test. In the mean while, I think there is a formal modeling tool that we can use.

roman-khimov commented 1 year ago

Adding MPT state root of the previous block would be great and not hard to do, it doesn't need to be signed by SVs as well. It's enough for every CN to have some local state root and include it into messages, if it's the same for every CN, then SV signature doesn't matter much (do we need SV role at all in this case is a separate matter). Well, just like was proposed in neo-project/neo#1526.

vncoelho commented 1 year ago

The advantage of the MPT may be that we could check the existence of a result easier. But maybe, for this case, hash will be enough.

roman-khimov commented 1 year ago

Inventing a scheme to capture block side-effects just for the consensus doesn't seem to be productive to me, that's why I'd strongly prefer reusing MPT. MPT itself is quite cheap for Neo.

Jim8y commented 1 year ago

I would love to use MPT as well, just not sure how much cost it will incur, if you all vote on MPT, then why not.

Jim8y commented 9 months ago

@Jim8y, I think you can close this one and move to an issue for us to discuss the idea.

There are possibilities such as sending the state along with the commit. We can append that to the block. There are possibilities to do in our round.

@vncoelho Updating the consensus is not gonna happen, the key of existing dbft model is consensus then execute, which means we will never get the state of the existing block during the consensus.

vncoelho commented 9 months ago

@Jim8y,

Execution results is not a decision made by consensus, @Jim8y. It is an informative layer. It is possible to change consensus to include that in one finality block still.

But the PR is not the correct place to design and discuss that. I think an issue is needed if we decide to move in that direction.

Jim8y commented 9 months ago

You don't, the reason of not execute transactions during the consensus is to avoid prolonged execution delay the consensus. You change that, you change everything. You can create an issue to discuss this, not necessarily to close this pr. The best case you can have is two block state finality, no better than that unless you want to reevaluate everything.

vncoelho commented 9 months ago

No @Jim8y ,

It is possible. State or execution does not lag decision making.

Jim8y commented 9 months ago

what state you have without execution? when we talk state here, we refer to transaction execution results.

vncoelho commented 9 months ago

We are doing something similar for sidechain NEOX, so soon we can apply it here.

Jim8y commented 9 months ago

We are doing something similar for sidechain NEOX, so soon we can apply it here.

please dont tell me its ' shadow block'