Zcash-compatible Bitcoin script impl

[dconnolly]

No description provided.

[hdevalence]

Two options:

1. Write a license-compatible implementation from scratch;

2. Write glue code around the parity-zcash code, tainting the total artefact's license but allowing us to replace it later without affecting the rest of the codebase.

[str4d]

Another option is to build libzcashconsensus from the zcashd codebase (which is technically doable but would require some legwork), and wrap that in a Rust API. This is equivalent to the bitcoinconsensus crate from the rust-bitcoin project (R.I.P. Tamas Blummer).

This would likely be a bit more work than wrapping the parity-zcash code, but you could probably provide the same API around both, and the license for libzcashconsensus is MIT.

[hdevalence]

Hmm, just to clarify, the suggestion is to extract the C++ consensus implementation from zcashd, provide FFI bindings to Rust as libzcashconsensus? I think that we would prefer not to do that, since at the moment we don't have any significant C++ dependencies (with the future exception of RocksDB, which we will isolate into zebra-storage internals) and I'm not sure we have the engineering resources to build and maintain C++ bindings, which is significantly more perilous than normal Rust.

[hdevalence]

some helpful pointers from @prestwich:

• Bitcoin script test vectors: https://github.com/bcoin-org/bcoin/blob/master/test/data/core-data/script-tests.json
• Parser for those tests: https://github.com/bcoin-org/bcoin/blob/master/test/script-test.js#L28-L68
• Rust implementation of Bitcoin script (CC0) https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/blockdata/script.rs

[yaahc]

relevant discord discussion: https://discordapp.com/channels/676527656170160146/716086297210650634/729816649242378261

[prestwich]

@yaahc mind reposting some context for those of us following who aren't on discord? :)

[yaahc]

@preswich Not sure how well this will paste but here goes nothing

---

Yaah (she/her)Yesterday at 2:50 PM
this is the relevant issue right? #180
GitHub
Zcash-compatible Bitcoin script impl · Issue #180 · ZcashFoundation...
durumcrustulumYesterday at 2:51 PM
correct yes
The current Script type is just a wrapper around a Vec with some Zcash(De)Serialize trait impls

zebra/zebra-chain/src/types.rs

Line 99 in 8281b90

pub struct Script(pub Vec<u8>);

GitHub
ZcashFoundation/zebra
An ongoing Rust implementation of a Zcash node. 🦓. Contribute to ZcashFoundation/zebra development by creating an account on GitHub.
str4dYesterday at 2:53 PM
Hoo boy, not envying you implementing this
durumcrustulumYesterday at 2:53 PM
we aren't, we're pulling in a dep
definitely uh, not trying to do that from scratch 😅
str4dYesterday at 2:53 PM
Sentiment still applies
durumcrustulumYesterday at 2:53 PM
eeep
str4dYesterday at 2:55 PM
We don't have SegWit (or several other NOP changes) from upstream, and we stripped out FindAndDelete etc. before launch, so you likely can't just depend on a Bitcoin Rust dependency.
durumcrustulumYesterday at 2:55 PM
I think this is the one
https://github.com/rust-bitcoin/rust-bitcoin/
GitHub
rust-bitcoin/rust-bitcoin
Rust Bitcoin library. Contribute to rust-bitcoin/rust-bitcoin development by creating an account on GitHub.
hrmpph yeah
str4dYesterday at 2:56 PM
Using that as a starting point, and keeping close enough to pull in updates from them, makes sense. But I doubt that the delta can be easily applied as a purely functional wrapper
(I'd be happy to be wrong!)
Some things (like NOP changes) could be wrapped to hide the internal behaviour with an actual nop, but IDK enough about how that implementation is designed. The C++ impl certainly isn't amenable to that (it internally iterates over the script, and there's no modular "handle one opcode" point that is treated as an opaque handler within the script iteration, just a giant switch statement)
Yaah (she/her)Yesterday at 3:06 PM

This library must not be used for consensus code

wheeeeww

[prestwich]

I think eeep is the only correct response ya

this is a really thorny issue as Zcash relies on an ancient fork of Script, and Script is full of terrible edge cases (like negative 0). If we use a Rust re-implementation we're more likely to cause consensus failures 🤔

[yaahc]

If we use a Rust re-implementation we're more likely to cause consensus failures 🤔

not if zcashd adopts the same rust impl :D

[prestwich]

if moving to a new Script implementation entirely, it would be a good idea to patch up sighash a bit. ZIP-243 hasn't been updated for the recent BIP-143 value issues.

Here's a list of changes in BIP-341. Numbers 2 and 3 are useful for Zcash:

In summary, the semantics of the BIP143 sighash types remain unchanged, except the following:

1. The way and order of serialization is changed.
2. The signature message commits to the scriptPubKey of the spent output and if the SIGHASH_ANYONECANPAY flag is not set, the message commits to the scriptPubKeys of all outputs spent by the transaction.
3. If the SIGHASH_ANYONECANPAY flag is not set, the message commits to the amounts of all transaction inputs.
4. The signature message commits to all input nSequence if SIGHASH_NONE or SIGHASH_SINGLE are set (unless SIGHASH_ANYONECANPAY is set as well).
5. The signature message includes commitments to the taproot-specific data spend_type and annex (if present).

[teor2345]

If we use a Rust re-implementation we're more likely to cause consensus failures 🤔

not if zcashd adopts the same rust impl :D

That would have to wait until at least the Canopy network upgrade, which is in November. We would need some kind of script implementation before then, to validate Sapling to Heartwood.

[yaahc]

That would have to wait until at least the Canopy network upgrade, which is in November. We would need some kind of script implementation before then, to validate Sapling to Heartwood.

Good point. I think we will need to come up with two plans, one for short term and one for long term.

Short Term

1. fork rust-bitcoin and use it for validation
   • This is going to cause the same consensus issues that reimplementing it from scratch would
   • we're not even sure that rust-bitcoin includes a full implementation of the runtime for scripts
2. reimplement in rust only the subset of bitcoin script that is currently used in existing transactions, document the fragility
3. wrap libzcashconsensus via ffi, either with a bindgen or cxx

Long Term

1. do a full reimplementation of the script impl from libzcashconsensus in rust and switch both zcashd and zebrad to this impl as part of the Canopy upgrade
2. stick with the wrapped c++ dependency for libzcashconcensus
3. stick with the rust-bitcoin impl

---

The best option seems to be 3 for short term and then 1 for long term, that gives us a fully valid and reliable script implementation at all times. If we run into issues with ffi and c++ or if we really don't want to have any c++ dependencies then I feel like the second best option would be 2 which then grows into 1. And for 2 we could even use bits and pieces of rust-bitcoin as a starting point, though it hardly matters at that point. I'm going to start with looking into FFI for now pending further feedback from other @ZcashFoundation/zebra-team members.

[dconnolly]

I think eeep is the only correct response ya

Voice of a generation, I ams

[teor2345]

• do a full reimplementation of the script impl from libzcashconsensus in rust and switch both zcashd and zebrad to this impl as part of the Canopy upgrade

How do we validate pre-Canopy blocks?
Zebra can set a mandatory checkpoint, but zcashd can't.

[teor2345]

In general, it might help to look through the list of BIPs and ZIPs in #253 to find any script changes or restrictions.

[yaahc]

How do we validate pre-Canopy blocks?
Zebra can set a mandatory checkpoint, but zcashd can't.

disclaimer: I'm not exactly an expert on the network upgrades or consensus, so this could easily be based on faulty assumptions.

I assume the zebra-script impl would be well tested against all pre canopy blocks. If we're worried about some of the last pre Canopy blocks failing to validate causing problems with the network update we could do a transition period where as part of the Canopy upgrade we include both script validators and continue to use the c++ script validator for all old blocks and use the rust validator for all new blocks, then we could go back and make sure all existing pre-canopy blocks successfully validate with zebra-script and once they do we could rip out the c++ script validator at our leasure.

[teor2345]

Using dual libraries works, as long as the new validator accepts all the old blocks.

Here's how we would do that hard fork at Canopy activation:

• use the old validator up to the Canopy height
• use the new validator at and after the Canopy upgrade height

That way, we support chain reorganisations across the activation height.

If the old validator rejects some old blocks, then new zcashd nodes will reject those old blocks during the initial sync. There are workarounds, but they involve hard-coded exceptions for some blocks or block ranges.

[yaahc]

@str4d went and setup a version of libzcashconsensus that is refactored to be more self contained which we should use for any zcash ffi based impls for this

https://github.com/str4d/zcash/tree/proofverifier-refactor

pull that branch from str4d's repo and then build just libzcashconsensus with this command:

CONFIGURE_FLAGS="--with-libs" ./zcutil/build.sh -j$(nproc)

[str4d]

You can disable more parts and build just libzcashconsensus.a:

CONFIGURE_FLAGS="--with-libs --without-daemon --without-utils --disable-tests --disable-bench" ./zcutil/build.sh -j$(nproc)

The static library ends up in src/.libs/libzcashconsensus.a. The relevant API is zcashconsensus_verify_script, which is declared in src/script/zcashconsensus.h (and would be redeclared in an extern "C" {} Rust block).

[yaahc]

extremely relevant reference impl for when someone wrapped the upstream version of bitcoinconsensus as a rust library

https://github.com/rust-bitcoin/rust-bitcoinconsensus/blob/master/build.rs