Allow all the Bitswap CIDs (v1) to pass regardless of used hash #482

bkontur · 2025-11-24T15:13:09Z

Relates to the: paritytech/polkadot-sdk#10301

Fully tested with paritytech/polkadot-bulletin-chain#100

Rationale

The only weak point is that BitswapRequestHandler expects a Blake2b hash (due to BlockT) here: https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/client/network/src/bitswap/mod.rs#L208-L209. However, this is not a real problem, because we initially create a default 32-byte hash and then replace it with the 32-byte hash from the CID. Since we only claim to support 32-byte hashes, it just works. 😊

Solution

Proposed by @dmitry-markin - remove CID filtering from litep2p and do filtering in PolkadotSDK - see more.

Follow-up

As a follow-up, I would refactor fn indexed_transaction / fn has_indexed_transaction here: https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/client/api/src/client.rs#L154-L163, and make the following change:

+        type ContentHash = [u8; 32];

	/// Get single indexed transaction by content hash.
	///
	/// Note that this will only fetch transactions
	/// that are indexed by the runtime with `storage_index_transaction`.
-	fn indexed_transaction(&self, hash: Block::Hash) -> sp_blockchain::Result<Option<Vec<u8>>>;
+	fn indexed_transaction(&self, hash: ContentHash) -> sp_blockchain::Result<Option<Vec<u8>>>;

	/// Check if transaction index exists.
