[0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt #16608

Sjors · 2019-08-14T11:44:03Z

Backport of #15911 for 0.18 branch.

The original PR changed rawtransaction_util.cpp, whereas the backport changes rawtransaction.cpp to avoid having to also backport #15638.

meshcollider

I have verified that the code changes here match those in the original PR (haven't actually re-reviewed the changes though, just verified the backport)

a678e06874e0004cd026e1969c27ecfc643ac289

src/rpc/rawtransaction.cpp

meshcollider · 2019-08-19T10:48:34Z

re-utACK 576580f

Only change was indentation fix

maflcko · 2019-08-19T12:27:44Z

ACK 576580f

Show signature and timestamp

Signature:

Timestamp of file with hash f5a11e9d65e3f6cfd270f092131edbd8b4b33243c1a93c5ebd8c3244f41517df -

…etcreatefundedpsbt 576580f [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0 (Sjors Provoost) 0942a60 [doc] rpc: remove "fallback to" from RBF default help (Sjors Provoost) ee950ec [rpc] walletcreatefundedpsbt: use wallet default RBF (Sjors Provoost) Pull request description: Backport of #15911 for 0.18 branch. The original PR changed `rawtransaction_util.cpp`, whereas the backport changes `rawtransaction.cpp` to avoid having to also backport #15638. ACKs for top commit: meshcollider: re-utACK 576580f MarcoFalke: ACK 576580f Tree-SHA512: 51c40fc4db4f8739e93e931aeac572779d5ade0bfeb3bcb0082301cf73b4f48e31d4ccbe40421dc65d0ec091d6393685c0c9b8f76584c59249f13642e80a6da5

fanquake · 2019-08-19T12:31:57Z

Weren't all the commits here missing the backport metadata?

Sjors · 2019-08-19T12:35:59Z

Probably, what backport metadata should be in a commit?

maflcko · 2019-08-19T12:37:25Z

Maybe we should mention the backport script in the contrib/devtools readme?

maflcko · 2019-08-19T12:40:24Z

The code itself was the same when I did the backport, but those were the commit differences (via range-diff):

1:  ee950ec465 ! 1:  1057254270 [rpc] walletcreatefundedpsbt: use wallet default RBF
    @@ -2,6 +2,9 @@
     
         [rpc] walletcreatefundedpsbt: use wallet default RBF
     
    +    Github-Pull: #15911
    +    Rebased-From: 4fcb698bc2bb74171cd3a14b94f9882d8e19e9fb
    +
      diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
      --- a/src/rpc/rawtransaction.cpp
      +++ b/src/rpc/rawtransaction.cpp
    @@ -84,6 +87,15 @@
      --- a/src/wallet/rpcwallet.cpp
      +++ b/src/wallet/rpcwallet.cpp
     @@
    +                                     {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
    +                                 },
    +                             },
    +-                            {"replaceable", RPCArg::Type::BOOL, /* default */ "false", "Marks this transaction as BIP125 replaceable.\n"
    ++                            {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n"
    +                             "                              Allows this transaction to be replaced by a transaction with higher fees"},
    +                             {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"},
    +                             {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
    +@@
      
          CAmount fee;
          int change_position;
    @@ -119,12 +131,12 @@
              block_height = self.nodes[0].getblockcount()
              unspent = self.nodes[0].listunspent()[0]
     -        psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":True}, False)
    -+        psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":False}, False)
    ++        psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False}, False)
              decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"])
              for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]):
     -           assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE)
     -           assert "bip32_derivs" not in psbt_in
    -+            assert(tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE)
    ++            assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE
     +            assert "bip32_derivs" not in psbt_in
              assert_equal(decoded_psbt["tx"]["locktime"], block_height+2)
      
