dusk-network / dips

Dusk Improvement Proposals
0 stars 1 forks source link

Proposal: Add DIPs #1

Open fed-franz opened 1 year ago

fed-franz commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

A couple of things we can still consider:

fed-franz commented 1 year ago

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 commented 1 year ago

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 commented 6 months ago

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

HDauven commented 6 months ago

@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.

fed-franz commented 5 months ago

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 commented 5 months ago

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.