LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #15039: wallet: Avoid leaking nLockTime fingerprint when anti-fee-sniping

Browse source 
fa48baf23e wallet: Avoid leaking locktime fingerprint when anti-fee-sniping (MarcoFalke)
453803adc9 [test] wallet_txn_clone: Correctly clone txin sequence (MarcoFalke)

Pull request description:

  The wallet sets the locktime to the current height of our active chain. This is fine, as long as our node is connected to other nodes. However, when we fall back and get stuck at a particular height (e.g. taking the wallet offline), the same (potentially unique) locktime is used for all transactions. This makes it easier for passive observers to cluster transactions by wallet.

  For reference, I visualized "locktime-reuse" with the data:
  * blocks 545k-555k (both inclusive)
  * locktimes<=60k
  * excluding coinbase txs

  ![distribution of height-based tx locktimes used at least twice](https://user-images.githubusercontent.com/6399679/50446163-b8256d80-0913-11e9-9832-40b76052b2b9.png)

Tree-SHA512: 2af259dd8f9f863312e2732d80ca8ba6a20c8d6d1c486b10a48479e1c85ccf13b0c38723740ebadde0f28d321cd9c133ad3e5d1e925472eb27681143bda2d0e7
This commit is contained in:
Wladimir J. van der Laan 2019-01-10 15:56:44 +01:00
commit cebe910718
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
4 changed files with 97 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
src/wallet/wallet.cpp
View file

@ -2516,6 +2516,65 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
return true;
}
static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain)
{
if (IsInitialBlockDownload()) {
return false;
}
constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60; // in seconds
if (chainActive.Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) {
return false;
}
return true;
}
/**
* Return a height-based locktime for new transactions (uses the height of the
* current chain tip unless we are not synced with the current chain
*/
static uint32_t GetLocktimeForNewTransaction(interfaces::Chain::Lock& locked_chain)
{
uint32_t locktime;
// Discourage fee sniping.
//
// For a large miner the value of the transactions in the best block and
// the mempool can exceed the cost of deliberately 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.
if (IsCurrentForAntiFeeSniping(locked_chain)) {
locktime = chainActive.Height();
// Secondly occasionally randomly pick a nLockTime even further back, so
// that transactions that are delayed after signing for whatever reason,
// e.g. high-latency mix networks and some CoinJoin implementations, have
// better privacy.
if (GetRandInt(10) == 0)
locktime = std::max(0, (int)locktime - GetRandInt(100));
} else {
// If our chain is lagging behind, we can't discourage fee sniping nor help
// the privacy of high-latency transactions. To avoid leaking a potentially
// unique "nLockTime fingerprint", set nLockTime to a constant.
locktime = 0;
}
assert(locktime <= (unsigned int)chainActive.Height());
assert(locktime < LOCKTIME_THRESHOLD);
return locktime;
}
OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend)
{
// If -changetype is specified, always use that change type.
@ -2570,37 +2629,8 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
CMutableTransaction txNew;
// Discourage fee sniping.
//
// For a large miner the value of the transactions in the best block and
// the mempool can exceed the cost of deliberately 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();
txNew.nLockTime = GetLocktimeForNewTransaction(locked_chain);
// Secondly occasionally randomly pick a nLockTime even further back, so
// that transactions that are delayed after signing for whatever reason,
// e.g. high-latency mix networks and some CoinJoin implementations, have
// better privacy.
if (GetRandInt(10) == 0)
txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100));
assert(txNew.nLockTime <= (unsigned int)chainActive.Height());
assert(txNew.nLockTime < LOCKTIME_THRESHOLD);
FeeCalculation feeCalc;
CAmount nFeeNeeded;
int nBytes;

1
test/functional/test_runner.py
View file

@ -174,6 +174,7 @@ BASE_SCRIPTS = [
'wallet_fallbackfee.py',
'feature_minchainwork.py',
'rpc_getblockstats.py',
'wallet_create_tx.py',
'p2p_fingerprint.py',
'feature_uacomment.py',
'wallet_coinbase_category.py',

35
test/functional/wallet_create_tx.py Executable file
View file

@ -0,0 +1,35 @@
#!/usr/bin/env python3
# Copyright (c) 2018 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
)
class CreateTxWalletTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = False
self.num_nodes = 1
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
def run_test(self):
self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled')
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200)
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex'])
assert_equal(tx['locktime'], 0)
self.log.info('Check that anti-fee-sniping is enabled when we mine a recent block')
self.nodes[0].generate(1)
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex'])
assert 0 < tx['locktime'] <= 201
if __name__ == '__main__':
CreateTxWalletTest().main()

2
test/functional/wallet_txn_clone.py
View file

@ -65,7 +65,7 @@ class TxnMallTest(BitcoinTestFramework):
# Construct a clone of tx1, to be malleated
rawtx1 = self.nodes[0].getrawtransaction(txid1, 1)
clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"]}]
clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"], "sequence": rawtx1["vin"][0]["sequence"]}]
clone_outputs = {rawtx1["vout"][0]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][0]["value"],
rawtx1["vout"][1]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][1]["value"]}
clone_locktime = rawtx1["locktime"]