-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Swaps] Updates to the Asset Swaps fees rationale and specification #63
base: asset_swaps
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for zcash-zips-qedit ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This makes updates based on the comments made on #50.
2e6ad47
to
3977a28
Compare
2786578
to
41b606a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice, added important comments.
zips/zip-0228.rst
Outdated
+-----------------------------+--------------------------+--------------------------------------------------+---------------------------------------------------------------------+ | ||
|``sizeProofsOrchard`` |``proofsOrchard`` |``byte[sizeProofsOrchard]`` |The aggregated zk-SNARK proof for all Actions in the Action Group. | | ||
+-----------------------------+--------------------------+--------------------------------------------------+---------------------------------------------------------------------+ | ||
|``4`` |``timeLimit`` |``uint32`` |The block number (in the future) after which the Actions of the | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consult @ConstanceBeguier on whether timeLimit
should be here or at the actionGroup
level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moving it to actionGroup
we require less changes and I don't see a case where we might need different limit for each action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This already is at the Action Group level. The Actions are inside the vActionsOrchard
field in this table, so all the Actions in this Action Group have the same timeLimit
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified the table format, it should now be clearer.
927ae9d
to
7d2576a
Compare
d68094e
to
55807eb
Compare
…ated. (#73) This changes the Spend Authorization Signature to use the `orchard_action_groups_digest`.
This PR makes clearer the Fees section of ZIP 228, and also adds a rationale section for the fees.
It also changes the specification of the computation of the spend authorization signature