From 2370fc570167723a3ff4e2864962c44802470078 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Sat, 7 Aug 2021 22:36:47 +0200 Subject: [PATCH 1/3] qa: add -paytxfee and -mintxfee interaction test Test that the interaction between the wallet parameters -paytxfee and -mintxfee function as intended. This has to be done using rpc tests rather than unit tests because it tests the actual parameters passed to the executables. --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/paytxfee.py | 82 +++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 qa/rpc-tests/paytxfee.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 02e858d96..71c7b5f20 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -155,6 +155,7 @@ testScripts = [ # 'nulldummy.py', 'import-rescan.py', 'harddustlimit.py', + 'paytxfee.py', # While fee bumping should work in Doge, these tests depend on free transactions, which we don't support. # Disable until we can do a full rewrite of the tests (possibly upstream), or revise fee schedule, or something 'bumpfee.py', diff --git a/qa/rpc-tests/paytxfee.py b/qa/rpc-tests/paytxfee.py new file mode 100644 index 000000000..feec0c45c --- /dev/null +++ b/qa/rpc-tests/paytxfee.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Dogecoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""PayTxFee QA test. + +# Tests wallet behavior of -paytxfee in relation to -mintxfee +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import * +from decimal import Decimal + +class PayTxFeeTest(BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.num_nodes = 4 + + def setup_nodes(self, split=False): + nodes = [] + + # node 0 has txindex to track txs + nodes.append(start_node(0, self.options.tmpdir, + ["-debug", '-txindex'])) + + # node 1 pays 0.1 DOGE on all txs due to implicit mintxfee = paytxfee + nodes.append(start_node(1, self.options.tmpdir, + ["-paytxfee=0.1", "-debug"])) + + # node 2 will always pay 1 DOGE on all txs because of explicit mintxfee + nodes.append(start_node(2, self.options.tmpdir, + ["-mintxfee=1", "-paytxfee=0.1", "-debug"])) + + # node 3 will always pay 0.1 DOGE on all txs despite explicit mintxfee of 0.01 + nodes.append(start_node(3, self.options.tmpdir, + ["-mintxfee=0.01", "-paytxfee=0.1", "-debug"])) + + return nodes + + def run_test(self): + + seed = 1000 # the amount to seed wallets with + amount = 995 # the amount to send back + targetAddress = self.nodes[0].getnewaddress() + + # mine some blocks and prepare some coins + self.nodes[0].generate(102) + self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), seed) + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), seed) + self.nodes[0].sendtoaddress(self.nodes[3].getnewaddress(), seed) + self.nodes[0].generate(1) + self.sync_all() + + # create transactions + txid1 = self.nodes[1].sendtoaddress(targetAddress, amount) + txid2 = self.nodes[2].sendtoaddress(targetAddress, amount) + txid3 = self.nodes[3].sendtoaddress(targetAddress, amount) + self.sync_all() + + # make sure correct fees were paid + tx1 = self.nodes[0].getrawtransaction(txid1, True) + tx2 = self.nodes[0].getrawtransaction(txid2, True) + tx3 = self.nodes[0].getrawtransaction(txid3, True) + + assert_equal(tx1['vout'][0]['value'] + tx1['vout'][1]['value'], Decimal("999.9")) + assert_equal(tx2['vout'][0]['value'] + tx2['vout'][1]['value'], Decimal("999")) + assert_equal(tx3['vout'][0]['value'] + tx3['vout'][1]['value'], Decimal("999.9")) + + # mine a block + self.nodes[0].generate(1); + self.sync_all() + + # make sure all fees were mined + block = self.nodes[0].getblock(self.nodes[0].getbestblockhash()) + coinbaseTx = self.nodes[0].getrawtransaction(block['tx'][0], True) + + assert_equal(coinbaseTx['vout'][0]['value'], Decimal("500001.2")) + +if __name__ == '__main__': + PayTxFeeTest().main() From 59f27ca73d52b23137818fcaba89a34cb1181130 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Sun, 15 Aug 2021 21:43:39 +0200 Subject: [PATCH 2/3] fees: remove careless override of -paytxfee in GetMinimumFee --- src/wallet/wallet.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9ad4fb7f0..fa8526686 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2865,19 +2865,24 @@ CAmount CWallet::GetMinimumFee(const CMutableTransaction& tx, unsigned int nTxBy CAmount nFeeNeeded = targetFee; // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == 0) { - int estimateFoundTarget = nConfirmTarget; - nFeeNeeded = pool.estimateSmartFee(nConfirmTarget, &estimateFoundTarget).GetFee(nTxBytes); - // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee - if (nFeeNeeded == 0) - nFeeNeeded = fallbackFee.GetFee(nTxBytes); + //int estimateFoundTarget = nConfirmTarget; + //nFeeNeeded = pool.estimateSmartFee(nConfirmTarget, &estimateFoundTarget).GetFee(nTxBytes); + //// ... unless we don't have enough mempool data for estimatefee, then use fallbackFee + //if (nFeeNeeded == 0) + // nFeeNeeded = fallbackFee.GetFee(nTxBytes); + + // Dogecoin: Drop the smart fee estimate, use GetRequiredFee + nFeeNeeded = GetRequiredFee(tx, nTxBytes); } // prevent user from paying a fee below minRelayTxFee or minTxFee - // Dogecoin: Drop the smart fee estimate, use GetRequiredFee - // nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(tx, nTxBytes)); - nFeeNeeded = GetRequiredFee(tx, nTxBytes); + // Dogecoin: as we're adapting minTxFee to never be higher than + // payTxFee unless explicitly set, this should be fine + nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(tx, nTxBytes)); + // But always obey the maximum if (nFeeNeeded > maxTxFee) nFeeNeeded = maxTxFee; + return nFeeNeeded; } From 07be86c14772b6c1cf660772c0fbfaf2cf256aaa Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Mon, 16 Aug 2021 00:11:04 +0200 Subject: [PATCH 3/3] qa: fix bumpfee now that paytxfee works --- qa/rpc-tests/bumpfee.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/bumpfee.py b/qa/rpc-tests/bumpfee.py index b23761b06..20a683922 100755 --- a/qa/rpc-tests/bumpfee.py +++ b/qa/rpc-tests/bumpfee.py @@ -210,9 +210,10 @@ def test_rebumping(rbf_node, dest_address): # check that re-bumping the original tx fails, but bumping the bumper succeeds rbf_node.settxfee(Decimal("10.00000000")) rbfid = create_fund_sign_send(rbf_node, {dest_address: 7.00000000}) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 150000000}) - assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000}) - rbf_node.bumpfee(bumped["txid"], {"totalFee": 200000000}) + bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1050000000}) + assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 11000}) + rbf_node.bumpfee(bumped["txid"], {"totalFee": 1100000000}) + rbf_node.settxfee(Decimal("0.00000000")) def test_rebumping_not_replaceable(rbf_node, dest_address):