Skip to content

refactor: move AssetMap from clarity into the clarity-types crate - #7513

Open
cylewitruk-stacks wants to merge 1 commit into
stacks-network:mainfrom
cylewitruk-stacks:chore/relocate-assetmap-to-clarity-types
Open

refactor: move AssetMap from clarity into the clarity-types crate#7513
cylewitruk-stacks wants to merge 1 commit into
stacks-network:mainfrom
cylewitruk-stacks:chore/relocate-assetmap-to-clarity-types

Conversation

@cylewitruk-stacks

@cylewitruk-stacks cylewitruk-stacks commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Move AssetMap and AssetMapEntry from clarity into the new clarity_types::effects module and give them their own AssetMapError error type.

The existing clarity::vm::contexts::{AssetMap, AssetMapEntry} paths remain available as re-exports for downstream compatibility (following the already established convention of re-exporting clarity-types types from clarity).

Chose effects as the map/entry types felt more like "side-effect/accumulator" types than core Clarity types.

See #7478 (comment) for more context.

Changes

  • Move AssetMap and AssetMapEntry to clarity_types::effects.
  • Add the narrow AssetMapError type for accumulation and merge failures.
  • Map AssetMapError into the existing Clarity VM error variants.
  • Re-export AssetMapError through clarity::vm::errors.
  • Preserve the existing AssetMap and AssetMapEntry compatibility paths.
  • Gate AssetMap::to_json behind the optional asset-map-json feature.
  • Preserve consensus-relevant overflow and pre-Epoch-4.0 stacking behavior.
  • Move and expand AssetMap tests, including coverage for:
    • atomic merges on overflow,
    • Epoch 4.0 stacking accumulation and overflow,
    • new principals and assets during merges,
    • NFT merging,
    • PoX action merging,
    • VM error conversion.

This is intended to be a dependency-boundary refactor with no consensus or runtime behavior changes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves asset-effect tracking from the Clarity VM into clarity-types while preserving compatibility re-exports and VM error behavior.

Changes:

  • Introduces effects::AssetMap, AssetMapEntry, and AssetMapError.
  • Preserves VM compatibility and maps new errors to existing VM errors.
  • Adds feature-gated JSON support and expanded asset-map tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
stackslib/src/chainstate/stacks/db/transactions.rs Converts burn-total errors into VM errors.
contrib/clarity-cli/Cargo.toml Enables asset-map JSON support.
clarity/src/vm/errors.rs Re-exports and converts AssetMapError.
clarity/src/vm/contexts.rs Re-exports the relocated asset-map types.
clarity/Cargo.toml Forwards the JSON feature.
clarity-types/src/tests/mod.rs Registers asset-map tests.
clarity-types/src/tests/asset_map.rs Tests merging, overflow, epoch, and JSON behavior.
clarity-types/src/lib.rs Exposes the effects module.
clarity-types/src/effects/mod.rs Defines the effects module interface.
clarity-types/src/effects/asset_map.rs Implements relocated asset tracking.
clarity-types/README.md Documents effects and JSON support.
clarity-types/Cargo.toml Adds the optional JSON feature.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 31747827433

Coverage decreased (-0.06%) to 86.584%

Details

  • Coverage decreased (-0.06%) from the base build.
  • Patch coverage: 28 uncovered changes across 2 files (308 of 336 lines covered, 91.67%).
  • 194 coverage regressions across 37 files.

Uncovered Changes

File Changed Covered %
clarity-types/src/effects/asset_map.rs 298 271 90.94%
clarity/src/vm/contexts.rs 11 10 90.91%
Total (4 files) 336 308 91.67%

Coverage Regressions

194 previously-covered lines in 37 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
stackslib/src/net/p2p.rs 34 74.57%
stackslib/src/net/inv/epoch2x.rs 21 79.09%
stacks-signer/src/v0/signer.rs 20 87.66%
stackslib/src/net/download/epoch2x.rs 15 59.69%
stackslib/src/net/relay.rs 8 74.81%
stackslib/src/burnchains/burnchain.rs 7 71.39%
stacks-node/src/nakamoto_node/miner.rs 7 86.56%
stacks-signer/src/v0/signer_state.rs 6 92.39%
clarity/src/vm/functions/crypto.rs 5 89.24%
libsigner/src/v0/messages.rs 5 91.36%

Coverage Stats

Coverage Status
Relevant Lines: 232804
Covered Lines: 201571
Line Coverage: 86.58%
Coverage Strength: 19327682.34 hits per line

💛 - Coveralls

@brice-stacks brice-stacks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants