fix(Receiver): Ignore unwanted blocks

[hannahhoward]

Goals

If Bitswap receives a block that isn't in it's wantlist, is should ignore it, and not add it to the block store

Implementation

Blocks get received in bitswap one of two ways. They're either added by the user (HasBlock) -- and in this case it seems like they should always get added to the blockstore. Or, they come in through the network -- through the ReceiveMessage function. It seems to me, and it's possible I'm missing something, this is the place to ignore an unwanted block. This perhaps naive implementation simply checks blocks received from the network against the global wantlist and ignores them if they aren't in the wantlist.

For Discussion

There are interesting questions about what exactly we want to do and not do with an unwanted block. This approach counts the block in statistics but doesn't provide it, add it to the block store, send it to sessions, or anything else.

The reason for counting in statistics is information was transferred and received, and duplicate blocks likely have been removed from the wantlist but were requested at some point, making them worth recording as duplicate blocks.

But it might merit further examination to figure out if this is the exact right set of things to "not do" if we get a block that's not in the want list.

fix #21 fix #22

[schomatis]

Is the broadcast wantlist (bcwl) always contained by the regular one? Should the session's tofetch queue be considered here? (although it seems outside of the scope of the Bitswap struct)

[hannahhoward]

@schomatis
Question 1: I believe so -- based on my read of wantmanager.go, every block that gets requested through WantBlocks ends up on the wl wantlist. bcwl is a subset, and blocks get added if no targets are passed to WantBlocks. See my comment here: #22 (comment)

Actually I'm curious if my read looks right to you.

Question 2: Ooh... good question. So toFetch is a list of keys that have been requested in code through the sessions GetBlocks/GetBlock methods but the request for those blocks has not yet gone out to the network. So should they be stored in the block store if they are received from the network? The situation I could see this being relevant is if at some point we decide to start optimistically sending blocks to peers based on the structure of the Merkledag. We don't do that at the moment, but maybe it's worth exploring? Either way, I wonder if this is a concern we need to solve for now, or we can wait till that becomes an optimization need.

[hannahhoward]

@schomatis are you ok with addressing toFetch later?

Seeking your LGTM + 1 more

[schomatis]

LGTM

@schomatis are you ok with addressing toFetch later?

Yes, this is definitely a step in the right direction, we can address the other issues later.

[michaelavila]

LGTM