Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from Aug 19, 2019

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Aug 14, 2019

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.

@fanquake fanquake added this to the 0.18.2 milestone Aug 14, 2019
@meshcollider meshcollider changed the title [backport] Use wallet RBF default for walletcreatefundedpsbt [0.18] Backport: Use wallet RBF default for walletcreatefundedpsbt Aug 16, 2019
Copy link
Contributor

@meshcollider meshcollider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

a678e06

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved
@Sjors Sjors force-pushed the 2018/08/backport-wallet-rbf-default branch from a678e06 to 576580f Compare August 17, 2019 10:53
@meshcollider
Copy link
Contributor

meshcollider commented Aug 19, 2019

re-utACK 576580f

Only change was indentation fix

@maflcko 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
Copy link
Member

maflcko commented Aug 19, 2019

ACK 576580f

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 576580fe8a063f21c0e903af1cf8f85cd6cb71d7
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUi90wwAwVZA8P1MHIw8biLLtDV+osnhJQ8yQCXAZG2ED4snoCbV5s2SczbTgksT
BWzEiCEQqf5s0lF1cgnCjkaYpIidpZ55O7iVSYTFnvlAd8Aw6cAH0Xe0V9VF+WhG
0tW3GBTIaOX4ZEfwMD0nSxdAorL1UWYt3HgDjaGNlZBWzScdFlTHzLb7LXKQqLL2
CprrQo/n2Ryp+riKB4998YserFpoudeHRcYzTR4aq0bmgfYEVtXZuxhlcz75BdQh
nw2Ehas82qqOtszuDUbvvOkZguwUfwEtoAxH3yUxsGUUwISOPmn2ekdE8u4WRxos
lgemrveGSKjPmEEhWKSETR97lTe6XLAjiaPtiClxAQSmBOhtuY0LqZpmhkh9B2Ru
R6QBnF+iZDAc7e0OQ7Bqug273Swhub5YbDS2qs2YL63EO+SKo528SNcznJRHBI8a
ZBdKWDxqMJysj5LkrW4oeIH312vcMK1wbBjiF5IZpmalShvHHux7oxPVFUpTEOfu
ywn7IU9C
=3lWE
-----END PGP SIGNATURE-----

Timestamp of file with hash f5a11e9d65e3f6cfd270f092131edbd8b4b33243c1a93c5ebd8c3244f41517df -

maflcko pushed a commit that referenced this pull request Aug 19, 2019
…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
@maflcko maflcko merged commit 576580f into bitcoin:0.18 Aug 19, 2019
@fanquake
Copy link
Member

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

@Sjors
Copy link
Member Author

Sjors commented Aug 19, 2019

Probably, what backport metadata should be in a commit?

@maflcko
Copy link
Member

maflcko commented Aug 19, 2019

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

@maflcko
Copy link
Member

maflcko commented Aug 19, 2019

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
Copy link
Contributor

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

@Sjors Sjors deleted the 2018/08/backport-wallet-rbf-default branch August 19, 2019 14:04
@maflcko
Copy link
Member

maflcko commented Aug 21, 2019

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

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants