chore: integrate with latest version of strata-p2p

[sistemd]

Description

Basic integration with the latest version of strata-p2p. Without byos (bring your own signer) feature, for now. Will be added in a follow-up.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactor
• New or updated tests
• Dependency Update

Notes to Reviewers

Checklist

• I have performed a self-review of my code.
• I have commented my code where necessary.
• I have updated the documentation if needed.
• My changes do not introduce new warnings.
• I have added (where necessary) tests that prove my changes are effective or that my feature works.
• New and existing tests pass with my changes.
• I have disclosed my use of AI in the body of this PR.

Related Issues

Part of STR-1650.

[storopoli]

We need to eliminate the inefficient hack that we, instead of replying the request-response in the oneshot channel, we drop it and we regossip whatever the requester wanted. This is inefficient and was a nasty shortcut that I had to take in Singapore early this year to try to deliver testnet in time.

This means that all of these functions that don't take any oneshot channel should now take it and stop doing OperatorDuty::PublishFoo:

• process_p2p_request: the mothership function that call all others.
• process_stake_chain_exchange_request
• process_deposit_setup_request
• process_musig2_nonces_exchange_request
• process_musig2_signatures_exchange_request

This is the relevant part of the codebase:

strata-bridge/crates/duty-tracker/src/contract_manager.rs

