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

fix readme #41

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix readme #41

wants to merge 2 commits into from

Conversation

Eikix
Copy link

@Eikix Eikix commented Jul 12, 2024

Fixing readme by:

  • deleting outdated information:
    • diagrams
    • paragraphs in MD files
    • notes from a meeting (don't belong in main imo). NB: someone can copy them from git history into their local notes
  • consolidating the three MD files into one for clarity it'll be easier to update it. If it grows too much we'll split it.
  • adding small mermaid diagrams (to be reworked, generated with chatgpt so might be bad)

@Eikix
Copy link
Author

Eikix commented Jul 12, 2024

Adding lists of TODOS for today:

  • add discovery protocol (left as TODO for now in the code)
  • make mermaid diagrams more informative
  • add general flow diagram for block propagation
  • get a review with Kasar Labs team then Starkware team @ShahakShama


As a design choice This Node results in some loss of generality, but seems like a reasonable given that libp2p is becoming a de-facto standard, and we see little value in re-implementing (and re-inventing) lower level network communication protocols.

Specifically, we identify nodes and address them using [Peer IDs](https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md), with key pairs derived using [Ed25519](https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#ed25519) scheme.
Copy link
Author

Choose a reason for hiding this comment

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

Is this still true?

Commit is old


### Identifying Nodes

An [`Identify` message](https://github.com/libp2p/specs/blob/master/identify/README.md) should include the following fields
Copy link
Author

Choose a reason for hiding this comment

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

Is this still true?

Commit is old


### Message Encoding

Use protobuf (`proto3`) for message encoding/decoding between peers.
Copy link
Author

Choose a reason for hiding this comment

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

Is this still true?

Use protobuf (`proto3`) for message encoding/decoding between peers.
Specific message schemas to be defined for each protocol.

Messages that tie into other messages in one flow, i.e. messages expecting a response and providing a response MUST include a `request_id` (a positive integer) that allows nodes to correlate messages in the same flow.
Copy link
Author

Choose a reason for hiding this comment

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

Is this true


Nodes SHOULD use semantic version matching when matching protocols, as described [here](https://docs.libp2p.io/concepts/protocols/#protocol-negotiation).

`agentVersion` is defined in the form of _agent-name/version_; e.g. `papyrus/0.1.0`.
Copy link
Author

Choose a reason for hiding this comment

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

Is this still true?


An [`Identify` message](https://github.com/libp2p/specs/blob/master/identify/README.md) should include the following fields

`protocolVersion` := `/starknet/` + version
Copy link
Author

Choose a reason for hiding this comment

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

Is this still true?

@Eikix Eikix force-pushed the docs/update_readme branch from 58ca808 to 53e85f1 Compare July 12, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant