feat: return any value type from execute_contract_allow_private

[hugocaillard]

The helper execute_contract_allow_private does not allow the private function to return any type and can throw PublicFunctionMustReturnResponse if the private function return a non-Response value.

On Clarinet, we get recurrent requests from the community and internal teams to add the ability to call private function in unit tests

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• N/A Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• N/A New clarity functions have corresponding PR in clarity-benchmarking repo
• N/A New integration test(s) added to bitcoin-tests.yml

[jcnelson]

I think this should be epoch-gated so we don't accidentally use it prior to epoch 2.5

[codecov]

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 83.07%. Comparing base (067633d) to head (929bf1b).
Report is 82 commits behind head on next.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #4520      +/-   ##
==========================================
- Coverage   83.22%   83.07%   -0.16%     
==========================================
  Files         452      453       +1     
  Lines      326058   327375    +1317     
  Branches      323      323              
==========================================
+ Hits       271359   271956     +597     
- Misses      54691    55411     +720     
  Partials        8        8              

Files	Coverage Δ
clarity/src/vm/callables.rs	97.63% <100.00%> (ø)
clarity/src/vm/contexts.rs	93.08% <72.72%> (-0.15%)	⬇️

... and 43 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 067633d...929bf1b. Read the comment docs.

[hugocaillard]

@jcnelson @kantai
Following your remarks (here and on slack), I pushed a new commit that only have effect is developer-mode (which clarinet uses)

            else if allow_private && cfg!(feature = "developer-mode") {
                self.commit()?;
                Ok(result)
            }

@jcnelson

I think this should be epoch-gated so we don't accidentally use it prior to epoch 2.5

Ideally, I'd like to be able to use it for any epoch in simnet

[kantai]

@jcnelson @kantai Following your remarks (here and on slack), I pushed a new commit that only have effect is developer-mode (which clarinet uses)

            else if allow_private && cfg!(feature = "developer-mode") {
                self.commit()?;
                Ok(result)
            }

@jcnelson

I think this should be epoch-gated so we don't accidentally use it prior to epoch 2.5

Ideally, I'd like to be able to use it for any epoch in simnet

Yep -- I agree with this, as long as its controlled with a compile flag, this does not need to be epoch gated.

My only comment is that I'd like to be a little more paranoid again, and introduce a new feature flag -- something specifically like allow-private. developer-mode is supposed to produce a compatible VM with the stacks-node, and I think we should just be extra-careful here.

[hugocaillard]

@kantai

as long as its controlled with a compile flag

Right now i'm using the cfg! macro, which, as I understand is slightly different from a compile flag (#[cgf(...)]). I could use a compile flag instead but it would make the code slightly less readable I think

    pub fn handle_tx_result(
        &mut self,
        result: Result<Value>,
        allow_private: bool,
    ) -> Result<Value> {
        if let Ok(result) = result {
            if let Value::Response(data) = result {
                if data.committed {
                    self.commit()?;
                } else {
                    self.roll_back()?;
                }
                Ok(Value::Response(data))
            } else {
                #[cfg(feature = "developer-mode")]
                if allow_private {
                    self.commit()?;
                    return Ok(result);
                }
                Err(
                    CheckErrors::PublicFunctionMustReturnResponse(TypeSignature::type_of(&result)?)
                        .into(),
                )
            }
        } else {
            self.roll_back()?;
            result
        }
    }

---

something specifically like allow-private

I'm starting to think that just "allow-private" is confusing. Because it should really be allow-private-to-return-any-type (or something like that)

Should I just introduce a flag like "devtools" or "clarinet"? I'm sure I can find tons of usage for such a flag in the future

[kantai]

Right now i'm using the cfg! macro, which, as I understand is slightly different from a compile flag (#[cgf(...)]). I could use a compile flag instead but it would make the code slightly less readable I think

Ah, yes, the cfg! macro is fine here. I agree it results in more readable code.

Should I just introduce a flag like "devtools" or "clarinet"? I'm sure I can find tons of usage for such a flag in the future

Yes -- either works.

[hugocaillard]

Done @kantai, I went for "devtools"

[kantai]

LGTM!

[jcnelson]

LGTM, but before merging, please add a comment to handle_tx_result() to indicate that allow_private is ignored unless the node is compiled with the devtools feature (the comment should include the line that they'd have to add to their Cargo.toml or the build flags to pass to cargo).

[hugocaillard]

Done @jcnelson

[kantai]

LGTM!