Skip to content

Conversation

@AmirH-A
Copy link

@AmirH-A AmirH-A commented Jun 10, 2024

  • resolve typos inside [portal-wire-protocol.md] and [implementation-details-overlay.md]

@AmirH-A
Copy link
Author

AmirH-A commented Jun 11, 2024

@KolbyML @pipermerriam

nodes = Container(total: uint8, enrs: List[ByteList[2048], max_length=32])
```

<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like some merge conflict leftovers

Copy link
Author

Choose a reason for hiding this comment

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

thank you ❤️

@AmirH-A
Copy link
Author

AmirH-A commented Jun 19, 2024

@kdeme

Comment on lines +132 to +135
- `total`: The total number of `Nodes` response messages being sent. Currently fixed to only 1 response message.
- `enrs`: List of byte strings, each of which is an RLP encoded ENR record.
_ Individual ENR records **MUST** correspond to one of the requested distances.
_ It is invalid to return multiple ENR records for the same `node_id`. \* The ENR record of the requesting node **SHOULD** be filtered out of the list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This unordered list went wrong.

And to add on that, all the unordered list changes are not needed in the first place as asterisks, pluses, and hyphens may all be used for this in markdown.

The same applies for the emphasis asterisks vs underscores.

Comment on lines -220 to +229
Upon *sending* this message, the requesting node **SHOULD** *listen* for an incoming uTP stream with the generated `connection_id`.
Upon _sending_ this message, the requesting node **SHOULD** _listen_ for an incoming uTP stream with the generated `connection_id`.

Upon *receiving* this message, the serving node **SHOULD** *initiate* a uTP stream with the received `connection_id`.
Upon _receiving_ this message, the serving node **SHOULD** _initiate_ a uTP stream with the received `connection_id`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

unneeded change as mentioned above.

Comment on lines -167 to +173
Upon *sending* this message with a `connection_id`, the sending node **SHOULD** *listen* for an incoming uTP stream with the generated `connection_id`.
Upon _sending_ this message with a `connection_id`, the sending node **SHOULD** _listen_ for an incoming uTP stream with the generated `connection_id`.

Upon *receiving* this message with a `connection_id`, the receiving node **SHOULD** *initiate* a uTP stream with the received `connection_id`.
Upon _receiving_ this message with a `connection_id`, the receiving node **SHOULD** _initiate_ a uTP stream with the received `connection_id`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

unneeded change as mentioned above.

Copy link
Member

Choose a reason for hiding this comment

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

The _ and - changes are required by the ethereum/EIPs linter. We should review and merge Pipers PR before this one though @kdeme

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, fair enough. Same linter should be setup on this repo then also.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Very good. I will be happy if I can help in this matter

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.

4 participants