bidirectional: target chain from event.caip2_id instead of dest by ppca · Pull Request #696 · sig-net/mpc

ppca · 2026-03-11T10:12:27Z

No description provided.

Copilot

Pull request overview

Adds Bitcoin as a recognized chain across the chain-signatures stack and introduces CAIP-2/deprecated chain-id parsing helpers to consistently derive target chains for bidirectional events.

Changes:

Extend mpc_primitives::Chain with Bitcoin plus associated timing/checkpoint config.
Add chain-id metadata + parsing helpers (from_caip2_chain_id, from_deprecated_chain_id) in mpc-crypto KDF and expose equivalent constructors in primitives.
Switch bidirectional event target-chain derivation from dest parsing to CAIP-2 parsing; add “bitcoin unsupported” handling in node RPC.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
chain-signatures/primitives/src/lib.rs	Adds `Chain::Bitcoin`, timing/checkpoint defaults, and chain-id parsing constructors bridging to `mpc_crypto::kdf::Chain`.
chain-signatures/node/src/stream/ops.rs	Introduces `SignBidirectionalEvent::target_chain()` (CAIP-2 based) and uses it during bidirectional respond processing.
chain-signatures/node/src/rpc.rs	Adds Bitcoin match arms with “unsupported” behavior in publish flow and client selection.
chain-signatures/node/src/protocol/signature.rs	Updates logging to reference `event.target_chain()` for bidirectional requests.
chain-signatures/crypto/src/kdf.rs	Refactors chain-id metadata, adds CAIP-2/deprecated chain-id parsing helpers, and derives `PartialEq/Eq` for `Chain`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

chain-signatures/node/src/stream/ops.rs

chain-signatures/node/src/protocol/signature.rs

chain-signatures/node/src/rpc.rs

chain-signatures/crypto/src/kdf.rs

…rocessed

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

chain-signatures/node/src/protocol/signature.rs

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T11:24:05Z

chain-signatures/crypto/src/kdf.rs

+            .find(|chain| chain.deprecated_chain_id() == chain_id)
    }
 }



New parsing helpers from_caip2_chain_id / from_deprecated_chain_id were added but aren’t covered by the existing unit tests in this module. Adding tests for expected mappings (and a negative case) would help prevent accidental regressions in chain-id strings and ensure the ALL list stays in sync with the enum variants.

Suggested change

#[cfg(test)]

mod tests {

use super::Chain;

#[test]

fn caip2_chain_id_mappings_are_correct() {

// Positive cases: ensure that each known CAIP-2 chain id maps back to the expected Chain.

assert_eq!(Chain::from_caip2_chain_id("near:mainnet"), Some(Chain::Near));

assert_eq!(Chain::from_caip2_chain_id("eip155:1"), Some(Chain::Ethereum));

assert_eq!(

Chain::from_caip2_chain_id("solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp"),

Some(Chain::Solana)

);

assert_eq!(

Chain::from_caip2_chain_id("bip122:000000000019d6689c085ae165831e93"),

Some(Chain::Bitcoin)

);

assert_eq!(

Chain::from_caip2_chain_id("polkadot:2034"),

Some(Chain::Hydration)

);

// Negative case: unknown CAIP-2 chain id must not map to any Chain.

assert_eq!(Chain::from_caip2_chain_id("unknown:chain"), None);

}

#[test]

fn deprecated_chain_id_mappings_are_correct() {

// Positive cases: ensure that each known deprecated chain id maps back to the expected Chain.

assert_eq!(Chain::from_deprecated_chain_id("0x18d"), Some(Chain::Near));

assert_eq!(Chain::from_deprecated_chain_id("0x1"), Some(Chain::Ethereum));

assert_eq!(

Chain::from_deprecated_chain_id("0x800001f5"),

Some(Chain::Solana)

);

assert_eq!(

Chain::from_deprecated_chain_id("bip122:000000000019d6689c085ae165831e93"),

Some(Chain::Bitcoin)

);

assert_eq!(

Chain::from_deprecated_chain_id("polkadot:2034"),

Some(Chain::Hydration)

);

// Negative case: unknown deprecated chain id must not map to any Chain.

assert_eq!(Chain::from_deprecated_chain_id("0xdeadbeef"), None);

}

}

Copilot · 2026-03-11T11:24:05Z

chain-signatures/node/src/stream/ops.rs

