Reject replacements that add new unconfirmed inputs

This commit is contained in:
Peter Todd 2015-10-30 00:04:00 -04:00
parent fc8c19a07c
commit b272ecfdb3
2 changed files with 59 additions and 4 deletions

View File

@ -35,11 +35,11 @@ class Test_ReplaceByFee(unittest.TestCase):
cls.proxy = bitcoin.rpc.Proxy()
@classmethod
def tearDownClass(cls):
# Make sure mining works
def mine_mempool(cls):
"""Mine until mempool is empty"""
mempool_size = 1
while mempool_size:
cls.proxy.call('generate',1)
cls.proxy.call('generate', 1)
new_mempool_size = len(cls.proxy.getrawmempool())
# It's possible to get stuck in a loop here if the mempool has
@ -47,10 +47,18 @@ class Test_ReplaceByFee(unittest.TestCase):
assert(new_mempool_size != mempool_size)
mempool_size = new_mempool_size
def make_txout(self, amount, scriptPubKey=CScript([1])):
@classmethod
def tearDownClass(cls):
# Make sure mining works
cls.mine_mempool()
def make_txout(self, amount, confirmed=True, scriptPubKey=CScript([1])):
"""Create a txout with a given amount and scriptPubKey
Mines coins as needed.
confirmed - txouts created will be confirmed in the blockchain;
unconfirmed otherwise.
"""
fee = 1*COIN
while self.proxy.getbalance() < amount + fee:
@ -72,6 +80,10 @@ class Test_ReplaceByFee(unittest.TestCase):
tx2_txid = self.proxy.sendrawtransaction(tx2, True)
# If requested, ensure txouts are confirmed.
if confirmed:
self.mine_mempool()
return COutPoint(tx2_txid, 0)
def test_simple_doublespend(self):
@ -276,5 +288,24 @@ class Test_ReplaceByFee(unittest.TestCase):
else:
self.fail()
def test_new_unconfirmed_inputs(self):
"""Replacements that add new unconfirmed inputs are rejected"""
confirmed_utxo = self.make_txout(1.1*COIN)
unconfirmed_utxo = self.make_txout(0.1*COIN, False)
tx1 = CTransaction([CTxIn(confirmed_utxo)],
[CTxOut(1.0*COIN, CScript([b'a']))])
tx1_txid = self.proxy.sendrawtransaction(tx1, True)
tx2 = CTransaction([CTxIn(confirmed_utxo), CTxIn(unconfirmed_utxo)],
tx1.vout)
try:
tx2_txid = self.proxy.sendrawtransaction(tx2, True)
except bitcoin.rpc.JSONRPCException as exp:
self.assertEqual(exp.error['code'], -26)
else:
self.fail()
if __name__ == '__main__':
unittest.main()

View File

@ -1009,6 +1009,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
LOCK(pool.cs);
CFeeRate newFeeRate(nFees, nSize);
set<uint256> setConflictsParents;
BOOST_FOREACH(const uint256 hashConflicting, setConflicts)
{
CTxMemPool::txiter mi = pool.mapTx.find(hashConflicting);
@ -1042,6 +1043,11 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
REJECT_INSUFFICIENTFEE, "insufficient fee");
}
BOOST_FOREACH(const CTxIn &txin, mi->GetTx().vin)
{
setConflictsParents.insert(txin.prevout.hash);
}
// For efficiency we simply sum up the pre-calculated
// fees/size-with-descendants values from the mempool package
// tracking; this does mean the pathological case of diamond tx
@ -1050,6 +1056,24 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
nConflictingSize += mi->GetSizeWithDescendants();
}
for (unsigned int j = 0; j < tx.vin.size(); j++)
{
// We don't want to accept replacements that require low
// feerate junk to be mined first. Ideally we'd keep track of
// the ancestor feerates and make the decision based on that,
// but for now requiring all new inputs to be confirmed works.
if (!setConflictsParents.count(tx.vin[j].prevout.hash))
{
// Rather than check the UTXO set - potentially expensive -
// it's cheaper to just check if the new input refers to a
// tx that's in the mempool.
if (pool.mapTx.find(tx.vin[j].prevout.hash) != pool.mapTx.end())
return state.DoS(0, error("AcceptToMemoryPool: replacement %s adds unconfirmed input, idx %d",
hash.ToString(), j),
REJECT_NONSTANDARD, "replacement-adds-unconfirmed");
}
}
// The replacement must pay greater fees than the transactions it
// replaces - if we did the bandwidth used by those conflicting
// transactions would not be paid for.