[ZIP 230] Adding back the assetBase field to the IssueNote structure#1081
[ZIP 230] Adding back the assetBase field to the IssueNote structure#1081vivek-arte wants to merge 1 commit into
Conversation
The merge of zcash#987 happened to delete the table entry for the Asset Base in the Issue Note. We are adding it back here.
| +-----------------------------+------------------------------+------------------------------------------------+---------------------------------------------------------------------+ | ||
| | 8 |``value`` |``uint64`` |The amount being issued in this note. | | ||
| +-----------------------------+------------------------------+------------------------------------------------+---------------------------------------------------------------------+ | ||
| | 32 |``assetBase`` |``byte[32]`` |The encoding of the Asset Base. | |
There was a problem hiding this comment.
The removal was an intentional choice by the ZIP Editors: 80ac162 (see commit message for rationale).
If you have alternative rationale for having it in the format, please provide it. Otherwise, close this PR.
There was a problem hiding this comment.
I see - there is a benefit in terms of reduced transaction size, and the removal of that consensus check.
The way the issuance bundle is structured, however, it is not possible to completely specify the constituent Issue Notes as they are parsed -- the issuer identifier (issuer) field is only parsed after the entire vIssueActions vector, but it is needed to be able to construct the Issue Note.
We can avoid this if we reorder the issuance bundle to start first with issuer, then the vIssueActions vector, then the issueAuthSig; accompanied by the appropriate change in the digest algorithm. (Or we can include the asset base in the Issue Note, as in this PR)
I prepared this change in #1086 , along with some other necessary corrections (such as the note size being changed to 115 bytes from 147 bytes, and the removal of the now extra consensus checks).
I'd appreciate if you could review and let me know what you think works best, and we can close one of these PRs.
|
Closing in favour of #1086 |
The merge of #987 happened to delete the table entry for the Asset Base in the Issue Note Description of ZIP 230. This PR adds it back.