From 81d2e29e435d242e1f23d9ef515c410ad82b33fd Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Mon, 14 Jul 2025 16:13:06 +0200 Subject: [PATCH 1/2] fix: credit origin chain ID on SignedFills --- crates/types/src/agg/order.rs | 14 ++++++++++---- crates/types/src/signing/error.rs | 5 +++++ crates/types/src/signing/fill.rs | 29 +++++++++++++++++++++-------- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/types/src/agg/order.rs b/crates/types/src/agg/order.rs index 4b200279..37800987 100644 --- a/crates/types/src/agg/order.rs +++ b/crates/types/src/agg/order.rs @@ -104,17 +104,23 @@ impl AggregateOrders { .collect() } - /// Get the aggregated Outputs for a given chain id. - pub fn outputs_for(&self, target_chain_id: u64) -> Vec { + /// Get the aggregated Outputs for a given destination chain id. + /// NOTE it is assumed that all Orders in the AggregateOrders + /// are from the same origin chain id. + pub fn outputs_for( + &self, + destination_chain_id: u64, + origin_chain_id: u64, + ) -> Vec { let mut o = Vec::new(); for ((chain_id, token), recipient_map) in &self.outputs { - if *chain_id == target_chain_id { + if *chain_id == destination_chain_id { for (recipient, amount) in recipient_map { o.push(RollupOrders::Output { token: *token, amount: U256::from(*amount), recipient: *recipient, - chainId: *chain_id as u32, + chainId: origin_chain_id as u32, }); } } diff --git a/crates/types/src/signing/error.rs b/crates/types/src/signing/error.rs index 12250dc2..76a07657 100644 --- a/crates/types/src/signing/error.rs +++ b/crates/types/src/signing/error.rs @@ -22,6 +22,11 @@ pub enum SigningError { "Target chain id is missing. Populate it by calling with_chain before attempting to sign" )] MissingChainId, + /// Missing origin chain id for a Fill. + #[error( + "Origin chain id is missing. Populate it by calling with_origin_chain_id before attempting to sign" + )] + MissingOriginChainId, /// Missing chain config for a specific chain. #[error("Target Order contract address is missing for chain id {0}. Populate it by calling with_chain before attempting to sign")] MissingOrderContract(u64), diff --git a/crates/types/src/signing/fill.rs b/crates/types/src/signing/fill.rs index af0b92d3..29899943 100644 --- a/crates/types/src/signing/fill.rs +++ b/crates/types/src/signing/fill.rs @@ -116,6 +116,7 @@ pub struct UnsignedFill<'a> { deadline: Option, nonce: Option, destination_chains: HashMap, + origin_chain_id: Option, } impl<'a> From<&'a AggregateOrders> for UnsignedFill<'a> { @@ -132,6 +133,7 @@ impl<'a> UnsignedFill<'a> { deadline: None, nonce: None, destination_chains: HashMap::new(), + origin_chain_id: None, } } @@ -145,7 +147,15 @@ impl<'a> UnsignedFill<'a> { Self { deadline: Some(deadline), ..self } } - /// Add the chain id and Order contract address to the UnsignedOrder. + /// Add the origin chain id to the UnsignedFill. + /// This is the rollup chain id from which the Orders originated, + /// to which the Fill should be credited. + /// MUST call this before signing, cannot be inferred. + pub fn with_origin_chain_id(self, origin_chain_id: u64) -> Self { + Self { origin_chain_id: Some(origin_chain_id), ..self } + } + + /// Add the chain id and Order contract address to the UnsignedFill. pub fn with_chain(mut self, chain_id: u64, order_contract_address: Address) -> Self { self.destination_chains.insert(chain_id, order_contract_address); self @@ -172,11 +182,11 @@ impl<'a> UnsignedFill<'a> { /// Sign the UnsignedFill for a specific destination chain. /// Use if Filling Orders with different signing keys on respective destination chains. /// # Warning ⚠️ - /// *All* Outputs MUST be filled on all destination chains, else the Order Inputs will not be transferred. + /// *All* Outputs MUST be filled on all destination chains, else the Order Inputs will not be transferred on the origin chain. /// Take care when using this function to produce SignedFills for every destination chain. pub async fn sign_for( &self, - chain_id: u64, + destination_chain_id: u64, signer: &S, ) -> Result { let now = Utc::now(); @@ -188,11 +198,14 @@ impl<'a> UnsignedFill<'a> { // get the destination order address let destination_order_address = self .destination_chains - .get(&chain_id) - .ok_or(SigningError::MissingOrderContract(chain_id))?; + .get(&destination_chain_id) + .ok_or(SigningError::MissingOrderContract(destination_chain_id))?; + + // get the origin chain id, or throw an error if not set + let origin_chain_id = self.origin_chain_id.ok_or(SigningError::MissingOriginChainId)?; - // get the outputs for the chain from the AggregateOrders - let outputs = self.orders.outputs_for(chain_id); + // get the outputs for the destination chain from the AggregateOrders + let outputs = self.orders.outputs_for(destination_chain_id, origin_chain_id); // generate the permitted tokens from the Outputs let permitted: Vec = outputs.iter().map(Into::into).collect(); @@ -202,7 +215,7 @@ impl<'a> UnsignedFill<'a> { permitted, deadline, nonce, - chain_id, + destination_chain_id, *destination_order_address, ); From ae68156a0e9143264a6b162c603be4bc3e0146a7 Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Mon, 14 Jul 2025 19:18:37 +0200 Subject: [PATCH 2/2] renaming --- crates/rpc/examples/filler.rs | 2 +- crates/types/src/agg/order.rs | 21 +++++----- crates/types/src/signing/error.rs | 6 +-- crates/types/src/signing/fill.rs | 64 +++++++++++++++++-------------- 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/crates/rpc/examples/filler.rs b/crates/rpc/examples/filler.rs index 05b4986f..fd29e589 100644 --- a/crates/rpc/examples/filler.rs +++ b/crates/rpc/examples/filler.rs @@ -160,7 +160,7 @@ where // produce an UnsignedFill from the AggregateOrder let mut unsigned_fill = UnsignedFill::from(&agg); // populate the Order contract addresses for each chain - for chain_id in agg.destination_chain_ids() { + for chain_id in agg.target_chain_ids() { unsigned_fill = unsigned_fill.with_chain( chain_id, self.constants diff --git a/crates/types/src/agg/order.rs b/crates/types/src/agg/order.rs index 37800987..cee8cf4d 100644 --- a/crates/types/src/agg/order.rs +++ b/crates/types/src/agg/order.rs @@ -97,30 +97,27 @@ impl AggregateOrders { } } - /// Get the unique destination chain ids for the aggregated outputs. - pub fn destination_chain_ids(&self) -> Vec { + /// Get the unique target chain ids for the aggregated outputs. + pub fn target_chain_ids(&self) -> Vec { HashSet::::from_iter(self.outputs.keys().map(|(chain_id, _)| *chain_id)) .into_iter() .collect() } - /// Get the aggregated Outputs for a given destination chain id. - /// NOTE it is assumed that all Orders in the AggregateOrders - /// are from the same origin chain id. - pub fn outputs_for( - &self, - destination_chain_id: u64, - origin_chain_id: u64, - ) -> Vec { + /// Get the aggregated Outputs for a given target chain id. + /// # Warning ⚠️ + /// All Orders in the AggregateOrders MUST have originated on the same rollup. + /// Otherwise, the aggregated Outputs will be incorrectly credited. + pub fn outputs_for(&self, target_chain_id: u64, ru_chain_id: u64) -> Vec { let mut o = Vec::new(); for ((chain_id, token), recipient_map) in &self.outputs { - if *chain_id == destination_chain_id { + if *chain_id == target_chain_id { for (recipient, amount) in recipient_map { o.push(RollupOrders::Output { token: *token, amount: U256::from(*amount), recipient: *recipient, - chainId: origin_chain_id as u32, + chainId: ru_chain_id as u32, }); } } diff --git a/crates/types/src/signing/error.rs b/crates/types/src/signing/error.rs index 76a07657..a2f067b8 100644 --- a/crates/types/src/signing/error.rs +++ b/crates/types/src/signing/error.rs @@ -22,11 +22,11 @@ pub enum SigningError { "Target chain id is missing. Populate it by calling with_chain before attempting to sign" )] MissingChainId, - /// Missing origin chain id for a Fill. + /// Missing rollup chain id for a Fill. #[error( - "Origin chain id is missing. Populate it by calling with_origin_chain_id before attempting to sign" + "Rollup chain id is missing. Populate it by calling with_ru_chain_id before attempting to sign" )] - MissingOriginChainId, + MissingRollupChainId, /// Missing chain config for a specific chain. #[error("Target Order contract address is missing for chain id {0}. Populate it by calling with_chain before attempting to sign")] MissingOrderContract(u64), diff --git a/crates/types/src/signing/fill.rs b/crates/types/src/signing/fill.rs index 29899943..95bd2dd2 100644 --- a/crates/types/src/signing/fill.rs +++ b/crates/types/src/signing/fill.rs @@ -112,11 +112,17 @@ impl From<&SignedFill> for FillPermit2 { /// An UnsignedFill is a helper type used to easily transform an AggregateOrder into a single SignedFill with correct permit2 semantics. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct UnsignedFill<'a> { + /// The rollup chain id from which the Orders originated. + ru_chain_id: Option, + /// The set of Orders to fill. Multiple Orders can be aggregated into a single Fill, + /// but they MUST all originate on the same rollup chain indicated by `ru_chain_id`. orders: Cow<'a, AggregateOrders>, + /// The deadline for the Fill, after which it cannot be mined. deadline: Option, + /// The Permit2 nonce for the Fill, used to prevent replay attacks. nonce: Option, - destination_chains: HashMap, - origin_chain_id: Option, + /// The (chain_id, order_contract_address) for each target chain on which Fills will be submitted. + target_chains: HashMap, } impl<'a> From<&'a AggregateOrders> for UnsignedFill<'a> { @@ -129,11 +135,11 @@ impl<'a> UnsignedFill<'a> { /// Get a new UnsignedFill from a set of AggregateOrders. pub fn new(orders: &'a AggregateOrders) -> Self { Self { + ru_chain_id: None, orders: orders.into(), deadline: None, nonce: None, - destination_chains: HashMap::new(), - origin_chain_id: None, + target_chains: HashMap::new(), } } @@ -147,21 +153,21 @@ impl<'a> UnsignedFill<'a> { Self { deadline: Some(deadline), ..self } } - /// Add the origin chain id to the UnsignedFill. + /// Add the rollup chain id to the UnsignedFill. /// This is the rollup chain id from which the Orders originated, /// to which the Fill should be credited. /// MUST call this before signing, cannot be inferred. - pub fn with_origin_chain_id(self, origin_chain_id: u64) -> Self { - Self { origin_chain_id: Some(origin_chain_id), ..self } + pub fn with_ru_chain_id(self, ru_chain_id: u64) -> Self { + Self { ru_chain_id: Some(ru_chain_id), ..self } } /// Add the chain id and Order contract address to the UnsignedFill. pub fn with_chain(mut self, chain_id: u64, order_contract_address: Address) -> Self { - self.destination_chains.insert(chain_id, order_contract_address); + self.target_chains.insert(chain_id, order_contract_address); self } - /// Sign the UnsignedFill, generating a SignedFill for each destination chain. + /// Sign the UnsignedFill, generating a SignedFill for each target chain. /// Use if Filling Orders with the same signing key on every chain. pub async fn sign( &self, @@ -169,24 +175,24 @@ impl<'a> UnsignedFill<'a> { ) -> Result, SigningError> { let mut fills = HashMap::new(); - // loop through each destination chain and sign the fills - for destination_chain_id in self.orders.destination_chain_ids() { - let signed_fill = self.sign_for(destination_chain_id, signer).await?; - fills.insert(destination_chain_id, signed_fill); + // loop through each target chain and sign the fills + for target_chain_id in self.orders.target_chain_ids() { + let signed_fill = self.sign_for(target_chain_id, signer).await?; + fills.insert(target_chain_id, signed_fill); } // return the fills Ok(fills) } - /// Sign the UnsignedFill for a specific destination chain. - /// Use if Filling Orders with different signing keys on respective destination chains. + /// Sign the UnsignedFill for a specific target chain. + /// Use if Filling Orders with different signing keys on respective target chains. /// # Warning ⚠️ - /// *All* Outputs MUST be filled on all destination chains, else the Order Inputs will not be transferred on the origin chain. - /// Take care when using this function to produce SignedFills for every destination chain. + /// *All* Outputs MUST be filled on all target chains, else the Order Inputs will not be transferred on the rollup. + /// Take care when using this function to produce SignedFills for every target chain. pub async fn sign_for( &self, - destination_chain_id: u64, + target_chain_id: u64, signer: &S, ) -> Result { let now = Utc::now(); @@ -195,17 +201,17 @@ impl<'a> UnsignedFill<'a> { // if deadline is None, populate it as now + 12 seconds (can only mine within the current block) let deadline = self.deadline.unwrap_or(now.timestamp() as u64 + 12); - // get the destination order address - let destination_order_address = self - .destination_chains - .get(&destination_chain_id) - .ok_or(SigningError::MissingOrderContract(destination_chain_id))?; + // get the target order address + let target_order_address = self + .target_chains + .get(&target_chain_id) + .ok_or(SigningError::MissingOrderContract(target_chain_id))?; - // get the origin chain id, or throw an error if not set - let origin_chain_id = self.origin_chain_id.ok_or(SigningError::MissingOriginChainId)?; + // get the rollup chain id, or throw an error if not set + let ru_chain_id = self.ru_chain_id.ok_or(SigningError::MissingRollupChainId)?; - // get the outputs for the destination chain from the AggregateOrders - let outputs = self.orders.outputs_for(destination_chain_id, origin_chain_id); + // get the outputs for the target chain from the AggregateOrders + let outputs = self.orders.outputs_for(target_chain_id, ru_chain_id); // generate the permitted tokens from the Outputs let permitted: Vec = outputs.iter().map(Into::into).collect(); @@ -215,8 +221,8 @@ impl<'a> UnsignedFill<'a> { permitted, deadline, nonce, - destination_chain_id, - *destination_order_address, + target_chain_id, + *target_order_address, ); // sign it