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

Updates to txid_digest #15

Merged
merged 10 commits into from
May 25, 2023
Merged

Updates to txid_digest #15

merged 10 commits into from
May 25, 2023

Conversation

vivek-arte
Copy link

This PR details the changes to the transaction digest algorithm for the ZSA protocol.

  • A new branch is added for hashing the information in a transaction related to issuance.
  • Some branches are added and modified in the orchard_digest subtree to account for the additional Asset Base value that needs to be hashed.

@netlify
Copy link

netlify bot commented May 10, 2023

Deploy Preview for zcash-zips-qedit ready!

Name Link
🔨 Latest commit 10c8332
🔍 Latest deploy log https://app.netlify.com/sites/zcash-zips-qedit/deploys/646f6248a70e880008df5f36
😎 Deploy Preview https://deploy-preview-15--zcash-zips-qedit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

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

zip-0226.rst Outdated
Comment on lines 333 to 357
txid_digest
├── header_digest
├── transparent_digest
│   ├── prevouts_digest
│   ├── sequence_digest
│   └── outputs_digest
├── sapling_digest
│   ├── sapling_spends_digest
│   │   ├── sapling_spends_compact_digest
│   │   └── sapling_spends_noncompact_digest
│   ├── sapling_outputs_digest
│   │   ├── sapling_outputs_compact_digest
│   │   ├── sapling_outputs_memos_digest
│   │   └── sapling_outputs_noncompact_digest
│   └── valueBalance
└── orchard_digest
│   ├── orchard_actions_compact_digest
│   ├── orchard_actions_assetid_digest
│   ├── orchard_actions_memos_digest
│   ├── orchard_actions_noncompact_digest
│   ├── flagsOrchard
│   ├── valueBalanceOrchard
│   └── anchorOrchard
└── issuance_digest

Choose a reason for hiding this comment

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

Let's do just top level here:

txid_digest
    ├── header_digest
    ├── transparent_digest
    ├── sapling_digest
    └── orchard_digest
    └── issuance_digest

Choose a reason for hiding this comment

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

Actually let's remove the big tree completely. Seems redundant.

Copy link
Author

Choose a reason for hiding this comment

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

I updated the tree, with the [UPDATED FOR ZSA] labels. Thought it seemed good for completeness to have the whole table mentioned once.

Maybe we could remove the issuance version of the tree from the Issuance ZIP (ZIP 227), since that is repeated information from the Transfer ZIP (ZIP 226).

Choose a reason for hiding this comment

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

It does looks good but we can't have this duplication, it will bite us if we will keep it.

Copy link
Author

Choose a reason for hiding this comment

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

Removed.

zip-0226.rst Outdated
===========

The transaction digest algorithm defined in ZIP 244 [#zip-0244]_ is modified by the ZSA protocol to add a new branch for issuance information, and add an additional hash for the Asset Base within the ``orchard_digest``. The details of these changes are described in this section. The overall tree structure of the hash is as follows, and the additional names not referenced in ZIP 244 are described in detail below::

Choose a reason for hiding this comment

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

Changes and additions to the Orchard protocol are styled in Bold.

Copy link
Author

Choose a reason for hiding this comment

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

As discussed, now using [UPDATED FOR ZSA] to highlight this.

Choose a reason for hiding this comment

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

Great.
Still, we don't need the big tree, it's a duplication over all the other information. Let's remove it.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the big tree.

zip-0226.rst Outdated Show resolved Hide resolved
zip-0226.rst Outdated Show resolved Hide resolved
zip-0226.rst Outdated
Comment on lines 388 to 398
T.4b: orchard_actions_assetid_digest
''''''''''''''''''''''''''''''''''''

A BLAKE2b-256 hash of the subset of Zcash Shielded Asset identifier data for all Orchard Actions belonging to the transaction. For each Action, the following elements are included in the hash::

T.4b.i: assetid (field encoding bytes)

The personalization field of this hash is set to::

"ZTxIdOrcActAHash"

Choose a reason for hiding this comment

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

Think we need to remove it as described above.

Copy link
Author

Choose a reason for hiding this comment

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

Removed.

zip-0226.rst Show resolved Hide resolved
Copy link

@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 good! approved with one comment.

zip-0226.rst Outdated
===========

The transaction digest algorithm defined in ZIP 244 [#zip-0244]_ is modified by the ZSA protocol to add a new branch for issuance information, and add an additional hash for the Asset Base within the ``orchard_digest``. The details of these changes are described in this section. The overall tree structure of the hash is as follows, and the additional names not referenced in ZIP 244 are described in detail below::

Choose a reason for hiding this comment

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

Great.
Still, we don't need the big tree, it's a duplication over all the other information. Let's remove it.

zip-0226.rst Outdated
Comment on lines 333 to 357
txid_digest
├── header_digest
├── transparent_digest
│   ├── prevouts_digest
│   ├── sequence_digest
│   └── outputs_digest
├── sapling_digest
│   ├── sapling_spends_digest
│   │   ├── sapling_spends_compact_digest
│   │   └── sapling_spends_noncompact_digest
│   ├── sapling_outputs_digest
│   │   ├── sapling_outputs_compact_digest
│   │   ├── sapling_outputs_memos_digest
│   │   └── sapling_outputs_noncompact_digest
│   └── valueBalance
└── orchard_digest
│   ├── orchard_actions_compact_digest
│   ├── orchard_actions_assetid_digest
│   ├── orchard_actions_memos_digest
│   ├── orchard_actions_noncompact_digest
│   ├── flagsOrchard
│   ├── valueBalanceOrchard
│   └── anchorOrchard
└── issuance_digest

Choose a reason for hiding this comment

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

It does looks good but we can't have this duplication, it will bite us if we will keep it.

@vivek-arte vivek-arte merged commit e7ab26e into zsa1 May 25, 2023
PaulLaux pushed a commit that referenced this pull request Oct 4, 2023
This details the changes to the transaction digest algorithm for the
ZSA protocol.
- A new branch is added for hashing the information in a transaction
related to issuance.
- Some branches are added and modified in the orchard_digest subtree to
account for the additional Asset Base value that needs to be hashed.
vivek-arte added a commit that referenced this pull request Feb 12, 2024
This details the changes to the transaction digest algorithm for the
ZSA protocol.
- A new branch is added for hashing the information in a transaction
related to issuance.
- Some branches are added and modified in the orchard_digest subtree to
account for the additional Asset Base value that needs to be hashed.
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