Skip to content

Conversation

@brice-stacks
Copy link

Add some framework for property testing the new functions.

@obycode obycode requested a review from kantai October 20, 2025 14:19
Copy link

@aaronb-stacks aaronb-stacks left a comment

Choose a reason for hiding this comment

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

This looks like a great start for property testing restrict-asssets.

I think there's kind of two (possibly three?) things missing though.

First, I think you'll want a way to generate "allowable" snippets. As in, generate a stx-transfer and the necessary allowance to add to the snippet. Then, you could proptest that the execution is "okay" in those cases.

Second, I think we also want to be able to mix in multiple asset snippets. Basically, we'd want to test something like the property "execution is identical with and without restrict-assets when all asset snippets have assets included in restrict assets", but also "execution is disallowed when any asset snippet is included without a matching asset allowance"

Finally, I think we want to test with a different tx-sender than just the transient one as well.

@brice-stacks brice-stacks marked this pull request as ready for review October 24, 2025 20:10
@brice-stacks brice-stacks requested review from a team as code owners October 24, 2025 20:10
@brice-stacks
Copy link
Author

@aaronb-stacks I've addressed those points and added some other tests.

aaronb-stacks
aaronb-stacks previously approved these changes Oct 27, 2025
Copy link

@aaronb-stacks aaronb-stacks left a comment

Choose a reason for hiding this comment

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

LGTM!

@brice-stacks
Copy link
Author

@aaronb-stacks I reverted that "optimization" related to our discussion about handling short returns.

aaronb-stacks
aaronb-stacks previously approved these changes Oct 28, 2025
@brice-stacks brice-stacks requested review from jacinta-stacks and removed request for kantai October 28, 2025 16:55
@brice-stacks brice-stacks removed the request for review from jacinta-stacks October 28, 2025 17:00
@brice-stacks brice-stacks added this pull request to the merge queue Oct 28, 2025
Merged via the queue into stacks-network:develop with commit 3e7ecdb Oct 28, 2025
302 of 309 checks passed
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 54.17607% with 406 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.43%. Comparing base (d622c3d) to head (5635f88).
⚠️ Report is 50 commits behind head on develop.

Files with missing lines Patch % Lines
clarity/src/vm/tests/proptest_utils.rs 63.23% 200 Missing ⚠️
clarity/src/vm/tests/post_conditions.rs 37.22% 199 Missing ⚠️
clarity/src/vm/tests/conversions.rs 54.54% 5 Missing ⚠️
clarity/src/vm/mod.rs 85.71% 2 Missing ⚠️

❌ Your project status has failed because the head coverage (48.43%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (d622c3d) and HEAD (5635f88). Click for more details.

HEAD has 50 uploads less than BASE
Flag BASE (d622c3d) HEAD (5635f88)
139 89
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #6606       +/-   ##
============================================
- Coverage    79.86%   48.43%   -31.43%     
============================================
  Files          573      574        +1     
  Lines       352853   354916     +2063     
============================================
- Hits        281798   171913   -109885     
- Misses       71055   183003   +111948     
Files with missing lines Coverage Δ
clarity/src/vm/contexts.rs 77.27% <ø> (-14.19%) ⬇️
clarity/src/vm/tests/crypto.rs 20.16% <ø> (-78.61%) ⬇️
clarity/src/vm/tests/mod.rs 46.06% <ø> (-12.36%) ⬇️
clarity/src/vm/mod.rs 76.59% <85.71%> (-14.69%) ⬇️
clarity/src/vm/tests/conversions.rs 28.74% <54.54%> (-71.26%) ⬇️
clarity/src/vm/tests/post_conditions.rs 25.00% <37.22%> (-75.00%) ⬇️
clarity/src/vm/tests/proptest_utils.rs 63.23% <63.23%> (ø)

... and 456 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d622c3d...5635f88. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants