From 15d8fea45c2225304aa340eceba2ca1a33d3553c Mon Sep 17 00:00:00 2001 From: jsolman Date: Thu, 17 Jan 2019 19:51:17 -0800 Subject: [PATCH 1/2] Add unit test to verify memory pool sort order and reverification order. Fixed sort order bug. --- neo.UnitTests/UT_MemoryPool.cs | 70 ++++++++++++++++++++++++++++++++++ neo/Ledger/MemoryPool.cs | 13 ++++--- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/neo.UnitTests/UT_MemoryPool.cs b/neo.UnitTests/UT_MemoryPool.cs index d98efc43aa..f68b4b7f48 100644 --- a/neo.UnitTests/UT_MemoryPool.cs +++ b/neo.UnitTests/UT_MemoryPool.cs @@ -271,6 +271,76 @@ public void BlockPersistMovesTxToUnverifiedAndReverification() _unit.UnverifiedSortedLowPrioTxCount.ShouldBeEquivalentTo(0); } + private void verifyTransactionsSortedDescending(IEnumerable transactions) + { + Transaction lastTransaction = null; + foreach (var tx in transactions) + { + if (lastTransaction != null) + { + if (lastTransaction.FeePerByte == tx.FeePerByte) + { + if (lastTransaction.NetworkFee == tx.NetworkFee) + lastTransaction.Hash.Should().BeGreaterThan(tx.Hash); + else + lastTransaction.NetworkFee.Should().BeGreaterThan(tx.NetworkFee); + } + else + { + lastTransaction.FeePerByte.Should().BeGreaterThan(tx.FeePerByte); + } + } + lastTransaction = tx; + } + } + + [TestMethod] + public void VerifySortOrderAndThatHighetFeeTransactionsAreReverifiedFirst() + { + AddLowPriorityTransactions(50); + AddHighPriorityTransactions(50); + + var sortedVerifiedTxs = _unit.GetSortedVerifiedTransactions().ToList(); + // verify all 100 transactions are returned in sorted order + sortedVerifiedTxs.Count.ShouldBeEquivalentTo(100); + verifyTransactionsSortedDescending(sortedVerifiedTxs); + + // move all to unverified + var block = new Block { Transactions = new Transaction[0] }; + _unit.UpdatePoolForBlockPersisted(block, Blockchain.Singleton.GetSnapshot()); + + _unit.SortedHighPrioTxCount.ShouldBeEquivalentTo(0); + _unit.SortedLowPrioTxCount.ShouldBeEquivalentTo(0); + _unit.UnverifiedSortedHighPrioTxCount.ShouldBeEquivalentTo(50); + _unit.UnverifiedSortedLowPrioTxCount.ShouldBeEquivalentTo(50); + + // We can verify the order they are re-verified by reverifying 2 at a time + while (_unit.UnVerifiedCount > 0) + { + _unit.GetVerifiedAndUnverifiedTransactions(out IEnumerable sortedVerifiedTransactions, + out IEnumerable sortedUnverifiedTransactions); + sortedVerifiedTransactions.Count().ShouldBeEquivalentTo(0); + var sortedUnverifiedArray = sortedUnverifiedTransactions.ToArray(); + verifyTransactionsSortedDescending(sortedUnverifiedArray); + var maxHighPriorityTransaction = sortedUnverifiedArray.First(); + var maxLowPriorityTransaction = sortedUnverifiedArray.First(tx => tx.IsLowPriority); + + // reverify 1 high priority and 1 low priority transaction + _unit.ReVerifyTopUnverifiedTransactionsIfNeeded(2, Blockchain.Singleton.GetSnapshot()); + var verifiedTxs = _unit.GetSortedVerifiedTransactions().ToArray(); + verifiedTxs.Length.ShouldBeEquivalentTo(2); + verifiedTxs[0].ShouldBeEquivalentTo(maxHighPriorityTransaction); + verifiedTxs[1].ShouldBeEquivalentTo(maxLowPriorityTransaction); + var blockWith2Tx = new Block { Transactions = new Transaction[2] { maxHighPriorityTransaction, maxLowPriorityTransaction }}; + // verify and remove the 2 transactions from the verified pool + _unit.UpdatePoolForBlockPersisted(blockWith2Tx, Blockchain.Singleton.GetSnapshot()); + _unit.SortedHighPrioTxCount.ShouldBeEquivalentTo(0); + _unit.SortedLowPrioTxCount.ShouldBeEquivalentTo(0); + } + _unit.UnverifiedSortedHighPrioTxCount.ShouldBeEquivalentTo(0); + _unit.UnverifiedSortedLowPrioTxCount.ShouldBeEquivalentTo(0); + } + [TestMethod] public void CapacityTestWithUnverifiedHighProirtyTransactions() { diff --git a/neo/Ledger/MemoryPool.cs b/neo/Ledger/MemoryPool.cs index 521701d0de..1b89e5d1b7 100644 --- a/neo/Ledger/MemoryPool.cs +++ b/neo/Ledger/MemoryPool.cs @@ -226,9 +226,10 @@ public IEnumerable GetVerifiedTransactions() _txRwLock.EnterReadLock(); try { - verifiedTransactions = _sortedHighPrioTransactions.Select(p => p.Transaction) - .Concat(_sortedLowPrioTransactions.Select(p => p.Transaction)).ToArray(); - unverifiedTransactions = _unverifiedTransactions.Select(p => p.Value.Transaction).ToArray(); + verifiedTransactions = _sortedHighPrioTransactions.Reverse().Select(p => p.Transaction) + .Concat(_sortedLowPrioTransactions.Reverse().Select(p => p.Transaction)).ToArray(); + unverifiedTransactions = _unverifiedSortedHighPriorityTransactions.Reverse().Select(p => p.Transaction) + .Concat(_unverifiedSortedLowPriorityTransactions.Reverse().Select(p => p.Transaction)).ToArray(); } finally { @@ -241,8 +242,8 @@ public IEnumerable GetSortedVerifiedTransactions() _txRwLock.EnterReadLock(); try { - return _sortedHighPrioTransactions.Select(p => p.Transaction) - .Concat(_sortedLowPrioTransactions.Select(p => p.Transaction)) + return _sortedHighPrioTransactions.Reverse().Select(p => p.Transaction) + .Concat(_sortedLowPrioTransactions.Reverse().Select(p => p.Transaction)) .ToArray(); } finally @@ -429,7 +430,7 @@ internal void UpdatePoolForBlockPersisted(Block block, Snapshot snapshot) DateTime reverifyCutOffTimeStamp = DateTime.UtcNow.AddSeconds(secondsTimeout); List reverifiedItems = new List(count); List invalidItems = new List(); - + // Since unverifiedSortedTxPool is ordered in an ascending manner, we take from the end. foreach (PoolItem item in unverifiedSortedTxPool.Reverse().Take(count)) { From 4a92c5d910009f5e612f10152e351701c895bfb8 Mon Sep 17 00:00:00 2001 From: jsolman Date: Thu, 17 Jan 2019 20:35:23 -0800 Subject: [PATCH 2/2] VerifyCanTransactionFitInPool works as intended. Also inadvertantly verified GetLowestFeeTransaction() works. --- neo.UnitTests/UT_MemoryPool.cs | 53 +++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/neo.UnitTests/UT_MemoryPool.cs b/neo.UnitTests/UT_MemoryPool.cs index 72c89576d6..b5167e9b16 100644 --- a/neo.UnitTests/UT_MemoryPool.cs +++ b/neo.UnitTests/UT_MemoryPool.cs @@ -113,28 +113,34 @@ long LongRandom(long min, long max, Random rand) return longRand % (max - min) + min; } - private Transaction CreateMockHighPriorityTransaction() + private Transaction CreateMockTransactionWithFee(long fee) { var mockTx = CreateRandomHashInvocationMockTransaction(); - long rNetFee = LongRandom(100000, 100000000, _random); // [0.001,1]) GAS (enough to be a high priority TX) - mockTx.SetupGet(p => p.NetworkFee).Returns(new Fixed8(rNetFee)); + mockTx.SetupGet(p => p.NetworkFee).Returns(new Fixed8(fee)); var tx = mockTx.Object; - tx.Inputs = new CoinReference[1]; - // Any input will trigger reading the transaction output and get our mocked transaction output. - tx.Inputs[0] = new CoinReference + if (fee > 0) { - PrevHash = UInt256.Zero, - PrevIndex = 0 - }; + tx.Inputs = new CoinReference[1]; + // Any input will trigger reading the transaction output and get our mocked transaction output. + tx.Inputs[0] = new CoinReference + { + PrevHash = UInt256.Zero, + PrevIndex = 0 + }; + } return tx; } + private Transaction CreateMockHighPriorityTransaction() + { + return CreateMockTransactionWithFee(LongRandom(100000, 100000000, _random)); + } + private Transaction CreateMockLowPriorityTransaction() { - var mockTx = CreateRandomHashInvocationMockTransaction(); - long rNetFee = LongRandom(0, 100000, _random); // [0,0.001] GAS a fee lower than the threshold of 0.001 GAS (not enough to be a high priority TX) - mockTx.SetupGet(p => p.NetworkFee).Returns(new Fixed8(rNetFee)); - return mockTx.Object; + long rNetFee = LongRandom(0, 100000, _random); + // [0,0.001] GAS a fee lower than the threshold of 0.001 GAS (not enough to be a high priority TX) + return CreateMockTransactionWithFee(rNetFee); } private void AddTransactions(int count, bool isHighPriority=false) @@ -350,6 +356,27 @@ public void VerifySortOrderAndThatHighetFeeTransactionsAreReverifiedFirst() _unit.UnverifiedSortedLowPrioTxCount.ShouldBeEquivalentTo(0); } + void VerifyCapacityThresholdForAttemptingToAddATransaction() + { + var sortedVerified = _unit.GetSortedVerifiedTransactions().ToArray(); + + var txBarelyWontFit = CreateMockTransactionWithFee(sortedVerified.Last().NetworkFee.GetData() - 1); + _unit.CanTransactionFitInPool(txBarelyWontFit).ShouldBeEquivalentTo(false); + var txBarelyFits = CreateMockTransactionWithFee(sortedVerified.Last().NetworkFee.GetData() + 1); + _unit.CanTransactionFitInPool(txBarelyFits).ShouldBeEquivalentTo(true); + } + + [TestMethod] + public void VerifyCanTransactionFitInPoolWorksAsIntended() + { + AddLowPriorityTransactions(100); + VerifyCapacityThresholdForAttemptingToAddATransaction(); + AddHighPriorityTransactions(50); + VerifyCapacityThresholdForAttemptingToAddATransaction(); + AddHighPriorityTransactions(50); + VerifyCapacityThresholdForAttemptingToAddATransaction(); + } + [TestMethod] public void CapacityTestWithUnverifiedHighProirtyTransactions() {