Apply suggestions

Co-Authored-By: Hong Minhee <hong.minhee@gmail.com> Co-Authored-By: Ko Chanhyuck <lime_bell@naver.com>
limebell · Feb 4, 2020 · ace2177 · ace2177
1 parent 6642bbd
commit ace2177
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 27 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -188,8 +188,10 @@ To be released.
     [[#759]]
  -  Fixed a bug where `BlockChain<T>` had rendered and evaluated actions in
     the genesis block during forking.  [[#763]]
- -  Fixed a bug where transactions which were not propagated sufficiently,
-    could not be included in a block when reorg happened.  [[#775]]
+ -  Fixed a `Swam<T>`'s bug that some `Transaction<T>`s had become excluded from
+    mining `Block<T>`s after reorg from α to β where a `Transaction<T>` was once
+    included by a `Block<T>` (α) and not included by an other `Block<T>` (β) for
+    the same `Index` due to the latency gap between nodes.  [[#775]]
 
 [#368]: https://github.com/planetarium/libplanet/issues/368
 [#570]: https://github.com/planetarium/libplanet/issues/570

diff --git a/Libplanet.Tests/Net/SwarmTest.cs b/Libplanet.Tests/Net/SwarmTest.cs
@@ -2437,8 +2437,10 @@ public async Task HandleReorgInSynchronizing()
             }
         }
 
-        [Fact(Timeout = Timeout)]
-        public async void RestageTransactionsAfterReorg()
+        [Theory(Timeout = Timeout)]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async void RestageTransactionsOnceLocallyMinedAfterReorg(bool restage)
         {
             var policy = new BlockPolicy<DumbAction>(new MinerReward(1));
             var minerA = CreateSwarm(TestUtils.MakeBlockChain(policy, new DefaultStore(null)));
@@ -2449,9 +2451,6 @@ public async void RestageTransactionsAfterReorg()
 
             try
             {
-                await StartAsync(minerA);
-                await StartAsync(minerB);
-
                 const string dumbItem = "item0.0";
                 var txA = minerA.BlockChain.MakeTransaction(
                     privateKeyA,
@@ -2460,35 +2459,45 @@ public async void RestageTransactionsAfterReorg()
                     privateKeyB,
                     new[] { new DumbAction(_fx1.Address2, dumbItem), });
 
-                // Make minerB's chain longer than minerA's chain.
-                var blockA = await minerA.BlockChain.MineBlock(minerA.Address);
-                var blockB = await minerB.BlockChain.MineBlock(minerB.Address);
-                var blockC = await minerB.BlockChain.MineBlock(minerB.Address);
+                if (!restage)
+                {
+                    minerB.BlockChain.StageTransactions(
+                        ImmutableHashSet<Transaction<DumbAction>>.Empty.Add(txA));
+                }
 
-                // Check each states.
-                await BootstrapAsync(minerA, minerB.AsPeer);
+                Log.Debug("Make minerB's chain longer than minerA's chain.");
+                Block<DumbAction> blockA = await minerA.BlockChain.MineBlock(minerA.Address);
+                Block<DumbAction> blockB = await minerB.BlockChain.MineBlock(minerB.Address);
+                Block<DumbAction> blockC = await minerB.BlockChain.MineBlock(minerB.Address);
 
                 Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address1));
                 Assert.Equal((Text)dumbItem, minerB.BlockChain.GetState(_fx1.Address2));
 
-                // Occur reorg.
+                await StartAsync(minerA);
+                await StartAsync(minerB);
+
+                await BootstrapAsync(minerA, minerB.AsPeer);
+
+                Log.Debug("Reorg occurrs.");
                 minerB.BroadcastBlock(blockC);
-                minerA.BlockAppended.Wait();
+                await minerA.BlockAppended.WaitAsync();
 
-                // Check sync.
                 Assert.Equal(minerA.BlockChain.Tip, minerB.BlockChain.Tip);
                 Assert.Equal(3, minerA.BlockChain.Count);
-                Assert.Null(minerA.BlockChain.GetState(_fx1.Address1));
+                Assert.Equal(
+                    restage ? null : (Text?)dumbItem,
+                    minerA.BlockChain.GetState(_fx1.Address1));
                 Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address2));
 
-                // Expect stage txs in unrendered blocks.
-                Assert.Contains(txA.Id, minerA.BlockChain.GetStagedTransactionIds());
+                Log.Debug("Check if txs in unrendered blocks staged again.");
+                Assert.Equal(
+                    restage,
+                    minerA.BlockChain.GetStagedTransactionIds().Contains(txA.Id));
 
                 await minerA.BlockChain.MineBlock(minerA.Address);
                 minerA.BroadcastBlock(minerA.BlockChain.Tip);
-                minerB.BlockAppended.Wait();
+                await minerB.BlockAppended.WaitAsync();
 
-                // Check sync.
                 Assert.Equal(minerA.BlockChain.Tip, minerB.BlockChain.Tip);
                 Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address1));
                 Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address2));

diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs
@@ -1184,11 +1184,24 @@ internal void Swap(BlockChain<T> other, bool render)
             {
                 long shorterHeight =
                     Math.Min(Count, other.Count) - 1;
-                for (long index = shorterHeight; index >= 0; --index)
+                Block<T> t = this[shorterHeight], o = other[shorterHeight];
+
+                while (true)
                 {
-                    if (this[index].Equals(other[index]))
+                    if (t.Equals(o))
+                    {
+                        topmostCommon = t;
+                        break;
+                    }
+
+                    if (t.PreviousHash is HashDigest<SHA256> tp &&
+                        o.PreviousHash is HashDigest<SHA256> op)
+                    {
+                        t = this[tp];
+                        o = other[op];
+                    }
+                    else
                     {
-                        topmostCommon = this[index];
                         break;
                     }
                 }
@@ -1231,11 +1244,11 @@ IEnumerable<TxId> GetTxIdsWithRange(BlockChain<T> chain, Block<T> start, Block<T
                     .SelectMany(x => chain[x].Transactions.Select(tx => tx.Id));
 
             // It assumes reorg is small size. If it was big, this may be heavy task.
-            var unstagedTxIds =
+            ImmutableHashSet<TxId> unstagedTxIds =
                 GetTxIdsWithRange(this, topmostCommon, Tip).ToImmutableHashSet();
-            var stageTxIds =
+            ImmutableHashSet<TxId> stageTxIds =
                 GetTxIdsWithRange(other, topmostCommon, other.Tip).ToImmutableHashSet();
-            var restageTxIds = unstagedTxIds.Except(stageTxIds);
+            ImmutableHashSet<TxId> restageTxIds = unstagedTxIds.Except(stageTxIds);
             Store.StageTransactionIds(restageTxIds);
 
             try