More comprehensive datacarrier configuration #33682

pull jotapea wants to merge 1 commits into bitcoin:30.x from jotapea:2025-10-expand_datacarrier_conf_default_revert changing 30 files +218 −109
  1. jotapea commented at 5:41 pm on October 22, 2025: none

    This PR expands the control and testing of datacarrier transactions by decoupling the total bytes limit from the policy configuration.


    While doing this, the historical money-first defaults for OP_RETURN outputs were reverted.

    The motivation for this PR is to continue allowing each node to decide/signal their mempool relay (and mining) policy. However, the open-data relaying capacity of v30.0 is acknowledged and build upon.

    The unlimited datacarrier policy is possible with the following configuration:

    0# open-data policy
    1datacarrier=1 # not required
    2datacarriersize=0
    3datacarriercount=0
    

    But even without the reversion of defaults, the main scope of this PR still applies.

    First, for more robust and complete testing. And second, to allow full backwards compatibility of datacarrier policy. Specifically, to allow limiting the amount of OP_RETURN outputs to just 1.

    0# money-first policy
    1datacarriersize=83
    2datacarriercount=1 # used to be hardcoded
    

    This configuration mimics the exact historical (default) datacarrier policy, which is not possible in v30.0.

    https://github.com/bitcoin/bitcoin/blob/52ede28a8adb2c2d44d7f800bbfbef8aed86070e/src/policy/policy.cpp#L140-L166

    Note that nDataOut, which counted the number of OP_RETURN outputs, was removed in #32406. Without a count check, v30.0 nodes with -datacarriersize=83 are still relaying transactions with multiple OP_RETURN outputs, transactions which all nodes prior to v30.0 will reject…

  2. Expanded unlimited datacarrier configuration and testing, with historical defaults 238e7357b7
  3. DrahtBot added the label Backport on Oct 22, 2025
  4. DrahtBot commented at 5:41 pm on October 22, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33682.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept NACK cedwies, waketraindev

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • whose any data-carrying raw scriptPubKey output is of this size or less. -> for which any data-carrying raw scriptPubKey output is of this size or less. [“whose any” is ungrammatical; “for which any” makes the relationship between transaction and its outputs clear]
    • // null is datacarrier=0, 0 is unlimited datacarriersize=0 -> // null means datacarrier disabled; 0 means datacarriersize is unlimited. [“null is datacarrier=0” is unclear/inaccurate English; expand to explain meaning of null vs 0]
    • // null is datacarrier=0, 0 is unlimited datacarriercount=0 -> // null means datacarrier disabled; 0 means datacarriercount is unlimited. [same issue as above; clarify the meanings of null and 0]

    drahtbot_id_5_m

  5. in test/functional/mempool_datacarrier.py:140 in 238e7357b7
    174+        self.test_null_data_transaction(node=self.nodes[4], data=over_maximum_data, outputs=1, success=False, expected_error="tx-size")
    175+
    176+        # TODO
    177+        self.log.info("Testing limits for null data outputs in a transaction.")
    178+        self.test_null_data_transaction(node=self.nodes[5], data=None, outputs=int(MAX_DATACARRIER_RELAY/100), success=True)
    179+        self.test_null_data_transaction(node=self.nodes[5], data=None, outputs=int(MAX_DATACARRIER_RELAY/10), success=False, expected_error="tx-size")
    


    jotapea commented at 5:50 pm on October 22, 2025:
    I must be missing something, because these added tests only work if any (2) previous tests are commented out. Getting the following error: bad-txns-inputs-missingorspent (-25).
  6. cedwies commented at 8:11 pm on October 22, 2025: none

    Concept NACK

    Adding a new startup option (-datacarriercount) increases policy heterogeneity across the network. As discussed in the motivation, more heterogeneity → less mempool overlap → more missing transactions during compact block reconstruction → slower propagation on the critical path. v30 moved in the opposite direction to improve relay quality. Reintroducing another dimension of variance cuts against that goal.

    v30 deliberately changed the meaning of -datacarriersize (aggregated budget). This PR changes the semantics again (back to budget/output) and adds a new flag. Even if each change is defensible on its own (needs discussion), I do not support flipping meanings across consecutive releases, as I think it is a recipe for misconfiguration and surprises for operators and tooling.

    If it would be reasonable to change defaults so soon after v30, I think we have to see concrete evidence that the new defaults measurably improve the network (e.g., better compact-block hit rates, fewer round trips, fewer time-to-first-seen outliers), rather than just restoring historical behavior.

    From your motivation:

    The usefulness is control of the number of outputs separated from the size per output. Having both be one is sloppy. Multiple nulldata outputs is a new feature, which should be configurable.

    “Sloppy” does not quantify any harm from the aggregate approach (DoS, CPU, bandwidth, propagation, …?). Without measurements, we’re trading a known-uniform policy for a more fragmented one on intuition.

    Summing up: The change which was brought with v30 was broadly discussed and deliberately chosen. I think that reverting the defaults now would be a bad idea. And even if, the path to do this should not be this PR and changing defaults should not be bundled with introducing a new knob.

  7. jotapea commented at 8:25 pm on October 22, 2025: none

    Much appreciated feedback @cedwies . As mentioned, the main features of the PR are:

    First, for more robust and complete testing. And second, to allow full backwards compatibility of datacarrier policy. Specifically, to allow limiting the amount of OP_RETURN outputs to just 1.

    So, I can easily do a separate PR keeping v30.0 defaults. But will not for now, in order to not fragment the conversation.

    (I truly believe reverting the defaults asap is a net positive (some damage can’t be undone) for the bitcoin-core team.)

    Thank you for being clear and up front about the deliberate changes in v30.0. Changes to defaults. That, should be a separate convesation.

  8. waketraindev commented at 8:59 pm on October 22, 2025: none

    Concept NACK

    Unhealthy for the network and peers. Relies on other peers to deliver already seen and rejected transactions upon confirmation and rebroadcast. Default extra pool sizes are too small to retain arbitrary rejected transactions, whether they use datacarrier=0 or otherwise.

  9. achow101 commented at 9:13 pm on October 22, 2025: member

    Concept NACK

    I don’t think any contributors are interested in re-litigating this. Please see all of the commentary that has already been made on this topic.

  10. achow101 closed this on Oct 22, 2025

  11. jotapea commented at 5:54 pm on October 24, 2025: none

    more heterogeneity → less mempool overlap → more missing transactions during compact block reconstruction → slower propagation on the critical path @cedwies (hopefully with @achow101 ’s permission), didn’t v30.0 increase this heterogeneity? i.e:

    v30.0 nodes with -datacarriersize=83 are still relaying transactions with multiple OP_RETURN outputs, transactions which all nodes prior to v30.0 will reject..

    Fixing #33690 would improve it for all users that don’t want to change their policies.

    Anyway, it is clear that the “Current Core Contributors” are way too invested to be individual thinkers, you are all repeating the same non-sense while actually being culpable of precisely what you are preaching: v30 has increased the heterogeneity in all ways it can be evaluated. Making next order considerations like compact block reconstruction and slower propagation on the critical path are practically irrelevant until the desired heterogeneity is achieved. AND this by itself doesn’t sound very decentralized, so not sure why it has become a “goal”.

    If you were intellectually sincere, you would AT LEAST rephrase it like: “v30 HOPES to achieve heterogeneity by taking a definite stand on open-data policy, while increasing this heterogeneity it in the meantime”.

    First principles people, you are becoming shitcoin-heads that just repeat buzzwords to appear smarter.


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: 2025-11-02 18:12 UTC

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