Exclude transactions with too-new timestamps from block template

Sources/Stratis.Bitcoin.Features.Miner/BlockDefinition.cs

@@ -214,10 +214,8 @@ namespace Stratis.Bitcoin.Features.Miner
             // transaction (which in most cases can be a no-op).
             this.IncludeWitness = false; //IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx;
 
-            // add transactions from the mempool
-            int nPackagesSelected;
-            int nDescendantsUpdated;
-            this.AddTransactions(out nPackagesSelected, out nDescendantsUpdated);
+            // Add transactions from the mempool
+            this.AddTransactions(out int nPackagesSelected, out int nDescendantsUpdated);
 
             this.LastBlockTx = this.BlockTx;
             this.LastBlockSize = this.BlockSize;
@@ -229,6 +227,9 @@ namespace Stratis.Bitcoin.Features.Miner
             var coinviewRule = this.ConsensusManager.ConsensusRules.GetRule<CoinViewRule>();
             this.coinbase.Outputs[0].Value = this.fees + coinviewRule.GetProofOfWorkReward(this.height);
             this.BlockTemplate.TotalFee = this.fees;
+            
+            // We need the fee details per transaction to be readily available in case we have to remove transactions from the block later.
+            this.BlockTemplate.FeeDetails = this.inBlock.Select(i => new { i.TransactionHash, i.Fee }).ToDictionary(d => d.TransactionHash, d => d.Fee);
 
             int nSerializeSize = this.block.GetSerializedSize();
             this.logger.LogDebug("Serialized size is {0} bytes, block weight is {1}, number of txs is {2}, tx fees are {3}, number of sigops is {4}.", nSerializeSize, coinviewRule.GetBlockWeight(this.block), this.BlockTx, this.fees, this.BlockSigOpsCost);

Sources/Stratis.Bitcoin.Features.Miner/Interfaces/IPosMinting.cs

@@ -3,6 +3,7 @@ using System.Threading;
 using System.Threading.Tasks;
 using NBitcoin;
 using Stratis.Bitcoin.Features.Miner.Staking;
+using Stratis.Bitcoin.Mining;
 
 namespace Stratis.Bitcoin.Features.Miner.Interfaces
 {
@@ -16,13 +17,13 @@ namespace Stratis.Bitcoin.Features.Miner.Interfaces
         /// Creates a coinstake transaction with kernel that satisfies POS staking target.
         /// </summary>
         /// <param name="utxoStakeDescriptions">List of UTXOs that are available in the wallet for staking.</param>
-        /// <param name="block">Template of the block that we are trying to mine.</param>
+        /// <param name="blockTemplate">Template of the block that we are trying to stake.</param>
         /// <param name="chainTip">Tip of the best chain.</param>
         /// <param name="searchInterval">Length of an unexplored block time space in seconds. It only makes sense to look for a solution within this interval.</param>
         /// <param name="fees">Transaction fees from the transactions included in the block if we mine it.</param>
         /// <param name="coinstakeContext">Information about coinstake transaction and its private key that is to be filled when the kernel is found.</param>
         /// <returns><c>true</c> if the function succeeds, <c>false</c> otherwise.</returns>
-        Task<bool> CreateCoinstakeAsync(List<UtxoStakeDescription> utxoStakeDescriptions, Block block, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext);
+        Task<bool> CreateCoinstakeAsync(List<UtxoStakeDescription> utxoStakeDescriptions, BlockTemplate blockTemplate, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext);
 
         /// <summary>
         /// Attempts to stake new blocks in a loop.

Sources/Stratis.Bitcoin.Features.Miner/Staking/DeStreamPosMinting.cs

@@ -59,7 +59,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
 
         /// <inheritdoc/>
         public override async Task<bool> CreateCoinstakeAsync(List<UtxoStakeDescription> utxoStakeDescriptions,
-            Block block, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext)
+            BlockTemplate blockTemplate, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext)
         {
             // Provides PrepareCoinStakeTransactions with fees amount
 
@@ -104,7 +104,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
                 new Money(ourWeight), ourPercent, new Money(this.networkWeight), TimeSpan.FromSeconds(expectedTime));
 
             this.logger.LogInformation("Staking block with transactions: {txIds}",
-                block.Transactions.Select(p => p.GetHash()).ToList());
+                blockTemplate.Block.Transactions.Select(p => p.GetHash()).ToList());
 
             this.rpcGetStakingInfoModel.ResumeStaking(ourWeight, expectedTime);
 
