Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RBF: Require unconfirmed inputs to come from a single conflicting transaction #29297

Conversation

petertodd
Copy link
Contributor

@petertodd petertodd commented Jan 23, 2024

Extends BIP-125 Rule #2 slightly. Rational: if we allow unconfirmed inputs to come from multiple conflicts, we could allow a desirable transaction, and an undesirable transaction, to be replaced by a single undesirable transaction.

This solves the same problem that #26451 was intended to solve, in a simpler way.

May not actually be worth adding to Bitcoin Core, as other solutions are coming. But I wrote the code for my replace-by-fee-rate work, where it fixes the infinite replacement cycle issue @murchandamus outlines here, so I figured I might as well open a pull-req.

Extends BIP-125 Rule #2 slightly. If we allow unconfirmed inputs to come from
multiple conflicts, we could allow a desirable transaction, and an undesirable
transaction, to be replaced by a single undesirable transaction.
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 23, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28886 (refactor: Replace sets of txiter with CTxMemPoolEntryRefs by TheCharlatan)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@morehouse
Copy link

morehouse commented Jan 24, 2024

As another benefit, I think this change helps prevent replacement cycling attacks against LN. The attacker would be unable to make their preimage spend depend on a second unconfirmed transaction, and therefore couldn't evict their preimage spend.

Edit: Actually, I think replacement cycling is still possible because the second parent doesn't need to be unconfirmed.

@petertodd
Copy link
Contributor Author

@morehouse

Actually, I think replacement cycling is still possible because the second parent doesn't need to be unconfirmed.

Heh, you nearly noticed the dumb mistake I made here. As Murch mentioned on the mailing list, this does not fix the infinite cycle problem with replace-by-fee-rate, as the second parent isn't unconfirmed.

However, if I change this pull req to only allowing fresh confirmed inputs, not including in an existing conflict, if transaction is being replaced that spends unconfirmed outputs, I think we've solved both, if only accidentally. :)

@petertodd
Copy link
Contributor Author

Closing. Turns out the exploit this was meant to solve doesn't actually exist: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2024-January/022314.html

@petertodd petertodd closed this Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants