Apply suggestions

Co-Authored-By: Hong Minhee <hong.minhee@gmail.com>
planetarium · Feb 3, 2020 · c483e0d · c483e0d
1 parent f6a2e2b
commit c483e0d
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 13 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
@@ -2438,7 +2438,7 @@ public async Task HandleReorgInSynchronizing()
         }
 
         [Fact(Timeout = Timeout)]
-        public async void RestageTransactionsAfterReorg()
+        public async void RestageTransactionsOnceLocallyMinedAfterReorg()
         {
             var policy = new BlockPolicy<DumbAction>(new MinerReward(1));
             var minerA = CreateSwarm(TestUtils.MakeBlockChain(policy, new DefaultStore(null)));
@@ -2462,31 +2462,29 @@ public async void RestageTransactionsAfterReorg()
                     privateKeyB,
                     new[] { new DumbAction(_fx1.Address2, dumbItem), });
 
-                // Make minerB's chain longer than minerA's chain.
+                Log.Debug("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);
 
-                // Check each states.
                 Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address1));
                 Assert.Equal((Text)dumbItem, minerB.BlockChain.GetState(_fx1.Address2));
 
-                // Occur reorg.
+                Log.Debug("Reorg occurred.");
                 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((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address2));
 
-                // Expect stage txs in unrendered blocks.
+                Log.Debug("Check txs in unrendered blocks staged");
                 Assert.Contains(txA.Id, minerA.BlockChain.GetStagedTransactionIds());
 
                 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);

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;
                     }
                 }