Improved scheme for Conflicts attribute storing and pricing

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)
             {
+                // It's possible that there are previously saved malicious conflict records for this transaction.
+                // These records are garbage in fact and can be removed in a separate flow, but at the same time
+                // it won't hurt to keep them.
+
                 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_MemoryPool.cs

@@ -736,74 +736,6 @@ public void TestUpdatePoolForBlockPersisted()
             _unit.VerifiedCount.Should().Be(0);
         }
 
-
-        [TestMethod]
-        public async Task Malicious_OnChain_Conflict()
-        {
-            // Arrange: prepare mempooled txs that have conflicts.
-            long txFee = 1;
-            var snapshot = GetSnapshot();
-            BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, senderAccount);
-            ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, settings: TestBlockchain.TheNeoSystem.Settings, gas: long.MaxValue);
-            engine.LoadScript(Array.Empty<byte>());
-
-            var tx1 = CreateTransactionWithFeeAndBalanceVerify(txFee);
-            var tx2 = CreateTransactionWithFeeAndBalanceVerify(txFee);
-
-            tx1.Signers[0].Account = UInt160.Parse("0x0001020304050607080900010203040506070809"); // Different sender
-
-            await NativeContract.GAS.Mint(engine, tx1.Sender, 100000, true); // balance enough for all mempooled txs
-            await NativeContract.GAS.Mint(engine, tx2.Sender, 100000, true); // balance enough for all mempooled txs
-
-            tx1.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = tx2.Hash } };
-
-            Assert.AreEqual(_unit.TryAdd(tx1, engine.Snapshot), VerifyResult.Succeed);
-
-            var block = new Block
-            {
-                Header = new Header()
-                {
-                    Index = 10000,
-                    MerkleRoot = UInt256.Zero,
-                    NextConsensus = UInt160.Zero,
-                    PrevHash = UInt256.Zero,
-                    Witness = new Witness() { InvocationScript = Array.Empty<byte>(), VerificationScript = Array.Empty<byte>() }
-                },
-                Transactions = new Transaction[] { tx1 },
-            };
-            _unit.UpdatePoolForBlockPersisted(block, engine.Snapshot);
-
-            _unit.SortedTxCount.Should().Be(0);
-
-            // Simulate persist tx1
-
-            Assert.AreEqual(_unit.TryAdd(tx2, engine.Snapshot), VerifyResult.Succeed);
-
-            byte[] onPersistScript;
-            using (ScriptBuilder sb = new())
-            {
-                sb.EmitSysCall(ApplicationEngine.System_Contract_NativeOnPersist);
-                onPersistScript = sb.ToArray();
-            }
-
-            TransactionState[] transactionStates;
-            using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.OnPersist, null, engine.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[]>();
-            }
-
-            // Test tx2 arrive
-
-            snapshot.Commit();
-
-            var senderProbe = CreateTestProbe();
-            senderProbe.Send(TestBlockchain.TheNeoSystem.Blockchain, tx2);
-            senderProbe.ExpectMsg<Blockchain.RelayResult>(p => p.Result == VerifyResult.InsufficientFunds); // should be Succedded.
-        }
-
         public static StorageKey CreateStorageKey(int id, byte prefix, byte[] key = null)
         {
             byte[] buffer = GC.AllocateUninitializedArray<byte>(sizeof(byte) + (key?.Length ?? 0));

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