Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Spec freeze: Mark main Waku v2 specs as stable #406

Closed
oskarth opened this issue Jun 23, 2021 · 16 comments
Closed

Spec freeze: Mark main Waku v2 specs as stable #406

oskarth opened this issue Jun 23, 2021 · 16 comments
Assignees

Comments

@oskarth
Copy link
Contributor

oskarth commented Jun 23, 2021

Problem

As we are getting more and more projects using Waku v2, there's a need for specs to be stabilized so they can be relied upon to be stable by end users. Furthermore, a minimal subset of specs have been essentially the same for many months.

See https://rfc.vac.dev/spec/1/#stable-specifications:

When draft specifications are used by third parties, they become stable specifications. Changes to stable specifications should be restricted to cosmetic ones, errata and clarifications. Stable specifications are contracts between editors, implementers, and end-users.

For example, WalletConnect wants to integrate with wallets, and for this protocol identifiers should be set. I.e. not /vac/waku/relay/2.0.0-beta2 but /vac/waku/relay/2.0.0.

Scope

The most immediate one is relay, but other candidates here are such specs as mentioned here: https://rfc.vac.dev/spec/10/#recommendations-for-clients i.e.

Request for feedback

It'd be good some thoughts from various client implementers in terms of:

  1. Requirements for updating spec
  • I understand WC wants relay and possibly store under stable identifiers
  1. Any thoughts on specs that SHOULD or SHOULD NOT be marked stable
  2. Finally, for stakeholders to take a look at specs before we end up freezing them, to make sure it meets their needs
  • Most immediately here would be Core and Desktop to have a look at above and see if they imagine any changes being required

cc @jm-clius @staheri14 @D4nte @richard-ramos (editors/client implementers)
cc @pedrouid @sbc64 WalletConnect
cc @cammellos @iurimatias Core and Desktop

@jm-clius
Copy link
Contributor

IMO those 4 specs are OK to move to stable (only those 4, for now)

@pedrouid
Copy link

Looks good to me too 👍 Those are the only specs we are currently relying on for WalletConnect

@staheri14
Copy link
Contributor

staheri14 commented Jun 23, 2021

Any thoughts on specs that SHOULD or SHOULD NOT be marked stable

Regarding the store protocol, I think there is still work to be done to mark it as stable.
For example,

Considering all these coming changes and issues, I think we should not consider the store specs stable atm.

@oskarth
Copy link
Contributor Author

oskarth commented Jun 24, 2021

Re Waku Message: #373 not sure how we want to deal with this uncertainty here

@D4nte
Copy link
Contributor

D4nte commented Jun 25, 2021

Re Waku Message: #373 not sure how we want to deal with this uncertainty here

I think there the question is simply: are we happy to use version: number to define some potential format for the payload or do we want to move to a different struct such as format: 'raw'|'status-enc'|'eth-sign' etc... to define that the payload may have a specific format.

IMHO, answering this question should be enough to make stable, we can then at a later stage define more formats.

@oskarth
Copy link
Contributor Author

oskarth commented Jun 25, 2021

do we want to move to a different struct such as format: 'raw'|'status-enc'|'eth-sign' etc

What would you specifically suggest here that would be a minimal diff? In terms of protobuf and data type change from current version field. What has worked in other tls/libp2p/noise etc specs?

@oskarth
Copy link
Contributor Author

oskarth commented Jun 28, 2021

Answering my own question, we could follow something like https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#keys

Looking at waku-org/js-waku#179 (comment) I think we need to clean up the encryption spec story, it is a bit diffuse right now with things in different places etc. So I think that should probably be in draft for now?

Ditto main spec, until we have completed discovery domain. There are no protocol identifiers so that should be fine.

@jm-clius do you think you could update the relay code and spec to be stable? i.e. remove beta prefix.

@staheri14 how much effort would you expect these to be? in general it'd be good if we could finish the basic store protocol up and mark it as stable and then move on. We can talk more about it in 1:1.

@D4nte
Copy link
Contributor

D4nte commented Jun 28, 2021

do we want to move to a different struct such as format: 'raw'|'status-enc'|'eth-sign' etc

What would you specifically suggest here that would be a minimal diff? In terms of protobuf and data type change from current version field. What has worked in other tls/libp2p/noise etc specs?

Note that from our discussion, my understanding on version 1 is not yet up to scratch so feel free to dismiss my comment.

@jm-clius
Copy link
Contributor

@jm-clius do you think you could update the relay code and spec to be stable? i.e. remove beta prefix.

Will do!

@staheri14
Copy link
Contributor

@oskarth I am working on addressing all the remaining issues of the store protocol. My estimation is that they are going to be done in a week or two.

@cammellos
Copy link
Contributor

I have looked at the specs, and looks good on our side, I haven't focused on store since it seems that it won't be marked as stable for now, but the rest look ok.
thanks

@staheri14
Copy link
Contributor

staheri14 commented Jul 21, 2021

Any thoughts on specs that SHOULD or SHOULD NOT be marked stable

Regarding the store protocol, I think there is still work to be done to mark it as stable.
For example,

Considering all these coming changes and issues, I think we should not consider the store specs stable atm.

The waku store protocol is ready to be marked stable. cc: @oskarth

  • Two items of the above checklist are completed.
  • After some investigation, the second item is crossed out, it turned out that the current choice of float64 is more precise than int64 because it accommodates sub-seconds in the fractional part (which contrasts with int64 that is usually used for seconds precision).
  • We also decided not to proceed with the third item of the checklist as it is more of an optimization problem (to handle multiple queries within one RPC) which can also be done through libp2p.

@oskarth
Copy link
Contributor Author

oskarth commented Jul 22, 2021

Thanks for the update @staheri14!

Question on timestamp: I see the rationale for float64, though it does strike me as a bit odd for some reason (maybe due to precision?). Did we consider the use of int64 nanosecond (or similar) considered? I believe the is this would accommodate sub-seconds. What is e.g. Nimbus using?

@oskarth
Copy link
Contributor Author

oskarth commented Aug 3, 2021

Brief update: general Waku v2 main spec improvements #444

@staheri14
Copy link
Contributor

staheri14 commented Aug 6, 2021

Question on timestamp: I see the rationale for float64, though it does strike me as a bit odd for some reason (maybe due to precision?). Did we consider the use of int64 nanosecond (or similar) considered? I believe the is this would accommodate
sub-seconds. What is e.g. Nimbus using?

@oskarth
Opened this issue to address your comment.

@jm-clius
Copy link
Contributor

jm-clius commented Jan 4, 2023

Reading through this, I believe we have covered most of the outstanding points and requirements in #528. Closing this as duplicate. cc @kaiserd

@jm-clius jm-clius closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants