Add option to opt into full-RBF when sending funds (rebase, original by petertodd) #8601

pull laanwj wants to merge 3 commits into bitcoin:master from laanwj:2016_08_full_rbf_option changing 2 files +15 −3

laanwj commented at 10:17 AM on August 26, 2016: member

Rebases #7132 Also adds the comment for #7132 (review)

Original pull has plenty of ACKs. After all this time we still have no way to opt in to RBF in the wallet, let's not try to bikeshed it to death this time.
Add option to opt into full-RBF when sending funds 152f45ba58
laanwj added the label Feature on Aug 26, 2016
laanwj added the label Wallet on Aug 26, 2016
laanwj cross-referenced this on Aug 26, 2016 from issue Add option to opt into full-RBF when sending funds by petertodd
laanwj renamed this:
~~Add option to opt into full-RBF when sending funds (rebase)~~
Add option to opt into full-RBF when sending funds (rebase, original by petertodd)
on Aug 26, 2016
wallet: Add BIP125 comment for MAXINT-1/-2 behavior 05fa823bf6

in src/wallet/wallet.cpp:None in 0c87520f28 outdated

2355 | @@ -2355,11 +2356,17 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
2356 |  
2357 |                  // Fill vin
2358 |                  //
2359 | -                // Note how the sequence number is set to max()-1 so that the
2360 | -                // nLockTime set above actually works.
2361 | +                // Note how the sequence number is set to non-maxint so that
2362 | +                // the nLockTime set above actually works.
2363 | +                /

MarcoFalke commented at 10:51 AM on August 26, 2016:

I don't think this is valid cpp :stuck_out_tongue:

laanwj force-pushed on Aug 26, 2016
luke-jr commented at 3:09 AM on August 27, 2016: member

I thought the commandline option was to be renamed to -walletrbf?
dcousens commented at 1:20 AM on August 29, 2016: contributor

utACK 05fa823
petertodd commented at 12:19 AM on August 31, 2016: contributor

utACK
laanwj commented at 1:22 PM on September 1, 2016: member

I thought the commandline option was to be renamed to -walletrbf?

Fine with me.
paveljanik commented at 3:18 PM on September 1, 2016: contributor

Why not rename the DEFAULT_.. as well to match the option name?
luke-jr commented at 6:50 PM on September 1, 2016: member

utACK b54c36703c4023114b05305b2edf15db88746fef as-is

http://codepad.org/fhQWSuAk could be included to address @paveljanik's suggestion
Rename `-optintofullrbf` option to `-walletrbf`
This makes it clear that this is a wallet option.
86726d8680
laanwj force-pushed on Sep 13, 2016
laanwj commented at 9:34 AM on September 13, 2016: member

Squashed in @luke-jr's patch
laanwj merged this on Sep 13, 2016
laanwj closed this on Sep 13, 2016
laanwj referenced this in commit 37ac67816a on Sep 13, 2016

in src/wallet/wallet.h:None in 86726d8680

  53 | @@ -53,6 +54,8 @@ static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true;
  54 |  static const bool DEFAULT_SEND_FREE_TRANSACTIONS = false;
  55 |  //! -txconfirmtarget default
  56 |  static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 2;
  57 | +//! -walletrbf default
  58 | +static const bool DEFAULT_WALLET_RBF = false;

rebroad commented at 2:18 AM on September 15, 2016:

why false by default?

MarcoFalke commented at 9:26 AM on September 15, 2016: member

Please see the linked pull in the description

On Thu, Sep 15, 2016 at 4:19 AM, R E Broadley notifications@github.com wrote:

@rebroad commented on this pull request.

In src/wallet/wallet.h #8601#pullrequestreview-83872:

@@ -53,6 +54,8 @@ static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; static const bool DEFAULT_SEND_FREE_TRANSACTIONS = false; //! -txconfirmtarget default static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 2; +//! -walletrbf default +static const bool DEFAULT_WALLET_RBF = false;

why false by default?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub #8601#pullrequestreview-83872, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv74fpRaN3BoDZKy7UTc-EN_RqpRnks5qqKs7gaJpZM4Jt8HS .
petertodd cross-referenced this on Sep 21, 2016 from issue Getting a txn-mempool-conflict by FrozenPrincess
MarcoFalke cross-referenced this on Nov 28, 2016 from issue Document RBF settings by unsystemizer
codablock referenced this in commit e01ea3dec1 on Sep 19, 2017
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7132 Add option to opt into full-RBF when sending funds #9232 Document RBF settings

Add option to opt into full-RBF when sending funds (rebase, original by petertodd) #8601

@rebroad commented on this pull request.