jet: add Bitcoin transaction environment (#331 redux) by delta1 · Pull Request #385 · BlockstreamResearch/rust-simplicity

delta1 · 2026-10-05T12:55:04Z

Redux of #331.

Covers the same ground: vendoring the bitcoin-env libsimplicity branch and adding the Rust BitcoinEnv/JetEnvironment plumbing.

Why a new branch instead of updating #331:

Vendored libsimplicity rev differs: this PR pins ba967b1a (the rev shipped with simplicity-lang 0.9.0, already on master);
Symbol version differs as a direct consequence: this PR's C/FFI layer targets rustsimplicity_0_8_*.
CoreEnv::EMPTY (added in b0a25c2) is not needed or referenced by this series and is intentionally omitted.

Three additional PRs stack on this one: #386 #387 #388

Don't actually build bitcoin yet; just refactor build.rs

For dumb "C polymorphism" reasons we have two structures in the C code named txEnv. We can call the corresponding Rust structures different things, but we need to access them from the C file env.c, which provides some C wrappers for FFI stuff. Since you can't have two different structs with the same name in one compilation unit, split it into two: add depend/bitcoin_env.c holding the Bitcoin txEnv wrappers, alongside the existing depend/env.c for Elements.

This is the more correct trait. When we update the Jet trait we will be forced to pick one, and we will pick Borrow.

Updates to BlockstreamResearch/simplicity#324 This PR does a couple things simultaneously: * Runs vendor-simplicity.sh and update-jets.sh * Updates the `Jet` trait to have associated transaction and environment types, and for the environment to be paramterized by the transaction type. * Changes the Core environment from () to CoreEnv::<Infallible> * Updates some fixed Core CMR/IHR vectors (this update to libsimplicity changes the benchmarks for the Core jets and thus changes these vectors) * Uncomments the commented-out symbols in simplicity-sys/src/c_jets/c_env/bitcoin.rs * Adds the "build bitcoin" lines to simplicity-sys/build.rs The update to the `Jet` trait is a bit noisy but ultimately mechanical: everywhere that we're generic over all J: Jet, we now also have to be generic over all T: Borrow<J::Transaction>, which leads to some extra line noise especially in unit tests where we have assert_* helper functions. The last four points are tiny diffs, thanks to the previous preparatory commits. The use of CoreEnv::<Infallible> as the core environment type is kinda fun. It means that it is impossible to execute any code which attempts to access the transaction in the environment. (No such code exists, since it would be nonsensical, but now we have some assurance that it won't exist by accident in the future.) Unfortunately this mixes mechanical and non-mechanical things in one commit. But the mechanical changes are exclusively in simplicity-sys/depend/ and src/jet/init/ and the non-mechanical changes are exclusively outside of those files, so it should be possible to review this.

Change the JetEnvironment impl for BitcoinEnv and ElementsEnv to be generic over T: Borrow<Transaction> instead of fixed to Arc<Transaction>. Update Policy::satisfy, get_satisfier, execute_successful, execute_unsuccessful, and serialize test helpers to accept ElementsEnv<impl Borrow<Transaction>>. This allows callers to construct environments without Arc overhead (for example, using a plain reference or owned value), and is required so that BitcoinEnv::c_jet_env can return a real CTxEnv from a borrowed transaction.

Now that we've updated the Jet trait to allow the transactions in environments to be arbitrary T: Borrow<Transaction>, we don't need to use Arc everywhere. In many cases we can use normal references.

This is a Rust type which can be used, among other things, to construct the transaction environment needed by C jets. Also provides accessors for the underlying transaction and input index, both of which are used (in the Elements version of this struct) in the policy satisfier.

rust-bitcoin 0.32 removed Witness::taproot_annex. Replace it with an inline BIP341 annex extraction: the last witness element that begins with 0x50 is the annex. This matches the Elements c_env helper and the BIP341 specification.

delta1 · 2026-10-05T13:38:53Z

Force pushed to fix the MSRV job, which required splitting the shared files into their own RustSimplicityCommon static lib that both Bitcoin and Elements link against, instead of duplicating them. This is because MSRV 1.74.0 forces -Wl,--whole-archive on every native static lib rustc links, which pulls every object from both archives and hits "multiple definition of" for every shared symbol.

delta1 · 2026-10-05T14:07:01Z

+/// Extracts the annex from a taproot witness stack per BIP341: if there are at
+/// least two witness elements and the last one starts with 0x50, it is the annex.
+/// (rust-bitcoin 0.32 removed `Witness::taproot_annex`.)
+fn get_annex(in_witness: &bitcoin::Witness) -> Option<&[u8]> {


This needs the same treatment as #384

Will do this together with any other review changes since the next 3 PRs will have to be rebased on top

ivanlele · 2026-10-06T10:31:26Z

+      rustsimplicity_0_8_free(analysis);
+    }
+    if (IS_OK(*error)) {
+      txEnv env = rustsimplicity_0_8_bitcoin_build_txEnv(tx, taproot, ix);


Is it an intentional design that we do not include genesisHash to txEnv?

It is a difference I see with Elements case. I think genesisHash inclusion was preventing replay attacks

The genesis hash for signatures is specific to elements. For bitcoin, the genesis hash is not traditionally included in the signatures. If we added it here, I think we would have to add it to libsimplicity as well.

Double checked with AI:

Confirmed against the vendored libsimplicity: the Bitcoin txEnv has no genesis hash and sigAllHash commits only to txHash, tapEnvHash and ix. That matches Bitcoin's own sighashes, which have no chain ID. Adding one would change Bitcoin Simplicity's consensus semantics, so it would need to be decided upstream rather than here.

I see, this is only Elements quirk

ivanlele · 2026-10-06T10:42:35Z

+            [
+                "bitcoin/exec.c",
+                "bitcoin/primitive.c",
+                // "bitcoin/checkSigHashAllTx1.c", // no sighashall test


Depending on the intention of this comment, we could remove it if this check is redundant. Or, we could add a TODO here to say that we will add the Bitcoin version later.

Either way, if somebody is even bored enough to do this, let's maybe do it in a follow-up, so we don't stall the review with small stuff.

ivanlele · 2026-10-06T11:09:35Z

Do we need this empty file

ivanlele · 2026-10-06T11:15:54Z

ACK, ran tests on 1f91d94.

Looks nice

KyrylR

AI-generated code review, posted by Codex at the GitHub account holder's request. The four inline findings are based on independent read-only reviews by exact models Claude Opus 5.5 (claude-opus-5-5, High) and Claude Mythos 5.1 (claude-mythos-5-1, High), with local reproductions by Codex. The account holder did not author these findings. This is a comment-only review.

KyrylR · 2026-10-07T10:41:43Z

+fn get_annex(in_witness: &bitcoin::Witness) -> Option<&[u8]> {
+    let last_item = in_witness.last()?;
+    if *last_item.first()? == TAPROOT_ANNEX_PREFIX {
+        Some(last_item)


AI review finding. Based on independent read-only reviews by Claude Opus 5.5 (claude-opus-5-5, High) and Claude Mythos 5.1 (claude-mythos-5-1, High). Posted by Codex at the account holder's request; this is not the account holder's own authored review.

[P1] Strip the annex marker before passing it to libsimplicity. Some(last_item) includes the leading 0x50, but the pinned compatible Signet node passes data()+1 and size()-1 to the same C environment. The Elements wrapper strips the marker too. bitcoin/env.c hashes the supplied annex bytes into txHash and sigAllHash. A local exact-head probe found the PR's full-element digest 261ca392… versus 87308987… with the node's stripped annex. An annex-bearing signature generated from this environment will therefore disagree with that node. Please pass the payload after 0x50 and add an annex-bearing regression test. This concerns the Simplicity C environment interface; BIP341's separate TapSighash rule includes the marker.

KyrylR · 2026-10-07T10:41:43Z

+/// (rust-bitcoin 0.32 removed `Witness::taproot_annex`.)
+fn get_annex(in_witness: &bitcoin::Witness) -> Option<&[u8]> {
+    let last_item = in_witness.last()?;
+    if *last_item.first()? == TAPROOT_ANNEX_PREFIX {


AI review finding. Based on independent read-only reviews by Claude Opus 5.5 (claude-opus-5-5, High) and Claude Mythos 5.1 (claude-mythos-5-1, High). Posted by Codex at the account holder's request; this is not the account holder's own authored review.

[P2] Require at least two witness items before recognizing an annex. This prefix check accepts a single witness item beginning 0x50, although BIP341 and the pinned node require at least two items. In the local exact-head probe, a one-item 0x50 witness changed the digest to the same value as a real two-item annex; a one-item ordinary witness did not. A key-path input in a mixed-input transaction can therefore change the Rust-side hash unexpectedly. Please guard on in_witness.len() >= 2 and test the one-item case. This expands the existing #384-related note on this helper.

KyrylR · 2026-10-07T10:41:44Z

+        script_cmr: Cmr,
+        control_block: ControlBlock,
+    ) -> Self {
+        let c_tx = c_env::new_tx(tx.borrow(), utxos);


AI review finding. Based on independent read-only reviews by Claude Opus 5.5 (claude-opus-5-5, High) and Claude Mythos 5.1 (claude-mythos-5-1, High). Posted by Codex at the account holder's request; this is not the account holder's own authored review.

[P2] Validate UTXO count and the current input index before building the C environment. c_env::new_tx uses tx.input.iter().zip(in_utxos.iter()), which silently truncates the C input array when the UTXO slice is short. A local exact-head probe changed the omitted second input and found the same digest; with both UTXOs supplied, the digest changed. This safe constructor also accepts ix >= tx.input.len(), violating the C build_txEnv precondition. Please reject mismatched lengths and an out-of-range index before allocation. The hash is wrong with a short slice today; downstream jet use could index outside the C input array.

Double check from GPT 6 Sol (Max effort) with Daybreak:

Short UTXO slice: confirmed; out-of-bounds consequence overstated. With two transaction inputs but one supplied UTXO, changing the omitted second input leaves the PR digest unchanged; with two UTXOs it changes. The safe BitcoinEnv::new also accepts ix=1 for a one-input transaction, despite the C build_txEnv precondition. However, the currently vendored build_txEnv does not dereference the indexed input, and every env->tx->input[env->ix] access found in bitcoinJets.c is preceded by a bounds check. The posted comment's statement that downstream jet use could index outside the C array is speculative and not supported by this revision. No out-of-bounds access was observed. The missing-UTXO hash defect and invalid-index acceptance remain valid.

KyrylR · 2026-10-07T10:41:44Z

+ *               unsigned char cmr[32]
+ *               unsigned char program[program_len]
+ */
+bool rustsimplicity_0_6_computeCmr( simplicity_err* error, unsigned char* cmr, rustsimplicity_0_6_callback_decodeJet decodeJet


AI review finding. Based on independent read-only reviews by Claude Opus 5.5 (claude-opus-5-5, High) and Claude Mythos 5.1 (claude-mythos-5-1, High). Posted by Codex at the account holder's request; this is not the account holder's own authored review.

[P2] Reconcile the vendored C files with their recorded source revision and symbol version. simplicity-HEAD-revision.txt still names ba967b1a, but the newly added C/header files are absent from that revision. This file uses rustsimplicity_0_6_* names alongside the repository's 0_8_* declarations; bitcoin/cmr.c calls the 0_8 CMR symbol, and bitcoin/exec.h declares a 0_6 symbol while exec.c defines 0_8. Standalone syntax checks of both cmr.c files fail with unknown or undeclared symbols. Cargo passes because it does not compile these files today. Please re-vendor from the actual upstream revision or normalize the files and compile-check them before they are wired in.

I wanted to ask the same question in the other PR, if vendor-simplicity.sh was called

LesterEvSe · 2026-10-07T12:30:21Z

simplicity-sys/vendor-simplicity.sh:93-96 only rewrites the version prefix in "$DEPEND_PATH/env.c" and "$DEPEND_PATH/wrapper.h". But this PR adds bitcoin_env.c which also hardcodes rustsimplicity_0_8_, so the script doesn't rewrite it.
Let's fix it by adding "$DEPEND_PATH/bitcoin_env.c" to the list.

Other than that that, I reviewed every commit manually, from a0b5435 to 1f91d94. LGTM!

LesterEvSe · 2026-10-07T12:58:20Z

AI finding (Claude Opus 5.5, Extra High): We pass each input's real scriptSig here, but the Signet node never sets it, so it hashes an empty one. As a result, input_script_sig_hash and input_script_sigs_hash will differ from the node whenever another input has a non-empty scriptSig. Elements Core has the same pattern. Is that intended (should we pass an empty scriptSig here), or should the node be fixed?

LesterEvSe · 2026-10-07T13:00:52Z

I also ran Claude Opus 5.5 Extra High on the entire PR, and it found the same problems as Kyrylo's AI review and marked them as blockers

apoelstra and others added 10 commits October 5, 2026 14:28

simplicity-sys: split up bitcoin and elements build

a0b5435

Don't actually build bitcoin yet; just refactor build.rs

simplicity-sys: define bitcoin transaction environment

0182d01

jet: switch from Deref to Borrow for transaction environments

87549f6

This is the more correct trait. When we update the Jet trait we will be forced to pick one, and we will pick Borrow.

jet: drop unnecessary Arcs in environments

1dfdf0f

Now that we've updated the Jet trait to allow the transactions in environments to be arbitrary T: Borrow<Transaction>, we don't need to use Arc everywhere. In many cases we can use normal references.

jet: populate annexes in Bitcoin environment

cae2c23

delta1 force-pushed the pr/bitcoin-tx-env-331-replacement branch from 9c7f56c to 1f91d94 Compare October 5, 2026 13:33

delta1 mentioned this pull request Oct 5, 2026

jet: populate Bitcoin CMR table #386

Open

delta1 commented Oct 5, 2026

View reviewed changes

ivanlele reviewed Oct 6, 2026

View reviewed changes

Comment thread src/jet/core/environment.rs

ivanlele Oct 6, 2026

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.

Do we need this empty file

ivanlele approved these changes Oct 6, 2026

View reviewed changes

KyrylR reviewed Oct 7, 2026

View reviewed changes

Conversation

delta1 commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

delta1 commented Oct 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivanlele commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KyrylR left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LesterEvSe commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LesterEvSe commented Oct 7, 2026

Uh oh!

LesterEvSe commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

delta1 commented Oct 5, 2026 •

edited

Loading

ivanlele commented Oct 6, 2026 •

edited

Loading

LesterEvSe commented Oct 7, 2026 •

edited

Loading