Merge #339: getnewblockhex: add ability to pass over transactions based on mempoo…

ed121e8 getnewblockhex: add ability to pass over transactions based on mempool age (Gregory Sanders)

qa/rpc-tests/mempool_packages.py

@@ -42,6 +42,25 @@ class MempoolPackagesTest(BitcoinTestFramework):
         return (txid, send_value)
 
     def run_test(self):
+
+        # Create transaction with 3-second block delay, should fail to enter the template
+        txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
+        block = self.nodes[0].getnewblockhex(required_age=3)
+        self.nodes[0].submitblock(block)
+        assert(txid in self.nodes[0].getrawmempool())
+        time.sleep(3)
+        block = self.nodes[0].getnewblockhex(required_age=3)
+        self.nodes[0].submitblock(block)
+        assert(txid not in self.nodes[0].getrawmempool())
+        # Once more with no delay (default is 0, just testing default arg)
+        txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
+        block = self.nodes[0].getnewblockhex(required_age=0)
+        self.nodes[0].submitblock(block)
+        assert(txid not in self.nodes[0].getrawmempool())
+        assert_raises_message(JSONRPCException, "required_wait must be non-negative.", self.nodes[0].getnewblockhex, -1)
+
+        print("Rest of entire test is disabled due to fee outputs etc")
+
         return #TODO
         ''' Mine some blocks and have them mature. '''
         self.nodes[0].generate(101)
@@ -252,5 +271,6 @@ class MempoolPackagesTest(BitcoinTestFramework):
         self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
         sync_blocks(self.nodes)
 
+
 if __name__ == '__main__':
     MempoolPackagesTest().main()

src/miner.cpp

@@ -127,7 +127,7 @@ void BlockAssembler::resetBlock()
     blockFinished = false;
 }
 
-std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx)
+std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx, int required_age_in_secs)
 {
     int64_t nTimeStart = GetTimeMicros();
 
@@ -172,7 +172,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     //addPriorityTxs(); addPackageTxs will take anything at any rate
     int nPackagesSelected = 0;
     int nDescendantsUpdated = 0;
-    addPackageTxs(nPackagesSelected, nDescendantsUpdated);
+    addPackageTxs(nPackagesSelected, nDescendantsUpdated, required_age_in_secs);
 
     int64_t nTime1 = GetTimeMicros();
 
@@ -416,8 +416,9 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, CTxMemP
 // Each time through the loop, we compare the best transaction in
 // mapModifiedTxs with the next transaction in the mempool to decide what
 // transaction package to work on next.
-void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated)
+void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int required_age_in_secs)
 {
+    int64_t current_time = GetTime();
     // mapModifiedTx will store sorted packages after they are modified
     // because some of their txs are already in the block
     indexed_modified_transaction_set mapModifiedTx;
@@ -472,16 +473,19 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
             }
         }
 
+        // Skip transactions that are under X seconds in mempool
+        if (iter->GetTime() > current_time - required_age_in_secs) {
+            continue;
+        }
+
         // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't
         // contain anything that is inBlock.
         assert(!inBlock.count(iter));
 
         uint64_t packageSize = iter->GetSizeWithAncestors();
-        CAmount packageFees = iter->GetModFeesWithAncestors();
         int64_t packageSigOpsCost = iter->GetSigOpCostWithAncestors();
         if (fUsingModified) {
             packageSize = modit->nSizeWithAncestors;
-            packageFees = modit->nModFeesWithAncestors;
             packageSigOpsCost = modit->nSigOpCostWithAncestors;
         }
 

src/miner.h

@@ -165,7 +165,7 @@ private:
 public:
     BlockAssembler(const CChainParams& chainparams);
     /** Construct a new block template with coinbase to scriptPubKeyIn */
-    std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx=true);
+    std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx=true, int required_age_in_secs=0);
 
 private:
     // utility functions
@@ -180,7 +180,7 @@ private:
     /** Add transactions based on feerate including unconfirmed ancestors
       * Increments nPackagesSelected / nDescendantsUpdated with corresponding
       * statistics from the package selection (for logging statistics). */
-    void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated);
+    void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int required_age_in_secs=0);
 
     // helper function for addPriorityTxs
     /** Test if tx will still "fit" in the block */

src/rpc/client.cpp

@@ -138,6 +138,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "bumpfee", 1, "options" },
     { "testproposedblock", 1, "acceptnonstd" },
     { "sendtomainchain", 2, "subtractfeefromamount"},
+    { "getnewblockhex", 0, "required_age"},
     // Echo with conversion (For testing only)
     { "echojson", 0, "arg0" },
     { "echojson", 1, "arg1" },

src/rpc/mining.cpp

@@ -162,19 +162,26 @@ UniValue generate(const JSONRPCRequest& request)
 
 UniValue getnewblockhex(const JSONRPCRequest& request)
 {
-    if (request.fHelp || request.params.size() != 0)
+    if (request.fHelp || request.params.size() > 1)
         throw runtime_error(
             "getnewblockhex\n"
             "\nGets hex representation of a proposed, unmined new block\n"
+            "\nArguments:\n"
+            "1. required_age    (numeric, optional, default=0) How many seconds a transaction must have been in the mempool to be inluded in the block proposal. This may help with faster block convergence among functionaries using compact blocks.\n"
             "\nResult\n"
             "blockhex      (hex) The block hex\n"
             "\nExamples:\n"
             + HelpExampleCli("getnewblockhex", "")
         );
 
+    int required_wait = !request.params[0].isNull() ? request.params[0].get_int() : 0;
+    if (required_wait < 0) {
+        throw JSONRPCError(RPC_INVALID_PARAMS, "required_wait must be non-negative.");
+    }
+
     CScript feeDestinationScript = Params().GetConsensus().mandatory_coinbase_destination;
     if (feeDestinationScript == CScript()) feeDestinationScript = CScript() << OP_TRUE;
-    std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(Params()).CreateNewBlock(feeDestinationScript));
+    std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(Params()).CreateNewBlock(feeDestinationScript, true, required_wait));
     if (!pblocktemplate.get())
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet keypool empty");
     {
@@ -1012,7 +1019,7 @@ static const CRPCCommand commands[] =
 
     { "generating",         "generate",               &generate,               true,  {"nblocks","maxtries"} },
     { "generating",         "combineblocksigs",       &combineblocksigs,       true,  {} },
-    { "generating",         "getnewblockhex",         &getnewblockhex,         true,  {} },
+    { "generating",         "getnewblockhex",         &getnewblockhex,         true,  {"required_age"} },
 
     { "util",               "estimatefee",            &estimatefee,            true,  {"nblocks"} },
     { "util",               "estimatepriority",       &estimatepriority,       true,  {"nblocks"} },