-
Notifications
You must be signed in to change notification settings - Fork 1k
Introduce StateRootInHeader
#4161
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// Copyright (C) 2015-2025 The Neo Project. | ||
// | ||
// BlockVersion.cs file belongs to the neo project and is free | ||
// software distributed under the MIT software license, see the | ||
// accompanying file LICENSE in the main directory of the | ||
// repository or http://www.opensource.org/licenses/mit-license.php | ||
// for more details. | ||
// | ||
// Redistribution and use in source and binary forms with or without | ||
// modifications are permitted. | ||
|
||
namespace Neo.Network.P2P.Payloads | ||
{ | ||
/// <summary> | ||
/// Represents the block version. | ||
/// </summary> | ||
public enum BlockVersion : byte | ||
{ | ||
/// <summary> | ||
/// Initial block version. | ||
/// </summary> | ||
V0, | ||
/// <summary> | ||
/// Block version 1 which adds PrevStateRoot field to the header. | ||
/// </summary> | ||
V1, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,16 +199,19 @@ public void Reset(byte viewNumber) | |
Snapshot?.Dispose(); | ||
Snapshot = neoSystem.GetSnapshotCache(); | ||
uint height = NativeContract.Ledger.CurrentIndex(Snapshot); | ||
var isFaun = neoSystem.Settings.IsHardforkEnabled(Hardfork.HF_Faun, height + 1); | ||
Block = new Block | ||
{ | ||
Header = new Header | ||
{ | ||
Version = isFaun ? (uint)BlockVersion.V1 : (uint)BlockVersion.V0, | ||
PrevHash = NativeContract.Ledger.CurrentHash(Snapshot), | ||
Index = height + 1, | ||
NextConsensus = Contract.GetBFTAddress( | ||
NeoToken.ShouldRefreshCommittee(height + 1, neoSystem.Settings.CommitteeMembersCount) ? | ||
NativeContract.NEO.ComputeNextBlockValidators(Snapshot, neoSystem.Settings) : | ||
NativeContract.NEO.GetNextBlockValidators(Snapshot, neoSystem.Settings.ValidatorsCount)) | ||
NativeContract.NEO.GetNextBlockValidators(Snapshot, neoSystem.Settings.ValidatorsCount)), | ||
PrevStateRoot = isFaun ? StateService.StatePlugin.GetStateRootHash(height) : null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this null here cause any issue? is the codebase robust enough to tolerate a null prevstateroot? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's checked. |
||
} | ||
}; | ||
TimePerBlock = neoSystem.GetTimePerBlock(); | ||
|
@@ -294,6 +297,8 @@ public void Deserialize(ref MemoryReader reader) | |
Block.Header.NextConsensus = reader.ReadSerializable<UInt160>(); | ||
if (Block.NextConsensus.Equals(UInt160.Zero)) | ||
Block.Header.NextConsensus = null; | ||
if (Block.Version == (uint)BlockVersion.V1) | ||
Block.Header.PrevStateRoot = reader.ReadSerializable<UInt256>(); | ||
ViewNumber = reader.ReadByte(); | ||
TransactionHashes = reader.ReadSerializableArray<UInt256>(ushort.MaxValue); | ||
Transaction[] transactions = reader.ReadSerializableArray<Transaction>(ushort.MaxValue); | ||
|
@@ -320,6 +325,8 @@ public void Serialize(BinaryWriter writer) | |
writer.Write(Block.Nonce); | ||
writer.Write(Block.PrimaryIndex); | ||
writer.Write(Block.NextConsensus ?? UInt160.Zero); | ||
if (Block.Version == (uint)BlockVersion.V1) | ||
writer.Write(Block.PrevStateRoot); | ||
writer.Write(ViewNumber); | ||
writer.Write(TransactionHashes ?? Array.Empty<UInt256>()); | ||
writer.Write(Transactions?.Values.ToArray() ?? Array.Empty<Transaction>()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,5 +7,8 @@ | |
"MaxBlockSize": 2097152, | ||
"MaxBlockSystemFee": 150000000000, | ||
"UnhandledExceptionPolicy": "StopNode" | ||
} | ||
}, | ||
"Dependency": [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add this extra dependency ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because dBFT should have an access to stateroot at every block, and stateroot is managed by StateService plugin. |
||
"StateService" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @neo-project/core There's a pitfall that may be moved to a separate issue: dBFT needs StateService because dBFT should have an access to stateroots, and stateroots are managed by StateService plugin. However, StateService plugin requires RPCServer plugin to be installed because of RPC method extensions. So in other words, dBFT requires RPC server which is not a proper dependency for dBFT. We may split StateService plugin functionality into two plugins: one part will manage stateroots, another part will expose additional RPC methods. Let me know if you agree or have other suggestions on how to solve this issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ref. #4190. |
||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,10 +102,35 @@ void ICommittingHandler.Blockchain_Committing_Handler(NeoSystem system, Block bl | |
IReadOnlyList<ApplicationExecuted> applicationExecutedList) | ||
{ | ||
if (system.Settings.Network != StateServiceSettings.Default.Network) return; | ||
StateStore.Singleton.UpdateLocalStateRootSnapshot(block.Index, | ||
|
||
// Rebuild MPT from scratch on node bootstrap if StateService plugin is not up-to-date wrt the current block height. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part is not clear to me, why rebuild here and why the plugin would be out of sync There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to avoid node resynchronization for existing CNs. We can skip doing this (frankly, It'd be even easier for us), but then CNs would need to enable the plugin and restart from genesis. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need the plugin to restart, not the whole node. Maybe we need this feature There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This may be moved to a separate issue, but for now it's implemented as is. I'm open to suggestions on a place where we can move this functionality, because right now it's not a proper place for it (right now we rebuild MPT after block processing first time when the node is started with StateService plugin in fact). |
||
if (block.Index > 0) | ||
{ | ||
using var stateSnapshot = StateStore.Singleton.GetSnapshot(); | ||
var persistedHeight = block.Index - 1; | ||
var stateRoot = stateSnapshot.GetStateRoot(persistedHeight); | ||
if (stateRoot is null) | ||
{ | ||
using var coreSnapshot = system.GetSnapshotCache(); | ||
StateStore.Singleton.UpdateLocalStateRootSnapshot(persistedHeight, | ||
coreSnapshot.Find(SeekDirection.Forward) | ||
.Where(si => si.Key.Id != NativeContract.Ledger.Id) | ||
.Select(si => new KeyValuePair<StorageKey, DataCache.Trackable>(si.Key, new DataCache.Trackable(si.Value, TrackState.Added))) | ||
.ToList()); | ||
StateStore.Singleton.UpdateLocalStateRoot(persistedHeight); | ||
} | ||
} | ||
|
||
var root = StateStore.Singleton.UpdateLocalStateRootSnapshot(block.Index, | ||
snapshot.GetChangeSet() | ||
.Where(p => p.Value.State != TrackState.None && p.Key.Id != NativeContract.Ledger.Id) | ||
.ToList()); | ||
if (system.Settings.IsHardforkEnabled(Hardfork.HF_Faun, block.Index + 1)) | ||
{ | ||
var nextH = system.HeaderCache[block.Index + 1]; | ||
if (nextH is not null && root != nextH.PrevStateRoot) | ||
throw new InvalidOperationException($"Stateroot mismatch at {block.Index}: expected {nextH.PrevStateRoot}, got {root}"); | ||
} | ||
} | ||
|
||
void ICommittedHandler.Blockchain_Committed_Handler(NeoSystem system, Block block) | ||
|
@@ -114,6 +139,14 @@ void ICommittedHandler.Blockchain_Committed_Handler(NeoSystem system, Block bloc | |
StateStore.Singleton.UpdateLocalStateRoot(block.Index); | ||
} | ||
|
||
public static UInt256 GetStateRootHash(uint index) | ||
{ | ||
using var snapshot = StateStore.Singleton.GetSnapshot(); | ||
var root = snapshot.GetStateRoot(index); | ||
if (root is null) return null; | ||
return root.RootHash; | ||
} | ||
|
||
private void CheckNetwork() | ||
{ | ||
var network = StateServiceSettings.Default.Network; | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still thinking that if we append at the end, we don't affect SC that checks the
Hashes.Length
without check the version