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

Peer want-list maintenance #1066

Open
benbierens opened this issue Jan 13, 2025 · 3 comments
Open

Peer want-list maintenance #1066

benbierens opened this issue Jan 13, 2025 · 3 comments
Assignees
Labels
Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details

Comments

@benbierens
Copy link
Contributor

benbierens commented Jan 13, 2025

A node keeps track of the wants of its peers. Currently this is a simple seq in peercontext.nim. A malicious peer could spam entries and break our node.

We should change this by adding the following functionality:

  • A max length for peer wantList, settable via conf option, defaulted to 100 (?)
  • When trying to add new entries to a full wantList, consider priority and replace low priority with high priority entries.
@benbierens benbierens self-assigned this Jan 13, 2025
@benbierens benbierens added the Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details label Jan 13, 2025
@cskiraly
Copy link
Contributor

Can you add a link to the related code portion?

Do we have a response message here to notify the sender about his want list being "chopped"? It might make sense to add that.

@benbierens
Copy link
Contributor Author

Can you add a link to the related code portion?

Do we have a response message here to notify the sender about his want list being "chopped"? It might make sense to add that.

Here's the seq in question:

peerWants*: seq[WantListEntry] # remote peers want lists

We don't have a response to tell a sender that their want entries are being dropped. I don't think bitswap has this in its spec. I'm not sure we should either: If we're being spammed, should we not do as little as possible? Also to keep in mind: Dmitriy recently made a chance where want entries are only stored if we don't have the block. If we do have it, we reply with a presence immediatelly but don't add the want to the peer's list. So the want list entries are only those blocks the peer wants which we don't currently have. (Once we have them, we should send a presence update and remove them from the want list. (I'm not sure if that's currently correctly implemented...))

I'm not sure 100 want entries is a sane default. Especially in a case where the full wantList is sent, it might not be enough.

@gmega
Copy link
Member

gmega commented Jan 14, 2025

I don't think we can cap the lists that way. In swarms where peers are connected mostly to neighbors that don't have the blocks you're interested in, you could easily run into a situation where you drop wantlist requests and then the requesting node never learns about their neighbors getting the block because the wantlist entries were dropped.

As for the spam and possibility for DoS: I'd argue it's probably better to drop requests for blocks belonging to datasets you're not actively interested in (i.e. those that you can't are legit), much like what happens in Bittorrent: you can't request blocks for file that a node doesn't know about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details
Projects
None yet
Development

No branches or pull requests

3 participants