fix: credit origin chain ID on SignedFills (#93)

Author: anna-carroll

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

crates/types/src/agg/order.rs

@@ -97,15 +97,18 @@ impl AggregateOrders {
         }
     }
 
-    /// Get the unique destination chain ids for the aggregated outputs.
-    pub fn destination_chain_ids(&self) -> Vec<u64> {
+    /// Get the unique target chain ids for the aggregated outputs.
+    pub fn target_chain_ids(&self) -> Vec<u64> {
         HashSet::<u64>::from_iter(self.outputs.keys().map(|(chain_id, _)| *chain_id))
             .into_iter()
             .collect()
     }
 
-    /// Get the aggregated Outputs for a given chain id.
-    pub fn outputs_for(&self, target_chain_id: u64) -> Vec<RollupOrders::Output> {
+    /// 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<RollupOrders::Output> {
         let mut o = Vec::new();
         for ((chain_id, token), recipient_map) in &self.outputs {
             if *chain_id == target_chain_id {
@@ -114,7 +117,7 @@ impl AggregateOrders {
                         token: *token,
                         amount: U256::from(*amount),
                         recipient: *recipient,
-                        chainId: *chain_id as u32,
+                        chainId: ru_chain_id as u32,
                     });
                 }
             }

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 rollup chain id for a Fill.
+    #[error(
+        "Rollup chain id is missing. Populate it by calling with_ru_chain_id before attempting to sign"
+    )]
+    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),

crates/types/src/signing/fill.rs

@@ -112,10 +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<u64>,
+    /// 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<u64>,
+    /// The Permit2 nonce for the Fill, used to prevent replay attacks.
     nonce: Option<u64>,
-    destination_chains: HashMap<u64, Address>,
+    /// The (chain_id, order_contract_address) for each target chain on which Fills will be submitted.
+    target_chains: HashMap<u64, Address>,
 }
 
 impl<'a> From<&'a AggregateOrders> for UnsignedFill<'a> {
@@ -128,10 +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(),
+            target_chains: HashMap::new(),
         }
     }
 
@@ -145,38 +153,46 @@ impl<'a> UnsignedFill<'a> {
         Self { deadline: Some(deadline), ..self }
     }
 
-    /// Add the chain id  and Order contract address to the UnsignedOrder.
+    /// 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_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<S: Signer>(
         &self,
         signer: &S,
     ) -> Result<HashMap<u64, SignedFill>, 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.
-    /// 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<S: Signer>(
         &self,
-        chain_id: u64,
+        target_chain_id: u64,
         signer: &S,
     ) -> Result<SignedFill, SigningError> {
         let now = Utc::now();
@@ -185,14 +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(&chain_id)
-            .ok_or(SigningError::MissingOrderContract(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 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 chain from the AggregateOrders
-        let outputs = self.orders.outputs_for(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<TokenPermissions> = outputs.iter().map(Into::into).collect();
 
@@ -202,8 +221,8 @@ impl<'a> UnsignedFill<'a> {
             permitted,
             deadline,
             nonce,
-            chain_id,
-            *destination_order_address,
+            target_chain_id,
+            *target_order_address,
         );
 
         // sign it