@@ -148,7 +148,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
                 workerContexts[workerIndex] = cwc;
 
                 workers[workerIndex] = Task.Run(() =>
-                    this.CoinstakeWorker(cwc, chainTip, block, minimalAllowedTime, searchInterval));
+                    this.CoinstakeWorker(cwc, chainTip, blockTemplate.Block, minimalAllowedTime, searchInterval));
             }
 
             await Task.WhenAll(workers).ConfigureAwait(false);
@@ -160,6 +160,23 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
             }
 
             this.logger.LogTrace("Worker #{0} found the kernel.", workersResult.KernelFoundIndex);
+            
+            // We have to make sure that we have no future timestamps in our transactions set.
+            // We ignore the coinbase (it gets its timestamp reset after the coinstake is created).
+            for (int i = blockTemplate.Block.Transactions.Count - 1; i >= 1; i--)
+            {
+                // We have not yet updated the header timestamp, so we use the coinstake timestamp directly here.
+                if (blockTemplate.Block.Transactions[i].Time <= coinstakeContext.CoinstakeTx.Time)
+                    continue;
+
+                // Update the total fees, with the to-be-removed transaction taken into account.
+                fees -= blockTemplate.FeeDetails[blockTemplate.Block.Transactions[i].GetHash()].Satoshi;
+
+                this.logger.LogDebug("Removing transaction with timestamp {0} as it is greater than coinstake transaction timestamp {1}. New fee amount {2}.", blockTemplate.Block.Transactions[i].Time, coinstakeContext.CoinstakeTx.Time, fees);
+                blockTemplate.Block.Transactions.Remove(blockTemplate.Block.Transactions[i]);
+            }
+
+            
             // Input to coinstake transaction.
             UtxoStakeDescription coinstakeInput = workersResult.KernelCoin;
 

Sources/Stratis.Bitcoin.Features.Miner/Staking/PosMinting.cs

-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
@@ -402,7 +402,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
                 this.rpcGetStakingInfoModel.NetStakeWeight = this.networkWeight;
 
                 // Trying to create coinstake that satisfies the difficulty target, put it into a block and sign the block.
