Ledger: change conflict records storage scheme

This commit is a part of neo-project#2907: it implements conflict records storage scheme discribed
in neo-project#2907 (comment).

The short scheme description:

Do not store the list of conflicting signers in the Ledger's conflict record value.
Instead, put each conflicting signer in the conflict record key so that the
reculting key is: {Prefix_Transaction, <conflict hash>, <signer>}, and the value
is {<block index>}.

As an optimisation, for each conflict record store the dummy stub where the
key is {Prefix_Transaction, <conflict hash>} and the value is empty. This optimisation
allows to reduce DB read requests during newly-pooled transaction verification for
those transactions that do not have any on-chained conflicts.

Also, IsTraceableBlock check is added for on-chain conflicts verification. It is
needed to avoid situations when untraceable on-chained conflict affects the
newly-pooled transaction verification.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

src/Neo/Ledger/Blockchain.cs

@@ -206,7 +206,7 @@ private void OnFillMemoryPool(IEnumerable<Transaction> transactions)
             {
                 if (NativeContract.Ledger.ContainsTransaction(snapshot, tx.Hash))
                     continue;
-                if (NativeContract.Ledger.ContainsConflictHash(snapshot, tx.Hash, tx.Signers.Select(s => s.Account)))
+                if (NativeContract.Ledger.ContainsConflictHash(snapshot, tx.Hash, tx.Signers.Select(s => s.Account), system.Settings.MaxTraceableBlocks))
                     continue;
                 // First remove the tx if it is unverified in the pool.
                 system.MemPool.TryRemoveUnVerified(tx.Hash, out _);

src/Neo/NeoSystem.cs

@@ -287,7 +287,7 @@ public bool ContainsTransaction(UInt256 hash)
         /// <returns><see langword="true"/> if the transaction conflicts with on-chain transaction; otherwise, <see langword="false"/>.</returns>
         public bool ContainsConflictHash(UInt256 hash, IEnumerable<UInt160> signers)
         {
-            return NativeContract.Ledger.ContainsConflictHash(StoreView, hash, signers);
+            return NativeContract.Ledger.ContainsConflictHash(StoreView, hash, signers, Settings.MaxTraceableBlocks);
         }
     }
 }

src/Neo/SmartContract/Native/LedgerContract.cs

@@ -47,13 +47,21 @@ internal override ContractTask OnPersist(ApplicationEngine engine)
             engine.Snapshot.Add(CreateStorageKey(Prefix_Block).Add(engine.PersistingBlock.Hash), new StorageItem(Trim(engine.PersistingBlock).ToArray()));
             foreach (TransactionState tx in transactions)
             {
+                // Remove possible previously saved malicious conflict records for the transaction (if any).
+                foreach (var (key, _) in engine.Snapshot.Find(CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash).ToArray()))
+                    engine.Snapshot.Delete(key);
+
                 engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash), new StorageItem(tx));
+
+                // Store transaction's conflicits.
                 var conflictingSigners = tx.Transaction.Signers.Select(s => s.Account);
                 foreach (var attr in tx.Transaction.GetAttributes<Conflicts>())
                 {
-                    var conflictRecord = engine.Snapshot.GetAndChange(CreateStorageKey(Prefix_Transaction).Add(attr.Hash),
-                        () => new StorageItem(new TransactionState { ConflictingSigners = Array.Empty<UInt160>() })).GetInteroperable<TransactionState>();
-                    conflictRecord.ConflictingSigners = conflictRecord.ConflictingSigners.Concat(conflictingSigners).Distinct().ToArray();
+                    engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(attr.Hash), new StorageItem(new TransactionState() { BlockIndex = engine.PersistingBlock.Index }));
+                    foreach (var signer in conflictingSigners)
+                    {
+                        engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(attr.Hash).Add(signer), new StorageItem(new TransactionState() { BlockIndex = engine.PersistingBlock.Index }));
+                    }
                 }
             }
             engine.SetState(transactions);
@@ -145,11 +153,24 @@ public bool ContainsTransaction(DataCache snapshot, UInt256 hash)
         /// <param name="snapshot">The snapshot used to read data.</param>
         /// <param name="hash">The hash of the conflicting transaction.</param>
         /// <param name="signers">The list of signer accounts of the conflicting transaction.</param>
+        /// <param name="maxTraceableBlocks">MaxTraceableBlocks protocol setting.</param>
         /// <returns><see langword="true"/> if the blockchain contains the hash of the conflicting transaction; otherwise, <see langword="false"/>.</returns>
-        public bool ContainsConflictHash(DataCache snapshot, UInt256 hash, IEnumerable<UInt160> signers)
+        public bool ContainsConflictHash(DataCache snapshot, UInt256 hash, IEnumerable<UInt160> signers, uint maxTraceableBlocks)
         {
-            var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable<TransactionState>();
-            return state is not null && state.Transaction is null && (signers is null || state.ConflictingSigners.Intersect(signers).Any());
+            // Check the dummy stub firstly to define whether there's exist at least one conflict record.
+            var stub = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable<TransactionState>();
+            if (stub is null || stub.Transaction is not null || !IsTraceableBlock(snapshot, stub.BlockIndex, maxTraceableBlocks))
+                return false;
+
+            // At least one conflict record is found, then need to check signers intersection.
+            foreach (var signer in signers)
+            {
+                var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash).Add(signer))?.GetInteroperable<TransactionState>();
+                if (state is not null && IsTraceableBlock(snapshot, state.BlockIndex, maxTraceableBlocks))
+                    return true;
+            }
+
+            return false;
         }
 
         /// <summary>

