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

Update tests for AssetBase derivation and key components #49

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

vivek-arte
Copy link

@vivek-arte vivek-arte commented Mar 21, 2023

This PR updates the test-vectors from the updates to the zcash-test-vectors repository (see here).

The keys test is also updated to now use the asset base from the test vectors instead of just using the native asset.

@what-the-diff
Copy link

what-the-diff bot commented Mar 21, 2023

PR Summary

  • Asset base derivation: The asset base is now derived from the issuance validating key and description for improved accuracy.
  • Error handling update: AssetBase::from_bytes() has been updated to return an error for incorrect input lengths instead of causing a panic.
  • NoteValue::asset_base() change: This function now returns a reference to itself for all note values, including native ones using AssetBase::default(). This resolves previous test failures in relation to asset bases.
  • Test vector updates: Adjustments have been made to the test vectors in src\note\asset_id\testing to align with the new changes.

Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, same comment for a concise PR description and git commit msg as in QED-it/zcash-test-vectors#7 (review)

Make sure to describe the change to the keys test.

@PaulLaux PaulLaux changed the title Making changes to the tests to account for asset base derivation from the asset identifier Update tests for AssetBase derivation and key components Mar 29, 2023
@vivek-arte vivek-arte merged commit f0b7948 into zsa1 Apr 18, 2023
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.

2 participants