Fix BlockChain<T>.Append() to disallow block with too many txs

CHANGES.md

@@ -6,6 +6,13 @@ Version 0.10.2
 
 To be released.
 
+ -  Fixed `BlockChain<T>.Append()` method's bug that it had accepted
+    a `Block<T>` having more `Transactions` than the number specified by
+    the `IBlockPolicy<T>.MaxTransactionsPerBlock` property.  Now it throws
+    `InvalidBlockException` instead for such case.  [[#1104]]
+
+[#1104]: https://github.com/planetarium/libplanet/pull/1104
+
 
 Version 0.10.1
 --------------

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<Transaction<DumbAction>>();
+            for (int i = 0; i <= maxTxs; i++)
+            {
+                Transaction<DumbAction> 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<BlockExceedingTransactionsException>(() =>
+                _blockChain.Append(block1)
+            );
+            Assert.IsAssignableFrom<InvalidBlockException>(e);
+            Assert.Equal(maxTxs, e.MaxTransactionsPerBlock);
+            Assert.Equal(manyTxs.Count, e.ActualTransactions);
+        }
+
         [Fact]
         public void AppendWithoutEvaluateActions()
         {

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

Libplanet/Blockchain/BlockChain.cs

@@ -565,8 +565,8 @@ public void Append(Block<T> block, StateCompleterSet<T>? stateCompleters = null)
         /// <exception cref="InvalidBlockBytesLengthException">Thrown when the block to mine is
         /// too long (according to <see cref="IBlockPolicy{T}.GetMaxBlockBytes(long)"/>) in bytes.
         /// </exception>
-        /// <exception cref="InvalidBlockException">Thrown when the given
-        /// <paramref name="block"/> is invalid, in itself or according to
+        /// <exception cref="InvalidBlockException">Thrown when the given <paramref name="block"/>
+        /// is invalid (e.g., too many transactions), in itself or according to
         /// the <see cref="Policy"/>.</exception>
         /// <exception cref="InvalidTxNonceException">Thrown when the
         /// <see cref="Transaction{T}.Nonce"/> 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(

Libplanet/Blocks/BlockExceedingTransactionsException.cs

@@ -0,0 +1,56 @@
+#nullable enable
+using System;
+using System.Runtime.Serialization;
+using Libplanet.Blockchain.Policies;
+
+namespace Libplanet.Blocks
+{
+    /// <summary>
+    /// The exception that is thrown when a <see cref="Block{T}"/> has too many
+    /// <see cref="Block{T}.Transactions"/> (i.e., more than the number specified by
+    /// <see cref="IBlockPolicy{T}.MaxTransactionsPerBlock"/>).
+    /// </summary>
+    [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));
+        }
+
+        /// <summary>
+        /// The actual number of transactions in the block.
+        /// </summary>
+        public int ActualTransactions { get; set; }
+
+        /// <summary>
+        /// The maximum allowed number of transactions per block.
+        /// </summary>
+        /// <seealso cref="IBlockPolicy{T}.MaxTransactionsPerBlock"/>
+        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);
+        }
+    }
+}

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;