src/Neo/SmartContract/Native/TransactionState.cs

@@ -32,8 +32,6 @@ public class TransactionState : IInteroperable
         /// </summary>
         public Transaction Transaction;
 
-        public UInt160[] ConflictingSigners;
-
         /// <summary>
         /// The execution state
         /// </summary>
@@ -47,7 +45,6 @@ IInteroperable IInteroperable.Clone()
             {
                 BlockIndex = BlockIndex,
                 Transaction = Transaction,
-                ConflictingSigners = ConflictingSigners,
                 State = State,
                 _rawTransaction = _rawTransaction
             };
@@ -58,7 +55,6 @@ void IInteroperable.FromReplica(IInteroperable replica)
             TransactionState from = (TransactionState)replica;
             BlockIndex = from.BlockIndex;
             Transaction = from.Transaction;
-            ConflictingSigners = from.ConflictingSigners;
             State = from.State;
             if (_rawTransaction.IsEmpty)
                 _rawTransaction = from._rawTransaction;
@@ -67,20 +63,21 @@ void IInteroperable.FromReplica(IInteroperable replica)
         void IInteroperable.FromStackItem(StackItem stackItem)
         {
             Struct @struct = (Struct)stackItem;
-            if (@struct.Count == 1)
-            {
-                ConflictingSigners = ((VM.Types.Array)@struct[0]).Select(u => new UInt160(u.GetSpan())).ToArray();
-                return;
-            }
             BlockIndex = (uint)@struct[0].GetInteger();
+
+            // Conflict record.
+            if (@struct.Count == 1) return;
+
+            // Fully-qualified transaction.
             _rawTransaction = ((ByteString)@struct[1]).Memory;
             Transaction = _rawTransaction.AsSerializable<Transaction>();
             State = (VMState)(byte)@struct[2].GetInteger();
         }
 
         StackItem IInteroperable.ToStackItem(ReferenceCounter referenceCounter)
         {
-            if (Transaction is null) return new Struct(referenceCounter) { new VM.Types.Array(referenceCounter, ConflictingSigners.Select(u => new ByteString(u.ToArray())).ToArray()) };
+            if (Transaction is null)
+                return new Struct(referenceCounter) { BlockIndex };
             if (_rawTransaction.IsEmpty)
                 _rawTransaction = Transaction.ToArray();
             return new Struct(referenceCounter) { BlockIndex, _rawTransaction, (byte)State };

tests/Neo.UnitTests/Ledger/UT_Blockchain.cs

@@ -135,7 +135,7 @@ public void TestMaliciousOnChainConflict()
             {
                 Header = new Header()
                 {
-                    Index = 10000,
+                    Index = 5, // allow tx1, tx2 and tx3 to fit into MaxValidUntilBlockIncrement.
                     MerkleRoot = UInt256.Zero,
                     NextConsensus = UInt160.Zero,
                     PrevHash = UInt256.Zero,
@@ -149,13 +149,26 @@ public void TestMaliciousOnChainConflict()
                 sb.EmitSysCall(ApplicationEngine.System_Contract_NativeOnPersist);
                 onPersistScript = sb.ToArray();
             }
-            TransactionState[] transactionStates;
             using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.OnPersist, null, snapshot, block, TestBlockchain.TheNeoSystem.Settings, 0))
             {
                 engine2.LoadScript(onPersistScript);
                 if (engine2.Execute() != VMState.HALT) throw new InvalidOperationException();
-                Blockchain.ApplicationExecuted application_executed = new(engine2);
-                transactionStates = engine2.GetState<TransactionState[]>();
+                engine2.Snapshot.Commit();
+            }
+            snapshot.Commit();
+
+            // Run PostPersist to update current block index in native Ledger.
+            // Relevant current block index is needed for conflict records checks.
+            byte[] postPersistScript;
+            using (ScriptBuilder sb = new())
+            {
+                sb.EmitSysCall(ApplicationEngine.System_Contract_NativePostPersist);
+                postPersistScript = sb.ToArray();
+            }
+            using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.PostPersist, null, snapshot, block, TestBlockchain.TheNeoSystem.Settings, 0))
+            {
+                engine2.LoadScript(postPersistScript);
+                if (engine2.Execute() != VMState.HALT) throw new InvalidOperationException();
                 engine2.Snapshot.Commit();
             }
             snapshot.Commit();

tests/Neo.UnitTests/Ledger/UT_TransactionState.cs

@@ -36,11 +36,7 @@ public void Initialize()
             };
             originTrimmed = new TransactionState
             {
-                ConflictingSigners = new UInt160[]
-            {
-                new UInt160(Crypto.Hash160(new byte[] { 1, 2, 3 })),
-                new UInt160(Crypto.Hash160(new byte[] { 4, 5, 6 }))
-            }
+                BlockIndex = 1,
             };
         }
 
@@ -67,10 +63,9 @@ public void TestDeserializeTrimmed()
             TransactionState dest = new();
             ((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null));
 
-            dest.BlockIndex.Should().Be(0);
+            dest.BlockIndex.Should().Be(originTrimmed.BlockIndex);
             dest.Transaction.Should().Be(null);
             dest.Transaction.Should().BeNull();
-            CollectionAssert.AreEqual(originTrimmed.ConflictingSigners, dest.ConflictingSigners);
         }
     }
 }