fix(hermes): doc sync p2p propagation #731

bkioshn · 2025-12-22T06:35:11Z

Description

Quick context:
Doc-sync messages weren't propagating between nodes. Turned out to be three issues caused by the new on doc commit:

Subscription conflicts - IPFS was auto-subscribing to topics that modules needed to control
Wrong CID format in messages - was sending dag-pb CIDs when protocol expects dag-cbor
Blocking operations - file operations were blocking the PubSub handler

What got fixed:
• Made the PubSub handler spawn async tasks so it doesn't block
• Fixed message format - now computes both CID types (dag-pb for storage, dag-cbor for protocol)
• Removed the auto-subscription logic that was causing conflicts
• Fixed the test detection (was using grep -q which was flaky, switched to wc -l)

Testing:
Run just test-pubsub-propagation from the p2p-testing dir. Should see all 5 nodes receive messages.

Related Issue(s)

The issues arise when trying to run test created from
#704
so I'm pointing this PR to this issue

Please confirm the following checks

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream module

WHAT THIS FIXES: - CID format: Use dag-cbor codec (0x51) instead of dag-pb (0x70) for protocol compliance - Message encoding: Publish CBOR-encoded payload::New with CID lists instead of raw document bytes - Subscription conflict: Remove auto-subscription that prevented doc-sync module from subscribing CHANGES: - hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs: * Import minicbor crate directly (not via cardano_chain_follower) * Compute both dag-pb CID (storage) and dag-cbor CID (protocol) in add_file() * Construct proper payload::New structure in publish() * Encode to CBOR before publishing to PubSub - hermes/bin/src/ipfs/mod.rs: * Remove auto-subscription to documents.new during bootstrap * Prevents subscription kind conflicts with doc-sync module - hermes/bin/src/ipfs/task.rs: * Add extensive documentation of blocking operations issue * Document root cause and recommended async solution REMAINING ISSUE: P2P propagation still fails due to blocking operations in async context. The doc_sync_topic_message_handler (added in 944360f) calls file_pin() and file_get() which use blocking_send()/blocking_recv(). This causes deadlock when called from async PubSub handler. NEXT STEPS: Convert file_pin() and file_get() to async versions. See detailed implementation guide in task.rs comments (lines 354-374). TEST: cd p2p-testing && just test-pubsub-propagation Currently FAILS with No Propagation due to blocking operations issue. Will PASS once async conversion is complete. Related: #691 (introduced blocking operations bug)

…e operations The doc_sync_topic_message_handler was blocking the PubSub event loop by using blocking_send/blocking_recv for IPFS file operations, preventing new messages from being processed while fetching content. Changes: - Add file_get_async() and file_pin_async() methods in ipfs/mod.rs that use send().await instead of blocking_send() - Update doc_sync_topic_message_handler() in ipfs/task.rs to spawn async task with tokio::spawn() for file operations - Add RECEIVED PubSub message with CID logging for test detection - Fix test-pubsub-propagation in p2p-testing/justfile to properly detect message reception using wc -l instead of grep -q Result: - PubSub handler no longer blocks - can process new messages while previous content is still being fetched - test-pubsub-propagation now passes with 100% success rate (5/5 nodes) - Resolves doc-sync P2P message propagation issue

bkioshn · 2025-12-22T08:35:47Z

hermes/bin/src/ipfs/task.rs

-            OnNewDocEvent::new(channel_name, &content),
-            crate::event::TargetApp::List(app_names.clone()),
-            crate::event::TargetModule::All,
+            // IMPORTANT: The message contains dag-cbor CIDs (CIDv1, codec 0x51), but IPFS storage


We are using cbor 0x51, not dag-cbor
https://github.com/multiformats/multicodec/blob/master/table.csv

We should ONLY every use 0x51 and no other CID. I see there is logic for both, only have 1.

bkioshn · 2025-12-22T09:11:15Z

hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs

 fn publish(
    ctx: &mut HermesRuntimeContext,
-    doc: DocData,
+    cid: &str,


I think this should be DocData so we don't need to construct the data here 🤔

We should never be using cid as a string, and always use its binary equivalent. Otherwise we are constantly encoding and decoding it everywhere. It should ONLY ever be a printable string if we intend to print it.
If we are using a CID, it should be a CID type that holds it.

@bkioshn, I think we only need the cid in this function.
DocData is already added to IPFS at this point here: https://github.com/input-output-hk/hermes/blob/fix/doc-sync-p2p-propagation/hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs#L185

bkioshn · 2025-12-22T09:24:33Z

hermes/bin/src/ipfs/task.rs

 }

