-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix: add hash verification for OnImport #3070
Conversation
* master: Fixed asp.net core project (neo-project#3067) Updated BLS12_381 (neo-project#3074) avoid nonsense exception messages. (neo-project#3063) Removed `MyGet` (neo-project#3071) Updated unit-test (neo-project#3073) add hash verification for OnImport (neo-project#3070) Make public ReadUserInput (neo-project#3068) Removed asp.net core (neo-project#3065) Enforce Line Endings in `.editorconfig` (neo-project#3060) Remove some warnings (neo-project#3057) Fixed workflow timeout-minutes (neo-project#3048) Fix: specify the error message (neo-project#3030) Removes `WebSocket`s from the network layer (neo-project#3039) set timeout for tests (neo-project#3046) Fix: Editconfig (neo-project#3023) Set project as nullable (neo-project#3042) Fix: fix equal (neo-project#3028) # Conflicts: # src/Neo.CLI/CLI/MainService.cs # src/Neo.CLI/Settings.cs # src/Neo/ProtocolSettings.cs
@@ -243,6 +243,7 @@ internal bool Verify(ProtocolSettings settings, DataCache snapshot) | |||
TrimmedBlock prev = NativeContract.Ledger.GetTrimmedBlock(snapshot, prevHash); | |||
if (prev is null) return false; | |||
if (prev.Index + 1 != index) return false; | |||
if (prev.Hash != prevHash) return false; |
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.
@shargon, what is the purpose of adding this check? Consensus nodes agree on parent block hash during dBFT process, so it can't be wrong if block witness is OK. And two lines below we check consensus nodes witness, so we're sure that block data are correct.
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.
@AnnaShaleva No special purpose, just make it align with the check below. It wont hurt to add, it wont hurt to remove all checks as well.
internal bool Verify(ProtocolSettings settings, DataCache snapshot, HeaderCache headerCache)
{
Header prev = headerCache.Last;
if (prev is null) return Verify(settings, snapshot);
if (primaryIndex >= settings.ValidatorsCount)
return false;
if (prev.Hash != prevHash) return false;
if (prev.index + 1 != index) return false;
if (prev.timestamp >= timestamp) return false;
return this.VerifyWitness(settings, snapshot, prev.nextConsensus, Witness, 3_00000000L, out _);
}
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.
Good, thanks!
* master: Made `MemoryStore` the default whithout `config.json` for `neo-cli` (neo-project#3085) Adding Devcontainer and link to codespace (neo-project#3075) Update & Consolidate nugets (neo-project#3083) Adding NNS to `neo-cli` (neo-project#3032) Add: add pull request template (neo-project#3081) Move to monorepo: Neo.Cryptography.BLS12_381 (neo-project#3077) Add: add a new verify result status code (neo-project#3076) Convert to Neo.Json and Neo.ConsoleService to `dotnet` standard 2.1 (neo-project#3044) Avoid IsExternalInit (neo-project#3079) Clean usings (neo-project#3078) Fixed asp.net core project (neo-project#3067) Updated BLS12_381 (neo-project#3074) avoid nonsense exception messages. (neo-project#3063) Removed `MyGet` (neo-project#3071) Updated unit-test (neo-project#3073) add hash verification for OnImport (neo-project#3070) Make public ReadUserInput (neo-project#3068) Removed asp.net core (neo-project#3065) Enforce Line Endings in `.editorconfig` (neo-project#3060) # Conflicts: # src/Neo.CLI/CLI/MainService.cs
Same as #3061 but more clear what is the problem