Skip to content

Commit

Permalink
Fix SyncBlockState hash collision issue (#460)
Browse files Browse the repository at this point in the history 
* Remove dependency on HashSet in SyncBlockState

* Add test for ProcessProofNodes
  • Loading branch information
@damian-orzechowski
damian-orzechowski authored Jan 20, 2025
1 parent 2bea3ca commit 2a09a23
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 17 deletions.
81 changes: 81 additions & 0 deletions src/Paprika.Tests/Chain/RawStateTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,5 +272,86 @@ public void CalcRootFromRlpMemoDataStorage()
newRootHash.Should().Be(checkKeccakOrRlp.Keccak);
}

[Test]
public void ProcessProofNodes()
{
using var remoteDb = PagedDb.NativeMemoryDb(32 * 1024, 2);
var merkle = new ComputeMerkleBehavior();

var remoteBlockchain = new Blockchain(remoteDb, merkle);

using var raw = remoteBlockchain.StartRaw();

var account1 = new Keccak(Convert.FromHexString("000aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbb11"));
var account2 = new Keccak(Convert.FromHexString("003aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbb22"));
var account3 = new Keccak(Convert.FromHexString("100aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbb33"));
var account4 = new Keccak(Convert.FromHexString("020aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbb44"));

raw.SetAccount(account1, new Account(Values.Balance1, Values.Nonce1));
raw.SetAccount(account2, new Account(Values.Balance2, Values.Nonce2));
raw.SetAccount(account3, new Account(Values.Balance3, Values.Nonce3));
raw.SetAccount(account4, new Account(Values.Balance4, Values.Nonce4));

raw.Commit(true, true);

var hashAccount3 = raw.GetHash(NibblePath.Parse("1"), false);
var hashAccount4 = raw.GetHash(NibblePath.Parse("02"), false);
var hashBranch2 = raw.GetHash(NibblePath.Parse("0"), false);

using var localDb = PagedDb.NativeMemoryDb(32 * 1024, 2);
var localBlockchain = new Blockchain(localDb, merkle);

using var syncRaw = localBlockchain.StartRaw();

syncRaw.CreateMerkleBranch(Keccak.Zero, NibblePath.Empty, [0, 1], [hashBranch2, hashAccount3], false);
syncRaw.CreateMerkleBranch(Keccak.Zero, NibblePath.Parse("0"), [0, 2], [Keccak.Zero, hashAccount4], false);

syncRaw.SetAccount(account1, new Account(Values.Balance1, Values.Nonce1));
syncRaw.SetAccount(account2, new Account(Values.Balance2, Values.Nonce2));

var localHash = syncRaw.RefreshRootHash();

localHash.Should().Be(raw.Hash);

Span<byte> packed = stackalloc byte[3 * 33];

NibblePath proofPath = NibblePath.Parse("0");

for (int i = proofPath.Length; i >= 0; i--)
{
var currentPath = proofPath.SliceTo(i);
packed[i * 33] = currentPath.Length;
currentPath.RawSpan.CopyTo(packed.Slice(i * 33 + 1));
}
syncRaw.ProcessProofNodes(Keccak.Zero, packed, 2);

syncRaw.Commit(false);

//2nd pass
using var syncRaw2 = localBlockchain.StartRaw();

//check proof nodes from 1st pass were not persisted during commit
syncRaw2.GetHash(NibblePath.Parse("0"), false).Should().Be(Keccak.EmptyTreeHash);

var hashBranch3 = raw.GetHash(NibblePath.Parse("00"), false);

syncRaw2.CreateMerkleBranch(Keccak.Zero, NibblePath.Empty, [0, 1], [hashBranch2, Keccak.Zero], false);
syncRaw2.CreateMerkleBranch(Keccak.Zero, NibblePath.Parse("0"), [0, 2], [hashBranch3, Keccak.Zero], false);

syncRaw2.SetAccount(account3, new Account(Values.Balance3, Values.Nonce3));
syncRaw2.SetAccount(account4, new Account(Values.Balance4, Values.Nonce4));

localHash = syncRaw2.RefreshRootHash();
localHash.Should().Be(raw.Hash);

syncRaw2.ProcessProofNodes(Keccak.Zero, packed, 2);
syncRaw2.Commit(false);

//check
using var syncRaw3 = localBlockchain.StartRaw();
//check proof nodes from 2nd pass were persisted during commit - part from root node - only persisted for storage tries
syncRaw3.GetHash(NibblePath.Parse("0"), false).Should().Be(hashBranch2);
}

private static Random GetRandom() => new(13);
}
4 changes: 4 additions & 0 deletions src/Paprika.Tests/Values.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ public static class Values
public static readonly UInt256 Balance0 = 13;
public static readonly UInt256 Balance1 = 17;
public static readonly UInt256 Balance2 = 19;
public static readonly UInt256 Balance3 = 23;
public static readonly UInt256 Balance4 = 29;

