diff --git a/src/Neo/Ledger/Blockchain.cs b/src/Neo/Ledger/Blockchain.cs index 52b3c37ce5..8acec8c974 100644 --- a/src/Neo/Ledger/Blockchain.cs +++ b/src/Neo/Ledger/Blockchain.cs @@ -206,6 +206,8 @@ private void OnFillMemoryPool(IEnumerable transactions) { if (NativeContract.Ledger.ContainsTransaction(snapshot, tx.Hash)) continue; + if (NativeContract.Ledger.ContainsConflictHash(snapshot, tx.Hash, tx.Signers.Select(s => s.Account))) + continue; // First remove the tx if it is unverified in the pool. system.MemPool.TryRemoveUnVerified(tx.Hash, out _); // Add to the memory pool @@ -337,6 +339,7 @@ private VerifyResult OnNewExtensiblePayload(ExtensiblePayload payload) private VerifyResult OnNewTransaction(Transaction transaction) { if (system.ContainsTransaction(transaction.Hash)) return VerifyResult.AlreadyExists; + if (system.ContainsConflictHash(transaction.Hash, transaction.Signers.Select(s => s.Account))) return VerifyResult.HasConflicts; return system.MemPool.TryAdd(transaction, system.StoreView); } @@ -391,8 +394,9 @@ private void OnTransaction(Transaction tx) { if (system.ContainsTransaction(tx.Hash)) SendRelayResult(tx, VerifyResult.AlreadyExists); - else - system.TxRouter.Forward(new TransactionRouter.Preverify(tx, true)); + else if (system.ContainsConflictHash(tx.Hash, tx.Signers.Select(s => s.Account))) + SendRelayResult(tx, VerifyResult.HasConflicts); + else system.TxRouter.Forward(new TransactionRouter.Preverify(tx, true)); } private void Persist(Block block) diff --git a/src/Neo/Ledger/MemoryPool.cs b/src/Neo/Ledger/MemoryPool.cs index 6ade372ee1..a966835fc3 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 HashSet(); + } + 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,49 @@ internal VerifyResult TryAdd(Transaction tx, DataCache snapshot) return VerifyResult.Succeed; } + /// + /// Checks whether there is no mismatch in Conflicts attributes between the current transaction + /// and mempooled unsorted transactions. If true, then these unsorted transactions will be added + /// into conflictsList. + /// + /// The current transaction 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(); + long conflictsFeeSum = 0; + // Step 1: check if `tx` was in Conflicts attributes of unsorted transactions. + if (_conflicts.TryGetValue(tx.Hash, out var conflicting)) + { + foreach (var hash in conflicting) + { + var unsortedTx = _unsortedTransactions[hash]; + if (unsortedTx.Tx.Signers.Select(s => s.Account).Contains(tx.Sender)) + conflictsFeeSum += unsortedTx.Tx.NetworkFee; + conflictsList.Add(unsortedTx); + } + } + // Step 2: check if unsorted transactions were in `tx`'s Conflicts attributes. + foreach (var hash in tx.GetAttributes().Select(p => p.Hash)) + { + if (_unsortedTransactions.TryGetValue(hash, out PoolItem unsortedTx)) + { + if (!tx.Signers.Select(p => p.Account).Intersect(unsortedTx.Tx.Signers.Select(p => p.Account)).Any()) return false; + conflictsFeeSum += unsortedTx.Tx.NetworkFee; + conflictsList.Add(unsortedTx); + } + } + // Network fee of tx have to be larger than the sum of conflicting txs network fees. + if (conflictsFeeSum != 0 && conflictsFeeSum >= tx.NetworkFee) + return false; + + // 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 +396,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 +414,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 HashSet 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 +459,67 @@ 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 { + Dictionary> conflicts = new Dictionary>(); // 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 _); + var conflictingSigners = tx.Signers.Select(s => s.Account); + foreach (var h in tx.GetAttributes().Select(a => a.Hash)) + { + if (conflicts.TryGetValue(h, out var signersList)) + { + signersList.AddRange(conflictingSigners); + continue; + } + signersList = conflictingSigners.ToList(); + conflicts.Add(h, signersList); + } } - // Add all the previously verified transactions back to the unverified transactions + // 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.TryGetValue(item.Tx.Hash, out var signersList) && signersList.Intersect(item.Tx.Signers.Select(s => s.Account)).Any()) || item.Tx.GetAttributes().Select(a => a.Hash).Intersect(persisted).Any()) + { + 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 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,10 +555,31 @@ 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); + 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 HashSet(); + } + 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); @@ -450,18 +595,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 9767fb919f..fa66b60981 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 b77adfe9c7..5b6cc6cc9c 100644 --- a/src/Neo/Ledger/TransactionVerificationContext.cs +++ b/src/Neo/Ledger/TransactionVerificationContext.cs @@ -12,6 +12,7 @@ using Neo.Persistence; using Neo.SmartContract.Native; using System.Collections.Generic; +using System.Linq; using System.Numerics; namespace Neo.Ledger @@ -50,15 +51,18 @@ 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; - if (balance < fee) return false; + BigInteger expectedFee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool; + foreach (var conflictTx in conflictingTxs.Where(c => c.Sender.Equals(tx.Sender))) + expectedFee -= (conflictTx.NetworkFee + conflictTx.SystemFee); + if (balance < expectedFee) return false; var oracle = tx.GetAttribute(); if (oracle != null && oracleResponses.ContainsKey(oracle.Id)) diff --git a/src/Neo/Ledger/VerifyResult.cs b/src/Neo/Ledger/VerifyResult.cs index 7858903ad2..94c9e78bf8 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/NeoSystem.cs b/src/Neo/NeoSystem.cs index c87b9e2630..2f66389672 100644 --- a/src/Neo/NeoSystem.cs +++ b/src/Neo/NeoSystem.cs @@ -278,5 +278,16 @@ public bool ContainsTransaction(UInt256 hash) if (MemPool.ContainsKey(hash)) return true; return NativeContract.Ledger.ContainsTransaction(StoreView, hash); } + + /// + /// Determines whether the specified transaction conflicts with some on-chain transaction. + /// + /// The hash of the transaction + /// The list of signer accounts of the transaction + /// if the transaction conflicts with on-chain transaction; otherwise, . + public bool ContainsConflictHash(UInt256 hash, IEnumerable signers) + { + return NativeContract.Ledger.ContainsConflictHash(StoreView, hash, signers); + } } } diff --git a/src/Neo/Network/P2P/Payloads/Conflicts.cs b/src/Neo/Network/P2P/Payloads/Conflicts.cs new file mode 100644 index 0000000000..db00b03259 --- /dev/null +++ b/src/Neo/Network/P2P/Payloads/Conflicts.cs @@ -0,0 +1,47 @@ +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) + { + // Only check if conflicting transaction is on chain. It's OK if the + // conflicting transaction was in the Conflicts attribute of some other + // on-chain transaction. + 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 b99449ac48..24581e2b18 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 in case of sender's match. /// 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 in case of sender's match. /// 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 41c69b507d..66dccb340b 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/Network/P2P/RemoteNode.ProtocolHandler.cs b/src/Neo/Network/P2P/RemoteNode.ProtocolHandler.cs index dfd21c63b8..ffbc53c1cb 100644 --- a/src/Neo/Network/P2P/RemoteNode.ProtocolHandler.cs +++ b/src/Neo/Network/P2P/RemoteNode.ProtocolHandler.cs @@ -318,7 +318,7 @@ private void OnInventoryReceived(IInventory inventory) switch (inventory) { case Transaction transaction: - if (!system.ContainsTransaction(transaction.Hash)) + if (!(system.ContainsTransaction(transaction.Hash) || system.ContainsConflictHash(transaction.Hash, transaction.Signers.Select(s => s.Account)))) system.TxRouter.Tell(new TransactionRouter.Preverify(transaction, true)); break; case Block block: diff --git a/src/Neo/SmartContract/Native/LedgerContract.cs b/src/Neo/SmartContract/Native/LedgerContract.cs index eec49da99a..cd405c098b 100644 --- a/src/Neo/SmartContract/Native/LedgerContract.cs +++ b/src/Neo/SmartContract/Native/LedgerContract.cs @@ -15,6 +15,7 @@ using Neo.Persistence; using Neo.VM; using System; +using System.Collections.Generic; using System.Linq; using System.Numerics; @@ -47,6 +48,13 @@ internal override ContractTask OnPersist(ApplicationEngine engine) foreach (TransactionState tx in transactions) { engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash), new StorageItem(tx)); + var conflictingSigners = tx.Transaction.Signers.Select(s => s.Account); + foreach (var attr in tx.Transaction.GetAttributes()) + { + var conflictRecord = engine.Snapshot.GetAndChange(CreateStorageKey(Prefix_Transaction).Add(attr.Hash), + () => new StorageItem(new TransactionState { ConflictingSigners = Array.Empty() })).GetInteroperable(); + conflictRecord.ConflictingSigners = conflictRecord.ConflictingSigners.Concat(conflictingSigners).Distinct().ToArray(); + } } engine.SetState(transactions); return ContractTask.CompletedTask; @@ -126,7 +134,22 @@ public bool ContainsBlock(DataCache snapshot, UInt256 hash) /// if the blockchain contains the transaction; otherwise, . public bool ContainsTransaction(DataCache snapshot, UInt256 hash) { - return snapshot.Contains(CreateStorageKey(Prefix_Transaction).Add(hash)); + var txState = GetTransactionState(snapshot, hash); + return txState != null; + } + + /// + /// Determine whether the specified transaction hash is contained in the blockchain + /// as the hash of conflicting transaction. + /// + /// The snapshot used to read data. + /// The hash of the conflicting transaction. + /// The list of signer accounts of the conflicting transaction. + /// if the blockchain contains the hash of the conflicting transaction; otherwise, . + public bool ContainsConflictHash(DataCache snapshot, UInt256 hash, IEnumerable signers) + { + var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable(); + return state is not null && state.Transaction is null && (signers is null || state.ConflictingSigners.Intersect(signers).Any()); } /// @@ -220,7 +243,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?.Transaction is null) return null; + return state; } /// diff --git a/src/Neo/SmartContract/Native/TransactionState.cs b/src/Neo/SmartContract/Native/TransactionState.cs index 5c7380f1b8..d3c7f644be 100644 --- a/src/Neo/SmartContract/Native/TransactionState.cs +++ b/src/Neo/SmartContract/Native/TransactionState.cs @@ -13,6 +13,7 @@ using Neo.VM; using Neo.VM.Types; using System; +using System.Linq; namespace Neo.SmartContract.Native { @@ -27,10 +28,12 @@ public class TransactionState : IInteroperable public uint BlockIndex; /// - /// The transaction. + /// The transaction, if the transaction is trimmed this value will be null /// public Transaction Transaction; + public UInt160[] ConflictingSigners; + /// /// The execution state /// @@ -44,6 +47,7 @@ IInteroperable IInteroperable.Clone() { BlockIndex = BlockIndex, Transaction = Transaction, + ConflictingSigners = ConflictingSigners, State = State, _rawTransaction = _rawTransaction }; @@ -54,6 +58,7 @@ 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; @@ -62,6 +67,11 @@ 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(); _rawTransaction = ((ByteString)@struct[1]).Memory; Transaction = _rawTransaction.AsSerializable(); @@ -70,6 +80,7 @@ void IInteroperable.FromStackItem(StackItem stackItem) 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 (_rawTransaction.IsEmpty) _rawTransaction = Transaction.ToArray(); return new Struct(referenceCounter) { BlockIndex, _rawTransaction, (byte)State }; diff --git a/tests/Neo.UnitTests/Ledger/UT_Blockchain.cs b/tests/Neo.UnitTests/Ledger/UT_Blockchain.cs index 130df54e60..1cd8c42360 100644 --- a/tests/Neo.UnitTests/Ledger/UT_Blockchain.cs +++ b/tests/Neo.UnitTests/Ledger/UT_Blockchain.cs @@ -6,6 +6,7 @@ using Neo.Persistence; using Neo.SmartContract; using Neo.SmartContract.Native; +using Neo.VM; using Neo.Wallets; using Neo.Wallets.NEP6; using System; @@ -97,5 +98,75 @@ private static Transaction CreateValidTx(DataCache snapshot, NEP6Wallet wallet, tx.Witnesses = data.GetWitnesses(); return tx; } + + [TestMethod] + public void TestMaliciousOnChainConflict() + { + var snapshot = TestBlockchain.TheNeoSystem.GetSnapshot(); + var walletA = TestUtils.GenerateTestWallet("123"); + var accA = walletA.CreateAccount(); + var walletB = TestUtils.GenerateTestWallet("456"); + var accB = walletB.CreateAccount(); + ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, settings: TestBlockchain.TheNeoSystem.Settings, gas: long.MaxValue); + engine.LoadScript(Array.Empty()); + + // Fake balance for accounts A and B. + var key = new KeyBuilder(NativeContract.GAS.Id, 20).Add(accA.ScriptHash); + var entry = snapshot.GetAndChange(key, () => new StorageItem(new AccountState())); + entry.GetInteroperable().Balance = 100_000_000 * NativeContract.GAS.Factor; + snapshot.Commit(); + + key = new KeyBuilder(NativeContract.GAS.Id, 20).Add(accB.ScriptHash); + entry = snapshot.GetAndChange(key, () => new StorageItem(new AccountState())); + entry.GetInteroperable().Balance = 100_000_000 * NativeContract.GAS.Factor; + snapshot.Commit(); + + // Create transactions: + // tx1 conflicts with tx2 and has the same sender (thus, it's a valid conflict and must prevent tx2 from entering the chain); + // tx2 conflicts with tx3 and has different sender (thus, this conflict is invalid and must not prevent tx3 from entering the chain). + var tx1 = CreateValidTx(snapshot, walletA, accA.ScriptHash, 0); + var tx2 = CreateValidTx(snapshot, walletA, accA.ScriptHash, 1); + var tx3 = CreateValidTx(snapshot, walletB, accB.ScriptHash, 2); + + tx1.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = tx2.Hash }, new Conflicts() { Hash = tx3.Hash } }; + + // Persist tx1. + var block = new Block + { + Header = new Header() + { + Index = 10000, + MerkleRoot = UInt256.Zero, + NextConsensus = UInt160.Zero, + PrevHash = UInt256.Zero, + Witness = new Witness() { InvocationScript = Array.Empty(), VerificationScript = Array.Empty() } + }, + Transactions = new Transaction[] { tx1 }, + }; + 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, 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(); + engine2.Snapshot.Commit(); + } + snapshot.Commit(); + + // Add tx2: must fail because valid conflict is alredy on chain (tx1). + senderProbe.Send(TestBlockchain.TheNeoSystem.Blockchain, tx2); + senderProbe.ExpectMsg(p => p.Result == VerifyResult.HasConflicts); + + // Add tx3: must succeed because on-chain conflict is invalid (doesn't have proper signer). + senderProbe.Send(TestBlockchain.TheNeoSystem.Blockchain, tx3); + senderProbe.ExpectMsg(p => p.Result == VerifyResult.Succeed); + } } } diff --git a/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs b/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs index 98f7b009fd..de5bff6ba0 100644 --- a/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs +++ b/tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs @@ -1,19 +1,21 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Numerics; +using System.Threading.Tasks; using Akka.TestKit.Xunit2; using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using Neo.Cryptography; using Neo.IO; using Neo.Ledger; using Neo.Network.P2P.Payloads; using Neo.Persistence; using Neo.SmartContract; using Neo.SmartContract.Native; -using System; -using System.Collections; -using System.Collections.Generic; -using System.Linq; -using System.Numerics; -using System.Threading.Tasks; +using Neo.VM; namespace Neo.UnitTests.Ledger { @@ -68,7 +70,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 +94,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 +243,215 @@ 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, 7, true); // balance enough for 7 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); + + var mp7 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp7 doesn't conflict with anyone + var tx6 = CreateTransactionWithFeeAndBalanceVerify(txFee); // in-block tx6 conflicts with mp7, but doesn't include sender of mp7 into signers list => even if tx6 is included into block, mp7 shouldn't be removed from the pool + tx6.Signers = new Signer[] { new Signer() { Account = new UInt160(Crypto.Hash160(new byte[] { 1, 2, 3 })) }, new Signer() { Account = new UInt160(Crypto.Hash160(new byte[] { 4, 5, 6 })) } }; + tx6.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp7.Hash } }; + _unit.TryAdd(mp7, engine.Snapshot); + + // Act: persist block and reverify all mempooled txs. + var block = new Block + { + Header = new Header(), + Transactions = new Transaction[] { tx1, tx2, tx3, tx4, tx5, tx6 }, + }; + _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(2); + _unit.GetSortedVerifiedTransactions().Select(tx => tx.Hash).Should().Contain(mp6.Hash); + _unit.GetSortedVerifiedTransactions().Select(tx => tx.Hash).Should().Contain(mp7.Hash); + _unit.UnverifiedSortedTxCount.Should().Be(0); + + // Cleanup: revert the balance. + await NativeContract.GAS.Burn(engine, UInt160.Zero, txFee * 7); + _ = 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 maliciousSender = new UInt160(Crypto.Hash160(new byte[] { 1, 2, 3 })); + 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 + _ = NativeContract.GAS.Mint(engine, maliciousSender, 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_1 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp2_1 conflicts with mp1 and has the same network fee + mp2_1.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp1.Hash } }; + _unit.TryAdd(mp2_1, engine.Snapshot); + var mp2_2 = CreateTransactionWithFeeAndBalanceVerify(txFee); // mp2_2 also conflicts with mp1 and has the same network fee as mp1 and mp2_1 + mp2_2.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp1.Hash } }; + _unit.TryAdd(mp2_2, 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 } }; + + var mp6 = CreateTransactionWithFeeAndBalanceVerify(mp2_1.NetworkFee + mp2_2.NetworkFee + 1); // mp6 conflicts with mp2_1 and mp2_2 and has larger network fee. + mp6.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp2_1.Hash }, new Conflicts() { Hash = mp2_2.Hash } }; + + var mp7 = CreateTransactionWithFeeAndBalanceVerify(txFee * 2 + 1); // mp7 doesn't conflicts with anyone, but mp8, mp9 and mp10malicious has smaller sum network fee and conflict with mp7. + var mp8 = CreateTransactionWithFeeAndBalanceVerify(txFee); + mp8.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp7.Hash } }; + var mp9 = CreateTransactionWithFeeAndBalanceVerify(txFee); + mp9.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp7.Hash } }; + var mp10malicious = CreateTransactionWithFeeAndBalanceVerify(txFee); + mp10malicious.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = mp7.Hash } }; + mp10malicious.Signers = new Signer[] { new Signer() { Account = maliciousSender, Scopes = WitnessScope.None } }; + + _unit.SortedTxCount.Should().Be(3); + _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_1, mp2_2 and mp3 but has lower network fee than mp3 => mp1 fails to be added + _unit.SortedTxCount.Should().Be(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp2_1, mp2_2, 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(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp2_1, mp2_2, 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(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp2_1, mp2_2, mp4 }); + + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp1 conflicts with mp2_1 and mp2_2 and has same network fee => mp2_1 and mp2_2 should be left in pool. + _unit.SortedTxCount.Should().Be(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp2_1, mp2_2, mp4 }); + + _unit.TryAdd(mp6, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp6 conflicts with mp2_1 and mp2_2 and has larger network fee than the sum of mp2_1 and mp2_2 fees => mp6 should be added. + _unit.SortedTxCount.Should().Be(2); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp6, mp4 }); + + _unit.TryAdd(mp1, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp1 conflicts with mp2_1 and mp2_2, but they are not in the pool now => mp1 should be added. + _unit.SortedTxCount.Should().Be(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp1, mp6, mp4 }); + + _unit.TryAdd(mp2_1, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp2_1 conflicts with mp1 and has same network fee => mp2_1 shouldn't be added to the pool. + _unit.SortedTxCount.Should().Be(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp1, mp6, mp4 }); + + _unit.TryAdd(mp5, engine.Snapshot).Should().Be(VerifyResult.HasConflicts); // mp5 conflicts with mp4 and has smaller network fee => mp5 fails to be added. + _unit.SortedTxCount.Should().Be(3); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp1, mp6, mp4 }); + + _unit.TryAdd(mp8, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp8, mp9 and mp10malicious conflict with mp7, but mo7 is not in the pool yet. + _unit.TryAdd(mp9, engine.Snapshot).Should().Be(VerifyResult.Succeed); + _unit.TryAdd(mp10malicious, engine.Snapshot).Should().Be(VerifyResult.Succeed); + _unit.SortedTxCount.Should().Be(6); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp1, mp6, mp4, mp8, mp9, mp10malicious }); + _unit.TryAdd(mp7, engine.Snapshot).Should().Be(VerifyResult.Succeed); // mp7 has larger network fee than the sum of mp8 and mp9 fees => should be added to the pool. + _unit.SortedTxCount.Should().Be(4); + _unit.GetVerifiedTransactions().Should().Contain(new List() { mp1, mp6, mp4, mp7 }); + + // Cleanup: revert the balance. + await NativeContract.GAS.Burn(engine, UInt160.Zero, 100); + _ = NativeContract.GAS.Mint(engine, UInt160.Zero, balance, true); + await NativeContract.GAS.Burn(engine, maliciousSender, 100); + _ = NativeContract.GAS.Mint(engine, maliciousSender, 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; @@ -525,6 +736,74 @@ 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()); + + 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(), VerificationScript = Array.Empty() } + }, + 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(); + } + + // Test tx2 arrive + + snapshot.Commit(); + + var senderProbe = CreateTestProbe(); + senderProbe.Send(TestBlockchain.TheNeoSystem.Blockchain, tx2); + senderProbe.ExpectMsg(p => p.Result == VerifyResult.InsufficientFunds); // should be Succedded. + } + public static StorageKey CreateStorageKey(int id, byte prefix, byte[] key = null) { byte[] buffer = GC.AllocateUninitializedArray(sizeof(byte) + (key?.Length ?? 0)); diff --git a/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs b/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs index db023d3eb8..0270d97891 100644 --- a/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs +++ b/tests/Neo.UnitTests/Ledger/UT_TransactionState.cs @@ -1,5 +1,6 @@ using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Neo.Cryptography; using Neo.IO; using Neo.Network.P2P.Payloads; using Neo.SmartContract; @@ -14,6 +15,8 @@ public class UT_TransactionState { TransactionState origin; + TransactionState originTrimmed; + [TestInitialize] public void Initialize() { @@ -31,6 +34,14 @@ 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 })) + } + }; } [TestMethod] @@ -39,11 +50,27 @@ public void TestDeserialize() var data = BinarySerializer.Serialize(((IInteroperable)origin).ToStackItem(null), 1024); var reader = new MemoryReader(data); - TransactionState dest = new TransactionState(); + TransactionState dest = new(); ((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null)); dest.BlockIndex.Should().Be(origin.BlockIndex); dest.Transaction.Hash.Should().Be(origin.Transaction.Hash); + dest.Transaction.Should().NotBeNull(); + } + + [TestMethod] + public void TestDeserializeTrimmed() + { + var data = BinarySerializer.Serialize(((IInteroperable)originTrimmed).ToStackItem(null), 1024); + var reader = new MemoryReader(data); + + TransactionState dest = new(); + ((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null)); + + dest.BlockIndex.Should().Be(0); + dest.Transaction.Should().Be(null); + dest.Transaction.Should().BeNull(); + CollectionAssert.AreEqual(originTrimmed.ConflictingSigners, dest.ConflictingSigners); } } } diff --git a/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs b/tests/Neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs index 27d584dca9..522b964ff9 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 0000000000..bbf46920aa --- /dev/null +++ b/tests/Neo.UnitTests/Network/P2P/Payloads/UT_Conflicts.cs @@ -0,0 +1,89 @@ +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Neo.IO; +using Neo.Network.P2P.Payloads; +using Neo.SmartContract.Native; +using Neo.SmartContract; +using Neo.VM; +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()); + + // Conflicting transaction is in the Conflicts attribute of some other on-chain transaction. + var conflict = new TransactionState(); + snapshot.Add(key, new StorageItem(conflict)); + Assert.IsTrue(test.Verify(snapshot, new Transaction())); + + // Conflicting transaction is on-chain. + snapshot.Delete(key); + conflict = new TransactionState + { + BlockIndex = 123, + Transaction = new Transaction(), + State = VMState.NONE + }; + snapshot.Add(key, new StorageItem(conflict)); + Assert.IsFalse(test.Verify(snapshot, new Transaction())); + + // There's no conflicting transaction at all. + 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 98c49f31c4..1df1340012 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]