Merge pull request planetarium#775 from moreal/bugfix/stage-after-reorg

Stage transactions not included in swapped chain
limebell · Feb 4, 2020 · 17d654f · 17d654f
2 parents 23dd63f + ace2177
commit 17d654f
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 12 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -188,6 +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 `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
@@ -241,6 +245,7 @@ To be released.
 [#767]: https://github.com/planetarium/libplanet/pull/767
 [#772]: https://github.com/planetarium/libplanet/pull/772
 [#774]: https://github.com/planetarium/libplanet/pull/774
+[#775]: https://github.com/planetarium/libplanet/pull/775
 
 
 Version 0.7.0

diff --git a/Libplanet.Tests/Net/SwarmTest.cs b/Libplanet.Tests/Net/SwarmTest.cs
@@ -2271,7 +2271,7 @@ public async Task PreloadAsyncCancellation(int cancelAfter)
             BlockChain<DumbAction> minerChain = _blockchains[0];
             BlockChain<DumbAction> receiverChain = _blockchains[1];
 
-            Guid receiverChainId = _blockchains[1].Id;
+            Guid receiverChainId = receiverChain.Id;
 
             (Address address, IEnumerable<Block<DumbAction>> blocks) =
                 await MakeFixtureBlocksForPreloadAsyncCancellationTest();
@@ -2437,6 +2437,81 @@ public async Task HandleReorgInSynchronizing()
             }
         }
 
+        [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)));
+            var minerB = CreateSwarm(TestUtils.MakeBlockChain(policy, new DefaultStore(null)));
+
+            var privateKeyA = new PrivateKey();
+            var privateKeyB = new PrivateKey();
+
+            try
+            {
+                const string dumbItem = "item0.0";
+                var txA = minerA.BlockChain.MakeTransaction(
+                    privateKeyA,
+                    new[] { new DumbAction(_fx1.Address1, dumbItem), });
+                var txB = minerB.BlockChain.MakeTransaction(
+                    privateKeyB,
+                    new[] { new DumbAction(_fx1.Address2, dumbItem), });
+
+                if (!restage)
+                {
+                    minerB.BlockChain.StageTransactions(
+                        ImmutableHashSet<Transaction<DumbAction>>.Empty.Add(txA));
+                }
+
+                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));
+
+                await StartAsync(minerA);
+                await StartAsync(minerB);
+
+                await BootstrapAsync(minerA, minerB.AsPeer);
+
+                Log.Debug("Reorg occurrs.");
+                minerB.BroadcastBlock(blockC);
+                await minerA.BlockAppended.WaitAsync();
+
+                Assert.Equal(minerA.BlockChain.Tip, minerB.BlockChain.Tip);
+                Assert.Equal(3, minerA.BlockChain.Count);
+                Assert.Equal(
+                    restage ? null : (Text?)dumbItem,
+                    minerA.BlockChain.GetState(_fx1.Address1));
+                Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address2));
+
+                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);
+                await minerB.BlockAppended.WaitAsync();
+
+                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));
+            }
+            finally
+            {
+                await StopAsync(minerA);
+                await StopAsync(minerB);
+
+                minerA.Dispose();
+                minerB.Dispose();
+            }
+        }
+
         [Fact(Timeout = Timeout)]
         public async Task CreateNewChainWhenBranchPointNotExist()
         {

diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs
@@ -1171,31 +1171,39 @@ internal void Swap(BlockChain<T> other, bool render)
             if (other?.Tip is null)
             {
                 throw new ArgumentException(
-                        $"The chain to be swapped is invalid. Id: {other?.Id}, Tip: {other?.Tip}",
-                        nameof(other));
+                    $"The chain to be swapped is invalid. Id: {other?.Id}, Tip: {other?.Tip}",
+                    nameof(other));
             }
 
             _logger.Debug(
                 "Swapping block chain. (from: {fromChainId}) (to: {toChainId})", Id, other.Id);
 
             // Finds the branch point.
             Block<T> topmostCommon = null;
-            if (render && !(Tip is null))
+            if (!(Tip is null))
             {
                 long shorterHeight =
                     Math.Min(Count, other.Count) - 1;
-                for (
-                    Block<T> t = this[shorterHeight], o = other[shorterHeight];
-                    t.PreviousHash is HashDigest<SHA256> tp &&
-                        o.PreviousHash is HashDigest<SHA256> op;
-                    t = this[tp], o = other[op]
-                )
+                Block<T> t = this[shorterHeight], o = other[shorterHeight];
+
+                while (true)
                 {
                     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
+                    {
+                        break;
+                    }
                 }
             }
 
@@ -1210,7 +1218,7 @@ t.PreviousHash is HashDigest<SHA256> tp &&
                 for (
                     Block<T> b = Tip;
                     !(b is null) && b.Index > (topmostCommon?.Index ?? -1) &&
-                        b.PreviousHash is HashDigest<SHA256> ph;
+                    b.PreviousHash is HashDigest<SHA256> ph;
                     b = this[ph]
                 )
                 {
@@ -1230,6 +1238,19 @@ t.PreviousHash is HashDigest<SHA256> tp &&
                 _logger.Debug($"Unrender for {nameof(Swap)}() is completed.");
             }
 
+            IEnumerable<TxId> GetTxIdsWithRange(BlockChain<T> chain, Block<T> start, Block<T> end)
+                => Enumerable
+                    .Range((int)start.Index + 1, (int)(end.Index - start.Index))
+                    .SelectMany(x => chain[x].Transactions.Select(tx => tx.Id));
+
+            // It assumes reorg is small size. If it was big, this may be heavy task.
+            ImmutableHashSet<TxId> unstagedTxIds =
+                GetTxIdsWithRange(this, topmostCommon, Tip).ToImmutableHashSet();
+            ImmutableHashSet<TxId> stageTxIds =
+                GetTxIdsWithRange(other, topmostCommon, other.Tip).ToImmutableHashSet();
+            ImmutableHashSet<TxId> restageTxIds = unstagedTxIds.Except(stageTxIds);
+            Store.StageTransactionIds(restageTxIds);
+
             try
             {
                 _rwlock.EnterWriteLock();

diff --git a/Menees.Analyzers.Settings.xml b/Menees.Analyzers.Settings.xml
@@ -2,5 +2,5 @@
 <Menees.Analyzers.Settings>
   <MaxLineColumns>100</MaxLineColumns>
   <MaxMethodLines>200</MaxMethodLines>
-  <MaxFileLines>2850</MaxFileLines>
+  <MaxFileLines>2900</MaxFileLines>
 </Menees.Analyzers.Settings>