From 352e6e25bb3153f1798993638b3eb4098017a76c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 12 May 2021 17:47:26 -0700 Subject: [PATCH] mempool: add additional test case for inherited RBF replacement In this commit, we add an additional test case for inherited RBF replacement. This test case asserts that if a parent is marked as being replaceable, but the child isn't, then the child can still be replaced as according to BIP 125 it shoudl _inhreit_ the replaceability of its parent. The addition of this test case was prompted by the recently discovered Bitcoin Core "CVE" [1]. It turns out that bitcoind doesn't properly implement BIP 125. Namely it fails to allow a child to "inherit" replaceability if its parent is also replaceable. Our implementation makes this trait rather explicit due to its recursive implementation. Kudos to the original implementer @wpaulino for getting this correct. [1]: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-May/018893.html. --- mempool/mempool_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 6d43cfd8..96d50544 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -1749,6 +1749,47 @@ func TestRBF(t *testing.T) { }, err: "", }, + { + // A transaction that doesn't signal replacement, can + // be replaced if the parent signals replacement. + name: "inherited replacement", + setup: func(ctx *testContext) (*btcutil.Tx, []*btcutil.Tx) { + coinbase := ctx.addCoinbaseTx(1) + + // Create an initial parent transaction that + // marks replacement, we won't be replacing + // this directly however. + coinbaseOut := txOutToSpendableOut(coinbase, 0) + outs := []spendableOutput{coinbaseOut} + parent := ctx.addSignedTx( + outs, 1, defaultFee, true, false, + ) + + // Now create a transaction that spends that + // parent transaction, which is marked as NOT + // being RBF-able. + parentOut := txOutToSpendableOut(parent, 0) + parentOuts := []spendableOutput{parentOut} + childNoReplace := ctx.addSignedTx( + parentOuts, 1, defaultFee, false, false, + ) + + // Now we'll create another transaction that + // replaces the *child* only. This should work + // as the parent has been marked for RBF, even + // though the child hasn't. + respendOuts := []spendableOutput{parentOut} + childReplace, err := ctx.harness.CreateSignedTx( + respendOuts, 1, defaultFee*3, false, + ) + if err != nil { + ctx.t.Fatalf("unable to create child tx: %v", err) + } + + return childReplace, []*btcutil.Tx{childNoReplace} + }, + err: "", + }, } for _, testCase := range testCases {