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

Proposal: Add DIPs #1

Open
fed-franz opened this issue Aug 18, 2023 · 10 comments · May be fixed by #15
Open

Proposal: Add DIPs #1

fed-franz opened this issue Aug 18, 2023 · 10 comments · May be fixed by #15
Labels
category:meta DIPs related to other categories or group thereof

Comments

@fed-franz
Copy link
Collaborator

Description

Introduce the use of Dusk Improvement Proposals (DIPs) to request/track changes to the Dusk protocol.

Details

  1. Incipit: this will be typically an informal discussion on external platforms (Discord, forums, etc...) where the idea of a DIP stems from;
  2. Issue: the idea is officially proposed as a potential DIP by filing a new Issue on the dips repository. Here the proposal is thoroughly discussed and refined. The Issue name should start with "DIP: " and summarize the proposal in few words;
  3. PR: the DIP is officially drafted in a Pull Request; the PR should add a new file named DIP-xxxx with a suitable DIP number. In this phase the DIP is formally described and formatted according to the DIP guidelines. The PR can include further discussion if needed. If necessary, a proof-of-concept implementation PR can be linked to the PR;
  4. Merge: the DIP is included in the official list and its implementation is added to the Dusk roadmap;
  5. Implementation: the DIP is implemented through one or more PRs in the code repo (typically rusk) which are linked to the DIP.

The DIP document will include a Status field to keep track of the proposal (Active/Rejected/In Progress/...) and is updated with relevant information when needed.

Notes

This should be DIP-0.

Examples to follow:

@marta-belles
Copy link

It looks pretty good! In step 3, I would avoid numbering DIPs at this stage, since PRs may be discarded/rejected. If that is the case, the accepted PRs will be numbered quite weirdly with gaps between one DIP number and another. I would suggest to number a DIP once accepted.

Besides this, it seems that in Ethereum they start the PRs with "Add/Update EIP", maybe that is something we would also like to consider (or change in the future if we see the need for it).

@fed-franz
Copy link
Collaborator Author

It looks pretty good! In step 3, I would avoid numbering DIPs at this stage, since PRs may be discarded/rejected. If that is the case, the accepted PRs will be numbered quite weirdly with gaps between one DIP number and another. I would suggest to number a DIP once accepted.

The PR is intended to be merged either way, independently of whether the DIP will indeed be implemented or not. I see your point, but the number must be chosen in the PR phase to be then merged into main. If, for some reason, another DIP is accepted before, the DIP number can change before merging.

Besides this, it seems that in Ethereum they start the PRs with "Add/Update EIP", maybe that is something we would also like to consider (or change in the future if we see the need for it).

Sure, whatever works for us. This is to be defined by this DIP.

@HDauven
Copy link
Member

HDauven commented Sep 24, 2023

I would suggest the following for the DIP structure:

  1. Preamble: contains metadata on the DIP:
    • DIP number
    • Title
    • Author(s)
    • Status
    • Category/Type (Core, Standards, Governance)
    • Creation date
  2. Abstract: Concise summary.
  3. Motivation: Describes the problem being addressed.
  4. Tech Spec: Detailed technical aspects, emphasis on protocol changes, data structures, API changes and cryptographic considerations.
  5. Rationale: Discussion on the trade-offs and design choices.
  6. Backward Compatible: Detailed description of incompatibilities.
  7. Test Cases and Implementation: Practical test cases and a possible reference implementation.
  8. Security Considerations: Highlight of the potential security implications, especially in the context of privacy.
  9. Updates/Amendments (optional): A section to document updates or amendments to a DIP.
  10. References: List of relevant material and discussions.

@HDauven
Copy link
Member

HDauven commented Sep 24, 2023

A couple of things we can still consider:

  • A review period: A formal review period before a DIP gets merged.
  • Rejection criteria: When does it make sense to reject a DIP? What is the process for resubmitting and appealing a rejected proposal?
  • Versioning, updates & amendments: Should we track changes to a DIP? Should it be possible to amend a DIP in case it's a good idea but there's for examples vulnerabilities that need to be ironed out?
  • Template & Guidelines: This will need to be further refined. I posted a proposed template outline in the post above.

