Skip to content

Asset ID#5

Merged
alexeykoren merged 10 commits intozsa1from
value-commitments
Dec 20, 2022
Merged

Asset ID#5
alexeykoren merged 10 commits intozsa1from
value-commitments

Conversation

@alexeykoren
Copy link
Copy Markdown

  • Asset id test vectors
  • ZSA value commitment building and test

@QED-it QED-it deleted a comment from alexeykoren Dec 14, 2022
Copy link
Copy Markdown

@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.

Added some comments. In addition,

  • Reformat the asset_id.py file using the IDE, it has some missing spaces etc.
  • please update the Orchard PR with the new result, if any.

is_native = i < 10
note_type = None if is_native else bytes(Point.rand(rand))
asset = None if is_native else bytes(Point.rand(rand))
asset_descr = None if is_native else randbytes(512)
Copy link
Copy Markdown

@PaulLaux PaulLaux Dec 14, 2022

Choose a reason for hiding this comment

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

Should probably encode to UTF-8 not that important.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok

Comment on lines +25 to +27
def value_commit_zsa(rcv: Scalar, v: Scalar, asset_id: Point):
return asset_id * v + group_hash(b"z.cash:Orchard-cv", b"r") * rcv

Copy link
Copy Markdown

@PaulLaux PaulLaux Dec 14, 2022

Choose a reason for hiding this comment

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

Remove this function and extend value_commit() to work with (rcv: Scalar, v: Scalar, asset: Point)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

also, should use asset not asset_id.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

assert value_commit(rcv, v) == VALUE_COMMITMENT_RANDOMNESS_BASE * rcv + VALUE_COMMITMENT_VALUE_BASE * v

# Test consistency of ValueCommit^{Orchard} with precomputed generators for non-native asset
def test_value_commit_zsa():
Copy link
Copy Markdown

@PaulLaux PaulLaux Dec 14, 2022

Choose a reason for hiding this comment

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

remove test_value_commit_zsa() and simply add another assert into test_value_commit().

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done


rcv = rcv_trapdoor(rand)
cv = value_commit(rcv, Scalar(np.v))
cv = value_commit(rcv, Scalar(np.v)) if is_native else value_commit_zsa(rcv, Scalar(np.v), asset_id(asset, asset_descr))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This change is redundant after unifying everything into value_commit()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed

PaulLaux
PaulLaux previously approved these changes Dec 20, 2022
Copy link
Copy Markdown

@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.

looks good. For /test-vectors/rust directory - remove all files from this PR except
orchard_asset_id.rs
orchard_key_components.rs
orchard_note_encryption.rs
(same as in /test_vectors/json folder)

All other files include just whitespace changes and we don't want them as part of our diff.

@PaulLaux
Copy link
Copy Markdown

conclude by copying the rs files into QED-it/orchard#34

@alexeykoren alexeykoren merged commit ee15a53 into zsa1 Dec 20, 2022
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.

3 participants