Updating Bitswap architecture diagram

[dgrisham]

Updating the architecture diagram as a part of the Bitswap spec update. Rendered to .png using ditaa.

[daviddias]

How can users make changes? will ditaa pick up on the .txt version?

[dgrisham]

@diasdavid Yes -- I just ran ditaa architecture.txt to generate the .png.

[dgrisham]

@diasdavid Hey! Updated the diagram based on your comment on Slack (about the Bitswap message version transformations).

[daviddias]

[dgrisham]

Gotcha, can you define 'network' for me then? My understanding was roughly: https://github.com/dgrisham/specs/blob/a8fb19f9e15e39d6a716cb65f96b47afe209a3e6/bitswap/README.md#network, which doesn't seem accurate now based on your comment.

[daviddias]

I see where the confusion came from. That Network box is this internal class "https://github.com/ipfs/js-ipfs-bitswap/blob/master/src/network.js". All the things that were in the diagram were internal pieces to Bitswap :)

[dgrisham]

Ah, gotcha, that also explains my confusion with the original diagram as well. I'll update the diagram to reflect this, thanks!

[daviddias]

thanks @dgrisham . Did you had time to look into this again?

[dgrisham]

@diasdavid Hey, sorry about that, just pushed with that fix. Let me know what you think.

[daviddias]

Not trying to be super pedantic, but I still see differences:

[dgrisham]

@diasdavid No worries, we want to be as accurate as we can here :) I suppose the main reason those parts of the diagram differ is that mine has the 'Peer using older Bitswap?' choice. My thought was that it made it more clear when looking at the incoming/outgoing messages, because on the original it seemed that the lines coming out of the Network module were ambiguous (i.e. it doesn't appear obvious from the diagram when you go to the Transform function and when you don't). When adding in that 'Peer using older Bitswap?' question, it made the most sense to conceptualize the messages as Outgoing and Incoming for conciseness, as opposed to conceptualizing them as /ipfs/bitswap/1.0.0 and /ipfs/bitswap/1.1.0.

The other reason I did this was that it seemed to be a bit more future-proof, because if we Bitswap gets updated to /ipfs/bitswap/1.2.0 and so on, the original diagram doesn't scale particularly well. (My diagram does refer to a specific version number though, that being /ipfs/bitswap/1.1.0, so it would still require a small change on updates.)

Does that make sense/what are your thoughts on this approach? Happy to change it back if you're unconvinced, but thought I'd share my reasoning.

Edit: Also, I just pushed an update because my previous push was missing a few things.

[dgrisham]

Oo, just realized one significant difference between our diagrams though. I'll push a change in a minute.

Edit: Actually, scratch that. The line going from the Network module to the Decision Engine said receive a block on the original, but the decision engine gets the entire Bitswap message, which is why I represented it the way I did in the diagram.

[vmx]

What's the current state? As I start looking into Bitswap it would be great to look at an updated diagram :)

[daviddias]

@dgrisham given the lack of response, I'm closing this PR and keeping the mododraw version of the diagram which I believe does a better representation of the architecture.

[dgrisham]

Hey @diasdavid that was my bad, I was thinking that your approval/review was the next step in this conversation, which is why I didn't respond to @vmx -- I should've pinged you since I thought that was the case. Let me know if you still want to move forward with this, the most recent version is here. Also fine to keep the older version if you still think that's better, but wanted to make sure this didn't get overlooked.