Merge pull request #2594 from patricklodder/1.14.5-wallet-min-change

wallet/fee: Update MIN_CHANGE, related tests and document why

qa/rpc-tests/bumpfee.py

@@ -70,6 +70,7 @@ class BumpFeeTest(BitcoinTestFramework):
         test_rebumping(rbf_node, dest_address)
         test_rebumping_not_replaceable(rbf_node, dest_address)
         test_unconfirmed_not_spendable(rbf_node, rbf_node_address)
+        test_dogecoin_wallet_minchange(rbf_node, dest_address)
         test_locked_wallet_fails(rbf_node, dest_address)
         print("Success")
 
@@ -276,6 +277,25 @@ def test_locked_wallet_fails(rbf_node, dest_address):
     assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first.",
                           rbf_node.bumpfee, rbfid)
 
+def test_dogecoin_wallet_minchange(rbf_node, dest_address):
+    input = Decimal("10.00000000")
+    min_change = Decimal("0.03000000")
+    min_fee = Decimal("0.01000000")
+    bumpfee = Decimal("0.001")
+    est_tx_size = Decimal("0.193")
+    destamount = input - min_change - min_fee * est_tx_size
+    rbfid = spend_one_input(rbf_node,
+                            input,
+                            {dest_address: destamount,
+                             get_change_address(rbf_node): min_change})
+    bumped_tx = rbf_node.bumpfee(rbfid)
+    assert_equal(bumped_tx["fee"], min_fee * est_tx_size + bumpfee)
+    newfee = int((input - destamount - min_fee - bumpfee / 2 ) * 100000000)
+    bumped_tx = rbf_node.bumpfee(bumped_tx["txid"], {"totalFee": newfee})
+    assert_equal(bumped_tx["fee"], input - destamount - min_fee - bumpfee / 2)
+    bumped_tx = rbf_node.bumpfee(bumped_tx["txid"])
+    assert_equal(bumped_tx["fee"], input - destamount)
+    rbf_node.settxfee(Decimal("0.00000000"))
 
 def create_fund_sign_send(node, outputs):
     rawtx = node.createrawtransaction([], outputs)

qa/rpc-tests/importprunedfunds.py

@@ -15,7 +15,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
         self.num_nodes = 2
 
     def setup_network(self, split=False):
-        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir)
+        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, [['-spendzeroconfchange=0'], None])
         connect_nodes_bi(self.nodes,0,1)
         self.is_network_split=False
         self.sync_all()
@@ -25,7 +25,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
         self.nodes[0].generate(101)
 
         self.sync_all()
-        
+
         # address
         address1 = self.nodes[0].getnewaddress()
         # pubkey
@@ -59,18 +59,18 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
 
         #Send funds to self
         txnid1 = self.nodes[0].sendtoaddress(address1, 0.1)
-        self.nodes[0].generate(1)
         rawtxn1 = self.nodes[0].gettransaction(txnid1)['hex']
-        proof1 = self.nodes[0].gettxoutproof([txnid1])
 
         txnid2 = self.nodes[0].sendtoaddress(address2, 0.05)
-        self.nodes[0].generate(1)
         rawtxn2 = self.nodes[0].gettransaction(txnid2)['hex']
-        proof2 = self.nodes[0].gettxoutproof([txnid2])
 
         txnid3 = self.nodes[0].sendtoaddress(address3, 0.025)
-        self.nodes[0].generate(1)
         rawtxn3 = self.nodes[0].gettransaction(txnid3)['hex']
+
+        self.nodes[0].generate(1)
+
+        proof1 = self.nodes[0].gettxoutproof([txnid1])
+        proof2 = self.nodes[0].gettxoutproof([txnid2])
         proof3 = self.nodes[0].gettxoutproof([txnid3])
 
         self.sync_all()

src/wallet/wallet.h

@@ -66,10 +66,44 @@ static const CAmount DEFAULT_TRANSACTION_MINFEE = RECOMMENDED_MIN_TX_FEE;
  * This way, replacements for fee bumps are transient rather than persisted.
  */
 static const CAmount WALLET_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 10;
+
+/*
+ * Dogecoin: Creating change outputs at exactly the dustlimit is counter-
+ * productive because it leaves no space to bump the fee up, so we make the
+ * MIN_CHANGE parameter higher than the MIN_FINAL_CHANGE parameter.
+ *
+ * When RBF is not a default policy, we need to scale for both that and CPFP,
+ * to have a facility for those that did not manually enable RBF, yet need to
+ * bump a fee for their transaction to get mined.
+ *
+ * Using bumpfee currently will add WALLET_INCREMENTAL_RELAY_FEE as a fixed
+ * increment, and CPFP would need the spending fee for at least 147 bytes
+ * (1 input, 1 output), and additional space for actually increasing the fee
+ * for both transactions.
+ *
+ * Because the change calculation is currently not taking into account feerate
+ * or transaction size, we assume that most transactions are < 1kb, leading
+ * to the following when planning for a replacements with 2x original fee:
+ *
+ * RBF: MIN_CHANGE = dust limit + min fee or
+ * CPFP: MIN_CHANGE = dust limit + 2 * min fee * 0.147 + min fee
+ *
+ * Where the CPFP requirement is higher than the RBF one to lead to the same
+ * result.
+ *
+ * This can be rounded up to the nearest multiple of RECOMMENDED_MIN_TX_FEE as:
+ *
+ * MIN_CHANGE = DEFAULT_DUST_LIMIT + 2 * RECOMMENDED_MIN_TX_FEE
+ *
+ * The MIN_FINAL_CHANGE parameter can stay equal to DEFAULT_DUST_LIMIT as this
+ * influences when the wallet will discard all remaining dust as fee instead of
+ * change.
+ */
 //! target minimum change amount
-static const CAmount MIN_CHANGE = RECOMMENDED_MIN_TX_FEE;
+static const CAmount MIN_CHANGE = DEFAULT_DUST_LIMIT + 2 * RECOMMENDED_MIN_TX_FEE;
 //! final minimum change amount after paying for fees
-static const CAmount MIN_FINAL_CHANGE = RECOMMENDED_MIN_TX_FEE;
+static const CAmount MIN_FINAL_CHANGE = DEFAULT_DUST_LIMIT;
+
 //! Default for -spendzeroconfchange
 static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true;
 //! Default for -sendfreetransactions