-
Notifications
You must be signed in to change notification settings - Fork 37.4k
mempool: Add the bypass_feerate_accuracy
option to testmempoolaccept
#25532
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
Conversation
`maxfeerate` becomes a member of an "options" object rather than a positional argument. The idea is that any new parameters in the future will also go into options.
05e97ba
to
972674c
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Could you (or perhaps @ariard, seeing the suggestion at #25434 (review)) provide a use case for bypassing the minimum feerate checks? |
Let's say your a LN node receiving a counterparty's signature for a HTLC-timeout (BOLT2's @luke-jr are you raising that #7533 should be opened again as a technical approach is more straighforward ? If so I invite you to do so and let contributors evaluate which approach is the better. Or could you qualify more your concerns e.g the maintenance cost or API misusage ? If you're raising the concern that Lightning shouldn't make safety-critical assumptions on transaction-relay policy, I'll let the burden on you to propose the design of a second-layer to scale bitcoin payments as much decentralized and privacy-preserving (once we have fix all the issues). |
@MarcoFalke has already reopened it as #20753.
It shouldn't, but that's beside the point I am making here. There's nothing inherent in the original Lightning design that necessarily makes such assumptions... |
@w0xlt in the PR description, for reviewers I would suggest (a) mentioning that only the last 3 commits are new here, and (b) adding a "why" motivation in the PR description, because if the why doesn't make sense then the what probably doesn't matter. |
This is for test_accepts only, and not allowed in an actual submission to mempool - see assert statements. Provide an option to bypass BIP68 nSequence and nLockTime checks. This means clients can use testmempoolaccept to check whether L2 transaction chains (which typically have timelock conditions) are valid without submitting them. Note that BIP112 and BIP65 are still checked since they are script (non-contextual) checks. This does not invalidate any signature or script caching. Co-authored-by: glozow <gloriajzhao@gmail.com>
Co-authored-by: glozow <gloriajzhao@gmail.com>
Test the bypass_timelock options in testmempoolaccept. This lets us bypass BIP68 relative locktime checks (in nSequence) and absolute locktime checks (in nLocktime). Co-authored-by: glozow <gloriajzhao@gmail.com>
OP_CSV and OP_CLTV script checks are still done, so setting bypass_timelocks=True doesn't mean that bad scripts pass. Co-authored-by: glozow <gloriajzhao@gmail.com>
AFAICT #20753 only allows to bypass
And on that point, I really think the original Lightning design should have underscore more that failure to propagate a time-sensitive transaction should be considered as a break of the security model. Of course, if you're thinking about an ideal world where every LN node is a miner that wouldn't be an issue. That is not the real world. Or if you're saying that a LN node shouldn't make expectations about confirming a transaction before a timelock expiration, I believe that's the foundation of the payment channel design you're questioning. |
972674c
to
7368c07
Compare
Using a different JSON string for no reason is not an improvement. |
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
Seems like there isn't a reason to disable the feerate checks altogether. Rather, the better approach is to allow CPFP in the package submitted for testing, and the fee-bump should be included. |
Closing for now as this builds on #25434 which still needs conceptual review. Can reopen when this is no longer blocked. |
This PR adds the the
bypass_feerate_accuracy
option totestmempoolaccept
RPC, which allows skipping the minimum mempool fee rate validation.This enables testing of HTLC-timeout transactions signed with 0-feerate (with BOLT9
option_anchors_zero_fee_htlc_tx
).For more context, check #25434 (review).
Since this PR is built on top of #25577 and #25434, only the last 4 commits are new here.