* 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`
* 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>
Greg Pfeil