+    pub fn target_chain(&self) -> anyhow::Result<Chain> {
+        let event_caip2_id = self.caip2_id();
+        Chain::from_caip2_chain_id(&event_caip2_id).ok_or_else(|| {
+            anyhow::anyhow!(
+                "unable to parse target chain from event caip2_id field: {event_caip2_id}"
+            )
+        })
+    }


SignBidirectionalEvent::target_chain currently clones the caip2_id String via self.caip2_id() just to parse it. This adds an avoidable allocation on a hot path (request processing). Consider returning a borrowed &str/Cow<'_, str> from caip2_id() (or matching on self in target_chain and borrowing the inner field) so parsing can operate on a reference.

volovyks

Let's not forget about docs. I was expecting that this PR would be mostly about documenting struct fields and flows.

volovyks · 2026-03-16T07:45:02Z

chain-signatures/node/src/stream/ops.rs

        }
    }
+
+    pub fn target_chain(&self) -> anyhow::Result<Chain> {


Returning Result<> here should not be required if we will parse string -> Cahin enum when constructing this Event.
It will reduce complexity (like event.target_chain()?;).

chain-signatures/primitives/src/lib.rs

chain-signatures/crypto/src/kdf.rs

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

chain-signatures/node/src/stream/ops.rs

+    pub fn target_chain(&self) -> Chain {
+        // we can directly unwrap because we've checked that the chain id is valid during event deserialization in the indexer
+        Chain::from_caip2_chain_id(&self.caip2_id()).unwrap()


chain-signatures/primitives/src/lib.rs

+    pub fn from_caip2_chain_id(chain_id: &str) -> Result<Self, ChainFromError> {
+        Self::iter()
+            .into_iter()
+            .find(|chain| chain.caip2_chain_id() == chain_id)
+            .ok_or_else(|| ChainFromError::UnknownCaip2Id(chain_id.to_string()))
+    }
+
+    pub fn from_deprecated_chain_id(chain_id: &str) -> Result<Self, ChainFromError> {
+        Self::iter()
+            .into_iter()
+            .find(|chain| chain.deprecated_chain_id() == chain_id)
+            .ok_or_else(|| ChainFromError::UnknownDeprecatedId(chain_id.to_string()))
+    }


chain-signatures/contract-sol/src/lib.rs

+    /// The program ID of the Solana program that emitted this event.
+    ///
+    /// Used by off-chain services (e.g., relayers, indexers) to filter and
+    /// verify events from the correct program.
+    ///
+    /// MUST be provided and MUST match the deployed program ID.


volovyks

Maybe we could rely a bit less on Copilot and address the suggestions from it before requesting a review 🙂

chain-signatures/contract-sol/src/lib.rs

chain-signatures/node/src/stream/ops.rs

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

chain-signatures/node/src/stream/ops.rs

+    pub fn target_chain(&self) -> Result<Chain, mpc_primitives::ChainFromError> {
+        // we can directly unwrap because we've checked that the chain id is valid during event deserialization in the indexer
+        Chain::from_caip2_chain_id(&self.caip2_id())
+    }


chain-signatures/primitives/src/lib.rs

+    pub fn from_caip2_chain_id(chain_id: &str) -> Result<Self, ChainFromError> {
+        Self::iter()
+            .into_iter()
+            .find(|chain| chain.caip2_chain_id() == chain_id)
+            .ok_or_else(|| ChainFromError::UnknownCaip2Id(chain_id.to_string()))
+    }


chain-signatures/contract-sol/src/lib.rs

+    /// The program ID of the Solana program that emitted this event.
+    ///
+    /// Used by MPC service to filter and verify events from the correct program.
+    ///
+    /// MUST match the deployed program ID.


target chain from event.caip2_id instead of dest

49313fa

ppca requested a review from Copilot March 11, 2026 10:12

Copilot started reviewing on behalf of ppca March 11, 2026 10:13 View session

ppca changed the title ~~target chain from event.caip2_id instead of dest~~ bidirectional: target chain from event.caip2_id instead of dest Mar 11, 2026

Copilot AI reviewed Mar 11, 2026

View reviewed changes

make sure bidirectional requests with invalid target chain won't be p…

6145363

…rocessed

ppca requested a review from Copilot March 11, 2026 11:01

Copilot started reviewing on behalf of ppca March 11, 2026 11:02 View session

ppca force-pushed the xiangyi/use_caip2_id_not_dest branch from 920b7a3 to 7361463 Compare March 11, 2026 11:05

remove unnecessary change

706c370

ppca force-pushed the xiangyi/use_caip2_id_not_dest branch from 7361463 to 706c370 Compare March 11, 2026 11:06

Copilot AI reviewed Mar 11, 2026

View reviewed changes

chain-signatures/node/src/protocol/signature.rs Outdated Show resolved Hide resolved

ppca requested a review from Copilot March 11, 2026 11:06

Copilot started reviewing on behalf of ppca March 11, 2026 11:07 View session

no unwrap

3981e2f

Copilot AI reviewed Mar 11, 2026

View reviewed changes

update bitcoin defaults

8e57bd8

ppca requested a review from Copilot March 11, 2026 11:18

Copilot started reviewing on behalf of ppca March 11, 2026 11:18 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Merge branch 'develop' into xiangyi/use_caip2_id_not_dest

65f987d

ppca marked this pull request as ready for review March 16, 2026 04:35

ppca requested review from ChaoticTempest and volovyks March 16, 2026 04:35

volovyks reviewed Mar 16, 2026

View reviewed changes

Xiangyi Zheng added 2 commits March 17, 2026 16:32

Merge branch 'develop' into xiangyi/use_caip2_id_not_dest

14afa78

merge develop and address comments

a9a5903

ppca requested a review from Copilot March 17, 2026 09:35

Copilot started reviewing on behalf of ppca March 17, 2026 09:36 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

ppca requested a review from volovyks March 17, 2026 10:14

volovyks reviewed Mar 17, 2026

View reviewed changes

chain-signatures/contract-sol/src/lib.rs Outdated Show resolved Hide resolved

chain-signatures/contract-sol/src/lib.rs Outdated Show resolved Hide resolved

chain-signatures/node/src/stream/ops.rs Outdated Show resolved Hide resolved

address comments

b4c3467

ppca requested a review from Copilot March 18, 2026 02:29

Copilot started reviewing on behalf of ppca March 18, 2026 02:29 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

+#[cfg(test)]
+mod tests {
+    use super::Chain;
+    #[test]
+    fn caip2_chain_id_mappings_are_correct() {
+        // Positive cases: ensure that each known CAIP-2 chain id maps back to the expected Chain.
+        assert_eq!(Chain::from_caip2_chain_id("near:mainnet"), Some(Chain::Near));
+        assert_eq!(Chain::from_caip2_chain_id("eip155:1"), Some(Chain::Ethereum));
+        assert_eq!(
+            Chain::from_caip2_chain_id("solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp"),
+            Some(Chain::Solana)
+        );
+        assert_eq!(
+            Chain::from_caip2_chain_id("bip122:000000000019d6689c085ae165831e93"),
+            Some(Chain::Bitcoin)
+        );
+        assert_eq!(
+            Chain::from_caip2_chain_id("polkadot:2034"),
+            Some(Chain::Hydration)
+        );
+        // Negative case: unknown CAIP-2 chain id must not map to any Chain.
+        assert_eq!(Chain::from_caip2_chain_id("unknown:chain"), None);
+    }
+    #[test]
+    fn deprecated_chain_id_mappings_are_correct() {
+        // Positive cases: ensure that each known deprecated chain id maps back to the expected Chain.
+        assert_eq!(Chain::from_deprecated_chain_id("0x18d"), Some(Chain::Near));
+        assert_eq!(Chain::from_deprecated_chain_id("0x1"), Some(Chain::Ethereum));
+        assert_eq!(
+            Chain::from_deprecated_chain_id("0x800001f5"),
+            Some(Chain::Solana)
+        );
+        assert_eq!(
+            Chain::from_deprecated_chain_id("bip122:000000000019d6689c085ae165831e93"),
+            Some(Chain::Bitcoin)
+        );
+        assert_eq!(
+            Chain::from_deprecated_chain_id("polkadot:2034"),
+            Some(Chain::Hydration)
+        );
+        // Negative case: unknown deprecated chain id must not map to any Chain.
+        assert_eq!(Chain::from_deprecated_chain_id("0xdeadbeef"), None);
+    }
+}

Conversation

ppca commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

volovyks left a comment

Choose a reason for hiding this comment

Uh oh!

volovyks Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

volovyks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants