From b4a735fc94c121084d407c96c8a693c473c03d29 Mon Sep 17 00:00:00 2001 From: moreal Date: Thu, 23 Jan 2020 12:32:21 +0900 Subject: [PATCH 1/3] Stage txs in unrendered block after reorg --- Libplanet.Tests/Net/SwarmTest.cs | 68 +++++++++++++++++++++++++++++- Libplanet/Blockchain/BlockChain.cs | 32 ++++++++------ Menees.Analyzers.Settings.xml | 2 +- 3 files changed, 88 insertions(+), 14 deletions(-) diff --git a/Libplanet.Tests/Net/SwarmTest.cs b/Libplanet.Tests/Net/SwarmTest.cs index d5526f8e0a2..5616099dcc8 100644 --- a/Libplanet.Tests/Net/SwarmTest.cs +++ b/Libplanet.Tests/Net/SwarmTest.cs @@ -2271,7 +2271,7 @@ public async Task PreloadAsyncCancellation(int cancelAfter) BlockChain minerChain = _blockchains[0]; BlockChain receiverChain = _blockchains[1]; - Guid receiverChainId = _blockchains[1].Id; + Guid receiverChainId = receiverChain.Id; (Address address, IEnumerable> blocks) = await MakeFixtureBlocksForPreloadAsyncCancellationTest(); @@ -2437,6 +2437,72 @@ public async Task HandleReorgInSynchronizing() } } + [Fact(Timeout = Timeout)] + public async void RestageTransactionsAfterReorg() + { + var policy = new BlockPolicy(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 + { + await StartAsync(minerA); + await StartAsync(minerB); + + 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), }); + + // 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. + await BootstrapAsync(minerA, minerB.AsPeer); + + Assert.Equal((Text)dumbItem, minerA.BlockChain.GetState(_fx1.Address1)); + Assert.Equal((Text)dumbItem, minerB.BlockChain.GetState(_fx1.Address2)); + + // Occur reorg. + minerB.BroadcastBlock(blockC); + minerA.BlockAppended.Wait(); + + // 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. + Assert.Contains(txA.Id, minerA.BlockChain.GetStagedTransactionIds()); + + await minerA.BlockChain.MineBlock(minerA.Address); + minerA.BroadcastBlock(minerA.BlockChain.Tip); + minerB.BlockAppended.Wait(); + + // 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)); + } + 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 index b9b64b5d7d3..fd7b83aecfc 100644 --- a/Libplanet/Blockchain/BlockChain.cs +++ b/Libplanet/Blockchain/BlockChain.cs @@ -1171,8 +1171,8 @@ internal void Swap(BlockChain 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( @@ -1180,20 +1180,15 @@ internal void Swap(BlockChain other, bool render) // Finds the branch point. Block topmostCommon = null; - if (render && !(Tip is null)) + if (!(Tip is null)) { long shorterHeight = Math.Min(Count, other.Count) - 1; - for ( - Block t = this[shorterHeight], o = other[shorterHeight]; - t.PreviousHash is HashDigest tp && - o.PreviousHash is HashDigest op; - t = this[tp], o = other[op] - ) + for (long index = shorterHeight; index >= 0; --index) { - if (t.Equals(o)) + if (this[index].Equals(other[index])) { - topmostCommon = t; + topmostCommon = this[index]; break; } } @@ -1210,7 +1205,7 @@ t.PreviousHash is HashDigest tp && for ( Block b = Tip; !(b is null) && b.Index > (topmostCommon?.Index ?? -1) && - b.PreviousHash is HashDigest ph; + b.PreviousHash is HashDigest ph; b = this[ph] ) { @@ -1230,6 +1225,19 @@ t.PreviousHash is HashDigest tp && _logger.Debug($"Unrender for {nameof(Swap)}() is completed."); } + IEnumerable GetTxIdsWithRange(BlockChain chain, Block start, Block 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. + var unstagedTxIds = + GetTxIdsWithRange(this, topmostCommon, Tip).ToImmutableHashSet(); + var stageTxIds = + GetTxIdsWithRange(other, topmostCommon, other.Tip).ToImmutableHashSet(); + var restageTxIds = unstagedTxIds.Except(stageTxIds); + Store.StageTransactionIds(restageTxIds); + try { _rwlock.EnterWriteLock(); diff --git a/Menees.Analyzers.Settings.xml b/Menees.Analyzers.Settings.xml index 381b2776609..cca8ce6048d 100644 --- a/Menees.Analyzers.Settings.xml +++ b/Menees.Analyzers.Settings.xml @@ -2,5 +2,5 @@ 100 200 - 2850 + 2900 From 6642bbd5309d42d1c3e2dadd4dc0cf02ed9842e4 Mon Sep 17 00:00:00 2001 From: moreal Date: Thu, 23 Jan 2020 12:49:29 +0900 Subject: [PATCH 2/3] Update changelog --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index e9298942d79..969f44d4282 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -188,6 +188,8 @@ To be released. [[#759]] - Fixed a bug where `BlockChain` 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]] [#368]: https://github.com/planetarium/libplanet/issues/368 [#570]: https://github.com/planetarium/libplanet/issues/570 @@ -241,6 +243,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 From ace2177feb1d401a53ffb09007f80c783e7182ff Mon Sep 17 00:00:00 2001 From: moreal Date: Mon, 3 Feb 2020 14:32:59 +0900 Subject: [PATCH 3/3] Apply suggestions Co-Authored-By: Hong Minhee Co-Authored-By: Ko Chanhyuck --- CHANGES.md | 6 ++-- Libplanet.Tests/Net/SwarmTest.cs | 47 ++++++++++++++++++------------ Libplanet/Blockchain/BlockChain.cs | 25 ++++++++++++---- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 969f44d4282..0eddf6f09af 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -188,8 +188,10 @@ To be released. [[#759]] - Fixed a bug where `BlockChain` 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`'s bug that some `Transaction`s had become excluded from + mining `Block`s after reorg from α to β where a `Transaction` was once + included by a `Block` (α) and not included by an other `Block` (β) 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 index 5616099dcc8..e886ab0115d 100644 --- 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(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>.Empty.Add(txA)); + } - // Check each states. - await BootstrapAsync(minerA, minerB.AsPeer); + Log.Debug("Make minerB's chain longer than minerA's chain."); + Block blockA = await minerA.BlockChain.MineBlock(minerA.Address); + Block blockB = await minerB.BlockChain.MineBlock(minerB.Address); + Block 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 index fd7b83aecfc..4d597dea754 100644 --- a/Libplanet/Blockchain/BlockChain.cs +++ b/Libplanet/Blockchain/BlockChain.cs @@ -1184,11 +1184,24 @@ internal void Swap(BlockChain other, bool render) { long shorterHeight = Math.Min(Count, other.Count) - 1; - for (long index = shorterHeight; index >= 0; --index) + Block 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 tp && + o.PreviousHash is HashDigest op) + { + t = this[tp]; + o = other[op]; + } + else { - topmostCommon = this[index]; break; } } @@ -1231,11 +1244,11 @@ IEnumerable GetTxIdsWithRange(BlockChain chain, Block start, Block 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 unstagedTxIds = GetTxIdsWithRange(this, topmostCommon, Tip).ToImmutableHashSet(); - var stageTxIds = + ImmutableHashSet stageTxIds = GetTxIdsWithRange(other, topmostCommon, other.Tip).ToImmutableHashSet(); - var restageTxIds = unstagedTxIds.Except(stageTxIds); + ImmutableHashSet restageTxIds = unstagedTxIds.Except(stageTxIds); Store.StageTransactionIds(restageTxIds); try