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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/wallet/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE);
unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET;
bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE;
bool fSendFreeTransactions = DEFAULT_SEND_FREE_TRANSACTIONS;
bool fOptIntoFullRbf = DEFAULT_OPT_INTO_FULL_RBF;

const char * DEFAULT_WALLET_DAT = "wallet.dat";
const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;
Expand Down Expand Up @@ -2321,11 +2322,11 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt

// Fill vin
//
// Note how the sequence number is set to max()-1 so that the
// nLockTime set above actually works.
// Note how the sequence number is set to non-maxint so that
// the nLockTime set above actually works.
BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins)
txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(),
std::numeric_limits<unsigned int>::max()-1));
std::numeric_limits<unsigned int>::max() - (fOptIntoFullRbf ? 2 : 1)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe constants for 1 and 2?

Copy link
Member

Choose a reason for hiding this comment

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

those numbers aren't special, the 1st number just has to be bigger than 1. Not sure if that's a candidate for constant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mqybe more documentation solves it. I just cannot look at that line without wondering what the heck 1 and 2 mean...

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. The comment should be revised further.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this comment:

"BIP125 defines opt-in RBF as any nSequence < maxint-1, so we use the highest possible value in that range (maxint-2) to avoid conflicting with other possible uses of nSequence, and in the spirit of "smallest posible change from prior behavior""

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Maybe we could introduce std::numeric_limits<unsigned int>::max()-1 as some constant somewhere. SEQUENCE_OPT_OUT = std::numeric_limits<unsigned int>::max()-1 is used in other places (mempool) as well.

And then comment here "Use any value less than SEQUENCE_OPT_OUT according to BIP125." or use the comment you wrote above.

Copy link
Contributor

Choose a reason for hiding this comment

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

fOptIntoFullRbf ? SEQUENCE_OPT_OUT -1 : SEQUENCE_OPT_OUT

It's clrearer to me than:

 std::numeric_limits<unsigned int>::max() - (fOptIntoFullRbf ? 2 : 1)

Specially with the ""Use any value less than SEQUENCE_OPT_OUT..." comment.


// Sign
int nIn = 0;
Expand Down Expand Up @@ -3210,6 +3211,7 @@ std::string CWallet::GetWalletHelpString(bool showDebug)
strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE));
strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET));
strUsage += HelpMessageOpt("-usehd", _("Use hierarchical deterministic key generation (HD) after bip32. Only has effect during wallet creation/first start") + " " + strprintf(_("(default: %u)"), DEFAULT_USE_HD_WALLET));
strUsage += HelpMessageOpt("-optintofullrbf", strprintf(_("Send transactions with full-RBF opt-in enabled (default: %u)"), DEFAULT_OPT_INTO_FULL_RBF));
strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup"));
strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT));
strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST));
Expand Down Expand Up @@ -3441,6 +3443,7 @@ bool CWallet::ParameterInteraction()
nTxConfirmTarget = GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
bSpendZeroConfChange = GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE);
fSendFreeTransactions = GetBoolArg("-sendfreetransactions", DEFAULT_SEND_FREE_TRANSACTIONS);
fOptIntoFullRbf = GetBoolArg("-optintofullrbf", DEFAULT_OPT_INTO_FULL_RBF);

return true;
}
Expand Down
3 changes: 3 additions & 0 deletions src/wallet/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ extern CFeeRate payTxFee;
extern unsigned int nTxConfirmTarget;
extern bool bSpendZeroConfChange;
extern bool fSendFreeTransactions;
extern bool fOptIntoFullRbf;

static const unsigned int DEFAULT_KEYPOOL_SIZE = 100;
//! -paytxfee default
Expand All @@ -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;
//! -optintofullrbf default
static const bool DEFAULT_OPT_INTO_FULL_RBF = false;
//! Largest (in bytes) free transaction we're willing to create
static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000;
static const bool DEFAULT_WALLETBROADCAST = true;
Expand Down