Skip to content

Commit

Permalink
Merge pull request #3834 from greymistcube/refactor/stricter-swap-con…
Browse files Browse the repository at this point in the history 
…dition

♻️ Stricter `Swap()` condition
  • Loading branch information
@greymistcube
greymistcube authored Jun 20, 2024
2 parents 48364a0 + 45bfff1 commit 1a9ce89
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 81 deletions.
15 changes: 9 additions & 6 deletions Libplanet.Net.Tests/SwarmTest.Preload.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ public async Task GetDemandBlockHashesDuringReorg()
}

BlockChain forked = minerChain.Fork(minerChain.Genesis.Hash);
while (forked.Count <= minerChain.Count)
while (forked.Count <= minerChain.Count + 1)
{
Block block = forked.ProposeBlock(
minerKey, CreateBlockCommit(forked.Tip));
Expand All @@ -921,7 +921,10 @@ public async Task GetDemandBlockHashesDuringReorg()
s.ReceivedBlockHashCount > minerChain.Count / 2)
{
receivedCount = s.ReceivedBlockHashCount;
minerChain.Swap(forked, render: false);
if (minerChain.Count < forked.Count)
{
minerChain.Swap(forked, render: false);
}
}
}),
cancellationToken: CancellationToken.None
Expand Down Expand Up @@ -1107,14 +1110,14 @@ public async Task ActionExecutionWithBranchpoint()
}

var forked = seedChain.Fork(seedChain[5].Hash);
seedChain.Swap(forked, false);
for (int i = 0; i < 10; i++)
{
Block block = seedChain.ProposeBlock(
seedKey, CreateBlockCommit(seedChain.Tip));
seedChain.Append(block, TestUtils.CreateBlockCommit(block));
Block block = forked.ProposeBlock(
seedKey, CreateBlockCommit(forked.Tip));
forked.Append(block, TestUtils.CreateBlockCommit(block));
}

seedChain.Swap(forked, false);
var actionExecutionCount = 0;

var progress = new ActionProgress<BlockSyncState>(state =>
Expand Down
56 changes: 37 additions & 19 deletions Libplanet.Net/Swarm.BlockCandidate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,28 +98,46 @@ private bool BlockCandidateProcess(
return false;
}

if (synced is { } syncedB
&& !syncedB.Id.Equals(BlockChain?.Id)
&& BlockChain.Tip.Index < syncedB.Tip.Index)
try
{
_logger.Debug(
"Swapping chain {ChainIdA} with chain {ChainIdB}...",
BlockChain.Id,
synced.Id
);
renderSwap = BlockChain.Swap(
synced,
render: render);
_logger.Debug(
"Swapped chain {ChainIdA} with chain {ChainIdB}",
// Although highly unlikely, current block chain's tip can change.
if (synced is { } syncedB
&& !syncedB.Id.Equals(BlockChain?.Id)
&& BlockChain.Tip.Index < syncedB.Tip.Index)
{
_logger.Debug(
"Swapping chain {ChainIdA} with chain {ChainIdB}...",
BlockChain.Id,
synced.Id
);
renderSwap = BlockChain.Swap(
synced,
render: render);
_logger.Debug(
"Swapped chain {ChainIdA} with chain {ChainIdB}",
BlockChain.Id,
synced.Id
);

renderSwap();
BroadcastBlock(BlockChain.Tip);
return true;
}
else
{
return false;
}
}
catch (Exception e)
{
_logger.Error(
e,
"{MethodName}() has failed to swap chain {ChainIdA} with chain {ChainIdB}",
nameof(BlockCandidateProcess),
BlockChain.Id,
synced.Id
);
synced.Id);
return false;
}

renderSwap();
BroadcastBlock(BlockChain.Tip);
return true;
}

