Skip to content

feat!: make TestEnvironment be unconstrained#8747

Merged
Thunkar merged 9 commits into
masterfrom
nv/unconstrained-tests
Sep 25, 2024
Merged

feat!: make TestEnvironment be unconstrained#8747
Thunkar merged 9 commits into
masterfrom
nv/unconstrained-tests

Conversation

@nventuro

Copy link
Copy Markdown
Contributor

TestEnvironment throws a gazillion warnings due to calling oracles in constrained functions. We could have unsafe blocks, but this would be misleading since we're not constraining anything, and there'd be risk of user error. The correct call is to make these functions be unconstrained.

However, since they work with mutable references, these themselves cannot be called from constrained functions. Nor should they - this would again require unsafe blocks, but we cannot constrain these results, and we don't anyway since tests are run in a non-adversarial environment.

There should be no difference between constrained and unconstrained tests (except unconstrained tests might be a smidge faster). Therefore, the simplest and most correct way forward is to make all tests and helpers unconstrained, which is what I do here. This ends up removing a ton of 'missing unsafe' warnings.

@nventuro nventuro enabled auto-merge (squash) September 24, 2024 19:25
@nventuro nventuro disabled auto-merge September 24, 2024 22:01

@sklppy88 sklppy88 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.

Looks nice ! I think overall we should be more explicit on the rule which you summed up really well, which is mut references cannot cross between constrained <-> unconstrained. Imo this is obvious with context but not so obvious without, and it took me even awhile to see how this affects the code, even if I understood the reason behind it.

@Thunkar Thunkar enabled auto-merge (squash) September 25, 2024 07:55
@AztecBot

AztecBot commented Sep 25, 2024

Copy link
Copy Markdown
Collaborator

Docs Preview

Hey there! 👋 You can check your preview at https://66f3e0893305781a4b646c50--aztec-docs-dev.netlify.app

@Thunkar Thunkar merged commit b9a1f59 into master Sep 25, 2024
@Thunkar Thunkar deleted the nv/unconstrained-tests branch September 25, 2024 10:14
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