-
Notifications
You must be signed in to change notification settings - Fork 38.8k
[TESTS] Allow tx_invalid.json tests to include flag rules for if_unset: [A,B,C] then_unset: [D] #22954
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
@MarcoFalke is their an automated tool to run to fix this PR? |
|
@JeremyRubin I think git will always consider context, but you can use |
|
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... |
7f42285 to
0441c20
Compare
|
failure is unrelated; has been rebased |
0441c20 to
a797c72
Compare
…t: [A,B,C] then_unset: [D]
a797c72 to
f32c6b7
Compare
glozow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept -0 as this seems to only be useful for #21702, and not otherwise beneficial.
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.