mempool: Add the `bypass_{csv,cltv}` option to `testmempoolaccept` #25570

w0xlt · 2022-07-08T04:51:30Z

This PR adds the the bypass_csv and bypass_cltv options to testmempoolaccept RPC, making it possible to bypass CSV / CLTV execution.

For more context, check #25434 (review).

Since this PR is built on top of #25577, #25434 and #25532, only the last 4 commits are new here.

`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.

DrahtBot · 2022-07-08T09:36:36Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25577 (mempool, refactor: add MemPoolBypass by w0xlt)
#25487 ([kernel 3b/n] Decouple {Dump,Load}Mempool from ArgsManager by dongcarl)
#24007 ([mempool] allow tx replacement by smaller witness by LarryRuane)
#23897 (refactor: Move calculation logic out from CheckSequenceLocksAtTip() by hebasto)

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.

bvbfan · 2022-07-08T14:44:15Z

One small note: multi-line comments are evil. Consider a valid code

/*
func(/*param*/ p);

comment captures entire function.
Consider

struct ProcessTxCriteria {
    bool test_accept;
    std::optional<MempoolTestCriteria> test_criteria;
} defaultTxCriterias = {};

ProcessTransaction(txr, const ProcessTxCriteria& criterias = defaultTxCriterias);

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>

w0xlt · 2022-07-15T20:57:10Z

Rebased on top of 972674c.

@bvbfan Good suggestion. A similar struct was implemented in 9e8703c

DrahtBot · 2022-07-18T15:52:12Z

🐙 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".}

fanquake · 2022-08-01T14:24:16Z

Since this PR is built on top of #25577, #25434 and #25532,

I'm going to close this for now. I don't think there is a need to have this PR open yet, when it's building on multiple other PRs, which themselves still need conceptual review. You can always link to branches with example future changes, from the base PRs.

w0xlt added 2 commits July 1, 2022 15:48

mempool: change maxfeerate to be a members of an options argument

199b314

`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.

mempool: make test_accept parameter explicit

563dd14

DrahtBot added the Mempool label Jul 8, 2022

w0xlt mentioned this pull request Jul 8, 2022

mempool: Add option to bypass contextual timelocks in testmempoolaccept #25434

Closed

This was referenced Jul 8, 2022

[mempool] allow tx replacement by smaller witness #24007

Closed

refactor: Move calculation logic out from CheckSequenceLocksAtTip() #23897

Merged

This was referenced Jul 9, 2022

mempool, refactor: add MemPoolBypass #25577

Closed

test: testmempoolaccept accepts transactions from fail_txs in feature_csv_activation.py #25567

Closed

w0xlt and others added 10 commits July 9, 2022 20:38

validation: Add MemPoolBypass for bypassing mempool checks

9e8703c

mempool: add MemPoolBypass parameter to ProcessNewPackage

14eff47

mempool: add MemPoolBypass parameter to AcceptToMemoryPool

420a3c4

mempool: add MemPoolBypass parameter to ProcessTransaction

17db819

mempool: add MemPoolBypass parameter to ATMPArgs

26ee1a4

validation: add m_bypass_{relative,absolute}_timelock

12ac626

rpc: add optional bypass_timelocks for testmempoolaccept

afc2998

Co-authored-by: glozow <gloriajzhao@gmail.com>

test: check if bypass_{absolute,relative}_timelock works

7ec6416

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>

test: check if bypass_timelocks do not affect scripts

a006b8f

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>

DrahtBot mentioned this pull request Jul 13, 2022

[kernel 3b/n] Decouple {Dump,Load}Mempool from ArgsManager #25487

Merged

w0xlt added 8 commits July 15, 2022 15:56

validation: Add bypass_feerate_accuracy to MempoolTestCriteria

20d049d

validation: Add bypass_feerate_accuracy validation logic

4bf7da5

rpc: add bypass_feerate_accuracy option to testmempoolaccept RPC

9ec3f3b

test: add functional tests for bypass_feerate_accuracy option

7368c07

validation: Add m_bypass_{csv,cltv} to MempoolTestCriteria

643126d

validation: add logic to bypass CSV/CLTV execution

7b4036e

rpc: add bypass_{csv,cltv} params to testmempoolaccept RPC

cbe36c0

test: add tests for bypass_cltv and bypass_csv

5fa1770

w0xlt force-pushed the bypass-op-1 branch from 12de6b9 to 5fa1770 Compare July 15, 2022 20:49

DrahtBot added the Needs rebase label Jul 18, 2022

fanquake closed this Aug 1, 2022

bitcoin locked and limited conversation to collaborators Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mempool: Add the `bypass_{csv,cltv}` option to `testmempoolaccept` #25570

mempool: Add the `bypass_{csv,cltv}` option to `testmempoolaccept` #25570

Uh oh!

w0xlt commented Jul 8, 2022 •

edited

Loading

Uh oh!

DrahtBot commented Jul 8, 2022 •

edited

Loading

Uh oh!

bvbfan commented Jul 8, 2022 •

edited

Loading

Uh oh!

w0xlt commented Jul 15, 2022

Uh oh!

DrahtBot commented Jul 18, 2022

Uh oh!

fanquake commented Aug 1, 2022

Uh oh!

Uh oh!

mempool: Add the bypass_{csv,cltv} option to testmempoolaccept #25570

mempool: Add the bypass_{csv,cltv} option to testmempoolaccept #25570

Uh oh!

Conversation

w0xlt commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

bvbfan commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w0xlt commented Jul 15, 2022

Uh oh!

DrahtBot commented Jul 18, 2022

Uh oh!

fanquake commented Aug 1, 2022

Uh oh!

Uh oh!

mempool: Add the `bypass_{csv,cltv}` option to `testmempoolaccept` #25570

mempool: Add the `bypass_{csv,cltv}` option to `testmempoolaccept` #25570

w0xlt commented Jul 8, 2022 •

edited

Loading

DrahtBot commented Jul 8, 2022 •

edited

Loading

bvbfan commented Jul 8, 2022 •

edited

Loading