Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Changed encoding of GetBlockHashesMsg and BlockHashesMsg #3949

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ To be released.
- (Libplanet.Net) Changed `BlockHashDownloadState` and `BlockDownloadState`
to be `Obsolete`. [[#3943]]
- Removed `Fork()` method from `BlockChain`. [[#3948]]
- Changed the return type for `BlockChain.FindNextHashes()`
to `IReadOnlyList<BlockHash>`. [[#3949]]

### Backward-incompatible network protocol changes

- (Libplanet.Net) Changed the encoding for `GetBlockHashesMsg` and
`BlockHashesMsg`. [[#3949]]

### Backward-incompatible storage format changes

### Added APIs
Expand Down Expand Up @@ -57,6 +62,7 @@ To be released.
[#3942]: https://github.com/planetarium/libplanet/pull/3942
[#3943]: https://github.com/planetarium/libplanet/pull/3943
[#3948]: https://github.com/planetarium/libplanet/pull/3948
[#3949]: https://github.com/planetarium/libplanet/pull/3949


Version 5.2.2
Expand Down
27 changes: 3 additions & 24 deletions src/Libplanet.Net/Messages/BlockHashesMsg.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,9 @@ namespace Libplanet.Net.Messages
{
internal class BlockHashesMsg : MessageContent
{
public BlockHashesMsg(long? startIndex, IEnumerable<BlockHash> hashes)
public BlockHashesMsg(IEnumerable<BlockHash> hashes)
{
StartIndex = startIndex;
Hashes = hashes.ToList();

if (StartIndex is null == Hashes.Any())
{
throw new ArgumentException(
"The startIndex can be null iff hashes are empty.",
nameof(startIndex)
);
}
}

public BlockHashesMsg(byte[][] dataFrames)
Expand All @@ -28,8 +19,7 @@ public BlockHashesMsg(byte[][] dataFrames)
var hashes = new List<BlockHash>(hashCount);
if (hashCount > 0)
{
StartIndex = BitConverter.ToInt64(dataFrames[1], 0);
for (int i = 2, end = hashCount + 2; i < end; i++)
for (int i = 1, end = hashCount + 1; i < end; i++)
{
hashes.Add(new BlockHash(dataFrames[i]));
}
Expand All @@ -38,12 +28,6 @@ public BlockHashesMsg(byte[][] dataFrames)
Hashes = hashes;
}

/// <summary>
/// The block index of the first hash in <see cref="Hashes"/>.
/// It is <see langword="null"/> iff <see cref="Hashes"/> are empty.
/// </summary>
public long? StartIndex { get; }

/// <summary>
/// The continuous block hashes, from the lowest index to the highest index.
/// </summary>
Expand All @@ -58,12 +42,7 @@ public override IEnumerable<byte[]> DataFrames
{
var frames = new List<byte[]>();
frames.Add(BitConverter.GetBytes(Hashes.Count()));
if (StartIndex is { } offset)
{
frames.Add(BitConverter.GetBytes(offset));
frames.AddRange(Hashes.Select(hash => hash.ToByteArray()));
}

frames.AddRange(Hashes.Select(hash => hash.ToByteArray()));
return frames;
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/Libplanet.Net/Messages/GetBlockHashesMsg.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Collections.Generic;
using Libplanet.Blockchain;
using Libplanet.Types.Blocks;
Expand All @@ -14,7 +13,7 @@ public GetBlockHashesMsg(BlockLocator locator)

public GetBlockHashesMsg(byte[][] dataFrames)
{
Locator = new BlockLocator(new BlockHash(dataFrames[1]));
Locator = new BlockLocator(new BlockHash(dataFrames[0]));
}

public BlockLocator Locator { get; }
Expand All @@ -26,9 +25,7 @@ public override IEnumerable<byte[]> DataFrames
get
{
var frames = new List<byte[]>();
frames.Add(BitConverter.GetBytes(1));
frames.Add(Locator.Hash.ToByteArray());
frames.Add(Array.Empty<byte>());
return frames;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Libplanet.Net/Messages/MessageContent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public enum MessageType : byte
/// <summary>
/// Request to query block hashes.
/// </summary>
GetBlockHashes = 0x04,
GetBlockHashes = 0x032,

/// <summary>
/// Inventory to transfer transactions.
Expand Down Expand Up @@ -76,7 +76,7 @@ public enum MessageType : byte
/// <summary>
/// Message containing demand block hashes with their index numbers.
/// </summary>
BlockHashes = 0x0e,
BlockHashes = 0x33,

/// <summary>
/// Request current chain status of the peer.
Expand Down
13 changes: 4 additions & 9 deletions src/Libplanet.Net/Swarm.MessageHandlers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,15 @@ private Task ProcessMessageHandlerAsync(Message message)
"Received a {MessageType} message locator [{LocatorHead}]",
nameof(GetBlockHashesMsg),
getBlockHashes.Locator.Hash);
BlockChain.FindNextHashes(
IReadOnlyList<BlockHash> hashes = BlockChain.FindNextHashes(
getBlockHashes.Locator,
FindNextHashesChunkSize
).Deconstruct(
out long? offset,
out IReadOnlyList<BlockHash> hashes
);
FindNextHashesChunkSize);
_logger.Debug(
"Found {HashCount} hashes after the branchpoint (offset: {Offset}) " +
"Found {HashCount} hashes after the branchpoint " +
"with locator [{LocatorHead}]",
hashes.Count,
offset,
getBlockHashes.Locator.Hash);
var reply = new BlockHashesMsg(offset, hashes);
var reply = new BlockHashesMsg(hashes);

return Transport.ReplyMessageAsync(reply, message.Identity, default);
}
Expand Down
27 changes: 10 additions & 17 deletions src/Libplanet/Blockchain/BlockChain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -662,11 +662,10 @@ public IImmutableSet<TxId> GetStagedTransactionIds()
/// <param name="locator">The <see cref="BlockLocator"/> to find the branching point
/// from.</param>
/// <param name="count">The Maximum number of <see cref="BlockHash"/>es to return.</param>
/// <returns>A tuple of the index of the branch point and <see cref="BlockHash"/>es
/// including the branch point <see cref="BlockHash"/>. If no branch point is found,
/// returns a tuple of <see langword="null"/> and an empty array of
/// <see cref="BlockHash"/>es.</returns>
public Tuple<long?, IReadOnlyList<BlockHash>> FindNextHashes(
/// <returns>A list of <see cref="BlockHash"/>es including
/// the branch point <see cref="BlockHash"/>. If no branch point is found,
/// returns an empty list of <see cref="BlockHash"/>es.</returns>
public IReadOnlyList<BlockHash> FindNextHashes(
BlockLocator locator,
int count = 500)
{
Expand All @@ -675,12 +674,12 @@ public IImmutableSet<TxId> GetStagedTransactionIds()

if (!(FindBranchpoint(locator) is { } branchpoint))
{
return new Tuple<long?, IReadOnlyList<BlockHash>>(null, Array.Empty<BlockHash>());
return Array.Empty<BlockHash>();
}

if (!(Store.GetBlockIndex(branchpoint) is { } branchpointIndex))
{
return new Tuple<long?, IReadOnlyList<BlockHash>>(null, Array.Empty<BlockHash>());
return Array.Empty<BlockHash>();
}

var result = new List<BlockHash>();
Expand All @@ -705,7 +704,7 @@ public IImmutableSet<TxId> GetStagedTransactionIds()
Store.ListChainIds().Count(),
stopwatch.ElapsedMilliseconds);

return new Tuple<long?, IReadOnlyList<BlockHash>>(branchpointIndex, result);
return result;
}

/// <summary>
Expand Down Expand Up @@ -1194,19 +1193,13 @@ internal void AppendStateRootHashPreceded(
{
_rwlock.EnterReadLock();

_logger.Debug(
"Finding a branchpoint with locator [{LocatorHead}]",
locator.Hash);
BlockHash hash = locator.Hash;
if (_blocks.ContainsKey(hash)
&& _blocks[hash] is Block block
&& hash.Equals(Store.IndexBlockHash(Id, block.Index)))
if (ContainsBlock(locator.Hash))
{
_logger.Debug(
"Found a branchpoint with locator [{LocatorHead}]: {Hash}",
locator.Hash,
hash);
return hash;
locator.Hash);
return locator.Hash;
}

_logger.Debug(
Expand Down
15 changes: 1 addition & 14 deletions test/Libplanet.Net.Tests/Messages/BlockHashesTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,11 @@ public void Dispose()
NetMQConfig.Cleanup(false);
}

[Fact]
public void Constructor()
{
Assert.Throws<ArgumentException>(() =>
new BlockHashesMsg(null, new[] { default(BlockHash) })
);
Assert.Throws<ArgumentException>(() =>
new BlockHashesMsg(123, new BlockHash[0])
);
}

[Fact]
public void Decode()
{
BlockHash[] blockHashes = GenerateRandomBlockHashes(100L).ToArray();
var msg = new BlockHashesMsg(123, blockHashes);
Assert.Equal(123, msg.StartIndex);
var msg = new BlockHashesMsg(blockHashes);
Assert.Equal(blockHashes, msg.Hashes);
var privateKey = new PrivateKey();
AppProtocolVersion apv = AppProtocolVersion.Sign(privateKey, 3);
Expand All @@ -48,7 +36,6 @@ public void Decode()
peer,
DateTimeOffset.UtcNow);
BlockHashesMsg restored = (BlockHashesMsg)messageCodec.Decode(encoded, true).Content;
Assert.Equal(msg.StartIndex, restored.StartIndex);
Assert.Equal(msg.Hashes, restored.Hashes);
}

Expand Down
2 changes: 1 addition & 1 deletion test/Libplanet.Net.Tests/Messages/NetMQMessageCodecTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private MessageContent CreateMessage(MessageContent.MessageType type)
case MessageContent.MessageType.BlockHeaderMessage:
return new BlockHeaderMsg(genesis.Hash, genesis.Header);
case MessageContent.MessageType.BlockHashes:
return new BlockHashesMsg(0, new[] { genesis.Hash });
return new BlockHashesMsg(new[] { genesis.Hash });
case MessageContent.MessageType.GetChainStatus:
return new GetChainStatusMsg();
case MessageContent.MessageType.ChainStatus:
Expand Down
17 changes: 4 additions & 13 deletions test/Libplanet.Tests/Blockchain/BlockChainTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,9 @@ public void RenderActionsAfterAppendComplete()
public void FindNextHashes()
{
var key = new PrivateKey();
long? offsetIndex;
IReadOnlyList<BlockHash> hashes;

_blockChain.FindNextHashes(
new BlockLocator(_blockChain.Genesis.Hash))
.Deconstruct(out offsetIndex, out hashes);
hashes = _blockChain.FindNextHashes(new BlockLocator(_blockChain.Genesis.Hash));
Assert.Single(hashes);
Assert.Equal(_blockChain.Genesis.Hash, hashes.First());
var block0 = _blockChain.Genesis;
Expand All @@ -451,19 +448,13 @@ public void FindNextHashes()
key, lastCommit: CreateBlockCommit(_blockChain.Tip));
_blockChain.Append(block3, CreateBlockCommit(block3));

_blockChain.FindNextHashes(new BlockLocator(block0.Hash))
.Deconstruct(out offsetIndex, out hashes);
Assert.Equal(0, offsetIndex);
hashes = _blockChain.FindNextHashes(new BlockLocator(block0.Hash));
Assert.Equal(new[] { block0.Hash, block1.Hash, block2.Hash, block3.Hash }, hashes);

_blockChain.FindNextHashes(new BlockLocator(block1.Hash))
.Deconstruct(out offsetIndex, out hashes);
Assert.Equal(1, offsetIndex);
hashes = _blockChain.FindNextHashes(new BlockLocator(block1.Hash));
Assert.Equal(new[] { block1.Hash, block2.Hash, block3.Hash }, hashes);

_blockChain.FindNextHashes(new BlockLocator(block0.Hash), count: 2)
.Deconstruct(out offsetIndex, out hashes);
Assert.Equal(0, offsetIndex);
hashes = _blockChain.FindNextHashes(new BlockLocator(block0.Hash), count: 2);
Assert.Equal(new[] { block0.Hash, block1.Hash }, hashes);
}

Expand Down
Loading