Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Include Bitswap into request response protocols #12262

Merged
merged 2 commits into from
Sep 15, 2022

Conversation

altonen
Copy link
Contributor

@altonen altonen commented Sep 14, 2022

Include bitswap protocol into the list of enabled request response protocols. Previously the code stored the bitswap into another vector stored in sc_network::config::Params but didn't extend the protocol vector that contained the protocols that were given to Behaviour, essentially leaving Bitswap disabled even if --ipfs-server was provided on the command line.

@altonen altonen added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Sep 14, 2022
@altonen
Copy link
Contributor Author

altonen commented Sep 15, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 3bef1ba into master Sep 15, 2022
@paritytech-processbot paritytech-processbot bot deleted the include-bitswap-into-protocols branch September 15, 2022 07:27
@alebaffa
Copy link

alebaffa commented Dec 4, 2022

Hi, sorry to comment on this after 3 months, but I also found this issue (still open) paritytech/polkadot-sdk#542 where it is said that

Ok, then request-response can't be used for implementing bitswap because it requires opening a new stream for each response.

So, this PR means that it is no more the case?
Thanks.

@sinkingsugar
Copy link

This didn't fix much.
substrate side we receive requests from ipfs clients, but ipfs clients never get substrate responses currently.

@altonen
Copy link
Contributor Author

altonen commented Dec 6, 2022

@alebaffa @sinkingsugar

Looking at this implementation of Bitswap protocol, it would lead me to believe we can implement the protocol over the request response protocol meaning there might be some discrepancy between the "right way" and our way of implementing the codec. I'll look into how we might go about fixing this.

@altonen
Copy link
Contributor Author

altonen commented Dec 6, 2022

Unfortunately it seems that the Bitswap protocol cannot be built on top of the request response protocol if we want to be compatible with go-ipfs and js-ipfs. There is a way for us to add support for the protocol while keeping it still as a generic protocol living above sc-network, issue paritytech/polkadot-sdk#524 tracks the progress of this

ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants