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

CIP-0080 | Transaction Serialization Deprecation Cycle #372

Merged
merged 8 commits into from
Mar 18, 2023

Conversation

JaredCorduan
Copy link
Contributor

@JaredCorduan JaredCorduan commented Nov 10, 2022

This CIP proposes a policy for how we manage the backwards compatibility of the binary format for transactions. The summary is:

  • We should strive to maintain backwards compatibility (which we've maintained since the Shelley phase began).
  • Serious flaws can be fixed immediately (at the next hard fork), and can break backwards compatibility.
  • Non-Serious flaws can be fixed (at the next hard fork), but the old format must be supported for at least six months with support ending at the next hard fork event after the six months have passed.

(current draft in branch)

@rphair rphair changed the title transaction serialization CIP for compatibility CIP-???? | Transaction Serialization for Compatibility Nov 11, 2022
@rphair rphair changed the title CIP-???? | Transaction Serialization for Compatibility CIP-???? | Transaction Serialization Deprecation Cycle Nov 11, 2022
@rphair
Copy link
Collaborator

rphair commented Nov 11, 2022

thanks @JaredCorduan ... not sure which one of these titles suit better; the "for Compatibility" title would be more familiar to the uninitiated (e.g. me) but all other things being equal it's probably better to have the PR title match the document.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Unless @KtorZ objects this could be a good time to get ahead of the changes to CIP document standards in progress, particularly the header preamble (https://github.com/cardano-foundation/CIPs/blob/cip-cps-rework/CIP-0001/README.md#header-preamble):

CIP-tx-serialization-deprecation-cycles/README.md Outdated Show resolved Hide resolved
Title: Transaction Serialization Deprecation Cycle
Authors: Jared Corduan <jared.corduan@iohk.io>
Status: Draft
Type: Standards Track
Copy link
Collaborator

@rphair rphair Nov 11, 2022

Choose a reason for hiding this comment

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

There will no longer be a Type: field but instead a Category: (https://github.com/cardano-foundation/CIPs/blob/cip-cps-rework/CIP-0001/README.md#categories) which I would think should be Ledger. But I don't see that on the list of categories, nor do I see a Core category that I thought I read during recent drafts of CIP-0001.

@KtorZ I must have missed something about how CIPs for core features are categorised & why they aren't on the current list of choices; what would we do in this case?

Copy link
Member

Choose a reason for hiding this comment

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

I'd urge the respective maintainers of the various areas of the protocol to register an appropriate category for their respective areas. It is obvious that the cardano-ledger wants to engage more with the CIP process; and @JaredCorduan has been a recurring point of contact there; So simply capturing that in a document similar to CIP-0035 would be the way to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JaredCorduan
Copy link
Contributor Author

thanks @JaredCorduan ... not sure which one of these titles suit better; the "for Compatibility" title would be more familiar to the uninitiated (e.g. me) but all other things being equal it's probably better to have the PR title match the document.

I'm happy with either, and thanks for keeping it consistent!

@KtorZ
Copy link
Member

KtorZ commented Nov 30, 2022

As discussed in the CIP meeting today, there's no "ledger" category registered yet in the CIP process. Ideally, we would want something similar to CIP-0035: Plutus Core Evolution for all areas of the protocol that engage with the CIP process.

Having said that, this CIP sounds like a good foundation for a "ledger" category -- even though the current document is mainly focused on CDDL format of on-transactions, I imagine it could be extended to define the overall strategy for changes in the ledger and how people can engage with the team on these topics.

@KtorZ KtorZ changed the title CIP-???? | Transaction Serialization Deprecation Cycle CIP-0080 | Transaction Serialization Deprecation Cycle Nov 30, 2022
@KtorZ KtorZ changed the title CIP-0080 | Transaction Serialization Deprecation Cycle CIP-0080? | Transaction Serialization Deprecation Cycle Nov 30, 2022
JaredCorduan and others added 6 commits March 18, 2023 11:29
Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
  - Add discussions
  - Add subtitles to 'Motivation' & 'Rationale'
  - Fix 'Path to Active' section.
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

@Ryun1 @rphair

I am doing some housekeeping today and I believe this CIP was already discussed and could be merged (almost) as-is. Given its informational nature and being pushed by the ledger team itself.

I did resolve the last "cosmetic" issues on the header, folder name and so on.

@KtorZ KtorZ added the State: Last Check Review favourable with disputes resolved; staged for merging. label Mar 18, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@KtorZ I agree and I cannot find anything wrong with it & don't think the Ledger team or anyone else are likely to either. I am therefore itching to merge this but guess that you wouldn't have assigned the Last Check label unless SOP is to do this at the meeting (just going to resolve merge conflict for now).

@Ryun1
Copy link
Collaborator

Ryun1 commented Mar 18, 2023

@KtorZ @rphair LGTM

@KtorZ KtorZ merged commit b0b6c71 into cardano-foundation:master Mar 18, 2023
@KtorZ KtorZ deleted the jc/tx-serialization branch March 18, 2023 21:34
@KtorZ
Copy link
Member

KtorZ commented Mar 18, 2023

@rphair I did put the last-check mainly as a reminder in case you guys didn't replied promptly. But I underestimated you both :p

@rphair
Copy link
Collaborator

rphair commented Mar 19, 2023

OK, I'd been thinking Last Check had been a flag for a final review at the meeting, but it seems we can merge these if in agreement online. Therefore @KtorZ perhaps also #421 #422 can be merged?

@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label May 2, 2023
@rphair rphair changed the title CIP-0080? | Transaction Serialization Deprecation Cycle CIP-0080 | Transaction Serialization Deprecation Cycle May 24, 2023
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
…dation#372)

* transaction serialization CIP for compatibility

* assign CIP number 80

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>

* separated hard and soft fork requirements

* Update CIP-tx-serialization-deprecation-cycles/README.md

* Move CIP-0080 in its dedicated folder.

* Minor edits

  - Add discussions
  - Add subtitles to 'Motivation' & 'Rationale'
  - Fix 'Path to Active' section.

* Update top-level README with CIP-0080 inclusion.

---------

Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Co-authored-by: Robert Phair <rphair@cosd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Ledger Proposals belonging to the 'Ledger' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants