Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose ScriptError on C++ side #195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sellout
Copy link
Collaborator

@sellout sellout commented Jan 30, 2025

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.

**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.
@sellout
Copy link
Collaborator Author

sellout commented Jan 30, 2025

Since property/fuzz tests generally fail, this change was crucial to thorough testing of #174. Without it, there are only two failure values from the C++ side.

This is also preliminary work for the deeper step-wise comparison between the interpreters, as ScriptError is the failure type within verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant