Remove the lock from SendersFeeMonitor and rename it to TransactionVerificationContext

src/neo/Consensus/ConsensusContext.cs

@@ -40,7 +40,7 @@ internal class ConsensusContext : IDisposable, ISerializable
         /// <summary>
         /// Store all verified unsorted transactions' senders' fee currently in the consensus context.
         /// </summary>
-        public SendersFeeMonitor SendersFeeMonitor = new SendersFeeMonitor();
+        public TransactionVerificationContext VerificationContext = new TransactionVerificationContext();
 
         public SnapshotView Snapshot { get; private set; }
         private KeyPair keyPair;
@@ -116,11 +116,11 @@ public void Deserialize(BinaryReader reader)
             if (TransactionHashes.Length == 0 && !RequestSentOrReceived)
                 TransactionHashes = null;
             Transactions = transactions.Length == 0 && !RequestSentOrReceived ? null : transactions.ToDictionary(p => p.Hash);
-            SendersFeeMonitor = new SendersFeeMonitor();
+            VerificationContext = new TransactionVerificationContext();
             if (Transactions != null)
             {
                 foreach (Transaction tx in Transactions.Values)
-                    SendersFeeMonitor.AddSenderFee(tx);
+                    VerificationContext.AddTransaction(tx);
             }
         }
 
@@ -266,7 +266,7 @@ internal void EnsureMaxBlockLimitation(IEnumerable<Transaction> txs)
             txs = txs.Take((int)maxTransactionsPerBlock);
             List<UInt256> hashes = new List<UInt256>();
             Transactions = new Dictionary<UInt256, Transaction>();
-            SendersFeeMonitor = new SendersFeeMonitor();
+            VerificationContext = new TransactionVerificationContext();
 
             // Expected block size
             var blockSize = GetExpectedBlockSizeWithoutTransactions(txs.Count());
@@ -285,7 +285,7 @@ internal void EnsureMaxBlockLimitation(IEnumerable<Transaction> txs)
 
                 hashes.Add(tx.Hash);
                 Transactions.Add(tx.Hash, tx);
-                SendersFeeMonitor.AddSenderFee(tx);
+                VerificationContext.AddTransaction(tx);
             }
 
             TransactionHashes = hashes.ToArray();

src/neo/Consensus/ConsensusService.cs

@@ -64,20 +64,24 @@ internal ConsensusService(IActorRef localNode, IActorRef taskManager, IActorRef
 
         private bool AddTransaction(Transaction tx, bool verify)
         {
-            if (verify && tx.Verify(context.Snapshot, context.SendersFeeMonitor.GetSenderFee(tx.Sender)) != VerifyResult.Succeed)
+            if (verify)
             {
-                Log($"Invalid transaction: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
-                RequestChangeView(ChangeViewReason.TxInvalid);
-                return false;
-            }
-            if (!NativeContract.Policy.CheckPolicy(tx, context.Snapshot))
-            {
-                Log($"reject tx: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
-                RequestChangeView(ChangeViewReason.TxRejectedByPolicy);
-                return false;
+                VerifyResult result = tx.Verify(context.Snapshot, context.VerificationContext);
+                if (result == VerifyResult.PolicyFail)
+                {
+                    Log($"reject tx: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
+                    RequestChangeView(ChangeViewReason.TxRejectedByPolicy);
+                    return false;
+                }
+                else if (result != VerifyResult.Succeed)
+                {
+                    Log($"Invalid transaction: {tx.Hash}{Environment.NewLine}{tx.ToArray().ToHexString()}", LogLevel.Warning);
+                    RequestChangeView(ChangeViewReason.TxInvalid);
+                    return false;
+                }
             }
             context.Transactions[tx.Hash] = tx;
-            context.SendersFeeMonitor.AddSenderFee(tx);
+            context.VerificationContext.AddTransaction(tx);
             return CheckPrepareResponse();
         }
 
@@ -433,7 +437,7 @@ private void OnPrepareRequestReceived(ConsensusPayload payload, PrepareRequest m
             context.Block.ConsensusData.Nonce = message.Nonce;
             context.TransactionHashes = message.TransactionHashes;
             context.Transactions = new Dictionary<UInt256, Transaction>();
-            context.SendersFeeMonitor = new SendersFeeMonitor();
+            context.VerificationContext = new TransactionVerificationContext();
             for (int i = 0; i < context.PreparationPayloads.Length; i++)
                 if (context.PreparationPayloads[i] != null)
                     if (!context.PreparationPayloads[i].GetDeserializedMessage<PrepareResponse>().PreparationHash.Equals(payload.Hash))

src/neo/Ledger/Blockchain.cs

@@ -283,15 +283,10 @@ private void OnFillMemoryPool(IEnumerable<Transaction> transactions)
             {
                 if (View.ContainsTransaction(tx.Hash))
                     continue;
-                if (!NativeContract.Policy.CheckPolicy(tx, currentSnapshot))
-                    continue;
                 // First remove the tx if it is unverified in the pool.
                 MemPool.TryRemoveUnVerified(tx.Hash, out _);
-                // Verify the the transaction
-                if (tx.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(tx.Sender)) != VerifyResult.Succeed)
-                    continue;
                 // Add to the memory pool
-                MemPool.TryAdd(tx.Hash, tx);
+                MemPool.TryAdd(tx, currentSnapshot);
             }
             // Transactions originally in the pool will automatically be reverified based on their priority.
 
@@ -355,11 +350,7 @@ private VerifyResult OnNewInventory(IInventory inventory)
         private VerifyResult OnNewTransaction(Transaction transaction)
         {
             if (ContainsTransaction(transaction.Hash)) return VerifyResult.AlreadyExists;
-            if (!MemPool.CanTransactionFitInPool(transaction)) return VerifyResult.OutOfMemory;
-            VerifyResult reason = transaction.Verify(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(transaction.Sender));
-            if (reason != VerifyResult.Succeed) return reason;
-            if (!MemPool.TryAdd(transaction.Hash, transaction)) return VerifyResult.OutOfMemory;
-            return VerifyResult.Succeed;
+            return MemPool.TryAdd(transaction, currentSnapshot);
         }
 
         protected override void OnReceive(object message)

src/neo/Ledger/MemoryPool.cs

@@ -71,7 +71,7 @@ public class MemoryPool : IReadOnlyCollection<Transaction>
         /// <summary>
         /// Store all verified unsorted transactions' senders' fee currently in the memory pool.
         /// </summary>
-        public SendersFeeMonitor SendersFeeMonitor = new SendersFeeMonitor();
+        private TransactionVerificationContext VerificationContext = new TransactionVerificationContext();
 
         /// <summary>
         /// Total count of transactions in the pool.
@@ -261,18 +261,21 @@ internal bool CanTransactionFitInPool(Transaction tx)
         /// <param name="hash"></param>
         /// <param name="tx"></param>
         /// <returns></returns>
-        internal bool TryAdd(UInt256 hash, Transaction tx)
+        internal VerifyResult TryAdd(Transaction tx, StoreView snapshot)
         {
             var poolItem = new PoolItem(tx);
 
-            if (_unsortedTransactions.ContainsKey(hash)) return false;
+            if (_unsortedTransactions.ContainsKey(tx.Hash)) return VerifyResult.AlreadyExists;
 
             List<Transaction> removedTransactions = null;
             _txRwLock.EnterWriteLock();
             try
             {
-                _unsortedTransactions.Add(hash, poolItem);
-                SendersFeeMonitor.AddSenderFee(tx);
+                VerifyResult result = tx.Verify(snapshot, VerificationContext);
+                if (result != VerifyResult.Succeed) return result;
+
+                _unsortedTransactions.Add(tx.Hash, poolItem);
+                VerificationContext.AddTransaction(tx);
                 _sortedTransactions.Add(poolItem);
 
                 if (Count > Capacity)
@@ -290,7 +293,8 @@ internal bool TryAdd(UInt256 hash, Transaction tx)
                     plugin.TransactionsRemoved(MemoryPoolTxRemovalReason.CapacityExceeded, removedTransactions);
             }
 
-            return _unsortedTransactions.ContainsKey(hash);
+            if (!_unsortedTransactions.ContainsKey(tx.Hash)) return VerifyResult.OutOfMemory;
+            return VerifyResult.Succeed;
         }
 
         private List<Transaction> RemoveOverCapacity()
@@ -315,7 +319,7 @@ private bool TryRemoveVerified(UInt256 hash, out PoolItem item)
                 return false;
 
             _unsortedTransactions.Remove(hash);
-            SendersFeeMonitor.RemoveSenderFee(item.Tx);
+            VerificationContext.RemoveTransaction(item.Tx);
             _sortedTransactions.Remove(item);
 
             return true;
@@ -343,7 +347,7 @@ internal void InvalidateVerifiedTransactions()
 
             // Clear the verified transactions now, since they all must be reverified.
             _unsortedTransactions.Clear();
-            SendersFeeMonitor = new SendersFeeMonitor();
+            VerificationContext = new TransactionVerificationContext();
             _sortedTransactions.Clear();
         }
 
@@ -413,23 +417,23 @@ private int ReverifyTransactions(SortedSet<PoolItem> verifiedSortedTxPool,
             List<PoolItem> reverifiedItems = new List<PoolItem>(count);
             List<PoolItem> invalidItems = new List<PoolItem>();
 
-            // Since unverifiedSortedTxPool is ordered in an ascending manner, we take from the end.
-            foreach (PoolItem item in unverifiedSortedTxPool.Reverse().Take(count))
+            _txRwLock.EnterWriteLock();
+            try
             {
-                if (item.Tx.VerifyForEachBlock(snapshot, SendersFeeMonitor.GetSenderFee(item.Tx.Sender)) == VerifyResult.Succeed)
+                // Since unverifiedSortedTxPool is ordered in an ascending manner, we take from the end.
+                foreach (PoolItem item in unverifiedSortedTxPool.Reverse().Take(count))
                 {
-                    reverifiedItems.Add(item);
-                    SendersFeeMonitor.AddSenderFee(item.Tx);
-                }
-                else // Transaction no longer valid -- it will be removed from unverifiedTxPool.
-                    invalidItems.Add(item);
+                    if (item.Tx.VerifyForEachBlock(snapshot, VerificationContext) == VerifyResult.Succeed)
+                    {
+                        reverifiedItems.Add(item);
+                        VerificationContext.AddTransaction(item.Tx);
+                    }
+                    else // Transaction no longer valid -- it will be removed from unverifiedTxPool.
+                        invalidItems.Add(item);
 
-                if (DateTime.UtcNow > reverifyCutOffTimeStamp) break;
-            }
+                    if (DateTime.UtcNow > reverifyCutOffTimeStamp) break;
+                }
 
-            _txRwLock.EnterWriteLock();
-            try
-            {
                 int blocksTillRebroadcast = BlocksTillRebroadcast;
                 // Increases, proportionally, blocksTillRebroadcast if mempool has more items than threshold bigger RebroadcastMultiplierThreshold
                 if (Count > RebroadcastMultiplierThreshold)
@@ -450,7 +454,7 @@ private int ReverifyTransactions(SortedSet<PoolItem> verifiedSortedTxPool,
                         }
                     }
                     else
-                        SendersFeeMonitor.RemoveSenderFee(item.Tx);
+                        VerificationContext.RemoveTransaction(item.Tx);
 
                     _unverifiedTransactions.Remove(item.Tx.Hash);
                     unverifiedSortedTxPool.Remove(item);

src/neo/Ledger/TransactionVerificationContext.cs

@@ -0,0 +1,37 @@
+using Neo.Network.P2P.Payloads;
+using Neo.Persistence;
+using Neo.SmartContract.Native;
+using System.Collections.Generic;
+using System.Numerics;
+
+namespace Neo.Ledger
+{
+    public class TransactionVerificationContext
+    {
+        /// <summary>
+        /// Store all verified unsorted transactions' senders' fee currently in the memory pool.
+        /// </summary>
+        private readonly Dictionary<UInt160, BigInteger> senderFee = new Dictionary<UInt160, BigInteger>();
+
+        public void AddTransaction(Transaction tx)
+        {
+            if (senderFee.TryGetValue(tx.Sender, out var value))
+                senderFee[tx.Sender] = value + tx.SystemFee + tx.NetworkFee;
+            else
+                senderFee.Add(tx.Sender, tx.SystemFee + tx.NetworkFee);
+        }
+
+        public bool CheckTransaction(Transaction tx, StoreView snapshot)
+        {
+            BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender);
+            senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool);
+            BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool;
+            return balance >= fee;
+        }
+
+        public void RemoveTransaction(Transaction tx)
+        {
+            if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender);
+        }
+    }
+}

src/neo/Network/P2P/Payloads/Transaction.cs

@@ -12,7 +12,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using System.Numerics;
 using Array = Neo.VM.Types.Array;
 
 namespace Neo.Network.P2P.Payloads
@@ -258,10 +257,10 @@ public JObject ToJson()
 
         bool IInventory.Verify(StoreView snapshot)
         {
-            return Verify(snapshot, BigInteger.Zero) == VerifyResult.Succeed;
+            return Verify(snapshot, null) == VerifyResult.Succeed;
         }
 
-        public virtual VerifyResult VerifyForEachBlock(StoreView snapshot, BigInteger totalSenderFeeFromPool)
+        public virtual VerifyResult VerifyForEachBlock(StoreView snapshot, TransactionVerificationContext context)
         {
             if (ValidUntilBlock <= snapshot.Height || ValidUntilBlock > snapshot.Height + MaxValidUntilBlockIncrement)
                 return VerifyResult.Expired;
@@ -270,9 +269,7 @@ public virtual VerifyResult VerifyForEachBlock(StoreView snapshot, BigInteger to
                 return VerifyResult.PolicyFail;
             if (NativeContract.Policy.GetMaxBlockSystemFee(snapshot) < SystemFee)
                 return VerifyResult.PolicyFail;
-            BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, Sender);
-            BigInteger fee = SystemFee + NetworkFee + totalSenderFeeFromPool;
-            if (balance < fee) return VerifyResult.InsufficientFunds;
+            if (!(context?.CheckTransaction(this, snapshot) ?? true)) return VerifyResult.InsufficientFunds;
             if (hashes.Length != Witnesses.Length) return VerifyResult.Invalid;
             for (int i = 0; i < hashes.Length; i++)
             {
@@ -282,9 +279,9 @@ public virtual VerifyResult VerifyForEachBlock(StoreView snapshot, BigInteger to
             return VerifyResult.Succeed;
         }
 
-        public virtual VerifyResult Verify(StoreView snapshot, BigInteger totalSenderFeeFromPool)
+        public virtual VerifyResult Verify(StoreView snapshot, TransactionVerificationContext context)
         {
-            VerifyResult result = VerifyForEachBlock(snapshot, totalSenderFeeFromPool);
+            VerifyResult result = VerifyForEachBlock(snapshot, context);
             if (result != VerifyResult.Succeed) return result;
             int size = Size;
             if (size > MaxTransactionSize) return VerifyResult.Invalid;

tests/neo.UnitTests/Ledger/UT_MemoryPool.cs

@@ -74,8 +74,8 @@ private Transaction CreateTransactionWithFee(long fee)
             var randomBytes = new byte[16];
             random.NextBytes(randomBytes);
             Mock<Transaction> mock = new Mock<Transaction>();
-            mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
-            mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
+            mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<TransactionVerificationContext>())).Returns(VerifyResult.Succeed);
+            mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<TransactionVerificationContext>())).Returns(VerifyResult.Succeed);
             mock.Object.Script = randomBytes;
             mock.Object.Sender = UInt160.Zero;
             mock.Object.NetworkFee = fee;
@@ -98,8 +98,8 @@ private Transaction CreateTransactionWithFeeAndBalanceVerify(long fee)
             random.NextBytes(randomBytes);
             Mock<Transaction> mock = new Mock<Transaction>();
             UInt160 sender = UInt160.Zero;
-            mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns((StoreView snapshot, BigInteger amount) => NativeContract.GAS.BalanceOf(snapshot, sender) >= amount + fee ? VerifyResult.Succeed : VerifyResult.InsufficientFunds);
-            mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<BigInteger>())).Returns(VerifyResult.Succeed);
+            mock.Setup(p => p.VerifyForEachBlock(It.IsAny<StoreView>(), It.IsAny<TransactionVerificationContext>())).Returns((StoreView snapshot, BigInteger amount) => NativeContract.GAS.BalanceOf(snapshot, sender) >= amount + fee ? VerifyResult.Succeed : VerifyResult.InsufficientFunds);
+            mock.Setup(p => p.Verify(It.IsAny<StoreView>(), It.IsAny<TransactionVerificationContext>())).Returns(VerifyResult.Succeed);
             mock.Object.Script = randomBytes;
             mock.Object.Sender = sender;
             mock.Object.NetworkFee = fee;