-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Add issuer-selected opt-in txn / pckg policy checks #29448
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
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. 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. |
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really clear to me what you're trying to achieve in this PR - some kind of way for transactions to specify what their ancestor/descendant limits are? There are no tests, the CI is failing, and a lot of the code comments are incorrect/irrelevant to the adjacent lines.
I also don't really understand the motivation from the PR description. There seems to be a large number of goals and issues that aren't clearly defined. Would you mind writing something a bit clearer and more concise?
Yeah some kind of way for transactions to specify what their ancestor / descendant / max weight / limits, in the hard limits of current mempool ones. I know no test and CI failing, it’s just proof-of-concept code to test how it would fit in a package world. E.g for LN, you could adjust your pinning / dust inflation risk exposure in function of each of your lightning peer, as an element negotiated as part of the funding flow.
I thought more about it. That approach is currently too much ambitious and not conservative enough. I'll do a write-up. |
did it #29454 |
All the values selected by the transaction issuers are implemented to respect current hard policy limits as of 27.0
Bitcoin Core version (e.g
MAX_PACKAGE_COUNT
orMAX_STANDARD_TX_WEIGHT
). As such introducing a new distinction in the Bitcoin ecosystem among tx-relay policy checks enforced by full-nodes hosts of a said full-node implementation/version and the the policy-check opt-ed by any transaction or second-layers use-cases. That way any significant on-chain economic traffic can be still processed by low-performance full-nodes hosts, without altering the DoS profile risks.E.g in the context of the Lightning Network, lightning nodes can adjust their pinning risk exposures affecting their channel funds safety differently in function of each lightning topological peers (- assuming the BOLT protocol upgrades its negotiation flow
open_channel
/accept_channel
). As such lightning peers can enforce a trade-off between their off chain HTLC throughputs and their tx-relay jamming exposure (e.g pinning or RC attacks).E.g in the context of collaborative custody management, distrusted stakeholders owning a set of pre-signed set of transactions can all commits to the same set of max tx size / max package limits, as such introducing more reliability on the worst amount of fee-bumping reserves that should be provisioned instead of hard limits like the current approach with v3.
Wait ! I’m lost reading this OP ?! Already too much information, what should I do ? Go to read Bitcoin Optech 10 articles series’s “Waiting for confirmation: a series about mempool and relay policy” as a starter and then come back to read or comment.
The opt-in mechanism uses a one bit flag in the nSequence field which is already a standard and consenus Bitcoin transaction field since the genesis block. The mechansim (
ParsePackageTopologicalLimits()
) check that bip68 is disabled to avoid conflicts of semantics, as the remaining nSequence field bits are interpreted as dynamic issuers-selected policy checks (currently implemented ancestor / descendant / max package limits size). The intrusive aspect of the mechanism is minimal and the interpreted field could be uplifted in other parts of standard transaction fields while conserving the same policy check enforcement semantics.Here the code excerpt that deserves a
doc/policy/
, a BIP or a banana.Issuer-selected fields getting economic popularity and not jeopardizing bitcoin security model and long-term decentralization equlibrium of the ecosystem of peer-to-peer full-nodes and solo mining operations could get a consensus enforcement, going through the traditional consensus design & development process (narrator note: no ones really knows or is sure there is something as traditional consensus development process...). The nSequence consensus field due to its 32 bits limitation is space limited, even if a lot can be achieved with bit-wise operations.
From a technical philosophy perspective, I think this introduction of issuer-selected transaction / package policy limits checks should should reduce the amount of "bargaing-on-policy" we have seen recently with
mempoolfullrbf
(LN-vs-zero-conf-acceptance business) or do-digital-art-on-chain-and-the-posterity-will-say-if-it-is-bansky-or-crap-waste colored coins and tokens experimentations.This patchset built on the v3 patchset and its followups (commit
37fdf5a4
), is already integrated in the package mempoolacceptance code flow. There is 1 logical conflict for now as the 1000 vbytes limit is enforced by default for v3 (which still
exposes a lot of loss of funds risk exposure for some time-sensitive flows like lightning considering "loophole" and NTA pinning scenario - see #28948). In the future, this limit can just become a "tx issuer-selected policy check" limit.
In the future, such transaction / package “issuer-selected” can be used as a signaling mechanism, e.g for replace-by-feerate dynamic window enforced at the mempool-level, for the use-cases who wishing to ensure their chain of transactions are minimal by not using CPFP.
For more discussions on mempool policy technical philosophy design and trade-offs on downstream use-cases, see the old email thread "On mempool consistency".
This patchset can be applied on top of any fork of bitcoin core 27.0 with the minimal of engineering effort by anyone wishing
to experiment with mempool policy for its use-case (I'll see if I have time to carry this patchset forward as I'm officially in
sabbatical from bitcoin core development taking the sun on the beach just yelling at them when the stuff is broken, we’re all btc investors).