private BlockChain AppendPreviousBlocks(
Expand Down
110 changes: 73 additions & 37 deletions Libplanet.Tests/Blockchain/BlockChainTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,10 @@ public void ForkAndSwapCanonicity()
{
// Fork is not canonical.
var workspace = _blockChain.Fork(_blockChain.Genesis.Hash);
var b = workspace.ProposeBlock(
new PrivateKey(),
lastCommit: CreateBlockCommit(workspace.Tip));
workspace.Append(b, CreateBlockCommit(b));
Assert.True(_blockChain.IsCanonical);
Assert.False(workspace.IsCanonical);

Expand Down Expand Up @@ -820,15 +824,15 @@ public void Swap(bool render)
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[0], "foo")),
DumbAction.Create((addresses[0], "2-0")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 2,
privateKey: privateKey),
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[1], "bar")),
DumbAction.Create((addresses[1], "2-1")),
},
timestamp: DateTimeOffset.MinValue.AddSeconds(3),
nonce: 3,
Expand All @@ -839,15 +843,15 @@ public void Swap(bool render)
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[2], "baz")),
DumbAction.Create((addresses[2], "3-0")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 4,
privateKey: privateKey),
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[3], "qux")),
DumbAction.Create((addresses[3], "3-1")),
},
timestamp: DateTimeOffset.MinValue.AddSeconds(4),
nonce: 5,
Expand All @@ -862,35 +866,63 @@ public void Swap(bool render)
_blockChain.Append(b, CreateBlockCommit(b));
}

Transaction[] txsB =
Transaction[][] txsB =
{
// block #2'
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[0], "fork-foo")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 2,
privateKey: privateKey),
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[1], "fork-bar")),
DumbAction.Create((addresses[2], "fork-baz")),
},
timestamp: DateTimeOffset.MinValue.AddSeconds(2),
nonce: 3,
privateKey: privateKey),
new[]
{
// block #2'
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[0], "2'-0")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 2,
privateKey: privateKey),
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[1], "2'-1-0")),
DumbAction.Create((addresses[2], "2'-1-1")),
},
timestamp: DateTimeOffset.MinValue.AddSeconds(2),
nonce: 3,
privateKey: privateKey),
},
new[]
{
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[0], "3'-0")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 4,
privateKey: privateKey),
},
new[]
{
_fx.MakeTransaction(
new[]
{
DumbAction.Create((addresses[0], "4'-0")),
},
timestamp: DateTimeOffset.MinValue,
nonce: 5,
privateKey: privateKey),
},
};

Block forkTip = fork.ProposeBlock(
miner, txsB.ToImmutableList(), CreateBlockCommit(fork.Tip));
fork.Append(forkTip, CreateBlockCommit(forkTip), render: true);
foreach (Transaction[] txs in txsB)
{
Block b = fork.ProposeBlock(
miner, txs.ToImmutableList(), CreateBlockCommit(fork.Tip));
fork.Append(b, CreateBlockCommit(b), render: true);
}

Guid previousChainId = _blockChain.Id;
_renderer.ResetRecords();
_blockChain.Swap(fork, render)(); // #3 -> #2 -> #1 -> #2'
_blockChain.Swap(fork, render)(); // #3 -> #2 -> #1 -> #2' -> #3' -> #4'

Assert.Empty(_blockChain.Store.IterateIndexes(previousChainId));
Assert.Empty(_blockChain.Store.ListTxNonces(previousChainId));
Expand All @@ -904,10 +936,8 @@ public void Swap(bool render)
.ToArray();
DumbAction[] actions = actionRenders.Select(r => ToDumbAction(r.Action)).ToArray();

int actionsCountA = txsA.Sum(
a => a.Sum(tx => tx.Actions.Count)
);
int actionsCountB = txsB.Sum(tx => tx.Actions.Count);
int actionsCountA = txsA.Sum(a => a.Sum(tx => tx.Actions.Count));
int actionsCountB = txsB.Sum(b => b.Sum(tx => tx.Actions.Count));

int totalBlockCount = (int)_blockChain.Tip.Index + 1;

