rollback global context in error case

[csgui]

Fix #549

In case of an error, the function context needs to be cleaned up, rolling back to a pristine state.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.67%. Comparing base (c4528e1) to head (cad1b1b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #621      +/-   ##
==========================================
- Coverage   85.67%   85.67%   -0.01%     
==========================================
  Files          46       46              
  Lines       24599    24609      +10     
  Branches    24599    24609      +10     
==========================================
+ Hits        21076    21084       +8     
  Misses       1672     1672              
- Partials     1851     1853       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Acaccia]

Is there any way we could have some tests to prove the efficiency of the change ?

[csgui]

Is there any way we could have some tests to prove the efficiency of the change ?

I was thinking about that too. Actually, we do have tests to verify the error path, for instance:

https://github.com/stacks-network/clarity-wasm/blob/main/clar2wasm/tests/lib_tests.rs#L4091

and

https://github.com/stacks-network/clarity-wasm/blob/main/clar2wasm/tests/lib_tests.rs#L4103

among others.

Also, in this PR, I added the instantiation of the global context for tests, which is needed to validate the changes.

A more detailed test would require additional work to validate context management, state verification, and a more rigorous assessment of the integration with stacks-core. This is a task that can be addressed separately, imo, since we are already covering it with some tests and also relying the validation of the changes here on fuzzing tests.

[Acaccia]

This is a task that can be addressed separately, imo, since we are already covering it with some tests and also relying the validation of the changes here on fuzzing tests.

Where are those tests actually? What are we covering?
To my knowledge, none of our existing tests fail due to this bug.

[csgui]

The tests that I linked above are impacted by this PR. That means a context was not initialized before, but now it is initialized and rolled back in error cases. Without the change on https://github.com/stacks-network/clarity-wasm/pull/621/files#diff-88585f598b79cb14968cf2e11093aacaa49eb5f7553ded728347b3ffafc8c410R500 they will fail.

The changes are only visible during runtime errors. Normal conditions remain unaffected by this PR, but this issue is hindering the fuzzing tests. Therefore, I suggest merging this PR, deploying a new release, and validating it also with fuzzing tests.

[Acaccia]

Fuzzing tests are not enough imho since they are out of our control.
What currently prevents another dev to come work on this project and reintroduce the bug?
We would notice it only after it’s merged into the main branch. And this is only if we run systematically the fuzzing tests after a merge to main. The fuzzing tests with the right conditions that trigger this specific bug.

[Acaccia]

The tests that I linked above are impacted by this PR. That means a context was not initialized before, but now it is initialized and rolled back in error cases. Without the change on https://github.com/stacks-network/clarity-wasm/pull/621/files#diff-88585f598b79cb14968cf2e11093aacaa49eb5f7553ded728347b3ffafc8c410R500 they will fail.

I just checked this, and made the change only in tests/lib_tests.rs, the tests don't fail at all.

[csgui]

Thinking about the things we can control, you're right. Let me take a stab at adding more tests for this PR.