Fix/pox5 contract reentrancy

[rob-stacks]

Description

This patch ensures no re-entrancy is used (abused) during pox-5 contract function calls.

A new error code ERR_REENTRANT_CALL has been added too.

Applicable issues

• fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• For new Clarity features or consensus changes, add property tests (see docs/property-testing.md)
• Changelog fragment(s) or "no changelog" label added (see changelog.d/README.md)
• Required documentation changes (e.g., rpc/openapi.yaml for RPC endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo

[coveralls]

Coverage Report for CI Build 26643304473

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.06%) to 86.017%

Details

• Coverage increased (+0.06%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 6298 coverage regressions across 90 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

6298 previously-covered lines in 90 files lost coverage.

Top 10 Files by Coverage Loss	Lines Losing Coverage	Coverage
stackslib/src/chainstate/burn/db/sortdb.rs	629	90.22%
stackslib/src/config/mod.rs	418	69.43%
stackslib/src/net/mod.rs	311	77.9%
stackslib/src/chainstate/stacks/index/storage.rs	277	82.27%
clarity/src/vm/database/clarity_db.rs	259	82.14%
stackslib/src/chainstate/stacks/boot/mod.rs	248	94.21%
clarity/src/vm/database/structures.rs	236	78.07%
stackslib/src/chainstate/stacks/miner.rs	218	83.4%
stackslib/src/burnchains/burnchain.rs	208	72.09%
stackslib/src/chainstate/nakamoto/signer_set.rs	199	69.19%

---

Coverage Stats

Relevant Lines:	225705
Covered Lines:	194144
Line Coverage:	86.02%
Coverage Strength:	18521890.32 hits per line

---

💛 - Coveralls

[hstove-stacks]

Can you run npx clarigen in contrib/core-contract-tests? You can also run npx clarigen -w to keep it running in watch mode.

[brice-stacks]

You will need to upgrade the clarigen packages in package.json to 4.1.6.

[rob-stacks]

@brice-stacks is it the reason why it failed and i needed to fix it manually?

[brice-stacks]

There's no guard around this trait call.

[brice-stacks]

This begin is unnecessary.

[brice-stacks]

What do you think about putting all calls to the traits inside a private function that does the `var-sets for you? It would be nice if we supporter lambda expressions so it could be generic, but without that, I think we'd need a private function for each trait function we call, so just 2 I think. My worry is that it is easy to forget to do this. Having a private function doesn't force us to use it, but it does ensure that if it is used, we do the var setting correctly.

So one function for example would look like this:

(define-private (guarded-validate-stake?
        (signer-manager <signer-manager-trait>)
        (staker principal)
        (first-index uint)
        (num-indexes uint)
        (amount-ustx uint)
        (amount-sats uint)
        (is-bond bool)
        (signer-calldata (optional (buff 500)))
    )
    (begin
        (var-set signer-manager-call-active true)
        (let ((result (contract-call? signer-manager validate-stake! staker first-index
                num-indexes amount-ustx amount-sats is-bond signer-calldata
            )))
            (var-set signer-manager-call-active false)
            result
        )
    )
)

Do others think that would be worthwhile? Any ideas for a better solution.