Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select contracts implementing the correct trait from the Clarinet project for argument generation #88

Merged
merged 11 commits into from
Jan 26, 2025

Conversation

BowTiedRadone
Copy link
Collaborator

@BowTiedRadone BowTiedRadone commented Jan 13, 2025

This PR adds support for arbitrary selection of trait reference parameters. The new logic for handling trait implementations in trait reference parameters is as follows:

  • If a trait reference parameter is found in public functions or invariants during invariant testing, or in test functions during property testing, Rendezvous will scan the user's Clarinet project for a contract that implements the referenced trait.
  • If trait implementations are found for that specific trait reference, they will be added to a selection pool. Rendezvous uses fast-check to randomly select one of them for each run, ensuring seed-based randomness.

To test contracts with trait reference parameters, users must select at least one trait implementation for the referenced trait and add it to their Clarinet project, either as a direct contract or a project requirement.

This PR resolves #65.

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

Interesting work on the draft implementation so far! Left some comments.

The next steps I see are:

  • Create (perhaps parameterized?) test(s) based on the draft/example in property.ts.
  • Add as many more test cases as possible to expand coverage.
  • Tweak the draft implementation to ensure all tests pass.
  • (Once tests provide decent coverage, we'll consider refactoring with a
    parsec-style library.)

citizen.tests.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
shared.types.ts Show resolved Hide resolved
shared.types.ts Outdated Show resolved Hide resolved
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch from fbbb156 to 74a086b Compare January 17, 2025 21:47
Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

Left questions and comments at the abstraction level. Let's make sure we get those right before proceeding.

ast1737030528990.json Outdated Show resolved Hide resolved
example/contracts/trait.clar Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch 2 times, most recently from e92b62a to ae0d20c Compare January 20, 2025 21:25
invariant.ts Outdated Show resolved Hide resolved
invariant.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch from c65fc8a to c189840 Compare January 21, 2025 14:13
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch from 85dce3b to 1d76720 Compare January 21, 2025 14:26
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch from 2b837d9 to bef6767 Compare January 21, 2025 14:35
invariant.ts Outdated Show resolved Hide resolved
invariant.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
property.ts Outdated Show resolved Hide resolved
shared.ts Outdated Show resolved Hide resolved
shared.ts Outdated Show resolved Hide resolved
traits.ts Outdated Show resolved Hide resolved
traits.ts Outdated Show resolved Hide resolved
Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

This is massive 🥇 After refactoring simnet and coming up with candidate alternatives for "enrich" this should be ready for review.

example/contracts/trait.clar Outdated Show resolved Hide resolved
ast1737030528990.json Outdated Show resolved Hide resolved
example/Clarinet.toml Show resolved Hide resolved
example/contracts/trait.tests.clar Outdated Show resolved Hide resolved
invariant.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
traits.ts Outdated Show resolved Hide resolved
This commit adds a utility function called `extractProjectTraitImplementations`.
It creates a pre-defined record that maps all the contracts in the simnet
instance to their implemented traits. With this change, Rendezvous no longer
needs to iterate through all contracts during argument generation. Instead, the
pre-defined record is passed to all utilities that return arbitraries.
BowTiedRadone added a commit that referenced this pull request Jan 23, 2025
@BowTiedRadone BowTiedRadone changed the title [DRAFT] Select contracts implementing the correct trait from the Clarinet project for argument generation Select contracts implementing the correct trait from the Clarinet project for argument generation Jan 23, 2025
@BowTiedRadone BowTiedRadone marked this pull request as ready for review January 23, 2025 22:41
@BowTiedRadone BowTiedRadone requested a review from a team as a code owner January 23, 2025 22:41
@BowTiedRadone
Copy link
Collaborator Author

Gigantic cleaning session happened here. At least a couple things to decide on before merging:

  • Which contract that imports traits should be added to the example folder? It could be a handmade contract, but it should showcase the functionality clearly.
  • Which contract that implements traits do we want to use? This can either be a handmade contract added to the project or specified as a requirement.

Ideally, we should have two trait implementations for the same trait, so Rendezvous can randomly pick one of them for any trait reference parameter.

@moodmosaic
Copy link
Member

Which contract that imports traits should be added to the example folder? It could be a handmade contract, but it should showcase the functionality clearly.

Any contract that shows a real-world use case and stays under 100 lines.

Which contract that implements traits do we want to use? This can either be a handmade contract added to the project or specified as a requirement.

I'd say both: a local handmade one and one added as a requirement.

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

This is a massive and tricky-to-implement PR. It's been carefully crafted, and I want to leave an approval noting that it's one of the most well-crafted PRs (of this size) I've reviewed in over a decade.

I've left a few comments, but they aren't blockers. It would be nice to address them, but we could merge and handle those separately if needed.

One thing to consider for this PR is whether to keep the full commit history or squash it into a single commit. Keeping the history preserves individual commit context, which can be helpful for debugging and provides a detailed timeline of changes. On the other hand, it can make the history noisier with too many intermediate commits.

Squashing into a single commit results in a cleaner, more concise history, making it easier to track high-level changes. It also simplifies git revert if we need to roll back due to blockers, which is especially useful since we're still < v1 and can afford breaking changes. However, squashing does lose the detailed commit trail, which can sometimes be useful for debugging.

As Linus Torvalds put it: "I want clean history, but that really means (a) clean and (b) history." (Source: https://www.mail-archive.com/[email protected]/msg39091.html)

citizen.tests.ts Show resolved Hide resolved
citizen.ts Show resolved Hide resolved
example/contracts/trait.clar Outdated Show resolved Hide resolved
invariant.ts Show resolved Hide resolved
shared.types.ts Show resolved Hide resolved
traits.types.ts Outdated Show resolved Hide resolved
traits.ts Outdated Show resolved Hide resolved
traits.ts Outdated Show resolved Hide resolved
traits.tests.ts Outdated Show resolved Hide resolved
This commit adds the following to the example project:

- `ft-transfer-many`: A contract that imports a trait and references it in a
function parameter.
- `rendezvous-token`: A contract that implements the referenced trait.

Additionally, it includes an invariant and a property test for the
ft-transfer-many contract to demonstrate the random selection of trait
implementations in action.
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch from 33432a1 to c76fa69 Compare January 24, 2025 18:26
example/Clarinet.toml Show resolved Hide resolved
example/Clarinet.toml Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.clar Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.clar Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.clar Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.clar Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.clar Outdated Show resolved Hide resolved
example/Clarinet.toml Outdated Show resolved Hide resolved
example/contracts/ft-transfer-many.tests.clar Outdated Show resolved Hide resolved
@BowTiedRadone BowTiedRadone force-pushed the feat/project-trait-selection branch 4 times, most recently from f70fdd0 to 669df31 Compare January 25, 2025 22:58
Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

@moodmosaic moodmosaic merged commit 092b6bf into master Jan 26, 2025
21 checks passed
@moodmosaic moodmosaic deleted the feat/project-trait-selection branch January 26, 2025 20:46
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.

Support for traits
2 participants