2:  0942a60c06 ! 2:  7ab99cf477 [doc] rpc: remove "fallback to" from RBF default help
    @@ -2,6 +2,9 @@
     
         [doc] rpc: remove "fallback to" from RBF default help
     
    +    Github-Pull: #15911
    +    Rebased-From: 9ed062b5685eb6227d694572cb0f7bfbcc151b36
    +
      diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
      --- a/src/wallet/rpcwallet.cpp
      +++ b/src/wallet/rpcwallet.cpp
    @@ -48,12 +51,3 @@
                                  {"totalFee", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "Total fee (NOT feerate) to pay, in satoshis.\n"
                  "                         In rare cases, the actual fee paid might be slightly higher than the specified\n"
                  "                         totalFee if the tx change output has to be removed because it is too close to\n"
    -@@
    -                                     {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
    -                                 },
    -                             },
    --                            {"replaceable", RPCArg::Type::BOOL, /* default */ "false", "Marks this transaction as BIP125 replaceable.\n"
    -+                            {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n"
    -                             "                              Allows this transaction to be replaced by a transaction with higher fees"},
    -                             {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"},
    -                             {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
3:  576580fe8a ! 3:  86bc93ec44 [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0
    @@ -2,19 +2,13 @@
     
         [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0
     
    +    Github-Pull: #15911
    +    Rebased-From: d6b3640ac732f6f66a8cb6761084d1beecc8a876
    +
      diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
      --- a/test/functional/rpc_psbt.py
      +++ b/test/functional/rpc_psbt.py
     @@
    - 
    - from decimal import Decimal
    - from test_framework.test_framework import BitcoinTestFramework
    --from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes_bi, disconnect_nodes, find_output, sync_blocks
    -+from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, connect_nodes_bi, disconnect_nodes, find_output, sync_blocks
    - 
    - import json
    - import os
    -@@
              self.num_nodes = 3
              self.extra_args = [
                  ["-walletrbf=1"],
    @@ -24,10 +18,10 @@
              ]
      
     @@
    -         psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":False}, False)
    +         psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False}, False)
              decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"])
              for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]):
    --            assert(tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE)
    +-            assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE
     +            assert_greater_than(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE)
                  assert "bip32_derivs" not in psbt_in
              assert_equal(decoded_psbt["tx"]["locktime"], block_height+2)

meshcollider · 2019-08-19T12:51:17Z

Yeah the Github-Pull: and Rebased-From: were missing

maflcko · 2019-08-21T13:01:10Z

See https://github.com/bitcoin-core/bitcoin-maintainer-tools#backport for a script to backport

fanquake added the Backport label Aug 14, 2019

fanquake added this to the 0.18.2 milestone Aug 14, 2019

Sjors mentioned this pull request Aug 14, 2019

Use wallet RBF default for walletcreatefundedpsbt #15911

Merged

fanquake added RPC/REST/ZMQ Wallet labels Aug 14, 2019

meshcollider changed the title ~~[backport] Use wallet RBF default for walletcreatefundedpsbt~~ [0.18] Backport: Use wallet RBF default for walletcreatefundedpsbt Aug 16, 2019

meshcollider reviewed Aug 16, 2019

View reviewed changes

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved

Sjors added 3 commits August 17, 2019 12:52

[rpc] walletcreatefundedpsbt: use wallet default RBF

ee950ec

[doc] rpc: remove "fallback to" from RBF default help

0942a60

[test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0

576580f

Sjors force-pushed the 2018/08/backport-wallet-rbf-default branch from a678e06 to 576580f Compare August 17, 2019 10:53

maflcko changed the title ~~[0.18] Backport: Use wallet RBF default for walletcreatefundedpsbt~~ [0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt Aug 19, 2019

maflcko merged commit 576580f into bitcoin:0.18 Aug 19, 2019

Sjors deleted the 2018/08/backport-wallet-rbf-default branch August 19, 2019 14:04

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt #16608

[0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt #16608

Uh oh!

Sjors commented Aug 14, 2019

Uh oh!

meshcollider left a comment

Uh oh!

Uh oh!

meshcollider commented Aug 19, 2019 •

edited

Loading

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

fanquake commented Aug 19, 2019

Uh oh!

Sjors commented Aug 19, 2019

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

meshcollider commented Aug 19, 2019

Uh oh!

maflcko commented Aug 21, 2019 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt #16608

[0.18] Backport #15911: Use wallet RBF default for walletcreatefundedpsbt #16608

Uh oh!

Conversation

Sjors commented Aug 14, 2019

Uh oh!

meshcollider left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meshcollider commented Aug 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

fanquake commented Aug 19, 2019

Uh oh!

Sjors commented Aug 19, 2019

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

maflcko commented Aug 19, 2019

Uh oh!

meshcollider commented Aug 19, 2019

Uh oh!

maflcko commented Aug 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

meshcollider commented Aug 19, 2019 •

edited

Loading

maflcko commented Aug 21, 2019 •

edited

Loading