Cut off transactions to fit to block's max size

planetarium#201

CHANGES.md

@@ -121,11 +121,14 @@ To be released.
     `Block<T>()` constructor.  [[#986]]
  -  Added `IBlockPolicy<T>.MaxTransactionsPerBlock` property.
     [[#1037], [#1050]]
+ -  Added `IBlockPolicy<T>.GetMaxBlockBytes()` method.  [[#201], [#1050]]
  -  `IBlockPolicy<T>.DoesTransactionFollowPolicy()` method became to take
     additional `BlockChain<T>` parameter as its context.  [[#1012]]
  -  Methods in `BlockPolicy<T>` class became `virtual`.  [[#1010]]
- -  Added `maxTransactionsPerBlock` option to both `BlockPolicy<T>()` overloaded
-    constructors.  [[#1037], [#1050]]
+ -  Added `int maxTransactionsPerBlock` option to both `BlockPolicy<T>()`
+    overloaded constructors.  [[#1037], [#1050]]
+ -  Added `int maxBlockBytes` and `int maxGenesisBytes` options to both
+    `BlockPolicy<T>()` overloaded constructors.  [[#201], [#1050]]
  -  `BlockPolicy<T>()` constructor's `doesTransactionFollowPolicy` parameter
     became `Func<Transaction<T>, BlockChain<T>, bool>` on .  [[#1012]]
  -  Added `cacheSize` optional parameter to `BlockSet<T>()` constructor.
@@ -254,6 +257,9 @@ To be released.
  -  Added `Transaction<T>.GenesisHash` property.  [[#796], [#878]]
  -  Added `IAccountStateDelta.UpdatedAddresses` property contains
     asset updates besides state updates.  [[#861], [#900]]
+ -  `BlockChain<T>.MineBlock()` method became to cut off transactions to include
+    to fit into the limitation configured by `IBlockPolicy.GetMaxBlockBytes()`.
+    [[#201], [#1050]]
  -  `Swarm<T>` became to ignore received transaction with different
     genesis hash.  [[#796], [#878]]
  -  `Swarm<T>` became to ignore invalid `BlockHeader`s immediately.  [[#898]]

Libplanet.Tests/Blockchain/BlockChainTest.ValidateNextBlock.cs

@@ -118,13 +118,7 @@ private void ValidateNextBlockInvalidStateRootHash()
         {
             IKeyValueStore stateKeyValueStore = new MemoryKeyValueStore(),
                 stateHashKeyValueStore = new MemoryKeyValueStore();
-            var policy = new BlockPolicy<DumbAction>(
-                blockAction: null,
-                blockInterval: TimeSpan.FromHours(3),
-                minimumDifficulty: 1024,
-                difficultyBoundDivisor: 128,
-                maxTransactionsPerBlock: 100
-            );
+            var policy = new BlockPolicy<DumbAction>(null, 3 * 60 * 60 * 1000);
             var stateStore = new TrieStateStore(stateKeyValueStore, stateHashKeyValueStore);
             // FIXME: It assumes that _fx.GenesisBlock doesn't update any states with transactions.
             //        Actually, it depends on BlockChain<T> to update states and it makes hard to

Libplanet.Tests/Blockchain/BlockChainTest.cs

@@ -29,6 +29,7 @@ namespace Libplanet.Tests.Blockchain
 {
     public partial class BlockChainTest : IDisposable
     {
+        private readonly ILogger _logger;
         private StoreFixture _fx;
         private BlockPolicy<DumbAction> _policy;
         private BlockChain<DumbAction> _blockChain;
@@ -38,7 +39,7 @@ public partial class BlockChainTest : IDisposable
 
         public BlockChainTest(ITestOutputHelper output)
         {
-            Log.Logger = new LoggerConfiguration()
+            Log.Logger = _logger = new LoggerConfiguration()
                 .MinimumLevel.Verbose()
                 .Enrich.WithThreadId()
                 .WriteTo.TestOutput(output)
@@ -76,22 +77,61 @@ public void Dispose()
         [Fact]
         public async void MineBlock()
         {
+            Func<long, int> getMaxBlockBytes = _blockChain.Policy.GetMaxBlockBytes;
             Assert.Equal(1, _blockChain.Count);
 
             Block<DumbAction> block = await _blockChain.MineBlock(_fx.Address1);
             block.Validate(DateTimeOffset.UtcNow);
             Assert.True(_blockChain.ContainsBlock(block.Hash));
             Assert.Equal(2, _blockChain.Count);
+            Assert.True(block.BytesLength <= getMaxBlockBytes(block.Index));
 
             Block<DumbAction> anotherBlock = await _blockChain.MineBlock(_fx.Address2);
             anotherBlock.Validate(DateTimeOffset.UtcNow);
             Assert.True(_blockChain.ContainsBlock(anotherBlock.Hash));
             Assert.Equal(3, _blockChain.Count);
+            Assert.True(anotherBlock.BytesLength <= getMaxBlockBytes(anotherBlock.Index));
 
             Block<DumbAction> block3 = await _blockChain.MineBlock(_fx.Address3, append: false);
             block3.Validate(DateTimeOffset.UtcNow);
             Assert.False(_blockChain.ContainsBlock(block3.Hash));
             Assert.Equal(3, _blockChain.Count);
+            Assert.True(block3.BytesLength <= getMaxBlockBytes(block3.Index));
+
+            // Tests if MineBlock() method automatically fits the number of transactions according
+            // to the right size.
+            DumbAction[] manyActions =
+                Enumerable.Repeat(new DumbAction(default, "_"), 200).ToArray();
+            PrivateKey signer = null;
+            int nonce = 0;
+            for (int i = 0; i < 100; i++)
+            {
+                if (i % 25 == 0)
+                {
+                    nonce = 0;
+                    signer = new PrivateKey();
+                }
+
+                Transaction<DumbAction> heavyTx = _fx.MakeTransaction(
+                    manyActions,
+                    nonce: nonce,
+                    privateKey: signer);
+                _blockChain.StageTransaction(heavyTx);
+            }
+
+            Block<DumbAction> block4 = await _blockChain.MineBlock(_fx.Address3, append: false);
+            block4.Validate(DateTimeOffset.UtcNow);
+            Assert.False(_blockChain.ContainsBlock(block4.Hash));
+            _logger.Debug(
+                $"{nameof(block4)}.{nameof(block4.BytesLength)} = {0}",
+                block4.BytesLength
+            );
+            _logger.Debug(
+                $"{nameof(getMaxBlockBytes)}({nameof(block4)}.{nameof(block4.Index)}) = {0}",
+                getMaxBlockBytes(block4.Index)
+            );
+            Assert.True(block4.BytesLength <= getMaxBlockBytes(block4.Index));
+            Assert.Equal(4, block4.Transactions.Count());
         }
 
         [Fact]

Libplanet.Tests/Blockchain/NullPolicy.cs

@@ -32,5 +32,7 @@ public long GetNextBlockDifficulty(BlockChain<T> blocks) =>
 
         public InvalidBlockException ValidateNextBlock(BlockChain<T> blocks, Block<T> nextBlock) =>
             _exceptionToThrow;
+
+        public int GetMaxBlockBytes(long index) => 1024 * 1024;
     }
 }

Libplanet.Tests/Blockchain/Policies/BlockPolicyTest.cs

@@ -29,10 +29,7 @@ public BlockPolicyTest(ITestOutputHelper output)
             _output = output;
             _policy = new BlockPolicy<DumbAction>(
                 blockAction: null,
-                blockInterval: TimeSpan.FromHours(3),
-                minimumDifficulty: 1024,
-                difficultyBoundDivisor: 128,
-                maxTransactionsPerBlock: 100
+                blockIntervalMilliseconds: 3 * 60 * 60 * 1000
             );
             _chain = new BlockChain<DumbAction>(
                 _policy,
@@ -50,7 +47,15 @@ public void Dispose()
         public void Constructors()
         {
             var tenSec = new TimeSpan(0, 0, 10);
-            var a = new BlockPolicy<DumbAction>(null, tenSec, 1024, 128, 100);
+            var a = new BlockPolicy<DumbAction>(
+                blockAction: null,
+                blockInterval: tenSec,
+                minimumDifficulty: 1024L,
+                difficultyBoundDivisor: 128,
+                maxTransactionsPerBlock: 100,
+                maxBlockBytes: 100 * 1024,
+                maxGenesisBytes: 1024 * 1024
+            );
             Assert.Equal(tenSec, a.BlockInterval);
 
             var b = new BlockPolicy<DumbAction>(null, 65000);
@@ -63,15 +68,42 @@ public void Constructors()
                 new TimeSpan(0, 0, 5),
                 c.BlockInterval);
 
-            Assert.Throws<ArgumentOutOfRangeException>(
-                () => new BlockPolicy<DumbAction>(null, tenSec.Negate(), 1024, 128, 100));
+            Assert.Throws<ArgumentOutOfRangeException>(() =>
+                new BlockPolicy<DumbAction>(
+                    blockAction: null,
+                    blockInterval: tenSec.Negate(),
+                    minimumDifficulty: 1024,
+                    difficultyBoundDivisor: 128,
+                    maxTransactionsPerBlock: 100,
+                    maxBlockBytes: 100 * 1024,
+                    maxGenesisBytes: 1024 * 1024
+                )
+            );
             Assert.Throws<ArgumentOutOfRangeException>(
                 () => new BlockPolicy<DumbAction>(null, -5));
 
             Assert.Throws<ArgumentOutOfRangeException>(() =>
-                new BlockPolicy<DumbAction>(null, tenSec, 0, 128, 100));
+                new BlockPolicy<DumbAction>(
+                    blockAction: null,
+                    blockInterval: tenSec,
+                    minimumDifficulty: 0,
+                    difficultyBoundDivisor: 128,
+                    maxTransactionsPerBlock: 100,
+                    maxBlockBytes: 100 * 1024,
+                    maxGenesisBytes: 1024 * 1024
+                )
+            );
             Assert.Throws<ArgumentOutOfRangeException>(() =>
-                new BlockPolicy<DumbAction>(null, tenSec, 1024, 1024, 100));
+                new BlockPolicy<DumbAction>(
+                    blockAction: null,
+                    blockInterval: tenSec,
+                    minimumDifficulty: 1024,
+                    difficultyBoundDivisor: 1024,
+                    maxTransactionsPerBlock: 100,
+                    maxBlockBytes: 100 * 1024,
+                    maxGenesisBytes: 1024 * 1024
+                )
+            );
         }
 
         [Fact]

Libplanet/Blockchain/BlockChain.cs

@@ -716,33 +716,86 @@ public async Task<Block<T>> MineBlock(
             long difficulty = Policy.GetNextBlockDifficulty(this);
             HashDigest<SHA256>? prevHash = Store.IndexBlockHash(Id, index - 1);
 
-            IEnumerable<Transaction<T>> stagedTransactions = Store
+            Transaction<T>[] stagedTransactions = Store
                 .IterateStagedTransactionIds()
                 .Select(Store.GetTransaction<T>)
                 .GroupBy(tx => tx.Signer)
                 .SelectMany(grp => grp.Select((tx, idx) => (tx, idx)))
                 .OrderBy(t => t.Item2)
-                .Select(t => t.Item1);
+                .Select(t => t.Item1)
+                .ToArray();
 
             var transactionsToMine = new List<Transaction<T>>();
 
+            // Makes an empty block to estimate the length of bytes without transactions.
+            var estimatedBytes = new Block<T>(
+                index: index,
+                difficulty: difficulty,
+                totalDifficulty: Tip.TotalDifficulty,
+                nonce: default,
+                miner: miner,
+                previousHash: prevHash,
+                timestamp: currentTime,
+                transactions: new Transaction<T>[0]
+            ).BytesLength;
+            int maxBlockBytes = Math.Max(Policy.GetMaxBlockBytes(index), 1);
+            var skippedSigners = new HashSet<Address>();
+
             foreach (Transaction<T> tx in stagedTransactions)
             {
                 if (!Policy.DoesTransactionFollowsPolicy(tx, this))
                 {
                     UnstageTransaction(tx);
                 }
-                else if (Store.GetTxNonce(Id, tx.Signer) <= tx.Nonce
-                         && tx.Nonce < GetNextTxNonce(tx.Signer))
+                else if (!skippedSigners.Contains(tx.Signer) &&
+                         Store.GetTxNonce(Id, tx.Signer) <= tx.Nonce &&
+                         tx.Nonce < GetNextTxNonce(tx.Signer))
                 {
-                    transactionsToMine.Add(tx);
                     if (transactionsToMine.Count >= maxTransactions)
                     {
+                        _logger.Information(
+                            "Not all staged transactions will be included in a block #{Index} to " +
+                            "be mined by {Miner}, because it reaches the maximum number of " +
+                            "acceptable transactions: {MaxTransactions}",
+                            index,
+                            miner,
+                            maxTransactions
+                        );
                         break;
                     }
+
+                    int txBytes = tx.Serialize(true).Length;
+                    if (estimatedBytes + txBytes > maxBlockBytes)
+                    {
+                        // Once someone's tx is excluded from a block, their later txs are also all
+                        // excluded in the block, because later nonces become invalid.
+                        skippedSigners.Add(tx.Signer);
+                        _logger.Information(
+                            "The {Signer}'s transactions after the nonce #{Nonce} will be " +
+                            "excluded in a block #{Index} to be mined by {Miner}, because it " +
+                            "takes too long bytes.",
+                            tx.Signer,
+                            tx.Nonce,
+                            index,
+                            miner
+                        );
+                        continue;
+                    }
+
+                    transactionsToMine.Add(tx);
+                    estimatedBytes += txBytes;
                 }
             }
 
+            _logger.Verbose(
+                "A block #{Index} to be mined by {Miner} will include {Transactions} " +
+                "transactions out of {StagedTransactions} staged transactions.",
+                index,
+                miner,
+                transactionsToMine.Count,
+                stagedTransactions.Length
+            );
+
             CancellationTokenSource cts = new CancellationTokenSource();
             CancellationTokenSource cancellationTokenSource =
                 CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, cts.Token);

Libplanet/Blockchain/Policies/BlockPolicy.cs

@@ -13,6 +13,8 @@ namespace Libplanet.Blockchain.Policies
     public class BlockPolicy<T> : IBlockPolicy<T>
         where T : IAction, new()
     {
+        private readonly int _maxBlockBytes;
+        private readonly int _maxGenesisBytes;
         private readonly Func<Transaction<T>, BlockChain<T>, bool> _doesTransactionFollowPolicy;
 
         /// <summary>
@@ -33,6 +35,10 @@ public class BlockPolicy<T> : IBlockPolicy<T>
         /// <see cref="DifficultyBoundDivisor"/>. 128 by default.</param>
         /// <param name="maxTransactionsPerBlock">Configures <see cref="MaxTransactionsPerBlock"/>.
         /// 100 by default.</param>
+        /// <param name="maxBlockBytes">Configures <see cref="GetMaxBlockBytes(long)"/> where
+        /// the block is not a genesis.  100 KiB by default.</param>
+        /// <param name="maxGenesisBytes">Configures <see cref="GetMaxBlockBytes(long)"/> where
+        /// the block is a genesis.  1 MiB by default.</param>
         /// <param name="doesTransactionFollowPolicy">
         /// A predicate that determines if the transaction follows the block policy.
         /// </param>
@@ -42,13 +48,17 @@ public BlockPolicy(
             long minimumDifficulty = 1024,
             int difficultyBoundDivisor = 128,
             int maxTransactionsPerBlock = 100,
+            int maxBlockBytes = 100 * 1024,
+            int maxGenesisBytes = 1024 * 1024,
             Func<Transaction<T>, BlockChain<T>, bool> doesTransactionFollowPolicy = null)
             : this(
                 blockAction,
                 TimeSpan.FromMilliseconds(blockIntervalMilliseconds),
                 minimumDifficulty,
                 difficultyBoundDivisor,
                 maxTransactionsPerBlock,
+                maxBlockBytes,
+                maxGenesisBytes,
                 doesTransactionFollowPolicy)
         {
         }
@@ -68,6 +78,10 @@ public BlockPolicy(
         /// <see cref="DifficultyBoundDivisor"/>.</param>
         /// <param name="maxTransactionsPerBlock">Configures <see cref="MaxTransactionsPerBlock"/>.
         /// </param>
+        /// <param name="maxBlockBytes">Configures <see cref="GetMaxBlockBytes(long)"/> where
+        /// the block is not a genesis.</param>
+        /// <param name="maxGenesisBytes">Configures <see cref="GetMaxBlockBytes(long)"/> where
+        /// the block is a genesis.</param>
         /// <param name="doesTransactionFollowPolicy">
         /// A predicate that determines if the transaction follows the block policy.
         /// </param>
@@ -77,6 +91,8 @@ public BlockPolicy(
             long minimumDifficulty,
             int difficultyBoundDivisor,
             int maxTransactionsPerBlock,
+            int maxBlockBytes,
+            int maxGenesisBytes,
             Func<Transaction<T>, BlockChain<T>, bool> doesTransactionFollowPolicy = null)
         {
             if (blockInterval < TimeSpan.Zero)
@@ -109,6 +125,8 @@ public BlockPolicy(
             MinimumDifficulty = minimumDifficulty;
             DifficultyBoundDivisor = difficultyBoundDivisor;
             MaxTransactionsPerBlock = maxTransactionsPerBlock;
+            _maxBlockBytes = maxBlockBytes;
+            _maxGenesisBytes = maxGenesisBytes;
             _doesTransactionFollowPolicy = doesTransactionFollowPolicy ?? ((_, __) => true);
         }
 
@@ -184,5 +202,7 @@ public virtual long GetNextBlockDifficulty(BlockChain<T> blocks)
 
             return Math.Max(nextDifficulty, MinimumDifficulty);
         }
+
+        public int GetMaxBlockBytes(long index) => index > 0 ? _maxBlockBytes : _maxGenesisBytes;
     }
 }

Libplanet/Blockchain/Policies/IBlockPolicy.cs

@@ -69,5 +69,17 @@ InvalidBlockException ValidateNextBlock(
         /// <returns>A right <see cref="Block{T}.Difficulty"/>
         /// for a new <see cref="Block{T}"/> to be mined.</returns>
         long GetNextBlockDifficulty(BlockChain<T> blocks);
+
+        /// <summary>
+        /// Gets the maximum length of a <see cref="Block{T}"/> in bytes.  It can vary depending on
+        /// a given <paramref name="index"/>, but should be deterministic; for the same
+        /// <paramref name="index"/>, the same value must be returned.
+        /// </summary>
+        /// <param name="index">An <see cref="Block{T}.Index"/> of a block to mine or receive.
+        /// </param>
+        /// <returns>The maximum length of a <see cref="Block{T}"/> in bytes to accept.</returns>
+        /// <remarks>If it returns less then 1, it is treated as 1, because there is no block
+        /// taking 0 bytes or negative length of bytes.</remarks>
+        int GetMaxBlockBytes(long index);
     }
 }