Take the training wheels off anti-fee-sniping

Now that the off-by-one error w/nLockTime txs issue has been fixed by
87550eef (75a4d512 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 files changed, 19 insertions, 9 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3f12d88e79..50d20485de 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1715,15 +1715,25 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend,
 
     // Discourage fee sniping.
     //
-    // However because of a off-by-one-error in previous versions we need to
-    // neuter it by setting nLockTime to at least one less than nBestHeight.
-    // Secondly currently propagation of transactions created for block heights
-    // corresponding to blocks that were just mined may be iffy - transactions
-    // aren't re-accepted into the mempool - we additionally neuter the code by
-    // going ten blocks back. Doesn't yet do anything for sniping, but does act
-    // to shake out wallet bugs like not showing nLockTime'd transactions at
-    // all.
-    txNew.nLockTime = std::max(0, chainActive.Height() - 10);
+    // For a large miner the value of the transactions in the best block and
+    // the mempool can exceed the cost of delibrately attempting to mine two
+    // blocks to orphan the current best block. By setting nLockTime such that
+    // only the next block can include the transaction, we discourage this
+    // practice as the height restricted and limited blocksize gives miners
+    // considering fee sniping fewer options for pulling off this attack.
+    //
+    // A simple way to think about this is from the wallet's point of view we
+    // always want the blockchain to move forward. By setting nLockTime this
+    // way we're basically making the statement that we only want this
+    // transaction to appear in the next block; we don't want to potentially
+    // encourage reorgs by allowing transactions to appear at lower heights
+    // than the next block in forks of the best chain.
+    //
+    // Of course, the subsidy is high enough, and transaction volume low
+    // enough, that fee sniping isn't a problem yet, but by implementing a fix
+    // now we ensure code won't be written that makes assumptions about
+    // nLockTime that preclude a fix later.
+    txNew.nLockTime = chainActive.Height();
 
     // Secondly occasionally randomly pick a nLockTime even further back, so
     // that transactions that are delayed after signing for whatever reason,