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

[ZSA] Fees for ZSA issuance. #667

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

[ZSA] Fees for ZSA issuance. #667

wants to merge 12 commits into from

Conversation

vivek-arte
Copy link
Contributor

@vivek-arte vivek-arte commented Jan 18, 2023

This PR includes the Fees ZIP modifications for the ZSA Protocol, and is a companion to PR#676.

  • This PR details the changes to ZIP 317 - Proportional Transfer Fee Mechanism, the latest ZIP on fees in Zcash, that would be required along with the upgrade to the ZSA Protocol.

The rendered version of the ZIP can be viewed here

See #930

zip-0317.rst Outdated Show resolved Hide resolved
zip-0317.rst Outdated
Comment on lines 274 to 301
:math:`issuance\_fee` :math:`1000000` zatoshis per issuance action (as defined below)
:math:`finalize\_fee` :math:`1000000` zatoshis per issuance action (as defined below)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this set a minimum (economic) value for each NFT, or the sum of any other asset?

Can we add a note explaining what that minimum is in ZEC?

zip-0317.rst Outdated Show resolved Hide resolved
zip-0317.rst Outdated Show resolved Hide resolved
zip-0317.rst Outdated Show resolved Hide resolved
@nuttycom nuttycom changed the title ZSA Protocol ZIPs - Fees [ZSA] Fees for ZSA issuance. Nov 1, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

What we'll want for this change is not just the delta to ZIP 0317, but a separate ZIP draft (which will likely be assigned a ZIP number in the 2xxx range) that describes the rationale for the fee changes and a delta to ZIP 0317, which will then be represented here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The fee changes are a consequence of the new transaction format, and so the updates to ZIP 317 should be described in ZIP 230 (or an update ZIP that 230 can reference, but since they're small changes it's probably easier to do it in 230). They can't be applied directly to ZIP 317 for us to be able to merge them now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made these changes in QED-it#76. The Fees material is now in ZIP 230, and will reflect in #854 once we merge it (very soon)

@nuttycom
Copy link
Contributor

nuttycom commented Nov 1, 2024

@vivek-arte this will need attention before the NU7 zip selection deadline (2024-11-05). See also #854 (comment)

zips/zip-0317.rst Outdated Show resolved Hide resolved
Comment on lines +11 to +14
Daniel Benarroch
Jonathan Rouach
Pablo Kogan
Vivek Arte
Copy link
Collaborator

Choose a reason for hiding this comment

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

If one of yous wants to add yourself as an Owner, that would be fine from my point of view. (Speaking as an existing Owner.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose this is not relevant for now, since we aren't editing ZIP 317

Jonathan Rouach
Pablo Kogan
Vivek Arte
Status: Draft
Copy link
Collaborator

@daira daira Nov 3, 2024

Choose a reason for hiding this comment

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

As a modification to ZIP 317, this would need to look like:

  Status: [Revision 0] Active, [Revision 1] Proposed

but only once it actually gets to Proposed status. (See ZIP 214 for an example using this convention for NU6 changes.) One of the reasons to put it in a 2xxx ZIP instead is that then it can be merged as Draft.

ZIP Editors: I will file some suggested wording for ZIP 0 codifying this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it like so based on how it was in ZIP 316 (though now I see that has been updated to Proposed)

Status: Revision 0: Active, Revision 1: Draft

- This PR rewrites ZIP 317 in order to specify which are the revisions
made for the ZSA Protocol.
- It also rewrites the issuance_fee as a multiple of the marginal_fee
for easier analysis.
- The finalization fee is also removed, based on our discussions.

---------

Co-authored-by: Daira-Emma Hopwood <[email protected]>
@@ -40,6 +52,8 @@ The goal of this ZIP is to change the conventional fees for transactions
by making them dependent on the number of inputs and outputs in a transaction,
and to get buy-in for this change from wallet developers, miners and Zcash users.

Revision 1 of this ZIP additionally defines the fee mechanism associated with the Orchard Zcash Shielded Assets (OrchardZSA) protocol as described in ZIP 226 [#zip-0226]_ and ZIP 227 [#zip-0227]_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't strictly need to be a new Revision of ZIP 317 because it does not affect v5 transactions, and applying different rules for v6 transactions is non-disruptive to the existing specification because v6 transactions don't currently exist.

@vivek-arte
Copy link
Contributor Author

The ZSA Fees updates have been moved to ZIP 230, and will reflect in #854 -- so I think this PR can be closed for now?

vivek-arte added a commit to QED-it/zips that referenced this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants