Skip to content

Conversation

ariard
Copy link

@ariard ariard commented Feb 19, 2024

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 or MAX_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.

 (sequence_field & SEQUENCE_ISSUER_SELECTED_LIMITS_DISABLE_FLAG) {
        return std::nullopt;
    }

    const uint32_t ancestor_limit = sequence_field & SEQUENCE_ISSUER_SELECTED_ANCESTOR_MASK;
    const uint32_t descendant_limit = sequence_field & SEQUENCE_ISSUER_SELECTED_DESCENDANT_MASK;
    const uint32_t weight_limit = (uint32_t)(sequence_field & SEQUENCE_ISSUER_SELECTED_WEIGHT_MASK)
     << SEQUENCE_ISSUER_SELECTED_WEIGHT_GRANULARITY;

    // This parsing logic can be adapted for forward-compatibility in matters of issuer-selected
    // policy limits:
    //      - encodes accumulated package SigOpCost
    //      - commit to package-level dynamic DUST_RELAY_TX_FEE or DEFAULT_INCREMENTAL_RELAY_FEE
    //      - opt-in if limits are strict (at the pckg-level) or composable (at the tx-level)
    //        to allow non-interactive composability among N chain of transaction issuers.

    IssuerSelectedLimits tx_issuer_selected_limits(ancestor_limit, descendant_limit, weight_limit);

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 mempool
acceptance 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).

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 19, 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:

  • #28970 ([WIP] p2p: opportunistically accept 1-parent-1-child packages by glozow)
  • #28031 (Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling by glozow)

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.

@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/21711206160

Copy link
Member

@glozow glozow left a 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?

@ariard
Copy link
Author

ariard commented Feb 19, 2024

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.

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.

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?

I thought more about it. That approach is currently too much ambitious and not conservative enough. I'll do a write-up.

@ariard
Copy link
Author

ariard commented Feb 19, 2024

did it #29454

@bitcoin bitcoin locked and limited conversation to collaborators Feb 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants