Merge pull request #3949 from greymistcube/refactor/message-encoding

♻️ Changed encoding of `GetBlockHashesMsg` and `BlockHashesMsg`

CHANGES.md

@@ -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
@@ -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

src/Libplanet.Net/Messages/BlockHashesMsg.cs

@@ -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)
@@ -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]));
  }
@@ -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>
@@ -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;
  }
  }

src/Libplanet.Net/Messages/GetBlockHashesMsg.cs

@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using Libplanet.Blockchain;
 using Libplanet.Types.Blocks;
@@ -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; }
@@ -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;
  }
  }

src/Libplanet.Net/Messages/MessageContent.cs

@@ -31,7 +31,7 @@ public enum MessageType : byte
  /// <summary>
  /// Request to query block hashes.
  /// </summary>
- GetBlockHashes = 0x04,
+ GetBlockHashes = 0x032,
 
  /// <summary>
  /// Inventory to transfer transactions.
@@ -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.

src/Libplanet.Net/Swarm.MessageHandlers.cs

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

src/Libplanet/Blockchain/BlockChain.cs

@@ -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)
  {
@@ -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>();
@@ -705,7 +704,7 @@ public IImmutableSet<TxId> GetStagedTransactionIds()
  Store.ListChainIds().Count(),
  stopwatch.ElapsedMilliseconds);
 
- return new Tuple<long?, IReadOnlyList<BlockHash>>(branchpointIndex, result);
+ return result;
  }
 
  /// <summary>
@@ -1190,34 +1189,19 @@ internal void AppendStateRootHashPreceded(
  /// <see langword="null"/>.</returns>
  internal BlockHash? FindBranchpoint(BlockLocator locator)
  {
- try
+ if (ContainsBlock(locator.Hash))
  {
- _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)))
- {
- _logger.Debug(
- "Found a branchpoint with locator [{LocatorHead}]: {Hash}",
- locator.Hash,
- hash);
- return hash;
- }
-
  _logger.Debug(
- "Failed to find a branchpoint locator [{LocatorHead}]",
+ "Found a branchpoint with locator [{LocatorHead}]: {Hash}",
+ locator.Hash,
  locator.Hash);
- return null;
- }
- finally
- {
- _rwlock.ExitReadLock();
+ return locator.Hash;
  }
+
+ _logger.Debug(
+ "Failed to find a branchpoint locator [{LocatorHead}]",
+ locator.Hash);
+ return null;
  }
 
  /// <summary>

test/Libplanet.Net.Tests/Messages/BlockHashesTest.cs

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

test/Libplanet.Net.Tests/Messages/NetMQMessageCodecTest.cs

@@ -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:

test/Libplanet.Tests/Blockchain/BlockChainTest.cs

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