Skip to content

Commit db6047d

Browse files
committed
Take the training wheels off anti-fee-sniping
Now that the off-by-one error w/nLockTime txs issue has been fixed by 87550ee (75a4d51 in the 0.11 branch) we can make the anti-fee-sniping protection create transactions with nLockTime set such that they're only valid in the next block, rather than an earlier block. There was also a concern about poor propagation, however testing with transactions with nLockTime = GetAdjustedTime()+1 as a proxy for nLockTime propagation, as well as a few transactions sent the moment blocks were received, has turned up no detectable issues with propagation. If you have a block at a given height you certainly have at least one peer with that block who will accept the transaction. That peer will certainly have other peers who will accept it, and soon essentially the whole network has the transaction. In particular, if a node recives a transaction that it rejects due to the tx being non-final, it will be accepted again later as it winds its way around the network.
1 parent 247b914 commit db6047d

File tree

1 file changed

+19
-9
lines changed

1 file changed

+19
-9
lines changed

src/wallet/wallet.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,15 +1715,25 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend,
17151715

17161716
// Discourage fee sniping.
17171717
//
1718-
// However because of a off-by-one-error in previous versions we need to
1719-
// neuter it by setting nLockTime to at least one less than nBestHeight.
1720-
// Secondly currently propagation of transactions created for block heights
1721-
// corresponding to blocks that were just mined may be iffy - transactions
1722-
// aren't re-accepted into the mempool - we additionally neuter the code by
1723-
// going ten blocks back. Doesn't yet do anything for sniping, but does act
1724-
// to shake out wallet bugs like not showing nLockTime'd transactions at
1725-
// all.
1726-
txNew.nLockTime = std::max(0, chainActive.Height() - 10);
1718+
// For a large miner the value of the transactions in the best block and
1719+
// the mempool can exceed the cost of delibrately attempting to mine two
1720+
// blocks to orphan the current best block. By setting nLockTime such that
1721+
// only the next block can include the transaction, we discourage this
1722+
// practice as the height restricted and limited blocksize gives miners
1723+
// considering fee sniping fewer options for pulling off this attack.
1724+
//
1725+
// A simple way to think about this is from the wallet's point of view we
1726+
// always want the blockchain to move forward. By setting nLockTime this
1727+
// way we're basically making the statement that we only want this
1728+
// transaction to appear in the next block; we don't want to potentially
1729+
// encourage reorgs by allowing transactions to appear at lower heights
1730+
// than the next block in forks of the best chain.
1731+
//
1732+
// Of course, the subsidy is high enough, and transaction volume low
1733+
// enough, that fee sniping isn't a problem yet, but by implementing a fix
1734+
// now we ensure code won't be written that makes assumptions about
1735+
// nLockTime that preclude a fix later.
1736+
txNew.nLockTime = chainActive.Height();
17271737

17281738
// Secondly occasionally randomly pick a nLockTime even further back, so
17291739
// that transactions that are delayed after signing for whatever reason,

0 commit comments

Comments
 (0)