refactor: Extract post-conditions from stackslib into separate crate - #7478
refactor: Extract post-conditions from stackslib into separate crate#7478jbencin-stacks wants to merge 13 commits into
stackslib into separate crate#7478Conversation
e65c562 to
856cdd7
Compare
…tion Post-conditions constrain the assets a transaction may move. That check is a static function of the declared post-conditions, an `AssetMap`, the origin principal and the epoch — it needs no database or chainstate, and nothing about it is specific to Clarity as a source language. Hosting it in `stackslib` made it unreachable from wasm, and hosting it in `clarity` would tie it to one contract language as we look at supporting others. Move `check_transaction_postconditions` and its `HashableClarityValue` helper into a new `stacks-postconditions` crate, leaving a thin adapter in `stackslib` that projects `StacksAccount` onto the origin principal the check needs. The crate still depends on `clarity` for `AssetMap` and `VmExecutionError`, so this does not yet decouple from the Clarity VM; it isolates the logic so that the remaining step is a change to one crate rather than a change spread across `stackslib`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The three epoch-activation rules for post-conditions (`Originator` mode and NFT `MaybeSent` requiring Stacks 3.4, `Staking`/`Pox` requiring Stacks 4.0) were duplicated verbatim between `process_transaction_precheck` and `StacksBlock::validate_transaction_static_epoch`. Like the checker moved in the previous commit they are pure functions of the post-conditions, the mode and the epoch, with no chainstate dependency. Add `check_post_conditions_supported_in_epoch`, returning a typed `UnsupportedPostCondition` rather than a formatted message so both call sites keep their existing error channel and log level. This collapses the two copies into one predicate, and matters for wasm consumers: because `check_transaction_postconditions` evaluates explicit post-conditions in every epoch, a caller running only that function would return a pass/fail verdict for a transaction mainnet rejects outright. Behavior is unchanged, including the message text at both call sites. The `Display` impl carries the "before Stacks X.Y" phrasing that `process_transaction_precheck` puts in its error, and `subject()` exposes the offending feature on its own so `validate_transaction_static_epoch` can keep naming the current epoch in its log. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Move the tests that exercise the checker in isolation — a hand-built `AssetMap` plus a list of post-conditions — alongside the code they cover. The `process_transaction` pipeline tests that reach the checker through the node (fee handling, receipts, rollback) stay in `stackslib`. This is a pure move. The only changes to the test bodies are mechanical: the `make_account` helper is gone, since the relocated check takes a `&PrincipalData` rather than a `&StacksAccount`, so each case passes the origin principal directly instead of fabricating a nonce and balance the check ignored. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`AssetMap::to_table()` excludes the stacking map and the pox-action set, so the
staking and PoX coverage checks are hand-rolled loops reachable only through
their own path. Several branches in them had no test:
- `Originator` mode for staking/PoX coverage. Every existing test used
`Deny`/`Allow`, leaving the non-origin skip in
`enforce_unchecked_assets_for_principal` unexercised.
- `PostConditionPrincipal::Standard` for `Staking`/`Pox`. All existing tests
used `::Origin`, so non-origin resolution through `to_principal_data` was
untested for these two variants.
- The `amount_staked == 0` skip in the staking coverage loop.
- What the epoch gate does *not* gate: explicit `Staking`/`Pox` post-conditions
are evaluated in every epoch, and only the coverage requirement is gated.
Existing pre-4.0 cases used empty post-condition lists.
Also cover `check_post_conditions_supported_in_epoch` on both sides of the
Stacks 3.4 and 4.0 boundaries, and add a test showing the admission and
asset-movement checks are independent, so callers need both.
Correct stale terminology in the PoX test comments, which referred to
"unstaking" / "MaybeUnstaked" from an earlier naming iteration; the condition
codes are `PoxConditionCode::{NotPerformed, MaybePerformed, Performed}` over
general PoX actions.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
856cdd7 to
cdbf6ca
Compare
benjamin-stacks
left a comment
There was a problem hiding this comment.
LGTM overall, just a few smaller things.
|
Tbh, I would prefer to see this and the transaction types which previously ended up under Of course, to keep the diff manageable, that could be achieved by just renaming this new crate to Thoughts? |
|
@cylewitruk-stacks I think that makes sense, but if we're trying to minimize diffs here, then I should move it to |
|
@cylewitruk-stacks Looking into this more, it seems that Having a larger number of smaller crates won't be an issue once we clean up the top level and organize them. Maybe we can have:
But let's save that for another PR |
Yeah, exactly what I was thinking :)
Right, so, I think that:
Assuming my assumptions hold, I think that would be the cleaner structural direction, and the moving of |
Yeah, eventually we'd like postconditions to be completely independent of Clarity so that we could use them with other languages (like Rust compiled to Wasm). My first attempt at this refactor moved them into I don't have a strong opinion one way or the other whether this in theory belongs in the same crate as the codec, but for now I don't think it makes sense because of the Clarity dependency. We can revisit later once the dependency is eliminated |
The |
Description
Extract post-conditions from
stackslibinto separate crate (stacks-postconditions), so they can be built for WASM targetsApplicable issues
Additional info (benefits, drawbacks, caveats)
Checklist
docs/property-testing.md)changelog.d/README.md)rpc/openapi.yamlfor RPC endpoints,event-dispatcher.mdfor new events)clarity-benchmarkingrepo