Expand All @@ -927,9 +957,11 @@ public void Swap(bool render)
Assert.Equal(0, actionRenders.Count(r => r.Unrender));
Assert.True(actionRenders.All(r => r.Render));

Assert.Equal("fork-foo", actions[0].Append?.Item);
Assert.Equal("fork-bar", actions[1].Append?.Item);
Assert.Equal("fork-baz", actions[2].Append?.Item);
Assert.Equal("2'-0", actions[0].Append?.Item);
Assert.Equal("2'-1-0", actions[1].Append?.Item);
Assert.Equal("2'-1-1", actions[2].Append?.Item);
Assert.Equal("3'-0", actions[3].Append?.Item);
Assert.Equal("4'-0", actions[4].Append?.Item);

RenderRecord.ActionBase[] blockActionRenders = _renderer.ActionRecords
.Where(r => IsMinerReward(r.Action))
Expand All @@ -943,7 +975,7 @@ public void Swap(bool render)
.GetAccountState(ReservedAddresses.LegacyAccount)
.GetState(minerAddress)
);
Assert.Single(blockActionRenders); // #1 -> #2'
Assert.Equal(3, blockActionRenders.Length); // #1 -> #2' -> #3' -> #4'
Assert.True(blockActionRenders.All(r => r.Render));
}
else
Expand Down Expand Up @@ -1024,11 +1056,11 @@ public void CleanupBlockCommitStore()
[SkippableTheory]
[InlineData(true)]
[InlineData(false)]
public void SwapForSameTip(bool render)
public void CannotSwapForSameHeightTip(bool render)
{
BlockChain fork = _blockChain.Fork(_blockChain.Tip.Hash);
IReadOnlyList<RenderRecord> prevRecords = _renderer.Records;
_blockChain.Swap(fork, render: render)();
Assert.Throws<ArgumentException>(() => _blockChain.Swap(fork, render: render)());

// Render methods should be invoked if and only if the tip changes
Assert.Equal(prevRecords, _renderer.Records);
Expand Down Expand Up @@ -1069,6 +1101,10 @@ public void ReorgIsUnableToHeterogenousChain(bool render)
chain2.Append(block2, CreateBlockCommit(block2));
}

Block extraBlock2 = chain2.ProposeBlock(
key, lastCommit: CreateBlockCommit(chain2.Tip));
chain2.Append(extraBlock2, CreateBlockCommit(extraBlock2));

Log.Logger.CompareBothChains(
LogEventLevel.Debug,
nameof(_blockChain),
Expand Down
35 changes: 16 additions & 19 deletions Libplanet/Blockchain/BlockChain.Swap.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,26 @@ internal System.Action Swap(
{
throw new ArgumentNullException(nameof(other));
}

if (Tip.Equals(other.Tip))
{
// If it's swapped for a chain with the same tip, it means there is no state change.
// Hence render is unnecessary.
render = false;
}
else
else if (other.Tip.Index <= Tip.Index)
{
_logger.Debug(
"The blockchain was reorged from " +
"{OldChainId} (#{OldTipIndex} {OldTipHash}) " +
"to {NewChainId} (#{NewTipIndex} {NewTipHash})",
Id,
Tip.Index,
Tip.Hash,
other.Id,
other.Tip.Index,
other.Tip.Hash);
throw new ArgumentException(
$"Given {nameof(other)}'s tip index ({other.Tip.Index}) must be " +
$"greater than the current one ({Tip.Index}) for {nameof(Swap)}.",
nameof(other));
}

System.Action renderSwap = () => { };
_logger.Debug(
"The blockchain was reorged from " +
"{OldChainId} (#{OldTipIndex} {OldTipHash}) " +
"to {NewChainId} (#{NewTipIndex} {NewTipHash})",
Id,
Tip.Index,
Tip.Hash,
other.Id,
other.Tip.Index,
other.Tip.Hash);

System.Action renderSwap = () => { };
_rwlock.EnterUpgradeableReadLock();
try
{
Expand Down

0 comments on commit 1a9ce89

Please sign in to comment.