Skip to content

Add Portal protocol versioning#3193

Merged
kdeme merged 1 commit intomasterfrom
portal-versioning
Apr 17, 2025
Merged

Add Portal protocol versioning#3193
kdeme merged 1 commit intomasterfrom
portal-versioning

Conversation

@kdeme
Copy link
Copy Markdown
Contributor

@kdeme kdeme commented Apr 4, 2025

Incomplete as it is currently not possible to always retrieve the ENR from a peer from which the node gets an incoming request.

The best solution that I see for this right now is to add ENRs to the discv5 SessionCache and thus make this retrievable for each node that has an ongoing session.

edit: currently fully implemented with v1 functionality enabled also, relies on status-im/nim-eth#790

@bhartnett
Copy link
Copy Markdown
Contributor

@kdeme Did you think we should ban nodes that have no compatible portal version?

@kdeme
Copy link
Copy Markdown
Contributor Author

kdeme commented Apr 17, 2025

@kdeme Did you think we should ban nodes that have no compatible portal version?

Yes, good point. I hadn't considered this but that would be a better way than just blocking in the addNode each time.
I will leave the PR like this for now considering we do not have banning enabled yet.

I think following steps would be then to test & enable banning. Then we alter the code to ban on incompatible version (with a small banned time period).

- Added portal versioning system based on field in ENR as per
Portal specifications
- Implemented Portal wire v1 functionality:
  - Accept codes
  - varint size prefix on FindContent uTP transfer
- Enabled Portal wire v0 + v1 versions
@kdeme kdeme force-pushed the portal-versioning branch from 463df04 to 2d66b29 Compare April 17, 2025 08:10
@kdeme kdeme marked this pull request as ready for review April 17, 2025 08:10
@kdeme kdeme merged commit 0c4c59a into master Apr 17, 2025
@kdeme kdeme deleted the portal-versioning branch April 17, 2025 09:23
@bhartnett
Copy link
Copy Markdown
Contributor

Yes, good point. I hadn't considered this but that would be a better way than just blocking in the addNode each time. I will leave the PR like this for now considering we do not have banning enabled yet.

I think following steps would be then to test & enable banning. Then we alter the code to ban on incompatible version (with a small banned time period).

Sure, makes sense. Can be done in a separate PR.

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.

2 participants