* bump to 0.3.1
* Add necessary components to toolchain
Apparently GitHub recently made changes so that these components aren’t
installed automatically for all toolchains, so they’re now explicit (as
they probably should have been all along).
* comment cleanups
---------
Co-authored-by: Greg Pfeil <greg@technomadic.org>
* 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