Lines 1135 to 1503 in a9e3973

	/// Processes incoming P2P requests from other operators and generates appropriate duty
	/// responses.
	///
	/// This method handles different types of P2P requests and generates the corresponding operator
	/// duties that need to be executed in response. The method supports the following request
	/// types:
	///
	/// # Request Types
	///
	/// - [`GetMessageRequest::StakeChainExchange`]: Requests for stake chain exchange data
	/// - Returns an [`OperatorDuty::PublishStakeChainExchange`] duty
	///
	/// - [`GetMessageRequest::DepositSetup`]: Requests for deposit setup information for a specific
	/// contract
	/// - Validates that the requested contract exists in active contracts
	/// - Returns an [`OperatorDuty::PublishDepositSetup`] duty with deposit and stake chain
	/// information
	///
	/// - [`GetMessageRequest::Musig2NoncesExchange`]: Requests for MuSig2 nonces for either graph
	/// or root transactions
	/// - For claim transactions: Returns an [`OperatorDuty::PublishGraphNonces`] duty with pegout
	/// graph data
	/// - For deposit request transactions: Returns an [`OperatorDuty::PublishRootNonce`] duty
	/// with deposit transaction data
	///
	/// - [`GetMessageRequest::Musig2SignaturesExchange`]: Requests for MuSig2 partial signatures
	/// - For claim transactions: Returns an [`OperatorDuty::PublishGraphSignatures`] duty with
	/// graph signature data
	/// - For deposit request transactions: Returns an [`OperatorDuty::PublishRootSignature`] duty
	/// with root signature data
	///
	/// # Arguments
	///
	/// * `req` - The P2P request from another operator
	///
	/// # Returns
	///
	/// Returns a vector of [`OperatorDuty`] instances that should be executed in response to the
	/// request. An empty vector indicates no duties are needed (e.g., when contract is not
	/// found or not ready).
	///
	/// # Errors
	///
	/// Returns [`ContractManagerErr::InvalidP2PRequest`] if:
	///
	/// - A deposit setup request is made for a non-existent contract
	/// - The request format is invalid or malformed
	async fn process_p2p_request(
	&mut self,
	req: GetMessageRequest,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	match req {
	GetMessageRequest::StakeChainExchange { .. } => {
	Ok(self.process_stake_chain_exchange_request())
	}
	GetMessageRequest::DepositSetup { scope, .. } => {
	self.process_deposit_setup_request(scope)
	}
	GetMessageRequest::Musig2NoncesExchange { session_id, .. } => {
	self.process_musig2_nonces_exchange_request(session_id)
	}
	GetMessageRequest::Musig2SignaturesExchange { session_id, .. } => {
	self.process_musig2_signatures_exchange_request(session_id)
	}
	}
	}
	fn process_stake_chain_exchange_request(&self) -> Option<OperatorDuty> {
	info!("received request for stake chain exchange");
	// TODO(proofofkeags): actually choose the correct stake chain
	// inputs based off the stake chain id we receive.
	Some(OperatorDuty::PublishStakeChainExchange)
	}
	fn process_deposit_setup_request(
	&self,
	scope: Scope,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	let deposit_txid = Txid::from_byte_array(*scope.as_ref());
	info!(%deposit_txid, "received request for deposit setup");
	let stake_chain_inputs = self
	.state
	.stake_chains
	.state()
	.get(self.cfg.operator_table.pov_p2p_key())
	.expect("our p2p key must exist in the operator table")
	.clone();
	if let Some(deposit_idx) = self
	.state
	.active_contracts
	.get(&deposit_txid)
	.map(|sm| sm.cfg().deposit_idx)
	{
	Ok(Some(OperatorDuty::PublishDepositSetup {
	deposit_txid,
	deposit_idx,
	stake_chain_inputs,
	}))
	} else {
	warn!(%deposit_txid, "received deposit setup request for unknown contract");
	Err(ContractManagerErr::InvalidP2PRequest(Box::new(
	GetMessageRequest::DepositSetup {
	scope,
	operator_pk: self.cfg.operator_table.pov_p2p_key().clone(),
	},
	)))
	}
	}
	fn process_musig2_nonces_exchange_request(
	&mut self,
	session_id: SessionId,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	let session_id_as_txid = Txid::from_byte_array(*session_id.as_ref());
	trace!(claims = ?self.state.claim_txids, "get nonces exchange");
	// First try to find by claim_txid
	if let Some(deposit_txid) = self.state.claim_txids.get(&session_id_as_txid) {
	if let Some(csm) = self.state.active_contracts.get(deposit_txid) {
	return Self::process_graph_nonces_request(session_id_as_txid, csm);
	}
	}
	// Try to find by deposit request txid
	if let Some(csm) = self
	.state
	.active_contracts
	.values()
	.find(|sm| sm.deposit_request_txid() == session_id_as_txid)
	{
	Ok(Self::process_root_nonces_request(session_id_as_txid, csm))
	} else {
	// otherwise ignore this message.
	warn!(txid=%session_id_as_txid, "received a musig2 nonces exchange for an unknown session");
	Ok(None)
	}
	}
	fn process_graph_nonces_request(
	claim_txid: Txid,
	csm: &ContractSM,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	info!(%claim_txid, "received request for graph nonces");
	if let ContractState::Requested {
	peg_out_graph_inputs,
	graph_nonces,
	..
	} = &csm.state().state
	{
	info!(%claim_txid, "received nag for graph nonces");
	let Some(graph_owner) = csm.state().state.claim_to_operator(&claim_txid) else {
	return Ok(None);
	};
	let input = peg_out_graph_inputs
	.get(&graph_owner)
	.expect("graph input must exist if claim_txid exists");
	let (pog_prevouts, pog_witnesses) = csm
	.pog()
	.get(&input.stake_outpoint.txid)
	.map(|pog| Ok((pog.musig_inpoints(), pog.musig_witnesses())))
	.unwrap_or_else(|| {
	csm.cfg()
	.build_graph(input)
	.map(|pog| (pog.musig_inpoints(), pog.musig_witnesses()))
	})?;
	// Get nonces from state if they exist for this claim txid
	let existing_nonces = graph_nonces
	.get(&claim_txid)
	.and_then(|session_nonces| {
	session_nonces.get(csm.cfg().operator_table.pov_p2p_key())
	})
	.cloned();
	Ok(Some(OperatorDuty::PublishGraphNonces {
	claim_txid,
	pog_prevouts,
	pog_witnesses,
	nonces: existing_nonces,
	}))
	} else {
	warn!(deposit_idx=%csm.cfg().deposit_idx, "nagged for nonces on a ContractSM that is not in a Requested state");
	Ok(None)
	}
	}
	fn process_root_nonces_request(
	deposit_request_txid: Txid,
	csm: &ContractSM,
	) -> Option<OperatorDuty> {
	info!(%deposit_request_txid, "received nag for root nonces");
	if let ContractState::Requested {
	graph_sigs,
	root_nonces,
	..
	} = &csm.state().state
	{
	if graph_sigs.len() != csm.cfg().operator_table.cardinality() {
	warn!(%deposit_request_txid, "aggregated graphs sigs incomplete, cannot publish root nonces");
	return None;
	}
	let witness = csm.cfg().deposit_tx.witnesses()[0].clone();
	// Get nonce from state if it exists for this operator
	let existing_nonce = root_nonces
	.get(csm.cfg().operator_table.pov_p2p_key())
	.cloned();
	Some(OperatorDuty::PublishRootNonce {
	deposit_request_txid,
	witness,
	nonce: existing_nonce,
	})
	} else {
	warn!(deposit_idx=%csm.cfg().deposit_idx, "nagged for nonces on a ContractSM that is not in a Requested state");
	None
	}
	}
	fn process_musig2_signatures_exchange_request(
	&mut self,
	session_id: SessionId,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	let session_id_as_txid = Txid::from_byte_array(*session_id.as_ref());
	trace!(claims = ?self.state.claim_txids, "get signatures exchange");
	// First try to find by claim_txid
	if let Some(deposit_txid) = self.state.claim_txids.get(&session_id_as_txid) {
	if let Some(csm) = self.state.active_contracts.get(deposit_txid) {
	return Self::process_graph_signatures_request(&self.cfg, session_id_as_txid, csm);
	}
	}
	// Try to find by deposit request txid
	if let Some(csm) = self
	.state
	.active_contracts
	.values()
	.find(|sm| sm.deposit_request_txid() == session_id_as_txid)
	{
	Ok(Self::process_root_signatures_request(
	session_id_as_txid,
	csm,
	))
	} else {
	// otherwise ignore this message.
	warn!(txid=%session_id_as_txid, "received a musig2 signatures exchange for an unknown session");
	Ok(None)
	}
	}
	fn process_graph_signatures_request(
	cfg: &ExecutionConfig,
	claim_txid: Txid,
	csm: &ContractSM,
	) -> Result<Option<OperatorDuty>, ContractManagerErr> {
	if let ContractState::Requested {
	peg_out_graph_inputs,
	graph_partials,
	agg_nonces,
	..
	} = &csm.state().state
	{
	info!(%claim_txid, "received nag for graph signatures");
	// Check if we already have our own partial signatures for this graph
	let our_p2p_key = cfg.operator_table.pov_p2p_key();
	let existing_partials = graph_partials
	.get(&claim_txid)
	.and_then(|session_partials| session_partials.get(our_p2p_key))
	.cloned();
	let Some(graph_owner) = csm.state().state.claim_to_operator(&claim_txid) else {
	return Ok(None);
	};
	let input = &peg_out_graph_inputs
	.get(&graph_owner)
	.expect("graph input must exist if claim_txid exists");
	let (pog_prevouts, pog_sighashes, pog_witnesses) = csm
	.pog()
	.get(&input.stake_outpoint.txid)
	.map(|cached_graph| {
	Ok((
	cached_graph.musig_inpoints(),
	cached_graph.musig_sighashes(),
	cached_graph.musig_witnesses(),
	))
	})
	.unwrap_or_else(|| {
	csm.cfg().build_graph(input).map(|pog| {
	(
	pog.musig_inpoints(),
	pog.musig_sighashes(),
	pog.musig_witnesses(),
	)
	})
	})?;
	let Some(aggnonces) = agg_nonces.get(&claim_txid).cloned() else {
	return Ok(None);
	};
	Ok(Some(OperatorDuty::PublishGraphSignatures {
	claim_txid,
	aggnonces,
	pog_prevouts,
	pog_sighashes,
	witnesses: pog_witnesses,
	partial_signatures: existing_partials,
	}))
	} else {
	warn!("nagged for nonces on a ContractSM that is not in a Requested state");
	Ok(None)
	}
	}
	fn process_root_signatures_request(
	deposit_request_txid: Txid,
	csm: &ContractSM,
	) -> Option<OperatorDuty> {
	info!(%deposit_request_txid, "received nag for root signatures");
	if let ContractState::Requested {
	root_nonces,
	root_partials,
	..
	} = &csm.state().state
	{
	info!(%deposit_request_txid, "received nag for root signatures");
	// Check if we already have our own root partial signature for this contract
	let our_p2p_key = csm.cfg().operator_table.pov_p2p_key();
	let existing_partial = root_partials.get(our_p2p_key).copied();
	let deposit_tx = &csm.cfg().deposit_tx;
	let sighash = deposit_tx.sighashes()[0];
	let witness = deposit_tx.witnesses()[0].clone();
	let aggnonce = csm
	.cfg()
	.operator_table
	.convert_map_p2p_to_btc(root_nonces.clone())
	.expect("valid operator table")
	.into_values()
	.sum();
	Some(OperatorDuty::PublishRootSignature {
	deposit_request_txid,
	aggnonce,
	sighash,
	witness,
	partial_signature: existing_partial,
	})
	} else {
	warn!("nagged for nonces on a ContractSM that is not in a Requested state");
	None
	}
	}

