Add issuer-selected opt-in txn / pckg policy checks #29448

pull ariard wants to merge 1 commits into bitcoin:master from ariard:2024-issuer-selected-policy changing 7 files +277 −24
  1. ariard commented at 3:26 am on February 19, 2024: member

    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.

     0 (sequence_field & SEQUENCE_ISSUER_SELECTED_LIMITS_DISABLE_FLAG) {
     1        return std::nullopt;
     2    }
     3
     4    const uint32_t ancestor_limit = sequence_field & SEQUENCE_ISSUER_SELECTED_ANCESTOR_MASK;
     5    const uint32_t descendant_limit = sequence_field & SEQUENCE_ISSUER_SELECTED_DESCENDANT_MASK;
     6    const uint32_t weight_limit = (uint32_t)(sequence_field & SEQUENCE_ISSUER_SELECTED_WEIGHT_MASK)
     7     << SEQUENCE_ISSUER_SELECTED_WEIGHT_GRANULARITY;
     8
     9    // This parsing logic can be adapted for forward-compatibility in matters of issuer-selected
    10    // policy limits:
    11    //      - encodes accumulated package SigOpCost
    12    //      - commit to package-level dynamic DUST_RELAY_TX_FEE or DEFAULT_INCREMENTAL_RELAY_FEE
    13    //      - opt-in if limits are strict (at the pckg-level) or composable (at the tx-level)
    14    //        to allow non-interactive composability among N chain of transaction issuers.
    15
    16    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).

  2. Add opt-in issuer-selected txn/pckg policy checks ff97ce6034
  3. DrahtBot commented at 3:26 am on February 19, 2024: contributor

    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.

  4. DrahtBot added the label CI failed on Feb 19, 2024
  5. DrahtBot commented at 4:20 am on February 19, 2024: 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

  6. glozow commented at 12:23 pm on February 19, 2024: member

    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?

  7. ariard commented at 4:47 pm on February 19, 2024: member

    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.

  8. ariard closed this on Feb 19, 2024

  9. ariard commented at 7:09 pm on February 19, 2024: member

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 07:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me