-                if (await this.StakeAndSignBlockAsync(utxoStakeDescriptions, posBlock, chainTip, blockTemplate.TotalFee, coinstakeTimestamp).ConfigureAwait(false))
+                if (await this.StakeAndSignBlockAsync(utxoStakeDescriptions, blockTemplate, chainTip, blockTemplate.TotalFee, coinstakeTimestamp).ConfigureAwait(false))
                 {
                     this.logger.LogTrace("New POS block created and signed successfully.");
                     await this.CheckStakeAsync(posBlock, chainTip).ConfigureAwait(false);
@@ -505,13 +505,15 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
         /// to be mined and signes it.
         /// </summary>
         /// <param name="utxoStakeDescriptions">List of UTXOs that are available in the wallet for staking.</param>
-        /// <param name="block">Template of the block that we are trying to mine.</param>
+        /// <param name="blockTemplate">Template of the block that we are trying to mine.</param>
         /// <param name="chainTip">Tip of the best chain.</param>
         /// <param name="fees">Transaction fees from the transactions included in the block if we mine it.</param>
         /// <param name="coinstakeTimestamp">Maximal timestamp of the coinstake transaction. The actual timestamp can be lower, but not higher.</param>
         /// <returns><c>true</c> if the function succeeds, <c>false</c> otherwise.</returns>
-        private async Task<bool> StakeAndSignBlockAsync(List<UtxoStakeDescription> utxoStakeDescriptions, PosBlock block, ChainedHeader chainTip, long fees, uint coinstakeTimestamp)
+        private async Task<bool> StakeAndSignBlockAsync(List<UtxoStakeDescription> utxoStakeDescriptions, BlockTemplate blockTemplate, ChainedHeader chainTip, long fees, uint coinstakeTimestamp)
         {
+            var block = blockTemplate.Block as PosBlock;
+            
             // If we are trying to sign something except proof-of-stake block template.
             if (!block.Transactions[0].Outputs[0].IsEmpty)
             {
@@ -526,8 +528,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
                 return true;
             }
 
-            var coinstakeContext = new CoinstakeContext();
-            coinstakeContext.CoinstakeTx = this.network.CreateTransaction();
+            var coinstakeContext = new CoinstakeContext { CoinstakeTx = this.network.CreateTransaction() };
             coinstakeContext.CoinstakeTx.Time = coinstakeTimestamp;
 
             // Search to current coinstake time.
@@ -539,26 +540,14 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
             this.lastCoinStakeSearchTime = searchTime;
             this.logger.LogTrace("Search interval set to {0}, last coinstake search timestamp set to {1}.", searchInterval, this.lastCoinStakeSearchTime);
 
-            if (await this.CreateCoinstakeAsync(utxoStakeDescriptions, block, chainTip, searchInterval, fees, coinstakeContext).ConfigureAwait(false))
+            if (await this.CreateCoinstakeAsync(utxoStakeDescriptions, blockTemplate, chainTip, searchInterval, fees, coinstakeContext).ConfigureAwait(false))
             {
                 uint minTimestamp = chainTip.Header.Time + 1;
                 if (coinstakeContext.CoinstakeTx.Time >= minTimestamp)
                 {
-                    // Make sure coinstake would meet timestamp protocol
-                    // as it would be the same as the block timestamp.
+                    // Make sure coinstake would meet timestamp protocol as it would be the same as the block timestamp.
                     block.Transactions[0].Time = block.Header.Time = coinstakeContext.CoinstakeTx.Time;
 
-                    // We have to make sure that we have no future timestamps in
-                    // our transactions set.
-                    for (int i = block.Transactions.Count - 1; i >= 0; i--)
-                    {
-                        if (block.Transactions[i].Time > block.Header.Time)
-                        {
-                            this.logger.LogTrace("Removing transaction with timestamp {0} as it is greater than coinstake transaction timestamp {1}.", block.Transactions[i].Time, block.Header.Time);
-                            block.Transactions.Remove(block.Transactions[i]);
-                        }
-                    }
-
                     block.Transactions.Insert(1, coinstakeContext.CoinstakeTx);
                     block.UpdateMerkleRoot();
 
@@ -576,7 +565,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
         }
 
         /// <inheritdoc/>
-        public virtual async Task<bool> CreateCoinstakeAsync(List<UtxoStakeDescription> utxoStakeDescriptions, Block block, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext)
+        public virtual async Task<bool> CreateCoinstakeAsync(List<UtxoStakeDescription> utxoStakeDescriptions, BlockTemplate blockTemplate, ChainedHeader chainTip, long searchInterval, long fees, CoinstakeContext coinstakeContext)
         {
             coinstakeContext.CoinstakeTx.Inputs.Clear();
             coinstakeContext.CoinstakeTx.Outputs.Clear();
@@ -647,7 +636,7 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
                 coinIndex += stakingUtxoCount;
                 workerContexts[workerIndex] = cwc;
 
-                workers[workerIndex] = Task.Run(() => this.CoinstakeWorker(cwc, chainTip, block, minimalAllowedTime, searchInterval));
+                workers[workerIndex] = Task.Run(() => this.CoinstakeWorker(cwc, chainTip, blockTemplate.Block, minimalAllowedTime, searchInterval));
             }
 
             await Task.WhenAll(workers).ConfigureAwait(false);
@@ -660,6 +649,21 @@ namespace Stratis.Bitcoin.Features.Miner.Staking
 
             this.logger.LogTrace("Worker #{0} found the kernel.", workersResult.KernelFoundIndex);
 
+            // We have to make sure that we have no future timestamps in our transactions set.
+            // We ignore the coinbase (it gets its timestamp reset after the coinstake is created).
+            for (int i = blockTemplate.Block.Transactions.Count - 1; i >= 1; i--)
+            {
+                // We have not yet updated the header timestamp, so we use the coinstake timestamp directly here.
+                if (blockTemplate.Block.Transactions[i].Time <= coinstakeContext.CoinstakeTx.Time)
+                    continue;
+
+                // Update the total fees, with the to-be-removed transaction taken into account.
+                fees -= blockTemplate.FeeDetails[blockTemplate.Block.Transactions[i].GetHash()].Satoshi;
+
+                this.logger.LogDebug("Removing transaction with timestamp {0} as it is greater than coinstake transaction timestamp {1}. New fee amount {2}.", blockTemplate.Block.Transactions[i].Time, coinstakeContext.CoinstakeTx.Time, fees);
+                blockTemplate.Block.Transactions.Remove(blockTemplate.Block.Transactions[i]);
+            }
+
             // Get reward for newly created block.
             long reward = fees + this.consensusManager.ConsensusRules.GetRule<PosCoinviewRule>().GetProofOfStakeReward(chainTip.Height + 1);
             if (reward <= 0)

Sources/Stratis.Bitcoin/Mining/BlockTemplate.cs

-using NBitcoin;
+using System.Collections.Generic;
+using NBitcoin;
 
 namespace Stratis.Bitcoin.Mining
 {
@@ -7,6 +8,8 @@ namespace Stratis.Bitcoin.Mining
         public Block Block { get; set; }
 
         public Money TotalFee { get; set; }
+        
+        public Dictionary<uint256, Money> FeeDetails { get; set; }
 
         public BlockTemplate(Network network)
         {