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.

Sign up for GitHub

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

Jump to bottom

Fix: add hash verification for OnImport #3070

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Fix: add hash verification for OnImport #3070

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c050f2
add hash verification for OnImport
Jim8y Jan 8, 2024
50b2495
Merge branch 'master' into fix-onimport
shargon Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Neo/Network/P2P/Payloads/Header.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

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.

Copy link
Contributor Author

@Jim8y Jim8y Jan 11, 2024 •
edited
Loading

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 _);
        }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, thanks!

if (prev.Header.timestamp >= timestamp) return false;
if (!this.VerifyWitnesses(settings, snapshot, 3_00000000L)) return false;
return true;
Expand Down