test: Move relevant tests to stacks-codec and add some new ones#7219
Open
jbencin-stacks wants to merge 12 commits into
Open
test: Move relevant tests to stacks-codec and add some new ones#7219jbencin-stacks wants to merge 12 commits into
stacks-codec and add some new ones#7219jbencin-stacks wants to merge 12 commits into
Conversation
|
|
|
|
Addresses the PR stacks-network#7200 reviewer comment about missing tests in stacks-codec by relocating tests whose dependencies are already satisfied by stacks-codec + stacks-common + clarity-types. Moves from stackslib/src/chainstate/stacks/transaction.rs: - test_transaction_payload_token_transfer (rstest) - clarity_version_codec_is_consistent (replaces the cross-crate test_clarity_versions rstest template with an inline #[case] enumeration so stacks-codec does not pick up a clarity dev-dep) - test_transaction_contract_call_codec - test_transaction_smart_contract_codec - test_transaction_payload_versioned_contracts_codec (rstest) - tx_stacks_transaction_payload_coinbase - tx_stacks_transaction_payload_nakamoto_coinbase - tx_stacks_transaction_payload_nakamoto_coinbase_alt_recipient - tx_stacks_transaction_payload_microblock_poison - tx_stacks_transaction_payload_invalid (and contract/function name variants) - tx_stacks_asset - tx_stacks_postcondition (and nft_maybe_sent variant) - tx_stacks_transaction_codec_originator_mode_and_nft_maybe_sent - tx_stacks_postcondition_invalid Moves from stackslib/src/util_lib/strings.rs: - tx_stacks_strings_codec (renamed stacks_strings_codec) - tx_stacks_string_invalid (renamed stacks_string_rejects_non_printable) Supporting changes: - Add a local copy of check_codec_and_corruption to stacks-codec so tests don't reach into stackslib's net codec helpers. - Add rstest plus testing-feature stacks-common/clarity-types as dev-dependencies on stacks-codec. - Prune now-unused imports in stackslib's test modules. stackslib retains StacksTransactionSigner, all sign/verify integration tests, and tx_stacks_transaction_codec (which depends on codec_all_transactions). 28 tests pass in stacks-codec; the remaining transaction tests still pass in stackslib.
Adds tests in stacks-codec that target boundaries the relocated stackslib tests did not cover. stacks-codec/src/transaction.rs additions: - Byte-value pins for the wire-format-critical enum discriminants (TransactionPayloadID, TransactionAnchorMode, TransactionPostConditionMode, TransactionVersion, TransactionAuthFlags, AssetInfoID, PostConditionPrincipalID) so any silent reordering trips a test instead of breaking consensus. - Exhaustive TenureChangeCause codec across all 7 variants plus rejection of unknown bytes (previously only BlockFound/Extended were covered indirectly via codec_all_transactions). - Roundtrip for TenureChangePayload (compares field-by-field since TenureChangeCause intentionally does not implement PartialEq). - Standalone StacksMicroblockHeader codec roundtrip (previously only exercised via PoisonMicroblock). - Fixed-size assertions for TokenTransferMemo and CoinbasePayload. - stacks_transaction_empty_post_conditions_codec roundtrip. - stacks_transaction_header_layout pins the version-then-chain_id ordering at the start of the serialized transaction. - Exhaustive FungibleConditionCode and NonfungibleConditionCode from_u8 roundtrips. stacks-codec/src/strings.rs additions: - Tab/newline acceptance. - 0x20/0x7e printable boundary, 0x1f/0x7f rejection. - Deserialize rejection of valid UTF-8 but non-printable bytes. - is_clarity_variable detection. - From<ClarityName> conversion.
jbencin-stacks
force-pushed
the
test/stacks-codec
branch
from
dff07e5 to
aff3408
Compare
- Refactor `tx_stacks_transaction_payload_microblock_poison` to use a local `header_bytes` closure with a documented byte-layout comment, removing the rustfmt-orphaned `// header_2` placement and making each variant (matching, equal, bad-parent) read as a header constructor call rather than three near-identical blocks of `extend_from_slice`. - Clarify the `stacks_string_deserialize_rejects_non_printable_bytes` body comment and tighten the assertion to `matches!` the error variant before checking the message substring. - Rename `fungible_/nonfungible_condition_code_roundtrip` to `..._from_u8_roundtrip` to reflect what they actually test (these codes are serialized inline, not via their own codec impl). - Add a `clarity_version_cases_are_exhaustive` test that pattern-matches on `ClarityVersion::latest()` to recover the compile-time exhaustiveness guarantee that the original cross-crate `test_clarity_versions` rstest template used to provide. - Extract the duplicated `check_codec_and_corruption` helper into a new `stacks_common::codec::testing` module gated on the `testing` feature. `stackslib::net::codec::test::check_codec_and_corruption` becomes a re-export; `stacks-codec` imports the canonical version directly, removing the local copies in `transaction.rs` and `strings.rs`.
Coverage Report for CI Build 26529130192Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.1%) to 85.602%Details
Uncovered Changes
Coverage Regressions6592 previously-covered lines in 115 files lost coverage.
Coverage Stats
💛 - Coveralls |
jbencin-stacks
force-pushed
the
test/stacks-codec
branch
from
fb35023 to
e682b28
Compare
Contributor
federico-stacks
left a comment
federico-stacks
left a comment
There was a problem hiding this comment.
LGTM!
You'll just need to fix https://github.com/stacks-network/stacks-core/actions/runs/26468364840/job/78037311978?pr=7219.
It is about updating this cargo lock files due to the new added dependency:
- clarity/fuzz/Cargo.lock
- stackslib/fuzz/Cargo.lock
Contributor
federico-stacks
left a comment
federico-stacks
left a comment
There was a problem hiding this comment.
heads-up: @jbencin-stacks CI was updated and you need to merge the develop branch otherwise the jobs will show as stuck.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Description
Following up on the refactoring work in #7200, this PR lowers the associated tests from
stackslibdown intostacks-codec, and also adds some new tests to address missing coverageNote
This PR may be easier to review by commit
Applicable issues
StacksTransactionintostacks-codec#7200Checklist
changelog.d/README.md)