-
Notifications
You must be signed in to change notification settings - Fork 356
CIP-0154? | Orthogonal Certificates #1021
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
base: master
Are you sure you want to change the base?
Conversation
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.
hey @yHSJ
Thanks for the proposal
See a few minor editorial comments
Overall, I think this is well written and motivated proposal
One thing i may suggest adding is some numbers on how many times these 'combination certificates' have been used on mainnet
as I suspect that these certificates havent been widely adopted, maybe in-part because hardware wallets don't support them
Thank you for submitting this! I think to allow for more discussions and for completeness sake, it would be good to mention the alternative option which we originally discussed, that would have one certificate with optional entries for stake & vote delegation. The upside of this would be that it's purely a CDDL change. |
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
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.
@yHSJ @WhatisRT it's great to see that you've preloaded this with Ledger feedback & looking forward to introducing this at tomorrow's CIP meeting (tagged Triage
): https://hackmd.io/@cip-editors/110
CIP-XXXX/README.md
Outdated
``` | ||
|
||
### Proposed CDDL | ||
While the current certificates may save a few bytes in the transaction when doing multiple actions in a single transaction, it greatly complicates certificates for both users and implementers. |
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.
For completeness, it would be worth highlighting how many bytes in the Rationale
section, to show that the original motivation for introducing those numerous certificates was likely a bit misguided.
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.
Good point, added the worst case numbers.
CIP-XXXX/README.md
Outdated
stake_registration_cert = (N, stake_credential, coin) | ||
stake_deregistration_cert = (N+1, stake_credential, coin) | ||
stake_delegation_cert = (N+2, stake_credential, pool_keyhash) | ||
vote_delegation_cert = (N+3, stake_credential, drep) |
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.
For backward-compatibility reason, it would be necessary here to specify the new indices, and makes sure that they don't clash with the current ones. This way, one could virtually still deserialise a compound certificate into two orthogonal ones; and ensure that we don't needlessly break all transaction builders once again.
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.
There is no need to break existing specification. All you trying to do is remove some certificates, while leaving others unchanged.
stake_registration_cert = (N, stake_credential, coin) | |
stake_deregistration_cert = (N+1, stake_credential, coin) | |
stake_delegation_cert = (N+2, stake_credential, pool_keyhash) | |
vote_delegation_cert = (N+3, stake_credential, drep) | |
stake_registration_cert = (7, stake_credential, coin) | |
stake_deregistration_cert = (8, stake_credential, coin) | |
stake_delegation_cert = (2, stake_credential, pool_keyhash) | |
vote_delegation_cert = (9, stake_credential, drep) |
Otherwise we would have to keep old certificates for one more era to avoid breakage, just to replace certificates with equivalent ones
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.
I decided to go with this in the CIP. I left them intentionally ambiguous so that others could weigh in on whether we want to simply rename existing certs or introduce new ones (for consistency or whatever)
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.
I am weighing in. Renaming (changing the tag) will require old certificates to be retained for a whole new era. So, it makes no sense to introduce new identical replacement certificates. Please adjust the CIP to retaon the old tags.
Actual names of cddl bindings are irrelevant and can be discussed at a any point, since they do not require a HF.
CIP-XXXX/README.md
Outdated
|
||
### Acceptance Criteria | ||
|
||
- [ ]Acceptance of the Ledger team, as well as alternative node implementation maintainers (Amaru, Dingo, and others) |
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 will also require a hard-fork, and the CIP can only be active once the hard-fork has been enacted.
Co-authored-by: Matthias Benkort <[email protected]>
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.
Confirmed as candidate at CIP meeting today, with some background given about how the proposal originated at a working group with the initial reviewers in common. Therefore @lehins it would be great timing for you to also enter the discussion & review.
Please @yHSJ also rename the containing directory to CIP-0154
🎉 & update the proposal link in the top comment.
CIP-XXXX/README.md
Outdated
|
||
While composite actions would require multiple certificates, multiple small certificates would only add a couple of bytes when compared to the composite certificates. | ||
|
||
## Path to Active |
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.
Note that hardforking into this can't immediately remove the current certificates, they can only be deprecated since otherwise downstream tooling would immediately break. So it needs to be done in two steps, implementing the new CDDL & deprecating the old one, and then removing the old one later.
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.
I asked a clarifying question about this in the Cardano scaling discord, but I'm not sure why this has become the process only for some changes.
CIP-XXXX/README.md
Outdated
stake_registration = (0, stake_credential) | ||
stake_deregistration = (1, stake_credential) |
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 two certificates are from the Shelley era and were already planned for removal. They were kept around in Conway for ease of transition and preventing breakage of tooling while this transition is under way.
CIP-XXXX/README.md
Outdated
stake_registration_cert = (N, stake_credential, coin) | ||
stake_deregistration_cert = (N+1, stake_credential, coin) | ||
stake_delegation_cert = (N+2, stake_credential, pool_keyhash) | ||
vote_delegation_cert = (N+3, stake_credential, drep) |
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.
There is no need to break existing specification. All you trying to do is remove some certificates, while leaving others unchanged.
stake_registration_cert = (N, stake_credential, coin) | |
stake_deregistration_cert = (N+1, stake_credential, coin) | |
stake_delegation_cert = (N+2, stake_credential, pool_keyhash) | |
vote_delegation_cert = (N+3, stake_credential, drep) | |
stake_registration_cert = (7, stake_credential, coin) | |
stake_deregistration_cert = (8, stake_credential, coin) | |
stake_delegation_cert = (2, stake_credential, pool_keyhash) | |
vote_delegation_cert = (9, stake_credential, drep) |
Otherwise we would have to keep old certificates for one more era to avoid breakage, just to replace certificates with equivalent ones
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.
Here is my general view on this CIP: I am not really sure if it was worth writing the CIP, since this is fairly simple concern and a ticket on ledger repo would have probably suffice. That being said, there is nothing wrong with creating a CIP, since it will give more visibility to the proposed change.
In a nutshell, we already had plans on removing couple of old certificates, namely stake_regestration
and stake_deregistration
, since they were kept around temporarily for backwards compatibility with prior era and allowed users for easier transition to new ones that require a deposit. The only real new thing that that this CIP requests is to remove few redundant certificate types that allow register and delegate at the same time.
In other words, in the next era, we just want to remove all of these certificates: https://github.com/IntersectMBO/cardano-ledger/blob/49623962445143680dd725ebbf812c37e099b65c/eras/conway/impl/cddl-files/conway.cddl#L677-L687
Just to confirm, this is what we want plus some consistent naming for the next era from this CIP
certificate = [
- stake_registration
- // stake_deregistration
- // stake_delegation
+ stake_delegation
// pool_registration
// pool_retirement
// reg_cert
// unreg_cert
// vote_deleg_cert
// stake_vote_deleg_cert
- // stake_reg_deleg_cert
- // vote_reg_deleg_cert
- // stake_vote_reg_deleg_cert
// auth_committee_hot_cert
// resign_committee_cold_cert
// reg_drep_cert
// unreg_drep_cert
// update_drep_cert]
If my understanding of this CIP is correct then you have my full support.
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.
@yHSJ @lehins @WhatisRT I think the remaining Ledger confirmation was the main thing we were looking for so I'm marking this Last Check
because I think @yHSJ all that remains are settling some cosmetic / clarity review points, given #1021 (review).
Therefore this should be ready with plenty of time to spare, since due to Builder Fest overlap our next meeting isn't for 3½ weeks: https://hackmd.io/@cip-editors/111
I'm ready to approve this in the meantime if @yHSJ you can update Path to Active at least according to these 2 suggestions (or equivalent):
#1021 (comment) (time separation between relevance for Haskell vs. future nodes)
#1021 (comment) (clarity about whether / how a hard fork is required, even if "doing it anyway")
Thanks for taking the time to review it! I agree, it's a rather simple change. That being said, I think it is important that, as an ecosystem, we move towards ledger changes being clearly documented in a CIP (or CIP like process) to help facilitate changes across all the different nodes. It's a big shift, but I truly believe it's important that the ledger is no longer owned by a single entity, but a shared resource. Also, this should be the first CIP in a family of "ledger simplification CIPs" that come about as a result of the node diversity workshop.
Yes, generally it's just removing redundant certificates to simplify the ledger (along with consistent naming).
It looks like you're missing a few of the "composite" certificates in that diff. For clarity sake, I am suggesting: removing:
introducing (or renaming, the arrows here represent equivalency):
Having the indexes of each of these "account management" certificates be sequential just feels like a simpler API. Ultimately, I am of the opinion it is better to have a more "painful" change in order to make the ledger as simple as possible for the future. |
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.
approved as per #1021 (review) 🚀
I just realized that we might never be able to remove those certificates, since some PlutusV3 scripts could depend on them. This means that if we completely remove these certificates, outputs that are locked by scripts that depend on these certificates being available:
will become unspendable. I am afraid that we might have to keep them around forever unless we have full support of all DApp writers out there that it is safe to remove these certificates. The only safe thing that I can think of is to disable such certificates, unless a transaction uses PlutusV3 scripts. Which I am not sure is that much better, since tooling would still have to support all those certificates 😢 |
@yHSJ the CIP meeting today recognised the rounds of approval but also confirmed that the last #1021 (comment) suggests an unaddressed backward compatibility problem (please @lehins correct us if wrong about that, because otherwise we were ready to sign off on this). I proposed that we might merge this if some mass deprecation warning were given to withdraw use of the affected certificates in advance of making this change... to be included very clearly in the Path to Active about how the developer community would be involved. I pointed out this could well leave it in Therefore we're updating the tags accordingly and editors will watch out for a solution that addresses both the Ledger feedback and our usual requirements for versioning and community acceptance. |
Yes, I do confirm that if we remove these four certificates:
there is a danger of locking funds in an unrecoverable manner, similar to what has happened in this issue: IntersectMBO/cardano-ledger#5009 and was also discussed in #1028 We still can and will remove these two certificates:
since backwards compatibility is achieved through translation of newer certificates that have the deposit to plutus context. Same trick unfortunately cannot be applied to the aforementioned four certificates, since their equivalent representation requires multiple simpler certificates. Naturally, we can and should also rename certificates in CDDL in more consistent manner.
There is certainly this option of communicating and warning the community about this. Question is whether this is worth the hassle. Naturally, this is not my call. My job is only to warn about the dangers and y'all deal with the community 😁 |
I thought I had commented this earlier, but apparently I didn't. This backwards compatibility issue makes me think that the original suggestion from the node diversity workshop is better in this context because it doesn't suffer from that, being purely a CDDL change. I referenced it in a previous post already: #1021 (comment) To give a bit more detail, the original proposal there was to make the CDDL mirror the structure we have in the Conway spec for certificates, see e.g. https://github.com/IntersectMBO/formal-ledger-specifications/blob/conway-v1.0/src/Ledger/Certs.lagda#L76 This also avoids the issue of non-linear growth when we add features and has some other upsides that the proposal here as as well. It might be a less intuitive interface for people outside the functional programming space, but that seems like an ok price to pay for backwards compatibility. |
pending resolution of backward compatibility issue
NOTE: This CIP was initially suggested and discussed in person during the Node Diversity Workshop in Paris, France, April 7-9, 2025.
The transaction is the most important interface in Cardano. Therefore, we should strive to make it as simple as possible without sacrificing functionality or security.
This CIP proposes a change to the ledger: replacing the certificates introduced in the Conway era with much orthogonal certificates, each scoped to a single action. This change significantly simplifies certificates, making them easier to use, document, and implement—which is especially important as alternative nodes are being implemented.
(rendered version)