Catch harmful arbitrary data. (missing code for #32359) #32368

pull Retropex wants to merge 4 commits into bitcoin:master from Retropex:data changing 9 files +304 −0
  1. Retropex commented at 8:20 pm on April 28, 2025: none

    Some people in the discussion of the PR #32359 are suggesting that OP_RETURN should be used instead of OP_IF, OP_FALSE data injections.

    IMO, PR #32359 should never be merged, as it would encourage the storage of arbitrary data on Bitcoin. But if the PR is still merged, then this code should be added, because large data insertions have no incentive to move to OP_RETURN.

    image

  2. script: Add CScript::DummyScriptBytes a3068674ff
  3. policy: GetScriptForTransactionInput to figure out P2SH, witness, taproot f30200fb0f
  4. Apply -limitdummyscriptdatasize cca03b982e
  5. QA: script_tests: Check GetScriptForTransactionInput and CScript::DummyScriptBytes() 7c35312964
  6. DrahtBot commented at 8:20 pm on April 28, 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/32368.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept NACK Sjors

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32359 (Remove arbitrary limits on OP_Return (datacarrier) outputs by petertodd)
    • #32133 (RFC: Accept non-std transactions in Testnet4 by default again by fjahr)
    • #31576 (test: Move script_assets_tests into its own suite 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.

  7. Retropex renamed this:
    Catch nocive arbitrary data. (missing code for #32359)
    Catch harmful arbitrary data. (missing code for #32359)
    on Apr 28, 2025
  8. DrahtBot added the label CI failed on Apr 28, 2025
  9. DrahtBot commented at 6:01 am on April 29, 2025: contributor

    🚧 At least one of the CI tasks failed. Task: https://github.com/bitcoin/bitcoin/runs/41302883721 LLM reason (✨ experimental): Test failure caused by an assertion error in test_framework/util.py: assert_equal(False, True) indicating a mismatch between expected and actual mempool transaction presence.

    Hints

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly 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.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

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

  10. Sjors commented at 7:22 pm on April 30, 2025: member

    NACK

    This is a duplicate of #29520 by the same author.

  11. maflcko commented at 7:58 am on May 5, 2025: member
    • This was opened with failing tests, with no further action for more than a week. (Fixing the tests, or at least running them to confirm they are passing at a minimum before opening a pull request should be trivial)
    • It is a duplicate of a closed pull request, which was open in total for more than half a year, but never had a single code-review ack and passing tests/CI at the same time. Also, outstanding feedback from reviewers, such as #29520 (review) or #29520 (comment) was never addressed.

    If you want to contribute in the future, please make sure to compile the code and run the tests before opening a pull request. Also, make sure to add tests for newly added features, or for changed features. Finally, addressing review feedback instead of ignoring it may encourage reviewers to take another look.

  12. maflcko closed this on May 5, 2025


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-05-05 12:12 UTC

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