diff --git a/src/Neo/Ledger/MemoryPool.cs b/src/Neo/Ledger/MemoryPool.cs index 6ade372ee10..a45df57c3b6 100644 --- a/src/Neo/Ledger/MemoryPool.cs +++ b/src/Neo/Ledger/MemoryPool.cs @@ -55,6 +55,10 @@ public class MemoryPool : IReadOnlyCollection /// private readonly Dictionary _unsortedTransactions = new(); /// + /// Store transaction hashes that conflict with verified mempooled transactions. + /// + private readonly Dictionary> _conflicts = new(); + /// /// Stores the verified sorted transactions currently in the pool. /// private readonly SortedSet _sortedTransactions = new(); @@ -275,7 +279,8 @@ private PoolItem GetLowestFeeTransaction(out Dictionary unsor } } - // Note: this must only be called from a single thread (the Blockchain actor) + // Note: this must only be called from a single thread (the Blockchain actor) and + // doesn't take into account conflicting transactions. internal bool CanTransactionFitInPool(Transaction tx) { if (Count < Capacity) return true; @@ -293,15 +298,31 @@ internal VerifyResult TryAdd(Transaction tx, DataCache snapshot) _txRwLock.EnterWriteLock(); try { - VerifyResult result = tx.VerifyStateDependent(_system.Settings, snapshot, VerificationContext); + if (!CheckConflicts(tx, out List conflictsToBeRemoved)) return VerifyResult.HasConflicts; + VerifyResult result = tx.VerifyStateDependent(_system.Settings, snapshot, VerificationContext, conflictsToBeRemoved.Select(c => c.Tx)); if (result != VerifyResult.Succeed) return result; _unsortedTransactions.Add(tx.Hash, poolItem); VerificationContext.AddTransaction(tx); _sortedTransactions.Add(poolItem); + foreach (var conflict in conflictsToBeRemoved) + { + if (TryRemoveVerified(conflict.Tx.Hash, out var _)) + VerificationContext.RemoveTransaction(conflict.Tx); + } + removedTransactions = conflictsToBeRemoved.Select(itm => itm.Tx).ToList(); + foreach (var attr in tx.GetAttributes()) + { + if (!_conflicts.TryGetValue(attr.Hash, out var pooled)) + { + pooled = new List(); + } + pooled.Add(tx.Hash); + _conflicts.AddOrSet(attr.Hash, pooled); + } if (Count > Capacity) - removedTransactions = RemoveOverCapacity(); + removedTransactions.AddRange(RemoveOverCapacity()); } finally { @@ -309,7 +330,7 @@ internal VerifyResult TryAdd(Transaction tx, DataCache snapshot) } TransactionAdded?.Invoke(this, poolItem.Tx); - if (removedTransactions != null) + if (removedTransactions.Count() > 0) TransactionRemoved?.Invoke(this, new() { Transactions = removedTransactions, @@ -320,6 +341,44 @@ internal VerifyResult TryAdd(Transaction tx, DataCache snapshot) return VerifyResult.Succeed; } + /// + /// Checks whether transaction conflicts with mempooled verified transactions + /// and can be added to the pool. If true, then transactions from conflictsList + /// should be removed from the verified structures (_unsortedTransactions and + /// _sortedTransactions). + /// + /// The that needs to be checked. + /// The list of conflicting verified transactions that should be removed from the pool if tx fits the pool. + /// True if transaction fits the pool, otherwise false. + private bool CheckConflicts(Transaction tx, out List conflictsList) + { + conflictsList = new(); + // Step 1: check if `tx` was in attributes of mempooled transactions. + if (_conflicts.TryGetValue(tx.Hash, out var conlicting)) + { + foreach (var hash in conlicting) + { + var existingTx = _unsortedTransactions[hash]; + if (existingTx.Tx.Signers.Select(s => s.Account).Contains(tx.Sender) && existingTx.Tx.NetworkFee > tx.NetworkFee) return false; + conflictsList.Add(existingTx); + } + } + // Step 2: check if mempooled transactions were in `tx`'s attributes. + foreach (var hash in tx.GetAttributes().Select(p => p.Hash)) + { + if (_unsortedTransactions.TryGetValue(hash, out PoolItem existingTx)) + { + if (!tx.Signers.Select(p => p.Account).Contains(existingTx.Tx.Sender)) return false; + if (existingTx.Tx.NetworkFee >= tx.NetworkFee) return false; + conflictsList.Add(existingTx); + } + } + // Step 3: take into account sender's conflicting transactions while balance check, + // this will be done in VerifyStateDependant. + + return true; + } + private List RemoveOverCapacity() { List removedTransactions = new(); @@ -332,7 +391,10 @@ private List RemoveOverCapacity() removedTransactions.Add(minItem.Tx); if (ReferenceEquals(sortedPool, _sortedTransactions)) + { + RemoveConflictsOfVerified(minItem); VerificationContext.RemoveTransaction(minItem.Tx); + } } while (Count > Capacity); return removedTransactions; @@ -347,9 +409,27 @@ private bool TryRemoveVerified(UInt256 hash, out PoolItem item) _unsortedTransactions.Remove(hash); _sortedTransactions.Remove(item); + RemoveConflictsOfVerified(item); + return true; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void RemoveConflictsOfVerified(PoolItem item) + { + foreach (var h in item.Tx.GetAttributes().Select(attr => attr.Hash)) + { + if (_conflicts.TryGetValue(h, out List conflicts)) + { + conflicts.Remove(item.Tx.Hash); + if (conflicts.Count() == 0) + { + _conflicts.Remove(h); + } + } + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool TryRemoveUnVerified(UInt256 hash, out PoolItem item) { @@ -374,28 +454,60 @@ internal void InvalidateVerifiedTransactions() _unsortedTransactions.Clear(); VerificationContext = new TransactionVerificationContext(); _sortedTransactions.Clear(); + _conflicts.Clear(); } // Note: this must only be called from a single thread (the Blockchain actor) internal void UpdatePoolForBlockPersisted(Block block, DataCache snapshot) { + var conflictingItems = new List(); _txRwLock.EnterWriteLock(); try { + HashSet conflicts = new HashSet(); // First remove the transactions verified in the block. + // No need to modify VerificationContext as it will be reset afterwards. foreach (Transaction tx in block.Transactions) { - if (TryRemoveVerified(tx.Hash, out _)) continue; - TryRemoveUnVerified(tx.Hash, out _); + if (!TryRemoveVerified(tx.Hash, out _)) TryRemoveUnVerified(tx.Hash, out _); + foreach (var h in tx.GetAttributes().Select(a => a.Hash)) + { + conflicts.Add(h); + } + } + + // Then remove the transactions conflicting with the accepted ones. + // No need to modify VerificationContext as it will be reset afterwards. + var persisted = block.Transactions.Select(t => t.Hash); + var stale = new List(); + foreach (var item in _sortedTransactions) + { + if (conflicts.Contains(item.Tx.Hash) || item.Tx.GetAttributes().Select(a => a.Hash).Intersect(persisted).Count() > 0) + { + stale.Add(item.Tx.Hash); + conflictingItems.Add(item.Tx); + } + } + foreach (var h in stale) + { + if (!TryRemoveVerified(h, out _)) TryRemoveUnVerified(h, out _); } - // Add all the previously verified transactions back to the unverified transactions + // Add all the previously verified transactions back to the unverified transactions and clear mempool conflicts list. InvalidateVerifiedTransactions(); } finally { _txRwLock.ExitWriteLock(); } + if (conflictingItems.Count() > 0) + { + TransactionRemoved?.Invoke(this, new() + { + Transactions = conflictingItems.ToArray(), + Reason = TransactionRemovalReason.Conflict, + }); + } // If we know about headers of future blocks, no point in verifying transactions from the unverified tx pool // until we get caught up. @@ -431,12 +543,32 @@ private int ReverifyTransactions(SortedSet verifiedSortedTxPool, // Since unverifiedSortedTxPool is ordered in an ascending manner, we take from the end. foreach (PoolItem item in unverifiedSortedTxPool.Reverse().Take(count)) { - if (item.Tx.VerifyStateDependent(_system.Settings, snapshot, VerificationContext) == VerifyResult.Succeed) + if (CheckConflicts(item.Tx, out List conflictsToBeRemoved) && + item.Tx.VerifyStateDependent(_system.Settings, snapshot, VerificationContext, conflictsToBeRemoved.Select(c => c.Tx)) == VerifyResult.Succeed) { reverifiedItems.Add(item); - VerificationContext.AddTransaction(item.Tx); - } - else // Transaction no longer valid -- it will be removed from unverifiedTxPool. + if (_unsortedTransactions.TryAdd(item.Tx.Hash, item)) + { + verifiedSortedTxPool.Add(item); + foreach (var attr in item.Tx.GetAttributes()) + { + if (!_conflicts.TryGetValue(attr.Hash, out var pooled)) + { + pooled = new List(); + } + pooled.Add(item.Tx.Hash); + _conflicts.AddOrSet(attr.Hash, pooled); + } + VerificationContext.AddTransaction(item.Tx); + foreach (var conflict in conflictsToBeRemoved) + { + if (TryRemoveVerified(conflict.Tx.Hash, out var _)) + VerificationContext.RemoveTransaction(conflict.Tx); + invalidItems.Add(conflict); + } + + } + } else // Transaction no longer valid -- it will be removed from unverifiedTxPool. invalidItems.Add(item); if (TimeProvider.Current.UtcNow > reverifyCutOffTimeStamp) break; @@ -450,18 +582,14 @@ private int ReverifyTransactions(SortedSet verifiedSortedTxPool, var rebroadcastCutOffTime = TimeProvider.Current.UtcNow.AddMilliseconds(-_system.Settings.MillisecondsPerBlock * blocksTillRebroadcast); foreach (PoolItem item in reverifiedItems) { - if (_unsortedTransactions.TryAdd(item.Tx.Hash, item)) + if (_unsortedTransactions.ContainsKey(item.Tx.Hash)) { - verifiedSortedTxPool.Add(item); - if (item.LastBroadcastTimestamp < rebroadcastCutOffTime) { _system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = item.Tx }, _system.Blockchain); item.LastBroadcastTimestamp = TimeProvider.Current.UtcNow; } } - else - VerificationContext.RemoveTransaction(item.Tx); _unverifiedTransactions.Remove(item.Tx.Hash); unverifiedSortedTxPool.Remove(item); diff --git a/src/Neo/Ledger/TransactionRemovalReason.cs b/src/Neo/Ledger/TransactionRemovalReason.cs index 9767fb919f8..fa66b609817 100644 --- a/src/Neo/Ledger/TransactionRemovalReason.cs +++ b/src/Neo/Ledger/TransactionRemovalReason.cs @@ -16,13 +16,18 @@ namespace Neo.Ledger public enum TransactionRemovalReason : byte { /// - /// The transaction was ejected since it was the lowest priority transaction and the memory pool capacity was exceeded. + /// The transaction was rejected since it was the lowest priority transaction and the memory pool capacity was exceeded. /// CapacityExceeded, /// - /// The transaction was ejected due to failing re-validation after a block was persisted. + /// The transaction was rejected due to failing re-validation after a block was persisted. /// NoLongerValid, + + /// + /// The transaction was rejected due to conflict with higher priority transactions with Conflicts attribute. + /// + Conflict, } } diff --git a/src/Neo/Ledger/TransactionVerificationContext.cs b/src/Neo/Ledger/TransactionVerificationContext.cs index b77adfe9c7c..021f71ad9c5 100644 --- a/src/Neo/Ledger/TransactionVerificationContext.cs +++ b/src/Neo/Ledger/TransactionVerificationContext.cs @@ -50,14 +50,19 @@ public void AddTransaction(Transaction tx) /// Determine whether the specified conflicts with other transactions. /// /// The specified . + /// The list of that conflicts with the specified one and are to be removed from the pool. /// The snapshot used to verify the . /// if the passes the check; otherwise, . - public bool CheckTransaction(Transaction tx, DataCache snapshot) + public bool CheckTransaction(Transaction tx, IEnumerable conflictingTxs, DataCache snapshot) { BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender); senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool); BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool; + foreach (var conflictTx in conflictingTxs) + { + fee -= conflictTx.NetworkFee + conflictTx.SystemFee; + } if (balance < fee) return false; var oracle = tx.GetAttribute(); diff --git a/src/Neo/Ledger/VerifyResult.cs b/src/Neo/Ledger/VerifyResult.cs index 7858903ad2c..94c9e78bf83 100644 --- a/src/Neo/Ledger/VerifyResult.cs +++ b/src/Neo/Ledger/VerifyResult.cs @@ -77,6 +77,11 @@ public enum VerifyResult : byte /// PolicyFail, + /// + /// Indicates that the failed to verify because it conflicts with on-chain or mempooled transactions. + /// + HasConflicts, + /// /// Indicates that the failed to verify due to other reasons. /// diff --git a/src/Neo/Network/P2P/Payloads/Conflicts.cs b/src/Neo/Network/P2P/Payloads/Conflicts.cs new file mode 100644 index 00000000000..9ef42e8500d --- /dev/null +++ b/src/Neo/Network/P2P/Payloads/Conflicts.cs @@ -0,0 +1,44 @@ +using Neo.IO; +using Neo.Json; +using Neo.Persistence; +using Neo.SmartContract.Native; +using System.IO; + +namespace Neo.Network.P2P.Payloads +{ + public class Conflicts : TransactionAttribute + { + /// + /// Indicates the conflict transaction hash. + /// + public UInt256 Hash; + + public override TransactionAttributeType Type => TransactionAttributeType.Conflicts; + + public override bool AllowMultiple => true; + + public override int Size => base.Size + Hash.Size; + + protected override void DeserializeWithoutType(ref MemoryReader reader) + { + Hash = reader.ReadSerializable(); + } + + protected override void SerializeWithoutType(BinaryWriter writer) + { + writer.Write(Hash); + } + + public override JObject ToJson() + { + JObject json = base.ToJson(); + json["hash"] = Hash.ToString(); + return json; + } + + public override bool Verify(DataCache snapshot, Transaction tx) + { + return !NativeContract.Ledger.ContainsTransaction(snapshot, Hash); + } + } +} diff --git a/src/Neo/Network/P2P/Payloads/Transaction.cs b/src/Neo/Network/P2P/Payloads/Transaction.cs index b99449ac48d..5babf5b8227 100644 --- a/src/Neo/Network/P2P/Payloads/Transaction.cs +++ b/src/Neo/Network/P2P/Payloads/Transaction.cs @@ -338,12 +338,13 @@ public JObject ToJson(ProtocolSettings settings) /// The used to verify the transaction. /// The snapshot used to verify the transaction. /// The used to verify the transaction. + /// The list of conflicting those fee should be excluded from sender's overall fee during -based verification. /// The result of the verification. - public VerifyResult Verify(ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context) + public VerifyResult Verify(ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context, IEnumerable conflictsList) { VerifyResult result = VerifyStateIndependent(settings); if (result != VerifyResult.Succeed) return result; - return VerifyStateDependent(settings, snapshot, context); + return VerifyStateDependent(settings, snapshot, context, conflictsList); } /// @@ -352,8 +353,9 @@ public VerifyResult Verify(ProtocolSettings settings, DataCache snapshot, Transa /// The used to verify the transaction. /// The snapshot used to verify the transaction. /// The used to verify the transaction. + /// The list of conflicting those fee should be excluded from sender's overall fee during -based verification. /// The result of the verification. - public virtual VerifyResult VerifyStateDependent(ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context) + public virtual VerifyResult VerifyStateDependent(ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context, IEnumerable conflictsList) { uint height = NativeContract.Ledger.CurrentIndex(snapshot); if (ValidUntilBlock <= height || ValidUntilBlock > height + settings.MaxValidUntilBlockIncrement) @@ -362,7 +364,7 @@ public virtual VerifyResult VerifyStateDependent(ProtocolSettings settings, Data foreach (UInt160 hash in hashes) if (NativeContract.Policy.IsBlocked(snapshot, hash)) return VerifyResult.PolicyFail; - if (!(context?.CheckTransaction(this, snapshot) ?? true)) return VerifyResult.InsufficientFunds; + if (!(context?.CheckTransaction(this, conflictsList, snapshot) ?? true)) return VerifyResult.InsufficientFunds; foreach (TransactionAttribute attribute in Attributes) if (!attribute.Verify(snapshot, this)) return VerifyResult.InvalidAttribute; diff --git a/src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs b/src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs index 41c69b507d4..66dccb340b2 100644 --- a/src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs +++ b/src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs @@ -33,6 +33,12 @@ public enum TransactionAttributeType : byte /// Indicates that the transaction is not valid before . /// [ReflectionCache(typeof(NotValidBefore))] - NotValidBefore = 0x20 + NotValidBefore = 0x20, + + /// + /// Indicates that the transaction conflicts with . + /// + [ReflectionCache(typeof(Conflicts))] + Conflicts = 0x21 } } diff --git a/src/Neo/SmartContract/Native/LedgerContract.cs b/src/Neo/SmartContract/Native/LedgerContract.cs index eec49da99a2..d686d4f3b27 100644 --- a/src/Neo/SmartContract/Native/LedgerContract.cs +++ b/src/Neo/SmartContract/Native/LedgerContract.cs @@ -38,6 +38,7 @@ internal override ContractTask OnPersist(ApplicationEngine engine) { TransactionState[] transactions = engine.PersistingBlock.Transactions.Select(p => new TransactionState { + Trimmed = false, BlockIndex = engine.PersistingBlock.Index, Transaction = p, State = VMState.NONE @@ -47,6 +48,10 @@ internal override ContractTask OnPersist(ApplicationEngine engine) foreach (TransactionState tx in transactions) { engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash), new StorageItem(tx)); + foreach (var attr in tx.Transaction.GetAttributes()) + { + engine.Snapshot.GetOrAdd(CreateStorageKey(Prefix_Transaction).Add(attr.Hash), () => new StorageItem(new TransactionState { Trimmed = true })); + } } engine.SetState(transactions); return ContractTask.CompletedTask; @@ -220,7 +225,9 @@ public Header GetHeader(DataCache snapshot, uint index) /// The with the specified hash. public TransactionState GetTransactionState(DataCache snapshot, UInt256 hash) { - return snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable(); + var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable(); + if (state is not null && state.Trimmed) return null; + return state; } /// diff --git a/src/Neo/SmartContract/Native/TransactionState.cs b/src/Neo/SmartContract/Native/TransactionState.cs index 5c7380f1b89..538b474a7e0 100644 --- a/src/Neo/SmartContract/Native/TransactionState.cs +++ b/src/Neo/SmartContract/Native/TransactionState.cs @@ -21,6 +21,11 @@ namespace Neo.SmartContract.Native /// public class TransactionState : IInteroperable { + /// + /// The transaction is trimmed. + /// To indicate this state is a placeholder for a conflict transaction. + /// + public bool Trimmed; /// /// The block containing this transaction. /// @@ -42,6 +47,7 @@ IInteroperable IInteroperable.Clone() { return new TransactionState { + Trimmed = Trimmed, BlockIndex = BlockIndex, Transaction = Transaction, State = State, @@ -52,6 +58,7 @@ IInteroperable IInteroperable.Clone() void IInteroperable.FromReplica(IInteroperable replica) { TransactionState from = (TransactionState)replica; + Trimmed = from.Trimmed; BlockIndex = from.BlockIndex; Transaction = from.Transaction; State = from.State; @@ -62,17 +69,20 @@ void IInteroperable.FromReplica(IInteroperable replica) void IInteroperable.FromStackItem(StackItem stackItem) { Struct @struct = (Struct)stackItem; - BlockIndex = (uint)@struct[0].GetInteger(); - _rawTransaction = ((ByteString)@struct[1]).Memory; + Trimmed = @struct[0].GetBoolean(); + if (Trimmed) return; + BlockIndex = (uint)@struct[1].GetInteger(); + _rawTransaction = ((ByteString)@struct[2]).Memory; Transaction = _rawTransaction.AsSerializable(); - State = (VMState)(byte)@struct[2].GetInteger(); + State = (VMState)(byte)@struct[3].GetInteger(); } StackItem IInteroperable.ToStackItem(ReferenceCounter referenceCounter) { + if (Trimmed) return new Struct(referenceCounter) { Trimmed }; if (_rawTransaction.IsEmpty) _rawTransaction = Transaction.ToArray(); - return new Struct(referenceCounter) { BlockIndex, _rawTransaction, (byte)State }; + return new Struct(referenceCounter) { Trimmed, BlockIndex, _rawTransaction, (byte)State }; } } } diff --git a/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs b/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs index 98f7b009fd1..80e50826d86 100644 --- a/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs +++ b/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs @@ -2,6 +2,7 @@ using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using Neo.Cryptography; using Neo.IO; using Neo.Ledger; using Neo.Network.P2P.Payloads; @@ -68,7 +69,7 @@ private Transaction CreateTransactionWithFee(long fee) var randomBytes = new byte[16]; random.NextBytes(randomBytes); Mock mock = new(); - mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny())).Returns(VerifyResult.Succeed); + mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())).Returns(VerifyResult.Succeed); mock.Setup(p => p.VerifyStateIndependent(It.IsAny())).Returns(VerifyResult.Succeed); mock.Object.Script = randomBytes; mock.Object.NetworkFee = fee; @@ -92,7 +93,7 @@ private Transaction CreateTransactionWithFeeAndBalanceVerify(long fee) random.NextBytes(randomBytes); Mock mock = new(); UInt160 sender = senderAccount; - mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny())).Returns((ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context) => context.CheckTransaction(mock.Object, snapshot) ? VerifyResult.Succeed : VerifyResult.InsufficientFunds); + mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())).Returns((ProtocolSettings settings, DataCache snapshot, TransactionVerificationContext context, IEnumerable conflictsList) => context.CheckTransaction(mock.Object, conflictsList, snapshot) ? VerifyResult.Succeed : VerifyResult.InsufficientFunds); mock.Setup(p => p.VerifyStateIndependent(It.IsAny())).Returns(VerifyResult.Succeed); mock.Object.Script = randomBytes; mock.Object.NetworkFee = fee; @@ -241,6 +242,168 @@ public async Task BlockPersistAndReverificationWillAbandonTxAsBalanceTransfered( _ = NativeContract.GAS.Mint(applicationEngine, sender, balance, true); } + [TestMethod] + public async Task UpdatePoolForBlockPersisted_RemoveBlockConflicts() + { + // Arrange: prepare mempooled and in-bock txs conflicting with each other. + 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()); + await NativeContract.GAS.Burn(engine, UInt160.Zero, balance); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, 6, true); // balance enough for 6 mempooled txs + + var mp1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp1 doesn't conflict with anyone + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.Succeed); + var tx1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // but in-block tx1 conflicts with mempooled mp1 => mp1 should be removed from pool after persist + tx1.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp1.Hash} }; + + var mp2 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp1 and mp2 don't conflict with anyone + _unit.TryAdd(mp2, engine.Snapshot); + var mp3 = CreateTransactionWithFeeAndBalanceVerify(txFee); + _unit.TryAdd(mp3, engine.Snapshot); + var tx2 = CreateTransactionWithFeeAndBalanceVerify(txFee); // in-block tx2 conflicts with mempooled mp2 and mp3 => mp2 and mp3 should be removed from pool after persist + tx2.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp2.Hash}, new Conflicts(){Hash = mp3.Hash} }; + + var tx3 = CreateTransactionWithFeeAndBalanceVerify(txFee); // in-block tx3 doesn't conflict with anyone + var mp4 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp4 conflicts with in-block tx3 => mp4 should be removed from pool after persist + mp4.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = tx3.Hash} }; + _unit.TryAdd(mp4, engine.Snapshot); + + var tx4 = CreateTransactionWithFeeAndBalanceVerify(txFee); // in-block tx4 and tx5 don't conflict with anyone + var tx5 = CreateTransactionWithFeeAndBalanceVerify(txFee); + var mp5 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp5 conflicts with in-block tx4 and tx5 => mp5 should be removed from pool after persist + mp5.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = tx4.Hash}, new Conflicts(){Hash = tx5.Hash} }; + _unit.TryAdd(mp5, engine.Snapshot); + + var mp6 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp6 doesn't conflict with anyone and noone conflicts with mp6 => mp6 should be left in the pool after persist + _unit.TryAdd(mp6, engine.Snapshot); + + _unit.SortedTxCount.Should().Be(6); + _unit.UnverifiedSortedTxCount.Should().Be(0); + + // Act: persist block and reverify all mempooled txs. + var block = new Block + { + Header = new Header(), + Transactions = new Transaction [] {tx1, tx2, tx3, tx4, tx5}, + }; + _unit.UpdatePoolForBlockPersisted(block, engine.Snapshot); + + // Assert: conflicting txs should be removed from the pool; the only mp6 that doesn't conflict with anyone should be left. + _unit.SortedTxCount.Should().Be(1); + _unit.GetSortedVerifiedTransactions().Select(tx => tx.Hash).Should().Contain(mp6.Hash); + _unit.UnverifiedSortedTxCount.Should().Be(0); + + // Cleanup: revert the balance. + await NativeContract.GAS.Burn(engine, UInt160.Zero, txFee * 6); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, balance, true); + } + + [TestMethod] + public async Task TryAdd_AddRangeOfConflictingTransactions() + { + // 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()); + await NativeContract.GAS.Burn(engine, UInt160.Zero, balance); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, 100, true); // balance enough for all mempooled txs + + var mp1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp1 doesn't conflict with anyone and not in the pool yet + + var mp2 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp2 conflicts with mp1 and has the same network fee + mp2.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp1.Hash} }; + _unit.TryAdd(mp2, engine.Snapshot); + + var mp3 = CreateTransactionWithFeeAndBalanceVerify(2 * txFee); // mp3 conflicts with mp1 and has larger network fee + mp3.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp1.Hash} }; + _unit.TryAdd(mp3, engine.Snapshot); + + var mp4 = CreateTransactionWithFeeAndBalanceVerify(3 * txFee); // mp4 conflicts with mp3 and has larger network fee + mp4.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp3.Hash} }; + + var malicious = CreateTransactionWithFeeAndBalanceVerify(3 * txFee); // malicious conflicts with mp3 and has larger network fee, but different sender + malicious.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp3.Hash} }; + malicious.Signers = new Signer[] { new Signer() { Account = new UInt160(Crypto.Hash160(new byte[]{1, 2, 3})), Scopes = WitnessScope.None } }; + + var mp5 = CreateTransactionWithFeeAndBalanceVerify(2 * txFee); // mp5 conflicts with mp4 and has smaller network fee + mp5.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp4.Hash} }; + + _unit.SortedTxCount.Should().Be(2); + _unit.UnverifiedSortedTxCount.Should().Be(0); + + // Act & Assert: try to add conlflicting transactions to the pool. + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp1 conflicts with mp2 and mp3 but has lower network fee than mp3 => mp1 fails to be added + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp2, mp3}); + + _unit.TryAdd(malicious, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // malicious conflicts with mp3, has larger network fee but malicious (different) sender => mp3 shoould be left in pool + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp2, mp3}); + + _unit.TryAdd(mp4, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp4 conflicts with mp3 and has larger network fee => mp3 shoould be removed from pool + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp2, mp4}); + + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp1 conflicts with mp2 and has same network fee => mp2 shoould be removed from pool + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp1, mp4}); + + _unit.TryAdd(mp5, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp1 conflicts with mp2 and has same network fee => mp2 shoould be removed from pool + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp1, mp4}); + + // Cleanup: revert the balance. + await NativeContract.GAS.Burn(engine, UInt160.Zero, 100); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, balance, true); + } + + [TestMethod] + public async Task TryRemoveVerified_RemoveVerifiedTxWithConflicts() + { + // 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()); + await NativeContract.GAS.Burn(engine, UInt160.Zero, balance); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, 100, true); // balance enough for all mempooled txs + + var mp1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp1 doesn't conflict with anyone and not in the pool yet + + var mp2 = CreateTransactionWithFeeAndBalanceVerify(2 * txFee); // mp2 conflicts with mp1 and has larger same network fee + mp2.Attributes = new TransactionAttribute[] { new Conflicts(){Hash = mp1.Hash} }; + _unit.TryAdd(mp2, engine.Snapshot); + + _unit.SortedTxCount.Should().Be(1); + _unit.UnverifiedSortedTxCount.Should().Be(0); + + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp1 conflicts with mp2 but has lower network fee + _unit.SortedTxCount.Should().Be(1); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp2}); + + // Act & Assert: try to invalidate verified transactions and push conflicting one. + _unit.InvalidateVerifiedTransactions(); + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp1 conflicts with mp2 but mp2 is not verified anymore + _unit.SortedTxCount.Should().Be(1); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp1}); + + var tx1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // in-block tx1 doesn't conflict with anyone and is aimed to trigger reverification + var block = new Block + { + Header = new Header(), + Transactions = new Transaction [] {tx1}, + }; + _unit.UpdatePoolForBlockPersisted(block, engine.Snapshot); + _unit.SortedTxCount.Should().Be(1); + _unit.GetVerifiedTransactions().Should().Contain(new List(){mp2}); // after reverificaion mp2 should be back at verified list; mp1 should be completely kicked off + } + private static void VerifyTransactionsSortedDescending(IEnumerable transactions) { Transaction lastTransaction = null; diff --git a/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs b/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs index db023d3eb89..1e3f765dd7d 100644 --- a/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs +++ b/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs @@ -14,6 +14,8 @@ public class UT_TransactionState { TransactionState origin; + TransactionState originTrimmed; + [TestInitialize] public void Initialize() { @@ -31,6 +33,10 @@ public void Initialize() } } } }; + originTrimmed = new TransactionState() + { + Trimmed = true, + }; } [TestMethod] @@ -44,6 +50,21 @@ public void TestDeserialize() dest.BlockIndex.Should().Be(origin.BlockIndex); dest.Transaction.Hash.Should().Be(origin.Transaction.Hash); + dest.Trimmed.Should().Be(false); + } + + [TestMethod] + public void TestDeserializeTrimmed() + { + var data = BinarySerializer.Serialize(((IInteroperable)originTrimmed).ToStackItem(null), 1024); + var reader = new MemoryReader(data); + + TransactionState dest = new TransactionState(); + ((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null)); + + dest.BlockIndex.Should().Be(0); + dest.Transaction.Should().Be(null); + dest.Trimmed.Should().Be(true); } } } diff --git a/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs b/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs index 27d584dca9a..b659de7d8c8 100644 --- a/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs +++ b/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs @@ -7,6 +7,7 @@ using Neo.SmartContract; using Neo.SmartContract.Native; using System; +using System.Collections.Generic; using System.Numerics; using System.Threading.Tasks; @@ -27,7 +28,7 @@ private Transaction CreateTransactionWithFee(long networkFee, long systemFee) var randomBytes = new byte[16]; random.NextBytes(randomBytes); Mock mock = new(); - mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny())).Returns(VerifyResult.Succeed); + mock.Setup(p => p.VerifyStateDependent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())).Returns(VerifyResult.Succeed); mock.Setup(p => p.VerifyStateIndependent(It.IsAny())).Returns(VerifyResult.Succeed); mock.Object.Script = randomBytes; mock.Object.NetworkFee = networkFee; @@ -60,12 +61,13 @@ public async Task TestDuplicateOracle() TransactionVerificationContext verificationContext = new(); var tx = CreateTransactionWithFee(1, 2); tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = Array.Empty() } }; - verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue(); + var conflicts = new List(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); verificationContext.AddTransaction(tx); tx = CreateTransactionWithFee(2, 1); tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = Array.Empty() } }; - verificationContext.CheckTransaction(tx, snapshot).Should().BeFalse(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeFalse(); } [TestMethod] @@ -79,15 +81,40 @@ public async Task TestTransactionSenderFee() TransactionVerificationContext verificationContext = new(); var tx = CreateTransactionWithFee(1, 2); - verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue(); + var conflicts = new List(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); verificationContext.AddTransaction(tx); - verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); verificationContext.AddTransaction(tx); - verificationContext.CheckTransaction(tx, snapshot).Should().BeFalse(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeFalse(); verificationContext.RemoveTransaction(tx); - verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); verificationContext.AddTransaction(tx); - verificationContext.CheckTransaction(tx, snapshot).Should().BeFalse(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeFalse(); + } + + [TestMethod] + public async Task TestTransactionSenderFeeWithConflicts() + { + var snapshot = TestBlockchain.GetTestSnapshot(); + ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, settings: TestBlockchain.TheNeoSystem.Settings, gas: long.MaxValue); + BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, UInt160.Zero); + await NativeContract.GAS.Burn(engine, UInt160.Zero, balance); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, 3+3+1, true); // balance is enough for 2 transactions and 1 GAS is left. + + TransactionVerificationContext verificationContext = new(); + var tx = CreateTransactionWithFee(1, 2); + var conflictingTx = CreateTransactionWithFee(1, 1); // costs 2 GAS + + var conflicts = new List(); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); + verificationContext.AddTransaction(tx); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); + verificationContext.AddTransaction(tx); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeFalse(); + + conflicts.Add(conflictingTx); + verificationContext.CheckTransaction(tx, conflicts, snapshot).Should().BeTrue(); // 1 GAS is left on the balance + 2 GAS is free after conflicts removal => enough for one more trasnaction. } } } diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Conflicts.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Conflicts.cs new file mode 100644 index 00000000000..1a17b79a468 --- /dev/null +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Conflicts.cs @@ -0,0 +1,74 @@ +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Neo.IO; +using Neo.Network.P2P.Payloads; +using Neo.SmartContract.Native; +using Neo.SmartContract; +using System; + +namespace Neo.UnitTests.Network.P2P.Payloads +{ + [TestClass] + public class UT_Conflicts + { + private const byte Prefix_Transaction = 11; + private static UInt256 _u = new UInt256(new byte[32] { + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01 + }); + + [TestMethod] + public void Size_Get() + { + var test = new Conflicts(){Hash = _u}; + test.Size.Should().Be(1 + 32); + } + + [TestMethod] + public void ToJson() + { + var test = new Conflicts(){Hash = _u}; + var json = test.ToJson().ToString(); + Assert.AreEqual(@"{""type"":""Conflicts"",""hash"":""0x0101010101010101010101010101010101010101010101010101010101010101""}", json); + } + + [TestMethod] + public void DeserializeAndSerialize() + { + var test = new Conflicts(){Hash = _u}; + + var clone = test.ToArray().AsSerializable(); + Assert.AreEqual(clone.Type, test.Type); + + // As transactionAttribute + byte[] buffer = test.ToArray(); + var reader = new MemoryReader(buffer); + clone = TransactionAttribute.DeserializeFrom(ref reader) as Conflicts; + Assert.AreEqual(clone.Type, test.Type); + + // Wrong type + buffer[0] = 0xff; + Assert.ThrowsException(() => + { + var reader = new MemoryReader(buffer); + TransactionAttribute.DeserializeFrom(ref reader); + }); + } + + [TestMethod] + public void Verify() + { + var test = new Conflicts(){ Hash = _u }; + var snapshot = TestBlockchain.GetTestSnapshot(); + var key = Ledger.UT_MemoryPool.CreateStorageKey(NativeContract.Ledger.Id, Prefix_Transaction, _u.ToArray()); + + snapshot.Add(key, new StorageItem()); + Assert.IsFalse(test.Verify(snapshot, new Transaction())); + + snapshot.Delete(key); + Assert.IsTrue(test.Verify(snapshot, new Transaction())); + } + } +} diff --git a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs index 98c49f31c4d..1df1340012b 100644 --- a/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Transaction.cs @@ -10,6 +10,7 @@ using Neo.VM; using Neo.Wallets; using System; +using System.Collections.Generic; using System.Linq; using System.Numerics; @@ -773,7 +774,7 @@ public void Transaction_Reverify_Hashes_Length_Unequal_To_Witnesses_Length() }; UInt160[] hashes = txSimple.GetScriptHashesForVerifying(snapshot); Assert.AreEqual(1, hashes.Length); - Assert.AreNotEqual(VerifyResult.Succeed, txSimple.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext())); + Assert.AreNotEqual(VerifyResult.Succeed, txSimple.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext(), new List())); } [TestMethod] @@ -1204,11 +1205,12 @@ public void Test_VerifyStateDependent() var key = NativeContract.GAS.CreateStorageKey(20, tx.Sender); var balance = snapshot.GetAndChange(key, () => new StorageItem(new AccountState())); balance.GetInteroperable().Balance = tx.NetworkFee; + var conflicts = new List(); - tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext()).Should().Be(VerifyResult.Invalid); + tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext(), conflicts).Should().Be(VerifyResult.Invalid); balance.GetInteroperable().Balance = 0; tx.SystemFee = 10; - tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext()).Should().Be(VerifyResult.InsufficientFunds); + tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext(), conflicts).Should().Be(VerifyResult.InsufficientFunds); var walletA = TestUtils.GenerateTestWallet("123"); var walletB = TestUtils.GenerateTestWallet("123"); @@ -1254,7 +1256,7 @@ public void Test_VerifyStateDependent() Assert.IsTrue(data.Completed); tx.Witnesses = data.GetWitnesses(); - tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext()).Should().Be(VerifyResult.Succeed); + tx.VerifyStateDependent(ProtocolSettings.Default, snapshot, new TransactionVerificationContext(), new List()).Should().Be(VerifyResult.Succeed); } [TestMethod]