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

Send pluralized wantBlock messages #1016

Merged
merged 11 commits into from
Dec 4, 2024

Conversation

benbierens
Copy link
Contributor

Previously:

    discard await allFinished(
      wantCids.mapIt(b.sendWantBlock(it, peerCtx)))

This gives 1 wantBlock message per entry in wantCid blockAddress. But, message format supports multiple addresses.

New:

    await b.sendWantBlock(wantCids, peerCtx)

Todo, not in this PR: initial calls to send wantHave and wantBlock messages can be pluralized as well.

@benbierens
Copy link
Contributor Author

This passes dist-tests.

@benbierens benbierens marked this pull request as ready for review December 3, 2024 09:33
@benbierens benbierens added the Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details label Dec 3, 2024
@benbierens benbierens self-assigned this Dec 3, 2024
Copy link
Member

@gmega gmega left a comment

Choose a reason for hiding this comment

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

Seems harmless to me, just need to fix the raises annotations.

@benbierens benbierens requested a review from gmega December 4, 2024 08:47
Copy link
Member

@gmega gmega left a comment

Choose a reason for hiding this comment

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

LGTM

@benbierens benbierens added this pull request to the merge queue Dec 4, 2024
Merged via the queue into master with commit 8e29939 Dec 4, 2024
15 checks passed
@benbierens benbierens deleted the feature/blkexc-pluralize-wantblock branch December 4, 2024 15:00
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

Successfully merging this pull request may close these issues.

2 participants