-/// Handler function for topic message streams (Doc Sync).
+/// Handler for Doc Sync PubSub messages on "*.new" topics.


Will add TODO here, since this function should be able to handle multiple topics (as mention // TODO: match the topic against a static list.)

stevenj · 2025-12-22T12:40:15Z

hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs

 fn publish(
    ctx: &mut HermesRuntimeContext,
-    doc: DocData,
+    cid: &str,


We should never be using cid as a string, and always use its binary equivalent. Otherwise we are constantly encoding and decoding it everywhere. It should ONLY ever be a printable string if we intend to print it.
If we are using a CID, it should be a CID type that holds it.

stevenj · 2025-12-22T12:41:11Z

hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs

+    // Messages contain CID lists, not document content - receivers fetch from IPFS.
+
+    // Step 1: Parse CID string
+    let cid_parsed = match cid.parse::<hermes_ipfs::Cid>() {


No, because it should never be a string, it should always be its bytes and we should only convert bytes to strong for display and never "to use".

stevenj · 2025-12-22T12:42:11Z

hermes/bin/src/runtime_extensions/hermes/doc_sync/host.rs

+
+            let multihash = multihash::Multihash::<64>::wrap(SHA2_256_CODE, &hash_digest)?;
+            let cbor_cid = hermes_ipfs::Cid::new_v1(CBOR_CODEC, multihash);
+            let cbor_cid_str = cbor_cid.to_string();


Don't return a string, return its bytes.

no30bit

LGTM! Great catches

Signed-off-by: bkioshn <[email protected]>

cong-or added 6 commits December 21, 2025 18:01

bkioshn assigned bkioshn and cong-or Dec 22, 2025

bkioshn added the squad: hermetics Hermes Backend, System Development & Integration Team label Dec 22, 2025

bkioshn added this to Catalyst Dec 22, 2025

github-project-automation bot moved this to New in Catalyst Dec 22, 2025

bkioshn commented Dec 22, 2025

View reviewed changes

bkioshn added the comment me Request for comments label Dec 22, 2025

bkioshn moved this from New to 🏗 In progress in Catalyst Dec 22, 2025

Merge branch 'main' into fix/doc-sync-p2p-propagation

538397e

bkioshn commented Dec 22, 2025

View reviewed changes

stevenj requested changes Dec 22, 2025

View reviewed changes

github-project-automation bot moved this from 🏗 In progress to 🔖 Ready in Catalyst Dec 22, 2025

no30bit reviewed Dec 22, 2025

View reviewed changes

bkioshn added 8 commits December 23, 2025 19:50

use type CID instead of string

7043008

Signed-off-by: bkioshn <[email protected]>

small cleanup

c23f932

Signed-off-by: bkioshn <[email protected]>

reverts commit 7043008

173d55a

Signed-off-by: bkioshn <[email protected]>

change cid string to CID

aaa66d3

Signed-off-by: bkioshn <[email protected]>

change ipfs-cid type string to bstr

bd3d5eb

Signed-off-by: bkioshn <[email protected]>

revert change

5402187

Signed-off-by: bkioshn <[email protected]>

Merge branch 'main' into fix/doc-sync-p2p-propagation

d10f78e

Signed-off-by: bkioshn <[email protected]>

fix linter

0a06de3

Signed-off-by: bkioshn <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(hermes): doc sync p2p propagation #731

fix(hermes): doc sync p2p propagation #731

Uh oh!

bkioshn commented Dec 22, 2025 •

edited

Loading

Uh oh!

bkioshn Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

bkioshn Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

no30bit Dec 22, 2025

Uh oh!

bkioshn Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

stevenj Dec 22, 2025

Uh oh!

no30bit left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(hermes): doc sync p2p propagation #731

Are you sure you want to change the base?

fix(hermes): doc sync p2p propagation #731

Uh oh!

Conversation

bkioshn commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Please confirm the following checks

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!

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!

no30bit left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bkioshn commented Dec 22, 2025 •

edited

Loading