public static readonly UInt256 Nonce0 = 23;
public static readonly UInt256 Nonce1 = 29;
public static readonly UInt256 Nonce2 = 31;
public static readonly UInt256 Nonce3 = 37;
public static readonly UInt256 Nonce4 = 41;
}
28 changes: 11 additions & 17 deletions src/Paprika/Chain/Blockchain.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,6 @@ public override string ToString() =>
private class SyncBlockState : BlockState
{
private readonly PooledSpanDictionary _proofKeys;
private readonly HashSet<ulong> _proofHashSet;

public SyncBlockState(Keccak parentStateRoot, IReadOnlyBatch batch, CommittedBlockState[] ancestors,
Blockchain blockchain) : base(parentStateRoot, batch, ancestors, blockchain)
Expand All @@ -1428,15 +1427,14 @@ public SyncBlockState(Keccak parentStateRoot, IReadOnlyBatch batch, CommittedBlo
_proofKeys.Dispose();

_proofKeys = new PooledSpanDictionary(Pool, true);
_proofHashSet = new HashSet<ulong>();
}

public override ReadOnlySpanOwnerWithMetadata<byte> Get(scoped in Key key)
{
var hash = GetHash(key);
var keyWritten = key.WriteTo(stackalloc byte[key.MaxByteLength]);

if (_proofHashSet.Contains(hash) && _proofKeys.TryGet(keyWritten, hash, out var span))
if (_proofKeys.TryGet(keyWritten, hash, out var span))
{
AcquireLease();
return new ReadOnlySpanOwner<byte>(span, this).WithDepth(0);
Expand All @@ -1450,17 +1448,16 @@ protected override void SetImpl(in Key key, in ReadOnlySpan<byte> payload, Entry
_hash = null;

var hash = GetHash(key);
var k = key.WriteTo(stackalloc byte[key.MaxByteLength]);
var keyWritten = key.WriteTo(stackalloc byte[key.MaxByteLength]);

if (type == EntryType.Proof || (type == EntryType.Persistent && _proofHashSet.Contains(hash)))
if (type == EntryType.Proof || (type == EntryType.Persistent && _proofKeys.TryGet(keyWritten, hash, out _)))
{
_proofKeys.Set(k, hash, payload, (byte)type);
_proofHashSet.Add(hash);
_proofKeys.Set(keyWritten, hash, payload, (byte)type);
return;
}

_filter.Add(hash);
dict.Set(k, hash, payload, (byte)type);
dict.Set(keyWritten, hash, payload, (byte)type);
}

protected override void SetImpl(in Key key, in ReadOnlySpan<byte> payload0, in ReadOnlySpan<byte> payload1, EntryType type, PooledSpanDictionary dict)
Expand All @@ -1469,22 +1466,20 @@ protected override void SetImpl(in Key key, in ReadOnlySpan<byte> payload0, in R
_hash = null;

var hash = GetHash(key);
var k = key.WriteTo(stackalloc byte[key.MaxByteLength]);
var keyWritten = key.WriteTo(stackalloc byte[key.MaxByteLength]);

if (type == EntryType.Proof || (type == EntryType.Persistent && _proofHashSet.Contains(hash)))
if (type == EntryType.Proof || (type == EntryType.Persistent && _proofKeys.TryGet(keyWritten, hash, out _)))
{
_proofKeys.Set(k, hash, payload0, payload1, (byte)type);
_proofHashSet.Add(hash);
_proofKeys.Set(keyWritten, hash, payload0, payload1, (byte)type);
return;
}

_filter.Add(hash);
dict.Set(k, hash, payload0, payload1, (byte)type);
dict.Set(keyWritten, hash, payload0, payload1, (byte)type);
}

protected override void CleanUp()
{
_proofHashSet.Clear();
_proofKeys.Dispose();
base.CleanUp();
}
Expand Down Expand Up @@ -1550,8 +1545,7 @@ public void ProcessProofNodes(Keccak accountKeccak, Span<byte> packedProofPaths,
var childHash = GetHash(childKey);

bool isPersisted;
//double check due to potential hash collision
if (_proofHashSet.Contains(childHash) && proofNodeResults.TryGetValue(childHash, out bool thisRoundPersisted))
if (proofNodeResults.TryGetValue(childHash, out bool thisRoundPersisted))
{
isPersisted = thisRoundPersisted;
}
Expand Down Expand Up @@ -1588,7 +1582,7 @@ public void ProcessProofNodes(Keccak accountKeccak, Span<byte> packedProofPaths,
: Key.Merkle(extChildPath);
var extChildHash = GetHash(extChildKey);

if (_proofHashSet.Contains(extChildHash) && proofNodeResults[extChildHash] || IsPersisted(accountKeccak, extChildPath))
if (proofNodeResults[extChildHash] || IsPersisted(accountKeccak, extChildPath))
{
proofNodeResults[hash] = true;
_preCommit.Set(k, hash, ext.WriteTo(nodeWorkingSpan), (byte)EntryType.Persistent);
Expand Down

0 comments on commit 2a09a23

Please sign in to comment.