From a43d8938bda22c47653876ffb722f4d8069f93bd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Wed, 25 Nov 2020 16:40:23 +0900 Subject: [PATCH] Fix BlockChain.Append() to disallow block with too many txs In the commit 1d357776a640839e34d4195299fc09238e4ce5d8, we added IBlockPolicy.MaxTransactionsPerBlock property, but it had only functioned as a default value for BlockChain.MineBlock() method's maxTransactions option. This had misled and been unintended, because BlockChain.Append() still had accept blocks violating its policy (Block having more Transactions than the number a policy allows still can be made with Block.Mine() method). Although this adds BlockExceedingTransactionsException class, since it would be released as a patch release, it's not public yet. We should make it public when it's ported to the upstream main (next major/minor release). --- CHANGES.md | 7 +++ Libplanet.Tests/Blockchain/BlockChainTest.cs | 33 +++++++++++ ...BlockExceedingTransactionsExceptionTest.cs | 28 ++++++++++ Libplanet/Blockchain/BlockChain.cs | 14 ++++- .../BlockExceedingTransactionsException.cs | 56 +++++++++++++++++++ .../InvalidBlockBytesLengthException.cs | 4 +- 6 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 Libplanet.Tests/Blocks/BlockExceedingTransactionsExceptionTest.cs create mode 100644 Libplanet/Blocks/BlockExceedingTransactionsException.cs diff --git a/CHANGES.md b/CHANGES.md index b579a516f29..90b90999354 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,13 @@ Version 0.10.2 To be released. + - Fixed `BlockChain.Append()` method's bug that it had accepted + a `Block` having more `Transactions` than the number specified by + the `IBlockPolicy.MaxTransactionsPerBlock` property. Now it throws + `InvalidBlockException` instead for such case. [[#1104]] + +[#1104]: https://github.com/planetarium/libplanet/pull/1104 + Version 0.10.1 -------------- diff --git a/Libplanet.Tests/Blockchain/BlockChainTest.cs b/Libplanet.Tests/Blockchain/BlockChainTest.cs index f5f0e4cbd4a..b149c438fda 100644 --- a/Libplanet.Tests/Blockchain/BlockChainTest.cs +++ b/Libplanet.Tests/Blockchain/BlockChainTest.cs @@ -704,6 +704,39 @@ public void AppendThrowsInvalidBlockBytesLengthException() Assert.Equal(block1.BytesLength, e.BlockBytesLength); } + [Fact] + public void AppendThrowsBlockExceedingTransactionsException() + { + PrivateKey signer = null; + int nonce = 0; + int maxTxs = _blockChain.Policy.MaxTransactionsPerBlock; + var manyTxs = new List>(); + for (int i = 0; i <= maxTxs; i++) + { + Transaction heavyTx = _fx.MakeTransaction( + nonce: nonce, + privateKey: signer); + manyTxs.Add(heavyTx); + } + + Assert.True(manyTxs.Count > maxTxs); + + var block1 = TestUtils.MineNext( + _blockChain.Genesis, + manyTxs, + difficulty: _blockChain.Policy.GetNextBlockDifficulty(_blockChain), + blockInterval: TimeSpan.FromSeconds(10) + ); + Assert.Equal(manyTxs.Count, block1.Transactions.Count()); + + var e = Assert.Throws(() => + _blockChain.Append(block1) + ); + Assert.IsAssignableFrom(e); + Assert.Equal(maxTxs, e.MaxTransactionsPerBlock); + Assert.Equal(manyTxs.Count, e.ActualTransactions); + } + [Fact] public void AppendWithoutEvaluateActions() { diff --git a/Libplanet.Tests/Blocks/BlockExceedingTransactionsExceptionTest.cs b/Libplanet.Tests/Blocks/BlockExceedingTransactionsExceptionTest.cs new file mode 100644 index 00000000000..bb1d165fe72 --- /dev/null +++ b/Libplanet.Tests/Blocks/BlockExceedingTransactionsExceptionTest.cs @@ -0,0 +1,28 @@ +using System.IO; +using System.Runtime.Serialization.Formatters.Binary; +using Libplanet.Blocks; +using Xunit; + +namespace Libplanet.Tests.Blocks +{ + public class BlockExceedingTransactionsExceptionTest + { + public void Serialization() + { + var e = new BlockExceedingTransactionsException(100, 50, "A message."); + var f = new BinaryFormatter(); + InvalidBlockBytesLengthException e2; + + using (var s = new MemoryStream()) + { + f.Serialize(s, e); + s.Seek(0, SeekOrigin.Begin); + e2 = (InvalidBlockBytesLengthException)f.Deserialize(s); + } + + Assert.Equal(e.Message, e2.Message); + Assert.Equal(e.ActualTransactions, e2.BlockBytesLength); + Assert.Equal(e.MaxTransactionsPerBlock, e2.MaxBlockBytesLength); + } + } +} diff --git a/Libplanet/Blockchain/BlockChain.cs b/Libplanet/Blockchain/BlockChain.cs index 3bc6d45d5c9..2762961ff53 100644 --- a/Libplanet/Blockchain/BlockChain.cs +++ b/Libplanet/Blockchain/BlockChain.cs @@ -565,8 +565,8 @@ public void Append(Block block, StateCompleterSet? stateCompleters = null) /// Thrown when the block to mine is /// too long (according to ) in bytes. /// - /// Thrown when the given - /// is invalid, in itself or according to + /// Thrown when the given + /// is invalid (e.g., too many transactions), in itself or according to /// the . /// Thrown when the /// is different from @@ -1042,6 +1042,16 @@ internal void Append( _logger.Debug("Trying to append block {blockIndex}: {block}", block?.Index, block); + if (block.Transactions.Count() is { } txCount && + txCount > Policy.MaxTransactionsPerBlock) + { + throw new BlockExceedingTransactionsException( + txCount, + Policy.MaxTransactionsPerBlock, + "The block to append has too many transactions." + ); + } + if (block.BytesLength > Policy.GetMaxBlockBytes(block.Index)) { throw new InvalidBlockBytesLengthException( diff --git a/Libplanet/Blocks/BlockExceedingTransactionsException.cs b/Libplanet/Blocks/BlockExceedingTransactionsException.cs new file mode 100644 index 00000000000..06c138d77bb --- /dev/null +++ b/Libplanet/Blocks/BlockExceedingTransactionsException.cs @@ -0,0 +1,56 @@ +#nullable enable +using System; +using System.Runtime.Serialization; +using Libplanet.Blockchain.Policies; + +namespace Libplanet.Blocks +{ + /// + /// The exception that is thrown when a has too many + /// (i.e., more than the number specified by + /// ). + /// + [Serializable] + internal class BlockExceedingTransactionsException : InvalidBlockException, ISerializable + { + public BlockExceedingTransactionsException( + int actualTransactions, + int maxTransactionsPerBlock, + string message + ) + : base( + $"{message}\n" + + $"Actual: {actualTransactions} txs\n" + + $"Allowed (max): {maxTransactionsPerBlock} txs") + { + ActualTransactions = actualTransactions; + MaxTransactionsPerBlock = maxTransactionsPerBlock; + } + + public BlockExceedingTransactionsException(SerializationInfo info, StreamingContext context) + : base(info.GetString(nameof(Message)) ?? string.Empty) + { + ActualTransactions = info.GetInt32(nameof(ActualTransactions)); + MaxTransactionsPerBlock = info.GetInt32(nameof(MaxTransactionsPerBlock)); + } + + /// + /// The actual number of transactions in the block. + /// + public int ActualTransactions { get; set; } + + /// + /// The maximum allowed number of transactions per block. + /// + /// + public int MaxTransactionsPerBlock { get; set; } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue(nameof(Message), Message); + info.AddValue(nameof(ActualTransactions), ActualTransactions); + info.AddValue(nameof(MaxTransactionsPerBlock), MaxTransactionsPerBlock); + } + } +} diff --git a/Libplanet/Blocks/InvalidBlockBytesLengthException.cs b/Libplanet/Blocks/InvalidBlockBytesLengthException.cs index b8ca7531f5f..db33f7344ac 100644 --- a/Libplanet/Blocks/InvalidBlockBytesLengthException.cs +++ b/Libplanet/Blocks/InvalidBlockBytesLengthException.cs @@ -29,8 +29,8 @@ string message ) : base( $"{message}\n" + - $"Actual: #{blockBytesLength} bytes\n" + - $"Allowed (max): #{maxBlockBytesLength}") + $"Actual: {blockBytesLength} bytes\n" + + $"Allowed (max): {maxBlockBytesLength} bytes") { BlockBytesLength = blockBytesLength; MaxBlockBytesLength = maxBlockBytesLength;