@fed-franz
Copy link
Collaborator Author

I would suggest the following for the DIP structure:

1. **Preamble**: contains metadata on the DIP:
   
   * DIP number
   * Title
   * Author(s)
   * Status
   * Category/Type (Core, Standards, Governance)
   * Creation date

2. **Abstract**: Concise summary.

3. **Motivation**: Describes the problem being addressed.

4. **Tech Spec**: Detailed technical aspects, emphasis on protocol changes, data structures, API changes and cryptographic considerations.

5. **Rationale**: Discussion on the trade-offs and design choices.

6. **Backward Compatible**: Detailed description of incompatibilities.

7. **Test Cases and Implementation**: Practical test cases and a possible reference implementation.

8. **Security Considerations**: Highlight of the potential security implications, especially in the context of privacy.

9. **Updates/Amendments** (optional): A section to document updates or amendments to a DIP.

10. **References**: List of relevant material and discussions.

I like it!
I'd remove the Amendments part and rather simply amend the DIP when needed. Otherwise, you would read a out-of-date description first, and then read the changes... seems overcomplicated to me. I think it's easier to have a single description with the latest proposal, duly motivated in the related section.

It might make sense to have an Updates sections, in case some "external" situation changes (e.g., a limitation was removed, or a new tool has been made available, things like that).

@fed-franz
Copy link
Collaborator Author

Answering to your points:

* A review period: A formal review period before a DIP gets merged.

I assume you refer to merging the DIP implementation to the codebase. In this case, more than a "period", I'd require a minimum number of reviewers (and possibly selected ones, not just any reviewer).
If you refer to merging the DIP in this repo, I don't think it's necessary, since a merged DIP does not imply it will be part of the protocol, but only that is sensible enough to be discussed.

* Rejection criteria: When does it make sense to reject a DIP? What is the process for resubmitting and appealing a rejected proposal?

Not sure it's possible to define a complete list of rejection criteria, but we might list some, like security flaws, or technical impediments. Is this really necessary though?
What I think it might be useful is some kind of internal "voting" on the acceptance of the DIP to see if we all agree (and possibly reject the DIP because many of us are not in favor).

* Versioning, updates & amendments: Should we track changes to a DIP? Should it be possible to amend a DIP in case it's a good idea but there's for examples vulnerabilities that need to be ironed out?

I think it should be possible to amend a DIP whenever it makes sense. We could update the description, and mention the change with its motivation in the Amendments section (so yeah, maybe it is useful if used this way).

* Template & Guidelines: This will need to be further refined. I posted a proposed template outline in the post above.

Both should be included in the PR for this issue (as new files).

@herr-seppia
Copy link
Member

@HDauven @autholykos Can this be considered implemented by #7 ?

@HDauven
Copy link
Member

HDauven commented Apr 4, 2024

@HDauven @autholykos Can this be considered implemented by #7 ?

In my opinion, yes. But it would be good if everyone has a review of what we currently have on main and provide feedback on it.

I still think we can have a good discussion on what constitutes DIP worthy.

@HDauven HDauven added the category:meta DIPs related to other categories or group thereof label Apr 19, 2024
@fed-franz
Copy link
Collaborator Author

I think we should have a file for each "approved" dip (by approved I mean that the issue is accepted as a dip, regardless of the actual outcome).
The first DIP should be this one, and it can include the info added by #7 plus missing details (like what should and should not be a DIP, as @HDauven mentioned).
I also think we should move on with this ASAP, so we can formalize all the other dips, some of which have already been implemented.

@fed-franz fed-franz linked a pull request Apr 26, 2024 that will close this issue
@fed-franz fed-franz linked a pull request Apr 26, 2024 that will close this issue
@fed-franz
Copy link
Collaborator Author

I created a draft of the DIP-1 file and opened a draft PR to add it.
I structured following the examples from BIP-1 and EIP-1.

@HDauven, @autholykos, and everyone else, feel free to contribute to the writing and editing of the document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:meta DIPs related to other categories or group thereof
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants