test: pass datacarriersize option for tests using large outputs (instead of acceptnonstdtxn)#25503
Merged
maflcko merged 3 commits intobitcoin:masterfrom Jun 30, 2022
Conversation
This assures that changing the internals of the helper function still leads to the expected outcome sizewise (preparation for the next commit).
Transactions with more than one datacarrier (OP_RETURN) output are never considered standard, i.e. this change is necessary in order to to get rid of the `acceptnonstdtxn` option for some tests.
…stead of `acceptnonstdtxn`) By specifying the `datacarriersize` option instead of the more generic `acceptnonstdtxn`, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy.
Member
|
Would it work to use witness programs of length 40 and then add a separate test fro datacarriersize? |
maflcko
approved these changes
Jun 29, 2022
Contributor
Author
Interesting idea, didn't even consider that. I think that would work, the only drawback is possibly that tests could slow down due to the quite high number of outputs (>1400 for reaching the same txouts serialized size in |
Member
|
yeah, your way might actually be preferable or at least a good first step. |
theStack
deleted the
202206-test-replace_acceptnonstdxn_with_datacarriersize_option
branch
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 30, 2022
…ing large outputs (instead of `acceptnonstdtxn`) 475aae8 test: pass `datacarriersize` option for tests using large outputs (instead of `acceptnonstdtxn`) (Sebastian Falbesoner) b1ba3ed test: let `gen_return_txouts` create a single large OP_RETURN output (Sebastian Falbesoner) f319287 test: assert serialized txouts size of `gen_return_txouts` helper (Sebastian Falbesoner) Pull request description: By specifying the `datacarriersize` option instead of the more generic `acceptnonstdtxn` for functional tests, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy. Transactions with more than one OP_RETURN output are [never considered standard](https://github.com/bitcoin/bitcoin/blob/749b80b29e875cc6afa1c2674cccdfd7115cc16a/src/policy/policy.cpp#L149-L153), i.e. we have to change the `gen_return_txouts` helper to create only a single output in order to get rid of the `acceptnonstdxtn` option. Note that on master there is currently no test using the `datacarriersize` parameter, so this PR indirectly also increases the test coverage. The change affects the tests `mempool_limit.py`, `mining_prioritisetransaction.py` (call `gen_return_txouts` directly) and `feature_maxuploadtarget.py` (calls `gen_return_txouts` indirectly via the `mine_large_block(...)` helper). Top commit has no ACKs. Tree-SHA512: c17f032e00d28f5e5880a4d378773fbc8b1995ea9c377f237598d412628fe117f497a44ebdfa8af8cd8a3b1e3127e0cf7692efbf5c833c713764a71a85301f23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
By specifying the
datacarriersizeoption instead of the more genericacceptnonstdtxnfor functional tests, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy. Transactions with more than one OP_RETURN output are never considered standard, i.e. we have to change thegen_return_txoutshelper to create only a single output in order to get rid of theacceptnonstdxtnoption. Note that on master there is currently no test using thedatacarriersizeparameter, so this PR indirectly also increases the test coverage.The change affects the tests
mempool_limit.py,mining_prioritisetransaction.py(callgen_return_txoutsdirectly) andfeature_maxuploadtarget.py(callsgen_return_txoutsindirectly via themine_large_block(...)helper).