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

Jim8y · 2023-10-10T06:42:03Z

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 · 2023-10-10T08:14:35Z

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 · 2023-10-10T09:29:51Z

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

Jim8y · 2023-10-10T09:33:13Z

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 · 2023-10-10T09:49:42Z

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 · 2023-10-10T09:50:53Z

src/DBFTPlugin/DBFTPlugin.cs

+        private static void OnCommitting(NeoSystem system, Block block, DataCache snapshot, IReadOnlyList<Blockchain.ApplicationExecuted> applicationExecutedList)
+        {
+            var serializedData = JsonSerializer.SerializeToUtf8Bytes(applicationExecutedList);
+            var hash = ComputeSha256Hash(serializedData);


Here, you are proposing the Sha256 (of all Executed List) on every payload for PrepareRequest.

vncoelho · 2023-10-10T09:51:47Z

src/DBFTPlugin/Messages/PrepareRequest.cs

@@ -19,13 +19,17 @@ public class PrepareRequest : ConsensusMessage
    {
        public uint Version;
        public UInt256 PrevHash;
+        // The execution result hash of the previous block transactions
+        // since we do not need to update it or search it, no merkel tree is needed


This is correct, in this case, it is not a storage, it is a protocol communication level that ensure nodes will not proceed.

vncoelho · 2023-10-10T09:55:49Z

@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 · 2023-10-10T09:58:23Z

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 · 2023-10-10T18:15:57Z

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 · 2023-10-10T19:23:07Z

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 · 2023-10-10T19:39:04Z

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 · 2023-10-11T00:40:20Z

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.

vncoelho

@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.

Jim8y · 2024-01-08T06:01:18Z

@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 · 2024-01-08T12:32:09Z

@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 · 2024-01-08T13:00:34Z

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 · 2024-01-08T15:28:18Z

No @Jim8y ,

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

Jim8y · 2024-01-08T15:43:31Z

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

vncoelho · 2024-01-08T16:29:51Z

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

Jim8y · 2024-01-08T16:56:52Z

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

please dont tell me its ' shadow block'

add the previous block execution result to the consensus process.

da2f3af

vncoelho reviewed Oct 10, 2023

View reviewed changes

Jim8y changed the title ~~Consensus: add the previous block execution result to the consensus process.~~ Consensus: Add MPT to the consensus. Oct 11, 2023

Merge branch 'master' into execution-result

f087094

Jim8y mentioned this pull request Oct 20, 2023

Enhance the limits neo-project/neo-vm#514

Merged

Jim8y changed the title ~~Consensus: Add MPT to the consensus.~~ Consensus: finding the best way to create a history of block executions results Oct 20, 2023

Jim8y mentioned this pull request Dec 31, 2023

Add: add previous block state to the block header neo-project/neo#3045

Closed

vncoelho reviewed Jan 8, 2024

View reviewed changes

Jim8y closed this Feb 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Jim8y commented Oct 10, 2023

vncoelho commented Oct 10, 2023

Jim8y commented Oct 10, 2023

Jim8y commented Oct 10, 2023

vncoelho commented Oct 10, 2023 •

edited

Loading

vncoelho Oct 10, 2023

vncoelho Oct 10, 2023

vncoelho commented Oct 10, 2023 •

edited

Loading

Jim8y commented Oct 10, 2023

roman-khimov commented Oct 10, 2023

vncoelho commented Oct 10, 2023

roman-khimov commented Oct 10, 2023

Jim8y commented Oct 11, 2023

vncoelho left a comment

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

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

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

Conversation

Jim8y commented Oct 10, 2023

vncoelho commented Oct 10, 2023

Jim8y commented Oct 10, 2023

Jim8y commented Oct 10, 2023

vncoelho commented Oct 10, 2023 • edited Loading

vncoelho Oct 10, 2023

Choose a reason for hiding this comment

vncoelho Oct 10, 2023

Choose a reason for hiding this comment

vncoelho commented Oct 10, 2023 • edited Loading

Jim8y commented Oct 10, 2023

roman-khimov commented Oct 10, 2023

vncoelho commented Oct 10, 2023

roman-khimov commented Oct 10, 2023

Jim8y commented Oct 11, 2023

vncoelho left a comment

Choose a reason for hiding this comment

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

vncoelho commented Jan 8, 2024

Jim8y commented Jan 8, 2024

vncoelho commented Oct 10, 2023 •

edited

Loading

vncoelho commented Oct 10, 2023 •

edited

Loading