-
Notifications
You must be signed in to change notification settings - Fork 3
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
Status old RFCs - moving to RFC Index #109
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.
Left a few comments that should be fixed. All specs should be in deprecated and placed in a status deprecated folder. (e.g.status/deprecated/1/client.md
) Also the .obsidian subfolder should be removed. And there are some links in specs living in this repo that link to specs.status, they should be updated.
status/1/client.md
Outdated
## Table of Contents | ||
|
||
- Abstract | ||
- Table of Contents | ||
- Introduction | ||
- Protocol layers | ||
- Protobuf | ||
- Components | ||
- P2P Overlay | ||
- Node discovery and roles | ||
- Bootstrapping | ||
- Discovery | ||
- Mobile nodes | ||
- Transport privacy and Whisper/Waku usage | ||
- Secure Transport | ||
- Data Sync | ||
- Payloads and clients | ||
- BIPs and EIPs Standards support | ||
- Security Considerations | ||
- Design Rationale | ||
- P2P Overlay | ||
- Why devp2p? Why not use libp2p? | ||
- What about other RLPx subprotocols like LES and Swarm? | ||
- Why do you use Whisper? | ||
- Why do you use Waku? |
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.
The table of contents and the list can be removed for all the specs.
status/1/client.md
Outdated
slug: 1 | ||
title: 1/CLIENT | ||
name: Client | ||
status: draft |
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.
Remove the slug and the number in front of the title for all the specs.
status/1/client.md
Outdated
|
||
A Mobile node is a Whisper and/or Waku node connecting to the network. | ||
Mobile nodes **MAY** relay messages. | ||
See 3/WHISPER-USAGE and 10/WAKU-USAGE for more details. |
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.
Remove the slugs for references in each spec.
status/9/ethereum-usage.md
Outdated
Status nodes rely on these Infura nodes | ||
to validate transaction integrity and report consistent history. | ||
|
||
[Key handling is described here](https://specs.status.im/spec/2-account.md) |
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.
Update links for specs being moved to this repo.
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.
Left a few comments. Also, there are links in the Status specs already living in this repo that have links to status.specs
that need to be updated (check all specs). For example, user account, located here
status/deprecated/1/client.md
Outdated
--- | ||
title: CLIENT | ||
name: Client | ||
status: draft |
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 should be deprecated
for every specification. Also the path directory, status/deprecated/1/client.md
, the number should be removed.
@@ -0,0 +1,139 @@ | |||
--- | |||
title: IPFS-gateway-for-Sticker-Pack |
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.
The directory path, status/deprecated/12/IPFS gateway for Sticker Pack.md
should be status/deprecated/12/IPFS-gateway-for-Sticker-Pack.md
. Check the other specs as well.
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.
Sorry mentioned this in an another comment but not here, the number in the directory path should be removed also.
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.
do you mean that the directory path should be the title?
- **Integrity**: The adversary should not cause either endpoint | ||
of a Status 1:1 chat to accept tampered data. | ||
|
||
These properties are ensured by Signal’s Double Ratchet. |
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.
These properties are ensured by Signal’s Double Ratchet. | |
These properties are ensured by [Signal’s Double Ratchet](https://signal.org/docs/specifications/doubleratchet/). |
The links should not have been removed. Add the missing links and also add a Reference section, and include all the links in the order they appear in the specs. This needs to be done on all the specs.
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 has not been resolved, I marked it as unresolved. RFCs within this repo should have a reference section if links are presented in the RFC. Also you removed links from the original RFCs, they should be added and put in the reference section as well.
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 don't remember manually removing these links, need to check.
|
||
### Account Generation | ||
|
||
See Account specification. |
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.
See Account specification. | |
See [Account specification](/status/deprecated/account.md). |
The link for the deprecated specifications should use the new link within this repo. My suggestion uses a path with the number removed, which should be removed for all specs before updating the links.
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.
Not resolved, should be done for each spec.
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.
all the other specs should have it - there was some conflict here but I resolved it now, pushing a new version now. Thanks!
Status nodes rely on these Infura nodes | ||
to validate transaction integrity and report consistent history. | ||
|
||
[Key handling is described here](https://rfc.vac.dev/status/9/ethereum-usage.md) |
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.
Update links.
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.
Should be relative to the rfc-index repo, e.g. status/deprecated/ethereum-usage.md
. Please check all specs.
The .obsidian directory should be removed. |
Co-authored-by: Jimmy Debe <[email protected]>
Co-authored-by: Jimmy Debe <[email protected]>
Co-authored-by: Jimmy Debe <[email protected]>
Co-authored-by: Jimmy Debe <[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.
Some comments were not resolved. Let me know if you have any questions. @Cofson
status/deprecated/6/payloads.md
Outdated
in a group chat. The details are covered in the [Group Chats specs](https://rfc.vac.dev/status/deprecated/7/group-chat.md). | ||
|
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 link should be a relative link, check my previous comment here
Status nodes rely on these Infura nodes | ||
to validate transaction integrity and report consistent history. | ||
|
||
[Key handling is described here](https://rfc.vac.dev/status/9/ethereum-usage.md) |
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.
Should be relative to the rfc-index repo, e.g. status/deprecated/ethereum-usage.md
. Please check all specs.
- **Integrity**: The adversary should not cause either endpoint | ||
of a Status 1:1 chat to accept tampered data. | ||
|
||
These properties are ensured by Signal’s Double Ratchet. |
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 has not been resolved, I marked it as unresolved. RFCs within this repo should have a reference section if links are presented in the RFC. Also you removed links from the original RFCs, they should be added and put in the reference section as well.
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.
Please check all specs for similar occurrences. @Cofson
Status clients run on a public, permissionless peer-to-peer network, | ||
as specified by the devP2P network protocols. | ||
devP2P provides a protocol for node discovery, | ||
and the RLPx Transport Protocol v5 is used for TCP-based communication. | ||
|
||
The client **SHOULD NOT** use Whisper V6. | ||
Instead, the client **SHOULD** use Waku V1 for privacy-preserving messaging | ||
and efficient bandwidth usage. | ||
|
||
### Node Discovery and Roles |
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 not sure where else this occurs but it looks like you did not copy the full spec. Here is this paragraph I see on specs.status.im:
" Status clients run on a public, permissionless peer-to-peer network, as specified by the devP2P network protocols. devP2P provides a protocol for node discovery which is in draft mode here. See more on node discovery and management in the next section.
To communicate between Status nodes, the RLPx Transport Protocol, v5 is used, which allows for TCP-based communication between nodes.
On top of this RLPx-based subprotocols are ran, the client SHOULD NOT use Whisper V6, the client SHOULD use Waku V1 for privacy-preserving messaging and efficient usage of a node’s bandwidth."
No description provided.