Skip to content

fix(tests): add missing min_cost parameter to evalTCOExpression FFI binding#355

Open
tvolk131 wants to merge 1 commit intoBlockstreamResearch:masterfrom
tvolk131:fix/evalTCOExpression-min-cost
Open

fix(tests): add missing min_cost parameter to evalTCOExpression FFI binding#355
tvolk131 wants to merge 1 commit intoBlockstreamResearch:masterfrom
tvolk131:fix/evalTCOExpression-min-cost

Conversation

@tvolk131
Copy link

Summary

The C function rustsimplicity_0_6_evalTCOExpression has a minCost parameter (of type ubounded) between len and budget, but the Rust FFI binding in the test module was missing this parameter. This caused an argument mismatch in the foreign function call, leading to undefined behavior at runtime.

  • Add min_cost: ubounded parameter to the simplicity_evalTCOExpression extern declaration in simplicity-sys/src/tests/ffi.rs to match the C signature in eval.h.
  • Pass 0 as min_cost in the simplicity_evalTCOProgram Rust wrapper, matching the behavior of the C inline evalTCOProgram helper in eval.h.
  • Fix budget.map() to budget.as_ref().map() in run_program (simplicity-sys/src/tests/mod.rs) — the previous code moved the Option<u32> value rather than borrowing it, which produced a dangling pointer instead of a valid pointer to the stack-local budget value.

Context

The C signatures (from depend/simplicity/eval.h):

simplicity_err rustsimplicity_0_6_evalTCOExpression(
    flags_type anti_dos_checks, UWORD* output, const UWORD* input,
    const dag_node* dag, type* type_dag, size_t len,
    ubounded minCost, const ubounded* budget, const txEnv* env);

static inline simplicity_err evalTCOProgram(
    const dag_node* dag, type* type_dag, size_t len,
    ubounded minCost, const ubounded* budget, const txEnv* env) {
  return rustsimplicity_0_6_evalTCOExpression(CHECK_ALL, NULL, NULL, dag, type_dag, len, minCost, budget, env);
}

Test plan

  • Verify the Rust FFI binding now matches the C function signature
  • Run the existing test suite (cargo test) to confirm no regressions

🤖 Generated with Claude Code

…FI binding

The C function `evalTCOExpression` gained a `minCost` parameter (between
`len` and `budget`), but the Rust FFI binding was not updated to match.
This caused undefined behavior due to argument mismatch in the foreign
function call.

Changes:
- Add `min_cost: ubounded` parameter to `simplicity_evalTCOExpression`
  extern declaration to match the C signature.
- Pass `0` as `min_cost` in the `simplicity_evalTCOProgram` wrapper,
  matching the C inline `evalTCOProgram` helper in eval.h.
- Fix `budget.map(|b| b as *const _)` to `budget.as_ref().map(|b| b as
  *const _)` in `run_program` to avoid moving the `Option<u32>` value
  and instead take a reference to its contents, producing a valid
  pointer to the stack-local budget value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 21, 2026 02:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes undefined behavior in the simplicity-sys test harness by aligning the Rust test-only FFI binding for evalTCOExpression with the actual C signature and correcting how an optional budget pointer is derived.

Changes:

  • Add the missing min_cost: ubounded parameter to the simplicity_evalTCOExpression extern declaration in simplicity-sys/src/tests/ffi.rs.
  • Update the simplicity_evalTCOProgram wrapper to pass min_cost = 0, matching the C inline helper behavior.
  • Fix run_program to use budget.as_ref().map(...) so the budget pointer refers to valid memory instead of casting the integer value into an address.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
simplicity-sys/src/tests/mod.rs Fixes construction of budget_ptr to correctly borrow the optional budget value when passing into FFI.
simplicity-sys/src/tests/ffi.rs Updates the FFI signature to include min_cost and adjusts the Rust wrapper call to match the C ABI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants