From 688de1cc8f335dbfd7074d8ed7a476a2bf34ec82 Mon Sep 17 00:00:00 2001 From: Igor Machado Coelho Date: Fri, 18 Jan 2019 22:57:38 -0200 Subject: [PATCH] Make PoolItem independent and add PoolItem tests (#562) * make poolitem independent * Merging * Multiply by -1 * Fix other * Fix Tx * Removing -1 extra multiplication * Fix * make PoolItem internal and added test class * Update PoolItem.cs * added comments for PoolItem variables * getting time from TimeProvider to allow testing * basic test * reset time provider * Add Hash comparison * Adding time provider again and equals * Fix arithmetic * Comment on PoolItem * Update PoolItem.cs * protecting tests against TimeProvider changes on fails * reusing setup part * fixed serialization properties * Improve generation of creating mock DateTime values. Implement hash comparison tests. * Adjust comment. --- neo.UnitTests/UT_MemoryPool.cs | 3 + neo.UnitTests/UT_PoolItem.cs | 152 +++++++++++++++++++++++++++++++++ neo/Ledger/MemoryPool.cs | 32 ------- neo/Ledger/PoolItem.cs | 63 ++++++++++++++ 4 files changed, 218 insertions(+), 32 deletions(-) create mode 100644 neo.UnitTests/UT_PoolItem.cs create mode 100644 neo/Ledger/PoolItem.cs diff --git a/neo.UnitTests/UT_MemoryPool.cs b/neo.UnitTests/UT_MemoryPool.cs index 8393528bd6..4f78b9b033 100644 --- a/neo.UnitTests/UT_MemoryPool.cs +++ b/neo.UnitTests/UT_MemoryPool.cs @@ -24,6 +24,9 @@ public class UT_MemoryPool [TestInitialize] public void TestSetup() { + // protect against external changes on TimeProvider + TimeProvider.ResetToDefault(); + if (TheNeoSystem == null) { var mockSnapshot = new Mock(); diff --git a/neo.UnitTests/UT_PoolItem.cs b/neo.UnitTests/UT_PoolItem.cs new file mode 100644 index 0000000000..3ef28356dc --- /dev/null +++ b/neo.UnitTests/UT_PoolItem.cs @@ -0,0 +1,152 @@ +using System; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Neo.Ledger; +using FluentAssertions; +using Neo.Network.P2P.Payloads; + +namespace Neo.UnitTests +{ + [TestClass] + public class UT_PoolItem + { + //private PoolItem uut; + private static readonly Random TestRandom = new Random(1337); // use fixed seed for guaranteed determinism + + [TestInitialize] + public void TestSetup() + { + var timeValues = new[] { + new DateTime(1968, 06, 01, 0, 0, 1, DateTimeKind.Utc), + }; + + var timeMock = new Mock(); + timeMock.SetupGet(tp => tp.UtcNow).Returns(() => timeValues[0]) + .Callback(() => timeValues[0] = timeValues[0].Add(TimeSpan.FromSeconds(1))); + TimeProvider.Current = timeMock.Object; + } + + [TestCleanup] + public void TestCleanup() + { + // important to leave TimeProvider correct + TimeProvider.ResetToDefault(); + } + + [TestMethod] + public void PoolItem_CompareTo_Fee() + { + int size1 = 50; + int netFeeSatoshi1 = 1; + var tx1 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshi1), size1); + int size2 = 50; + int netFeeSatoshi2 = 2; + var tx2 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshi2), size2); + + PoolItem pitem1 = new PoolItem(tx1.Object); + PoolItem pitem2 = new PoolItem(tx2.Object); + + Console.WriteLine($"item1 time {pitem1.Timestamp} item2 time {pitem2.Timestamp}"); + // pitem1 < pitem2 (fee) => -1 + pitem1.CompareTo(pitem2).Should().Be(-1); + // pitem2 > pitem1 (fee) => 1 + pitem2.CompareTo(pitem1).Should().Be(1); + } + + [TestMethod] + public void PoolItem_CompareTo_Hash() + { + int sizeFixed = 50; + int netFeeSatoshiFixed = 1; + + for (int testRuns = 0; testRuns < 30; testRuns++) + { + var tx1 = GenerateMockTxWithFirstByteOfHashGreaterThanOrEqualTo(0x80, new Fixed8(netFeeSatoshiFixed), sizeFixed); + var tx2 = GenerateMockTxWithFirstByteOfHashLessThanOrEqualTo(0x79,new Fixed8(netFeeSatoshiFixed), sizeFixed); + + PoolItem pitem1 = new PoolItem(tx1.Object); + PoolItem pitem2 = new PoolItem(tx2.Object); + + // pitem2 < pitem1 (fee) => -1 + pitem2.CompareTo(pitem1).Should().Be(-1); + + // pitem1 > pitem2 (fee) => 1 + pitem1.CompareTo(pitem2).Should().Be(1); + } + + // equal hashes should be equal + var tx3 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshiFixed), sizeFixed, new byte[] {0x13, 0x37}); + var tx4 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshiFixed), sizeFixed, new byte[] {0x13, 0x37}); + PoolItem pitem3 = new PoolItem(tx3.Object); + PoolItem pitem4 = new PoolItem(tx4.Object); + + pitem3.CompareTo(pitem4).Should().Be(0); + pitem4.CompareTo(pitem3).Should().Be(0); + } + + [TestMethod] + public void PoolItem_CompareTo_Equals() + { + int sizeFixed = 500; + int netFeeSatoshiFixed = 10; + var tx1 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshiFixed), sizeFixed, new byte[] {0x13, 0x37}); + var tx2 = MockGenerateInvocationTx(new Fixed8(netFeeSatoshiFixed), sizeFixed, new byte[] {0x13, 0x37}); + + PoolItem pitem1 = new PoolItem(tx1.Object); + PoolItem pitem2 = new PoolItem(tx2.Object); + + // pitem1 == pitem2 (fee) => 0 + pitem1.CompareTo(pitem2).Should().Be(0); + pitem2.CompareTo(pitem1).Should().Be(0); + } + + public Mock GenerateMockTxWithFirstByteOfHashGreaterThanOrEqualTo(byte firstHashByte, Fixed8 networkFee, int size) + { + Mock mockTx; + do + { + mockTx = MockGenerateInvocationTx(networkFee, size); + } while (mockTx.Object.Hash >= new UInt256(TestUtils.GetByteArray(32, firstHashByte))); + + return mockTx; + } + + public Mock GenerateMockTxWithFirstByteOfHashLessThanOrEqualTo(byte firstHashByte, Fixed8 networkFee, int size) + { + Mock mockTx; + do + { + mockTx = MockGenerateInvocationTx(networkFee, size); + } while (mockTx.Object.Hash <= new UInt256(TestUtils.GetByteArray(32, firstHashByte))); + + return mockTx; + } + + + // Generate Mock InvocationTransaction with different sizes and prices + public static Mock MockGenerateInvocationTx(Fixed8 networkFee, int size, byte[] overrideScriptBytes=null) + { + var mockTx = new Mock(); + mockTx.CallBase = true; + mockTx.SetupGet(mr => mr.NetworkFee).Returns(networkFee); + mockTx.SetupGet(mr => mr.Size).Returns(size); + + var tx = mockTx.Object; + // use random bytes in the script to get a different hash since we cannot mock the Hash + byte[] randomBytes; + if (overrideScriptBytes != null) + randomBytes = overrideScriptBytes; + else + { + randomBytes = new byte[16]; + TestRandom.NextBytes(randomBytes); + } + tx.Script = randomBytes; + tx.Attributes = new TransactionAttribute[0]; + tx.Inputs = new CoinReference[0]; + tx.Outputs = new TransactionOutput[0]; + tx.Witnesses = new Witness[0]; + return mockTx; + } + } +} diff --git a/neo/Ledger/MemoryPool.cs b/neo/Ledger/MemoryPool.cs index c6df72891a..0d857495de 100644 --- a/neo/Ledger/MemoryPool.cs +++ b/neo/Ledger/MemoryPool.cs @@ -14,38 +14,6 @@ namespace Neo.Ledger { public class MemoryPool : IReadOnlyCollection { - private class PoolItem : IComparable - { - public readonly Transaction Tx; - public readonly DateTime Timestamp; - public DateTime LastBroadcastTimestamp; - - public PoolItem(Transaction tx) - { - Tx = tx; - Timestamp = DateTime.UtcNow; - LastBroadcastTimestamp = Timestamp; - } - - public int CompareTo(Transaction otherTx) - { - if (otherTx == null) return 1; - // Fees sorted ascending - int ret = Tx.FeePerByte.CompareTo(otherTx.FeePerByte); - if (ret != 0) return ret; - ret = Tx.NetworkFee.CompareTo(otherTx.NetworkFee); - if (ret != 0) return ret; - // Transaction hash sorted descending - return otherTx.Hash.CompareTo(Tx.Hash); - } - - public int CompareTo(PoolItem otherItem) - { - if (otherItem == null) return 1; - return CompareTo(otherItem.Tx); - } - } - // Allow a reverified transaction to be rebroadcasted if it has been this many block times since last broadcast. private const int BlocksTillRebroadcastLowPriorityPoolTx = 30; private const int BlocksTillRebroadcastHighPriorityPoolTx = 10; diff --git a/neo/Ledger/PoolItem.cs b/neo/Ledger/PoolItem.cs new file mode 100644 index 0000000000..13dc9b9344 --- /dev/null +++ b/neo/Ledger/PoolItem.cs @@ -0,0 +1,63 @@ +using Neo.Network.P2P.Payloads; +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; +using Akka.Util.Internal; +using Neo.Network.P2P; +using Neo.Persistence; +using Neo.Plugins; + +namespace Neo.Ledger +{ + /// + /// Represents an item in the Memory Pool. + /// + // Note: PoolItem objects don't consider transaction priority (low or high) in their compare CompareTo method. + /// This is because items of differing priority are never added to the same sorted set in MemoryPool. + /// + internal class PoolItem : IComparable + { + /// + /// Internal transaction for PoolItem + /// + public readonly Transaction Tx; + + /// + /// Timestamp when transaction was stored on PoolItem + /// + public readonly DateTime Timestamp; + + /// + /// Timestamp where this transaction was last broadcast to other nodes + /// + public DateTime LastBroadcastTimestamp; + + internal PoolItem(Transaction tx) + { + Tx = tx; + Timestamp = TimeProvider.Current.UtcNow; + LastBroadcastTimestamp = Timestamp; + } + + public int CompareTo(Transaction otherTx) + { + if (otherTx == null) return 1; + // Fees sorted ascending + int ret = Tx.FeePerByte.CompareTo(otherTx.FeePerByte); + if (ret != 0) return ret; + ret = Tx.NetworkFee.CompareTo(otherTx.NetworkFee); + if (ret != 0) return ret; + // Transaction hash sorted descending + return otherTx.Hash.CompareTo(Tx.Hash); + } + + public int CompareTo(PoolItem otherItem) + { + if (otherItem == null) return 1; + return CompareTo(otherItem.Tx); + } + } +}