[TESTS] Allow tx_invalid.json tests to include flag rules for if_unset: [A,B,C] then_unset: [D] #22954

pull JeremyRubin wants to merge 1 commits into bitcoin:master from JeremyRubin:if_unset_then_unset changing 2 files +52 −4
  1. JeremyRubin commented at 11:14 PM on September 11, 2021: contributor

    This allows an optional tx_invalid.json parameter at position 3 with the format:

    [{"if_unset": ["A", "D"], "then_unset": ["B", "C"]}]
    

    During the one-by-one flag mutation (patched in #22948 to be all combinations, hence the support of multiple) which checks that if disable required flags ["A", "D"] are all unset then the flags ["B", "C"] should become unset as well. The unsetting logic loops until the flags are unset (so rules like if_unset A then_unset B and if_unset B then_unset D end up with B and D unset if A is unset and B is set initially).

    The current use case of this modification is:

    [{"if_unset": ["DEFAULT_CHECK_TEMPLATE_VERIFY_HASH"], "then_unset": ["DISCOURAGE_UPGRADABLE_NOPS"]}]
    

    This allows us to write use cases wherein we expect DEFAULT_CHECK_TEMPLATE_VERIFY_HASH to be enabled for the tx to be invalid, but desire for it to be valid (and not discouraged) when it is off. After activation of such a soft fork, we can remove such a rule.

    We do not want to do this via a TrimFlags rule as it should not happen across all transactions, only the ones we specify.

    It would also be possible to have a list of allowed unsettable flags (e.g., just DISCOURAGE_UPGRADABLE_NOPS and other standardness rules), but it is not required. While the allowing of multiple rules is perhaps extra, this keeps the format flexible for future needs.

  2. fanquake added the label Tests on Sep 11, 2021
  3. DrahtBot commented at 3:27 PM on September 12, 2021: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  4. DrahtBot cross-referenced this on Sep 12, 2021 from issue [Tests] Compute the Power Set of all flags instead of one by one exclusion by JeremyRubin
  5. DrahtBot cross-referenced this on Nov 18, 2021 from issue scripted-diff: Use clang-tidy syntax for C++ named arguments by maflcko
  6. DrahtBot cross-referenced this on Nov 18, 2021 from issue scripted-diff: Use clang-tidy syntax for C++ named arguments (tests only) by maflcko
  7. DrahtBot added the label Needs rebase on Dec 1, 2021
  8. JeremyRubin commented at 6:14 PM on December 12, 2021: contributor

    @MarcoFalke is their an automated tool to run to fix this PR?

  9. maflcko commented at 7:49 AM on December 13, 2021: member

    @JeremyRubin I think git will always consider context, but you can use patch for an automated approach to ignore changes in adjacent lines:

    $ git show -U0 7f4228566eafd91c3201e794ef2d82ca884db8c6 | patch -p1 --dry-run  
    checking file src/test/data/tx_invalid.json
    checking file src/test/transaction_tests.cpp
    Hunk [#1](/github-metadata-backup-bitcoin-bitcoin/1/) succeeded at 289 (offset 1 line).
    Hunk [#2](/github-metadata-backup-bitcoin-bitcoin/2/) succeeded at 297 (offset 1 line).
    Hunk [#3](/github-metadata-backup-bitcoin-bitcoin/3/) succeeded at 391 (offset 1 line).
    Hunk [#4](/github-metadata-backup-bitcoin-bitcoin/4/) succeeded at 394 (offset 1 line).
    Hunk [#5](/github-metadata-backup-bitcoin-bitcoin/5/) succeeded at 411 (offset 1 line).
    
  10. JeremyRubin commented at 8:22 AM on December 13, 2021: contributor

    i was asking because i think the conflict arose out of a scripted diff PR of yours that was merged, I was curious if you reckon that it will fix these conflicts too...

  11. maflcko cross-referenced this on Dec 15, 2021 from issue Code style PRs after v0.18 branch split by jnewbery
  12. JeremyRubin force-pushed on Dec 17, 2021
  13. DrahtBot removed the label Needs rebase on Dec 17, 2021
  14. JeremyRubin commented at 8:35 PM on December 17, 2021: contributor

    failure is unrelated; has been rebased

  15. JeremyRubin cross-referenced this on Jan 3, 2022 from issue Implement BIP-119 Validation (CheckTemplateVerify) by JeremyRubin
  16. JeremyRubin force-pushed on Jan 5, 2022
  17. [TESTS] Allow tx_invalid.json tests to include flag rules for if_unset: [A,B,C] then_unset: [D] f32c6b7a7c
  18. JeremyRubin force-pushed on Jan 6, 2022
  19. DrahtBot cross-referenced this on Jun 5, 2022 from issue test: Unit tests for taproot/tapscript coverage in `interpreter.cpp` by david-bakin
  20. DrahtBot cross-referenced this on Oct 4, 2022 from issue refactor: Remove duplicated test code by aureleoules
  21. glozow commented at 6:45 PM on October 12, 2022: member

    Concept -0 as this seems to only be useful for #21702, and not otherwise beneficial.

  22. JeremyRubin closed this on Dec 16, 2022

  23. bitcoin locked this on Dec 16, 2023

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: 2026-05-20 06:53 UTC