-	fn has_indexed_transaction(&self, hash: Block::Hash) -> sp_blockchain::Result<bool> {
-	fn has_indexed_transaction(&self, hash: ContentHash) -> sp_blockchain::Result<bool> {

Ultimately, we index transactions by their content hash, which is a [u8; 32] here:
https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/primitives/io/src/lib.rs#L1494,
and it is generated as a [u8; 32] here:
https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/frame/transaction-storage/src/lib.rs#L239-L242.

bkontur · 2025-11-24T15:16:41Z

basically I came to this, when I was debugging and so the empty CIDs here:

2025-11-24 11:21:22.132 DEBUG tokio-runtime-worker sub-libp2p::bitswap: [Parachain] handle bitswap request from PeerId("12D3KooWBwoqGyM5gmqQPs9xV2Lq8pZCatPEWTtPn3z7pJg44F8r") for []
2025-11-24 11:21:22.156 DEBUG tokio-runtime-worker sub-libp2p::bitswap: [Parachain] handle bitswap request from PeerId("12D3KooWBwoqGyM5gmqQPs9xV2Lq8pZCatPEWTtPn3z7pJg44F8r") for []
2025-11-24 11:21:32.136 DEBUG tokio-runtime-worker sub-libp2p::bitswap: [Parachain] handle bitswap request from PeerId("12D3KooWBwoqGyM5gmqQPs9xV2Lq8pZCatPEWTtPn3z7pJg44F8r") for []
2025-11-24 11:21:32.165 DEBUG tokio-runtime-worker sub-libp2p::bitswap: [Parachain] handle bitswap request from PeerId("12D3KooWBwoqGyM5gmqQPs9xV2Lq8pZCatPEWTtPn3z7pJg44F8r") for []
2025-11-24 11:21:42.138 DEBUG tokio-runtime-worker sub-libp2p::bitswap: [Parachain] handle bitswap request from PeerId("12D3KooWBwoqGyM5gmqQPs9xV2Lq8pZCatPEWTtPn3z7pJg44F8r") for []

And the reason was, that they were just filtered out, because of sha2 :)

lexnv · 2025-11-24T15:20:21Z

src/protocol/libp2p/bitswap/mod.rs

                    1 => WantType::Have,
-                    _ => return None,
+                    _ => {
+                        tracing::warn!(


nit: Since these warnings can be quite easily user-generated, and not necesarily something we support for our bitswap logic, I would downgrade them to debug to avoid poluting the logs

src/protocol/libp2p/bitswap/mod.rs

lexnv · 2025-11-24T15:28:13Z

src/protocol/libp2p/bitswap/mod.rs

+                if code != u64::from(Code::Blake2b256)
+                    && code != u64::from(Code::Sha2_256)
+                    && code != u64::from(Code::Keccak256)


Would it be better to extend this functionality and make it configurable via bitswap::Config?

Then, we can add a group of supported hash codes:

// In config: Config { ... supported_hash_codes: HashSet<u64>, } impl Default for Config { supported_hash_codes: [u64::from(Code::Blake2b256)].iter().collect(), } /// Builder to set supported hash codes for bitswap requests. /// /// Defaults to supporting only `Code::Blake2b256`. fn with_supported_hash_codes(&self, codes: impl Iter<Item = Code>) { self.supported_hash_codes.extend(codes.map(u64::from)); } // In bitswap: if self.supported_hash_codes.contains(..)

Would it be better to extend this functionality and make it configurable via bitswap::Config?

@lexnv Yes, yes, that would be much better. And where do I set those hard-coded values then?
Here?
https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/client/network/src/litep2p/shim/bitswap.rs#L47

let (config, handle) = Config::new() .with_supported_hash_codes(blake) .with_supported_hash_codes(sha2) .with_supported_hash_codes(keccak) .build();

Exactly 👍

Thanks for the suggestions, folks!
I implemented what you suggested ~~but with one minor details:~~

~~default_hash_codes are blake, sha2 and Keccak256 -- to maintain backward compatibility with the present code~~

@lexnv , PTAL at the new code

The code is not compiling, let me know when that's fixed 🙏

Cargo.toml

…rted multihashes

src/protocol/libp2p/bitswap/config.rs

bkontur · 2025-11-25T08:22:44Z

src/protocol/libp2p/bitswap/config.rs

+    /// ```
+    pub fn with_supported_hash_codes(
+        mut self,
+        codes: impl IntoIterator<Item = multihash::Code>,


@lexnv hmm, this way, we would need to add multihash to the sc-network, what about just re-exporting here in this file:

pub use multihash::Code; or pub use multihash::Code as MultihashCode;

pub use multihash::Code as MultihashCode; sounds good to me

src/protocol/libp2p/bitswap/mod.rs

lexnv · 2025-11-25T09:11:09Z

src/protocol/libp2p/bitswap/config.rs

    pub(super) cmd_rx: Receiver<BitswapCommand>,
+
+    /// Supported multihash codes for CID validation.
+    pub(super) supported_hash_codes: std::collections::HashSet<u64>,


Let's import this 🙏

lexnv · 2025-11-25T09:14:10Z

src/protocol/libp2p/bitswap/config.rs

+    /// ```
+    pub fn with_supported_hash_codes(


nit: Let's mention we are operating with Blake2b256 by default

lexnv · 2025-11-25T09:15:08Z

src/protocol/libp2p/bitswap/config.rs

+    /// Set supported multihash codes for bitswap CID validation.
+    /// # Example
+    ///
+    /// ```rust


This will probably compile the example, maybe we want to fix the doc example with some imports behind # so they don't get rendered in docsrs 🙏

lexnv · 2025-11-25T09:16:38Z

src/protocol/libp2p/bitswap/mod.rs

            event_tx: config.event_tx,
+            supported_hash_codes: config.supported_hash_codes,


Nit: let's add a debug in the fn new to know the supported hashes, just in case we start hitting Unsupported multihash code: {code} for cid: {cid}; or add there the full supported list (since it shouldn't be that big)

lexnv · 2025-11-25T09:17:48Z

src/protocol/libp2p/bitswap/mod.rs

+                if !self.supported_hash_codes.contains(&code) {
+                    tracing::debug!(
+                        target: LOG_TARGET,
+                        "Unsupported multihash code: {code} for cid: {cid}"


Suggested change

"Unsupported multihash code: {code} for cid: {cid}"

code,

cid,

expected = ?self.supported_hash_codes,

"Unsupported multihash"

lexnv · 2025-11-25T09:18:12Z

.gitignore

@@ -1 +1,2 @@
 /target
+.idea


nit: This is unrelated to the PR

Cargo.toml

dmitry-markin

IMO we can simplify the PR substantially.

I don't think we should complicate the litep2p Bitswap configuration with a multihash code filter. It would be more straightforward to pass CIDs as is to the client code, that knows anyway what codes/codecs it supports. The client code can then decide whether to respond with a DontHave message straight away, or silently drop such request.

bkontur · 2025-11-25T10:00:06Z

IMO we can simplify the PR substantially.

I don't think we should complicate the litep2p Bitswap configuration with a multihash code filter. It would be more straightforward to pass CIDs as is to the client code, that knows anyway what codes/codecs it supports. The client code can then decide whether to respond with a DontHave message straight away, or silently drop such request.

I like this also, so basically, you are saying, we just remove the CID filter from litep2p and we do this filtering in the PolkadotSDK part, right?

Is anybody else using litep2p bitswap handler? Just asking, that we allow all CIDs now :)

bkontur · 2025-11-25T10:01:13Z

IMO we can simplify the PR substantially.
I don't think we should complicate the litep2p Bitswap configuration with a multihash code filter. It would be more straightforward to pass CIDs as is to the client code, that knows anyway what codes/codecs it supports. The client code can then decide whether to respond with a DontHave message straight away, or silently drop such request.

I like this also, so basically, you are saying, we just remove the CID filter from litep2p and we do this filtering in the PolkadotSDK part, right?

Is anybody else using litep2p bitswap handler? Just asking, that we allow all CIDs now :)

@lexnv wdyt?

dmitry-markin · 2025-11-25T10:06:49Z

Is anybody else using litep2p bitswap handler?

That's unlikely, especially because it is not generic (only supports old transaction storage BLAKE2b). In any case, the client code should have the match/check and not rely on the library returning only one magic number. We will also mention this change in the release notes.

bkontur · 2025-11-25T10:14:09Z

@x3c41a ok, so:

let's revert all these changes here
the only change here would be that we remove original CID filtering, we allow to pass all the CIDs
when merged, release litep2p

then open a new PolkadotSDK PR:

add supported_hash_codes to the BitswapServer and in the new() just allow all three
basically, move this CID filtering somewhere here : https://github.com/paritytech/polkadot-sdk/blob/e588acf5e12b14c68331d2e08afe7c12d6671df9/substrate/client/network/src/litep2p/shim/bitswap.rs#L63-L64

.into_iter()
.filter(|(cid, want_type)| filter_cid(..)) // <- better name
.map(|(cid, want_type)| {

bump PolkadotSDK with released litep2p

bkontur · 2025-11-25T11:05:12Z

@x3c41a please change this import: https://github.com/paritytech/litep2p/pull/482/files#diff-944be4d6fcd2bfd09565edd9fffef4d804041dbcb562b25703878eb004cd418aR32 to pub use multihash::Code;

src/protocol/libp2p/bitswap/mod.rs

bkontur · 2025-11-25T11:38:57Z

@lexnv @dmitry-markin please, check now, can we merge and release? Or what is the release process here?

x3c41a · 2025-11-25T11:54:19Z

@lexnv @dmitry-markin please, check now, can we merge and release? Or what is the release process here?

+1, I am also curious how the release process look like.
I plan to release 0.12.2. I see that you manually add Changelog and add chore: prefix to each release PR.
Is chore a tool or something?

lexnv · 2025-11-25T13:03:31Z

The release process is highlighted in:

https://github.com/paritytech/litep2p/blob/master/RELEASING.md

There's nothing else you guys need to do, we'll handle the release once this PR is merged and bundle with a few other fixes we have in flight. Will let you know when this happens 🙏

Is chore a tool or something?

We generate the changelogs manually, and prefix the PRs with chore or release or similar to distinguish between legitimate features/fixes and release processes

src/protocol/libp2p/bitswap/mod.rs

lexnv

LGTM!

The PR title and description would need to change to reflect that we are passing through the CIDs without validation and that users should handle it on their own. (ie, we are no longer adding sha2256 / keccack256 hashing support)

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

bkontur · 2025-11-25T13:13:13Z

const blockSizes = chunks.map(c => BigInt(c.length))

LGTM!

The PR title and description would need to change to reflect that we are passing through the CIDs without validation and that users should handle it on their own. (ie, we are no longer adding sha2256 / keccack256 hashing support)

let me clean it

src/protocol/libp2p/bitswap/mod.rs

dmitry-markin · 2025-11-26T08:38:18Z

Hey @x3c41a, can you setup commit signing with GitHub verified key as per Parity Yubikey guide? We can't merge the PR now because the commits are not signed.

Merging is blocked
Commits must have verified signatures.

dmitry-markin · 2025-11-26T08:40:12Z

@x3c41a Note that multihash::Code is re-exported in litep2p as litep2p::types::multihash::Code.

## [0.12.2] - 2025-11-28 This release allows all Bitswap CIDs (v1) to pass regardless of the used hash. It also enhances local address checks in the transport manager. ### Changed - transport-service: Enhance logging with protocol names ([#485](#485)) - bitswap: Reexports for CID ([#486](#486)) - Allow all the Bitswap CIDs (v1) to pass regardless of used hash ([#482](#482)) - transport/manager: Enhance local address checks ([#480](#480)) cc paritytech/polkadot-bulletin-chain#119 --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

Relates to paritytech/litep2p#482 TODO: - [x] bump new litep2p version once it's released --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Branislav Kontur <bkontur@gmail.com>

Allow Bitswap CIDs with Blake2b256, Sha2_256 and Keccak256 hashing

3a5d74e

bkontur mentioned this pull request Nov 24, 2025

Full IPFS CID codec/hash support - passing CidConfig over TransactionExtension paritytech/polkadot-bulletin-chain#100

Open

1 task

bkontur requested review from dmitry-markin and lexnv November 24, 2025 15:15

lexnv reviewed Nov 24, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

lexnv reviewed Nov 24, 2025

View reviewed changes

x3c41a reviewed Nov 25, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

x3c41a added 4 commits November 25, 2025 04:53

supported CID version is verified with if let, also downgraded logs

373c125

added supported_hash_codes to bitswap config, improved checking suppo…

f693e82

…rted multihashes

fmt

5d4f429

extended hashset:

9a2309f

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/config.rs Outdated Show resolved Hide resolved

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/config.rs Outdated Show resolved Hide resolved

bkontur commented Nov 25, 2025

View reviewed changes

bkontur added 2 commits November 25, 2025 09:22

Update src/protocol/libp2p/bitswap/config.rs

892c32c

Update src/protocol/libp2p/bitswap/config.rs

768c784

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

6988e77

lexnv reviewed Nov 25, 2025

View reviewed changes

dmitry-markin reviewed Nov 25, 2025

View reviewed changes

reverted all files

87e1e14

removed CID filtering - will be implemented in SDK

da93ece

x3c41a added 3 commits November 25, 2025 12:09

removed import

539bea9

reverted import as MultihashCode

339d1f9

order

c3b8dec

bkontur commented Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Show resolved Hide resolved

Update src/protocol/libp2p/bitswap/mod.rs

6cf8406

bkontur requested review from dmitry-markin and lexnv November 25, 2025 11:38

x3c41a mentioned this pull request Nov 25, 2025

Added CID filtering to Bitswap server paritytech/polkadot-sdk#10416

Merged

1 task

dmitry-markin approved these changes Nov 25, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

lexnv approved these changes Nov 25, 2025

View reviewed changes

Update src/protocol/libp2p/bitswap/mod.rs

22e2fd8

Co-authored-by: Dmitry Markin <dmitry@markin.tech>

bkontur changed the title ~~Allow Bitswap CIDs with Blake2b256, Sha2_256 and Keccak256 hashing~~ Allow all the Bitswap CIDs (v1) to pass regardless of used hash Nov 25, 2025

Merge branch 'master' into bko-bitswap-fix

df67fe6

dmitry-markin reviewed Nov 26, 2025

View reviewed changes

src/protocol/libp2p/bitswap/mod.rs Outdated Show resolved Hide resolved

minor: remove public export of MultihashCode

f3be94a

bkontur mentioned this pull request Nov 26, 2025

[Release] v1.1.0 paritytech/polkadot-bulletin-chain#119

Open

7 tasks

dmitry-markin merged commit 3390588 into master Nov 26, 2025
8 checks passed

dmitry-markin deleted the bko-bitswap-fix branch November 26, 2025 08:48

lexnv mentioned this pull request Nov 26, 2025

chore: Release litep2p v0.12.2 #484

Merged

		event_tx: config.event_tx,
		supported_hash_codes: config.supported_hash_codes,

-                        "Unsupported multihash code: {code} for cid: {cid}"
+                        code,
+                        cid,
+                        expected = ?self.supported_hash_codes,
+                        "Unsupported multihash"

Allow all the Bitswap CIDs (v1) to pass regardless of used hash #482

Allow all the Bitswap CIDs (v1) to pass regardless of used hash #482

Uh oh!

Conversation

bkontur commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Solution

Follow-up

Uh oh!

bkontur commented Nov 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

x3c41a Nov 25, 2025 • edited by bkontur Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmitry-markin left a comment

Choose a reason for hiding this comment

Uh oh!

bkontur commented Nov 25, 2025

Uh oh!

bkontur commented Nov 25, 2025

Uh oh!

dmitry-markin commented Nov 25, 2025

Uh oh!

bkontur commented Nov 25, 2025 • edited by x3c41a Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bkontur commented Nov 25, 2025

Uh oh!

Uh oh!

bkontur commented Nov 25, 2025

Uh oh!

x3c41a commented Nov 25, 2025

Uh oh!

lexnv commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

lexnv left a comment

Choose a reason for hiding this comment

Uh oh!

bkontur commented Nov 25, 2025

Uh oh!

bkontur commented Nov 24, 2025 •

edited

Loading

x3c41a Nov 25, 2025 •

edited by bkontur

Loading

bkontur commented Nov 25, 2025 •

edited by x3c41a

Loading

lexnv commented Nov 25, 2025 •

edited

Loading

dmitry-markin commented Nov 26, 2025 •

edited

Loading