* Move `set_vch` to simplify later diffs
This makes no changes other than swapping the positions of the `set_vch`
and `serialize` operations on `ScriptNum`.
* Extract integer ↔︎ `Vec<u8>` `fn`s from `ScriptNum`
__NB__: I recommend ignoring whitespace when reviewing this commit.
`ScriptNum` was used one of three ways:
1. convert a Rust integer to a stack value
2. load a stack value into Rust
3. do arithmetic on stack values
Combining these into one interface added extra complexity. For example,
`getint()` would clamp the value to `i32` range, but that range could
only be violated if arithmetic was performed on `ScriptNum`s, and
`getint` was never used on the result of arithmetic.
This extracts `parse_num` and `serialize_num` changing the three
patterns as follows:
1. `ScriptNum::from(_).getvch()` is now `serialize_num(_)`, and
2. `ScriptNum::new(_).and_then(getint())` is now `parse_num(_)`,
3. `ScriptNum::new(_)` … `getvch()` remains the same.
* Make `ScriptNum` a process
We never held a `ScriptNum` – we create one from the stack, perform an
operation, and serialize the result. This eliminates the type in favor
of operations that take a closure on `i64`s.
The operations that exist can’t possibly hit the various bounds that
were checked on `ScriptNum` operations, so they were removed.
As part of the above work, this introduces `cast_from_bool`, which is
then used to replace all instances of `VCH_TRUE`/`FALSE`.
* Move some constants in preparation
This makes some minor changes to constants to facilitate splitting out a
stepwise interpreter.
* Extract a step function from the interpreter
This is generally useful for testing, but specifically, we want to be
able to run this side-by-side with the C++ interpreter, and check that
every bit of our state matches along the way.
This change is as minimal as possible, to avoid divergence until after
it can be compared against C++. E.g., the massive `match opcode {…}`
block that has been moved should only change the dereferencing of
`op_count`.
* Add a `State` struct to make stepping easier
* Expose step interpreter
* Add a stepwise comparison interpreter
The C++ changes aren’t in place yet, so this is currently just an A/A test.
This changes our closures into structs containing a function, because
that’s how we can pass around functions with universally-quantified
lifetimes.
* Make interpreters more flexible
Previously the `ZcashScript` impls didn’t use `self`, so the types were
just tags. However, with the new `StepwiseInterpreter`, they need
`self`.
This also removes `RustInterpreter` in favor of a `rust_interpreter`
function that instantiates an appropriate `StepwiseInterpreter`.
* Add a function for C++/Rust comparison interpreter
* Fix fuzzer
* Clean up `use` in lib.rs
* Fix weird indentation
* Make various fields non-`pub`
* Add a `new` constructor for `Stack`
* Remove incorrect comment
* Appease Clippy
Adds `Default` impls for `Stack` and `State`.
* Rename `State::manual` to `State::from_parts`
* Add ECC & myself (Greg Pfeil) as authors
* Have the Rust impl correctly report “high s” sigs
There is a bug in the C++ side where the error is not set correctly on a
“high s” signature. The Rust side had mirrored this bug, but this
eliminates the bug in the Rust.
* Remove extra byte from sig before low-s check
This doesn’t seem to have any effect on the semantics, as the DER-formatted signature includes
lengths that ensure it will ignore extra bytes, but the C++ code removes the extra byte, so the Rust
should as well.
* Change some comments
Co-authored-by: Daira-Emma Hopwood <daira@electriccoin.co>
* Appease `rustfmt`
* Have OP_DUP match the C++ impl more closely
* Address the second half of @daira’s #174 review
* Eliminate mutation from `Opcode` parsing
This now splits slices and returns the remaining pieces rather than
modifying the arguments.
* Remove obsolete comment
* Address PR comments
* Address additional comments on #174
---------
Co-authored-by: Daira-Emma Hopwood <daira@electriccoin.co>
**This changes the C++ implementation.**
Provides richer errors, which also gives more precision when comparing
against the Rust impl.
This also removes the (now unused) `zcash_script_error_t`. The only case
other than `zcash_script_ERR_OK` that was still in use was
`zcash_script_ERR_VERIFY_SCRIPT`, so that case has been added to
`ScriptError`.
This avoids changing the Rust API, but potentially `Error` and
`ScriptError` on the Rust side could be collapsed into one `enum`. It
would just be a breaking change.
* Change type of `lock_time` parameter
This is a breaking change. Lock times are stored in tx as `u32`, but
this API expected `i64`, forcing conversions on the caller. This change
brings the API into alignment with the tx representation.
* Update the `lock_time` type in the fuzz test
* [DRAFT]
* Rearrange Rust impl to match C++ more closely
The only changes here other than moving chunks of code around are
- moving `evaluate` out of `impl Script`, which required changing
`&self` to `script: &Script`; and
- unifying `ExecutionOptions` with `VerificationFlags`.
* Rename Rust identifiers to match C++
For easier side-by-side comparison.
* Connected the new API, but fails
* Existing unit tests succeed
* The rest of the owl
* Reverting to C++ style, and some other changes
* Appease Clippy
* Replace `ScriptNum` panics with error case
The C++ impl uses exceptions for `ScriptNum`, but catches them.
* Add some shallow property tests
These tests run both the C++ and Rust impls. One uses completely
arbitrary inputs, and the other limits script_sig to data pushes.
* Add shallow fuzz testing
* Preserve richer errors on the Rust side
For now, the underlying errors are discarded when comparing against the
C++ results, but there are corresponding changes on the C++ side in a
separate branch.
* Address @nuttycom’s review comments
- remove `uint256` module
- create a specific type for the C++/Rust comparison implementation
- rename some identifiers
- rephrase some comments
* Some changes to ease zebrad integration
- Switch from `log` to `tracing` for `warn!`,
- Export `SignedOutputs`, which is needed to create a `HashType`, and
- Upgrade zcash_primitives to the same version used by zebrad.
* Appease Clippy
* Remove dependency on zcash_primitives
This was only needed for the `TxVersion` enum. However, the `StrictEnc`
flag is a proxy for the v5 tx requirements, so instead of checking the
`TxVersion` explicitly, we expect callers to include `StrictEnc` for
verification of v5 transactions.
* Moving testing dependencies
libfuzzer-sys is Linux-specific, and it & proptest are only used for tests.
* Normalize Rust errors in comparison interpreter
This was a minor oversight, but this correction should only eliminate
false mismatches.
* Address @nuttycom’s PR feedback
* Eliminate a `panic!`
This `panic!` appears to be unreachable in the current implementation, but there is no need for it.
It doesn’t introduce any new failure cases.
Thanks to @conradoplg for noticing it.
* Use (`Try`)`From` for `ScriptNum` conversions
This also makes the `ScriptNum` field private so that `bn.0` can’t
extract the unconstrained `i64` value.
* Remove some `From` instances that do too much
* Subtract from `OP_RESERVED` instead of `OP_1 - 1`
`OP_RESERVED` is in the ‘0’ offset position of the `OP_n` opcodes. Just
use this even though it isn’t obviously a number to improve readability.
---------
Co-authored-by: Sean Bowe <ewillbefull@gmail.com>
Co-authored-by: Conrado Gouvea <conradoplg@gmail.com>
* Add a basic rust-toolchain.toml
Matches the one in Zebra, although I think pinning to a specific release
(e.g., `channel = "1.82"`) would help with consistency, perhaps avoiding
issues like
https://github.com/ZcashFoundation/zcash_script/pull/174#issuecomment-2445336306
* Remove redundant toolchain info from GH workflow
[actions-rs/toolchain doesn’t support TOML-formatted
rust-toolchain files](actions-rs/toolchain#126), but it’s unnecessary anyway.
- actions-rs/cargo will pick up the rust-toolchain.toml, so we usually
don’t need to mention the toolchain at all;
- the Windows build just runs `rustup target add x86_64-pc-windows-msvc`
directly; and
- where we want to build with multiple toolchains (in a matrix), there
are some slightly-awkward conditionals.
This also makes some other changes:
- `fail-fast` is disabled because it hides useful & distinct build
results; and
- `rustup component add` for clippy and rustfmt are removed because
they’re in the rust-toolchain.toml toolchain, and we want to make sure
they are, so that they’re available to developers.
* Pin rustup channel to "1.81"
Newer versions until 1.85 (current nightly) have some breakage wrt C++
linking.
* Have bindgen target correct rustc version
It should match the version in rust-toolchain.toml. Unfortunately, it’s
not possible to reference that directly, so this adds comments to remind
contributors to update them together.
* Address Str4d’s comments on #171
Notably, `HashType` has changed incompatibly, so
ZcashFoundation/zebra#8751 will need to be updated.
* Apply suggestions from code review
Co-authored-by: Jack Grigg <thestr4d@gmail.com>
* Restrict bitflags used for `HashType` in v5 tx
---------
Co-authored-by: Jack Grigg <thestr4d@gmail.com>
* Move C++ bindings out of lib.rs
Have lib.rs re-export them, but make room for the upcoming Rust implementation.
* Provide a Rustier wrapper for zcash_script
This adds a `Script` trait that exposes slightly Rustier types in order
to have a common interface for the existing C++ implementation as well
as the upcoming Rust implementation (and a third instance that runs both
and checks that the Rust result matches the C++ one).
The module structure (interpreter.rs, zcash_script.rs) and locations of
definitions are intended to mirror the structure of the C++ code, especially as
we get the Rust implementation in place, for easier comparison. That
organization is very likely to change once everything has been checked.
* Address review feedback
Thanks to @nuttycom and @arya2 for getting the closure to work.
* Additional cleanup
* Use `try_from`/`_into` instead of `as`
* Address review feedback
* Widen the `Unknown` error type
This should fix the Windows build.
* switch to MSVC, fixes to support it
* fix linking errors
* document how to patch zcash source
* update hash due to nightly breakage; don't use deprecated bindgen function
* update patch with str4d's upstream suggestion
---------
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
* Update dependencies to match zcashd v5.5.0
* Update dependencies to match Zebra main branch
* Update release instructions
* Add module docs for the build script to avoid warnings
* Update bridge file list to match the latest zcashd
* Ignore some emacs temporary files
* Standardise directory include paths in Cargo.toml
* Add extra info to cxx_gen errors
* Add additional Rust dependencies needed to compile
* Ignore some expected clippy lints
* Silence a C compiler macro redefinition warning
* Standardise directory paths in build.rs
* fix cxxbridge code generation
* Update Cargo.lock
* Use include!() for bridge.rs
* Add a changelog entry for the next release
* Fix a warning by adding docs for the crate
* Remove previous depend/zcash
* Squashed 'depend/zcash/' content from commit eb80047476
git-subtree-dir: depend/zcash
git-subtree-split: eb80047476e9c0db3524f647d412faf8d4a584ee
* Update depend/zcash to v5.5.0
```sh
git subtree add -P depend/zcash https://github.com/zcash/zcash.git v5.5.0 --squash
git rm depend/zcash/Cargo.toml
```
---------
Co-authored-by: Conrado Gouvea <conradoplg@gmail.com>
* Add extra dependency update steps to the README
* Use correct dependency steps from Cargo.toml
* Explain how to make new dependencies work
* Simplify instructions
* Add "check all open PRs before releasing"
* Add "run cargo-release" to the release instructions
Co-authored-by: Arya <aryasolhi@gmail.com>
---------
Co-authored-by: Arya <aryasolhi@gmail.com>
Conrado Gouvea
Greg Pfeil
dependabot[bot]
Dimitris Apostolou
Alfredo Garcia
Arya
teor