If you are stuck or feel confused, we can always schedule a pairing session on a synchronous meeting.
Let me know if you want to do that.

[Zk2u]

generally absolutely excellent code quality, very good job. just a couple of small things on top of what jose said

[storopoli]

ACK 7d5ef4e

One thing that I want to have an answer is: We renamed the OperatorDutys that are to PublishFoo to SendFoo whenever we get a request from a peer and we sent the specific info to that specific peer only using the oneshot channel. Great! But, this is important, we still have the regular gossiping with PublishFoo stuff whenever we see DRTs and other stuff on chain, right? I cannot see that in the PR. I just want to be sure.

[sistemd]

ACK 7d5ef4e

One thing that I want to have an answer is: We renamed the OperatorDutys that are to PublishFoo to SendFoo whenever we get a request from a peer and we sent the specific info to that specific peer only using the oneshot channel. Great! But, this is important, we still have the regular gossiping with PublishFoo stuff whenever we see DRTs and other stuff on chain, right? I cannot see that in the PR. I just want to be sure.

Yes - a gossip is just SendFoo with peer set to None.

[sistemd]

@Zk2u Heads up: because you requested changes, I can't merge this PR until you've approved.

[sistemd]

It seems like the CI keeps timing out, or I'm maybe missing something. I keep getting "operation cancelled".

