From 1d2380df56e2a5471a306a86fa76649075f12517 Mon Sep 17 00:00:00 2001 From: Michi Lumin Date: Wed, 21 Jul 2021 20:58:12 +0100 Subject: [PATCH 1/2] p2p: Reduce BIP125 replace by fee increment value --- qa/pull-tester/rpc-tests.py | 2 +- qa/rpc-tests/bumpfee.py | 79 +++++++++++++++++++------------------ src/wallet/rpcwallet.cpp | 8 +++- src/wallet/wallet.h | 2 +- 4 files changed, 49 insertions(+), 42 deletions(-) diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index b3cf10cd3..f7a2deb0d 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -155,7 +155,7 @@ testScripts = [ 'import-rescan.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', + 'bumpfee.py', 'rpcnamedargs.py', 'listsinceblock.py', 'p2p-leaktests.py', diff --git a/qa/rpc-tests/bumpfee.py b/qa/rpc-tests/bumpfee.py index 3c819369f..e8deb3adf 100755 --- a/qa/rpc-tests/bumpfee.py +++ b/qa/rpc-tests/bumpfee.py @@ -45,16 +45,16 @@ class BumpFeeTest(BitcoinTestFramework): peer_node, rbf_node = self.nodes rbf_node_address = rbf_node.getnewaddress() - # fund rbf node with 10 coins of 0.001 btc (100,000 satoshis) + # fund rbf node with 25 outputs of 10 DOGE print("Mining blocks...") peer_node.generate(70) self.sync_all() for i in range(25): - peer_node.sendtoaddress(rbf_node_address, 1.0000) + peer_node.sendtoaddress(rbf_node_address, 10.0000) self.sync_all() peer_node.generate(1) self.sync_all() - assert_equal(rbf_node.getbalance(), Decimal("25")) + assert_equal(rbf_node.getbalance(), Decimal("250")) print("Running tests") dest_address = peer_node.getnewaddress() @@ -75,7 +75,7 @@ class BumpFeeTest(BitcoinTestFramework): def test_simple_bumpfee_succeeds(rbf_node, peer_node, dest_address): - rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.90000000}) + rbfid = create_fund_sign_send(rbf_node, {dest_address: 8.00000000}) rbftx = rbf_node.gettransaction(rbfid) sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() @@ -99,7 +99,7 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address): # Create a transaction with segwit output, then create an RBF transaction # which spends it, and make sure bumpfee can be called on it. - segwit_in = next(u for u in rbf_node.listunspent() if u["amount"] == Decimal("0.001")) + segwit_in = next(u for u in rbf_node.listunspent() if u["amount"] == Decimal("10.0000000")) segwit_out = rbf_node.validateaddress(rbf_node.getnewaddress()) rbf_node.addwitnessaddress(segwit_out["address"]) segwitid = send_to_witness( @@ -108,15 +108,15 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address): utxo=segwit_in, pubkey=segwit_out["pubkey"], encode_p2sh=False, - amount=Decimal("0.0009"), + amount=Decimal("9.00000000"), sign=True) rbfraw = rbf_node.createrawtransaction([{ 'txid': segwitid, 'vout': 0, "sequence": BIP125_SEQUENCE_NUMBER - }], {dest_address: Decimal("0.0005"), - get_change_address(rbf_node): Decimal("0.0003")}) + }], {dest_address: Decimal("5.00000000"), + get_change_address(rbf_node): Decimal("3.00000000")}) rbfsigned = rbf_node.signrawtransaction(rbfraw) rbfid = rbf_node.sendrawtransaction(rbfsigned["hex"]) assert rbfid in rbf_node.getrawmempool() @@ -128,7 +128,7 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address): def test_nonrbf_bumpfee_fails(peer_node, dest_address): # cannot replace a non RBF transaction (from node which did not enable RBF) - not_rbfid = create_fund_sign_send(peer_node, {dest_address: 0.90000000}) + not_rbfid = create_fund_sign_send(peer_node, {dest_address: 9.00000000}) assert_raises_jsonrpc(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) @@ -144,7 +144,7 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): "address": utxo["address"], "sequence": BIP125_SEQUENCE_NUMBER } for utxo in utxos] - output_val = sum(utxo["amount"] for utxo in utxos) - Decimal("0.001") + output_val = sum(utxo["amount"] for utxo in utxos) - Decimal("10.00000000") rawtx = rbf_node.createrawtransaction(inputs, {dest_address: output_val}) signedtx = rbf_node.signrawtransaction(rawtx) signedtx = peer_node.signrawtransaction(signedtx["hex"]) @@ -156,8 +156,8 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address): # cannot bump fee if the transaction has a descendant # parent is send-to-self, so we don't have to check which output is change when creating the child tx - parent_id = create_fund_sign_send(rbf_node, {rbf_node_address: 0.00050000}) - tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000}) + parent_id = create_fund_sign_send(rbf_node, {rbf_node_address: 5.00000000}) + tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 2.00000000}) tx = rbf_node.signrawtransaction(tx) txid = rbf_node.sendrawtransaction(tx["hex"]) assert_raises_jsonrpc(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) @@ -166,65 +166,66 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address) def test_small_output_fails(rbf_node, dest_address): # cannot bump fee with a too-small output rbfid = spend_one_input(rbf_node, - Decimal("1.00000000"), - {dest_address: 0.80000000, - get_change_address(rbf_node): Decimal("0.10000000")}) + Decimal("10.00000000"), + {dest_address: 8.00000000, + get_change_address(rbf_node): Decimal("1.00000000")}) rbf_node.bumpfee(rbfid, {"totalFee": 200000000}) rbfid = spend_one_input(rbf_node, - Decimal("1.00000000"), - {dest_address: 0.800000000, + Decimal("10.00000000"), + {dest_address: 8.00000000, get_change_address(rbf_node): Decimal("1.00000000")}) assert_raises_jsonrpc(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 200000001}) def test_dust_to_fee(rbf_node, dest_address): # check that if output is reduced to dust, it will be converted to fee - # the bumped tx sets fee=9900, but it converts to 10,000 + # the bumped tx sets fee=99,000,000, but it converts to 100,000,000 rbfid = spend_one_input(rbf_node, - Decimal("1.00000000"), - {dest_address: 0.80000000, - get_change_address(rbf_node): Decimal("0.10000000")}) + Decimal("10.00000000"), + {dest_address: 8.00000000, + get_change_address(rbf_node): Decimal("1.00000000")}) fulltx = rbf_node.getrawtransaction(rbfid, 1) - bumped_tx = rbf_node.bumpfee(rbfid, {"totalFee": 19900000}) + bumped_tx = rbf_node.bumpfee(rbfid, {"totalFee": 199000000}) full_bumped_tx = rbf_node.getrawtransaction(bumped_tx["txid"], 1) - assert_equal(bumped_tx["fee"], Decimal("0.00020000")) + assert_equal(bumped_tx["fee"], Decimal("2.00000000")) assert_equal(len(fulltx["vout"]), 2) assert_equal(len(full_bumped_tx["vout"]), 1) #change output is eliminated def test_settxfee(rbf_node, dest_address): + # Dogecoin: Increment is fixed, so this test tests for settxfee not making a difference # check that bumpfee reacts correctly to the use of settxfee (paytxfee) # increase feerate by 2.5x, test that fee increased at least 2x - rbf_node.settxfee(Decimal("0.01000000")) - rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.90000000}) + rbf_node.settxfee(Decimal("2.00000000")) + rbfid = create_fund_sign_send(rbf_node, {dest_address: 3.00000000}) rbftx = rbf_node.gettransaction(rbfid) - rbf_node.settxfee(Decimal("0.00002500")) + rbf_node.settxfee(Decimal("5.00000000")) bumped_tx = rbf_node.bumpfee(rbfid) - assert bumped_tx["fee"] > 2 * abs(rbftx["fee"]) + assert_equal(bumped_tx["fee"], abs(rbftx["fee"]) + Decimal("0.50000000")) rbf_node.settxfee(Decimal("0.00000000")) # unset paytxfee def test_rebumping(rbf_node, dest_address): # check that re-bumping the original tx fails, but bumping the bumper succeeds - rbf_node.settxfee(Decimal("0.01000000")) - rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.90000000}) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000000}) + 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": 2000000}) + rbf_node.bumpfee(bumped["txid"], {"totalFee": 200000000}) def test_rebumping_not_replaceable(rbf_node, dest_address): # check that re-bumping a non-replaceable bump tx fails - rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.90000000}) - bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000000, "replaceable": False}) + rbfid = create_fund_sign_send(rbf_node, {dest_address: 7.00000000}) + bumped = rbf_node.bumpfee(rbfid, {"totalFee": 150000000, "replaceable": False}) assert_raises_jsonrpc(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], - {"totalFee": 20000000}) + {"totalFee": 200000000}) def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): # check that unconfirmed outputs from bumped transactions are not spendable - rbfid = create_fund_sign_send(rbf_node, {rbf_node_address: 0.90000000}) + rbfid = create_fund_sign_send(rbf_node, {rbf_node_address: 7.00000000}) rbftx = rbf_node.gettransaction(rbfid)["hex"] assert rbfid in rbf_node.getrawmempool() bumpid = rbf_node.bumpfee(rbfid)["txid"] @@ -259,7 +260,9 @@ def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): def test_bumpfee_metadata(rbf_node, dest_address): - rbfid = rbf_node.sendtoaddress(dest_address, 0.90000000, "comment value", "to value") + # Use a weirder value because otherwise the existing change outputs are consumed without leaving a change + # output on the TX, and then bumpfee() fails because there's no change. + rbfid = rbf_node.sendtoaddress(dest_address, 5.10000000, "comment value", "to value") bumped_tx = rbf_node.bumpfee(rbfid) bumped_wtx = rbf_node.gettransaction(bumped_tx["txid"]) assert_equal(bumped_wtx["comment"], "comment value") @@ -267,7 +270,7 @@ def test_bumpfee_metadata(rbf_node, dest_address): def test_locked_wallet_fails(rbf_node, dest_address): - rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.90000000}) + rbfid = create_fund_sign_send(rbf_node, {dest_address: 8.00000000}) rbf_node.walletlock() assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first.", rbf_node.bumpfee, rbfid) @@ -296,7 +299,7 @@ def get_change_address(node): dummy transaction, calls fundrawtransaction to give add an input and change output, then returns the change address.""" dest_address = node.getnewaddress() - dest_amount = Decimal("0.00012345") + dest_amount = Decimal("1.23450000") rawtx = node.createrawtransaction([], {dest_address: dest_amount}) fundtx = node.fundrawtransaction(rawtx) info = node.decoderawtransaction(fundtx["hex"]) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c7c665a8c..3a6c48dd7 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2830,6 +2830,10 @@ UniValue bumpfee(const JSONRPCRequest& request) // Calculate the expected size of the new transaction. int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); + // Doge: Round txSize up to nearest 1kB + if (txSize % 1024 != 0) { + txSize = txSize + 1024 - (txSize % 1024); + } const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx); // optional parameters @@ -2896,9 +2900,9 @@ UniValue bumpfee(const JSONRPCRequest& request) if (specifiedConfirmTarget) { nNewFee = CWallet::GetMinimumFee(*wtx.tx, maxNewTxSize, newConfirmTarget, mempool, CAmount(0)); } - // otherwise use the regular wallet logic to select payTxFee or default confirm target + // otherwise bump the fee by 1 DOGE. else { - nNewFee = CWallet::GetMinimumFee(*wtx.tx, maxNewTxSize, newConfirmTarget, mempool); + nNewFee = nOldFee + walletIncrementalRelayFee.GetFeePerK(); } nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 329923aa3..e3a041e7f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -56,7 +56,7 @@ static const CAmount DEFAULT_TRANSACTION_MINFEE = COIN; //mlumin 5/2021: adding a minimum Wallet fee vs relay, currently still 1 COIN, to be reduced. static const unsigned int DEFAULT_MIN_WALLET_TX_FEE = COIN; //! minimum recommended increment for BIP 125 replacement txs -static const CAmount WALLET_INCREMENTAL_RELAY_FEE = COIN * 5; +static const CAmount WALLET_INCREMENTAL_RELAY_FEE = COIN/10 * 5; //! target minimum change amount static const CAmount MIN_CHANGE = COIN; //! final minimum change amount after paying for fees From 14a2e1ba9644d0cc59c1f773d330a8f2674ab54b Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Mon, 26 Jul 2021 22:40:39 +0100 Subject: [PATCH 2/2] consensus: Fix a rare crash bug Fix a rare crash bug where no best chain can be activated, and therefore when trying to find the height of the best chain via the last block triggers a null pointer dereference. --- src/validation.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 35778a2e7..c81ab3c20 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2573,7 +2573,11 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } while (pindexNewTip != pindexMostWork); - CheckBlockIndex(chainparams.GetConsensus(pindexNewTip->nHeight)); + if (pindexNewTip != NULL) { + CheckBlockIndex(chainparams.GetConsensus(pindexNewTip->nHeight)); + } else { + CheckBlockIndex(chainparams.GetConsensus(0)); + } // Write changes periodically to disk, after relay. if (!FlushStateToDisk(state, FLUSH_STATE_PERIODIC)) {