[storopoli]

tests::gossipsub::all_to_all_multiple_ids is probably having a bug in the tests or the new code introduced a deadlock somehow.
It is consistently timing out..

Out of date

[sistemd]

I've discovered what the issue is.

ERROR strata_p2p::swarm: Failed to create signed gossipsub message e="Signature length is not 64 bytes"

So the problem is that strata-p2p assumes that all signatures coming from libp2p::identity::Keypair are 64 bytes: https://github.com/alpenlabs/strata-p2p/blob/1e66b4f2298976c48a638717f4e6ad5a448c0975/crates/p2p/src/signer.rs#L75-L80. But for the secp256k variant, this is not the case, it actually returns a DER-encoded signature which is ~70 bytes (DER size is actually variable).

One potential fix on the strata-bridge side is to switch from secp256k to a variant which actually does return 64 bytes, such as Ed25519. But this seems to me like a bug in strata-p2p. It should be able to handle variable-size signatures returned by libp2p::identity::Keypair.

LMK how you would like to proceed. CC @storopoli

[storopoli]

One potential fix on the strata-bridge side is to switch from secp256k to a variant which actually does return 64 bytes, such as Ed25519. But this seems to me like a bug in strata-p2p. It should be able to handle variable-size signatures returned by libp2p::identity::Keypair

No it's not a bug.

From the strata-p2p v2 specs

LibP2P implementations must support Ed25519 ****keys and signatures. Ed25519 is the [most well-implemented and supported authentication method in LibP2P](https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#key-types); and the only one that is mandatory to be supported in the protocol.

Ed25519 is generally preferred over ECDSA on secp256k1 (what the v1 of this spec used) for authentication for several important reasons:

• Faster performance: Ed25519 provides significantly faster signature verification (2-3x faster than ECDSA on secp256k1) and generation speed.
• Smaller signatures: Ed25519 produces 64-byte signatures compared to ECDSA on secp256k1's variable-length signatures (typically 70-72 bytes).
• Deterministic signatures: Ed25519 is inherently deterministic, eliminating the need for a secure random number generator during signing. ECDSA on secp256k1 by default is not, only if paired with the [RFC 6979](https://www.rfc-editor.org/rfc/rfc6979) for ECDSA deterministic nonces.

Hence, strata-bridge should use Ed25519

[storopoli]

ACK b39cc79

I suggest you or @Rajil1213 run a whole self-hosted docker workflow and see if just bridge-in works with the operators gossiping the DRT and checking if there aren't any nags or if the amount of nags is acceptable.

[sistemd]

@storopoli What I can see in the code is that the P2P keys ultimately come from alpen core's OperatorKeys type, which at the moment only supports secp256k1 keys generated from the same bip32 seed. So either the strata-p2p dep needs to change, or the alpen dep needs to change, or the P2P key should come from somewhere else now? I think it'd be fine for this key to be ephemeral, but I need your confirmation before I make that change.

[storopoli]

@storopoli What I can see in the code is that the P2P keys ultimately come from alpen core's OperatorKeys type, which at the moment only supports secp256k1 keys generated from the same bip32 seed. So either the strata-p2p dep needs to change, or the alpen dep needs to change, or the P2P key should come from somewhere else now? I think it'd be fine for this key to be ephemeral, but I need your confirmation before I make that change.

Ok I was able to quickly PR ed25519 support for strata-key-deriv in alpenlabs/alpen#1203. Let's wait that to be merged?