fix: better error messages

Author: troykessler

rust/sealevel/programs/ism/composite-ism/src/account_metas.rs

@@ -4,7 +4,7 @@ use solana_program::{account_info::AccountInfo, instruction::AccountMeta, pubkey
 
 use crate::{
     accounts::{derive_domain_pda, DomainIsmAccount, IsmNode},
-    metadata::{parse_aggregation_ranges, sub_metadata},
+    metadata::{parse_aggregation_ranges, parse_routing_amount, sub_metadata},
 };
 
 /// Returns `true` if the ISM tree contains any `RateLimited` node.
@@ -90,14 +90,7 @@ pub fn required_accounts_for_node(
             lower,
             upper,
         } => {
-            const AMOUNT_OFFSET: usize = 32;
-            const AMOUNT_END: usize = 64;
-            if message.body.len() < AMOUNT_END {
-                return vec![];
-            }
-            let Ok(amount): Result<[u8; 32], _> =
-                message.body[AMOUNT_OFFSET..AMOUNT_END].try_into()
-            else {
+            let Some(amount) = parse_routing_amount(&message.body) else {
                 return vec![];
             };
             let sub_ism = if amount >= *threshold { upper } else { lower };

rust/sealevel/programs/ism/composite-ism/src/error.rs

@@ -43,6 +43,14 @@ pub enum Error {
     RateLimitedInDomainIsm = 19,
     #[error("Routing ISM is not allowed inside a domain PDA")]
     RoutingInDomainIsm = 20,
+    #[error("Domain PDA must be writable when ISM tree contains a RateLimited node")]
+    DomainPdaNotWritable = 21,
+    #[error("Expected system program account")]
+    InvalidSystemProgram = 22,
+    #[error("Storage PDA account does not match expected derived address")]
+    InvalidStoragePda = 23,
+    #[error("Domain PDA account does not match expected derived address for this origin")]
+    InvalidDomainPda = 24,
 }
 
 impl From<Error> for ProgramError {

rust/sealevel/programs/ism/composite-ism/src/metadata.rs

@@ -76,6 +76,16 @@ pub fn sub_metadata(metadata: &[u8], range: MetadataRange) -> &[u8] {
     &metadata[range.start as usize..range.end as usize]
 }
 
+/// Parses the 32-byte big-endian U256 amount from a warp-route message body.
+///
+/// Warp-route (TokenMessage) layout: `[recipient (32 bytes)][amount (32 bytes)][metadata]`.
+/// Returns `None` if the body is too short.
+pub(crate) fn parse_routing_amount(body: &[u8]) -> Option<[u8; 32]> {
+    const AMOUNT_OFFSET: usize = 32;
+    const AMOUNT_END: usize = 64;
+    body.get(AMOUNT_OFFSET..AMOUNT_END)?.try_into().ok()
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -154,4 +164,26 @@ mod test {
         metadata.extend_from_slice(&[0u8; 4]);
         assert!(parse_aggregation_ranges(&metadata, 1).is_err());
     }
+
+    #[test]
+    fn test_parse_routing_amount_exact_body() {
+        let mut body = [0u8; 64];
+        body[63] = 42; // amount = 42 in the low byte
+        let amount = parse_routing_amount(&body).unwrap();
+        assert_eq!(amount[31], 42);
+    }
+
+    #[test]
+    fn test_parse_routing_amount_body_longer_than_64() {
+        let mut body = vec![0u8; 100];
+        body[32] = 1; // high byte of amount
+        let amount = parse_routing_amount(&body).unwrap();
+        assert_eq!(amount[0], 1);
+    }
+
+    #[test]
+    fn test_parse_routing_amount_body_too_short() {
+        assert!(parse_routing_amount(&[0u8; 63]).is_none());
+        assert!(parse_routing_amount(&[]).is_none());
+    }
 }

rust/sealevel/programs/ism/composite-ism/src/metadata_spec.rs

@@ -5,6 +5,7 @@ use solana_program::{account_info::AccountInfo, pubkey::Pubkey};
 use crate::{
     accounts::{derive_domain_pda, load_domain_ism, IsmNode},
     error::Error,
+    metadata::parse_routing_amount,
 };
 
 /// Describes the metadata a relayer must supply for this composite ISM tree.
@@ -32,68 +33,6 @@ pub enum MetadataSpec {
 
 /// Resolves the [`MetadataSpec`] for an ISM node given the message.
 ///
-/// Routing/AmountRouting are resolved inline. `Routing` nodes require a
-/// domain PDA account to be provided; use [`spec_for_node_with_pdas`] when
-/// `Routing` nodes may be present in the tree.
-#[allow(dead_code)]
-pub(crate) fn spec_for_node(
-    node: &IsmNode,
-    message: &HyperlaneMessage,
-) -> Result<MetadataSpec, Error> {
-    match node {
-        IsmNode::MultisigMessageId {
-            validators,
-            threshold,
-        } => Ok(MetadataSpec::MultisigMessageId {
-            validators: validators.clone(),
-            threshold: *threshold,
-        }),
-
-        IsmNode::Aggregation {
-            threshold,
-            sub_isms,
-        } => {
-            let sub_specs = sub_isms
-                .iter()
-                .map(|sub| spec_for_node(sub, message))
-                .collect::<Result<Vec<_>, _>>()?;
-            Ok(MetadataSpec::Aggregation {
-                threshold: *threshold,
-                sub_specs,
-            })
-        }
-
-        IsmNode::AmountRouting {
-            threshold,
-            lower,
-            upper,
-        } => {
-            const AMOUNT_OFFSET: usize = 32;
-            const AMOUNT_END: usize = 64;
-            if message.body.len() < AMOUNT_END {
-                return Err(Error::InvalidMessageBody);
-            }
-            let amount: [u8; 32] = message.body[AMOUNT_OFFSET..AMOUNT_END]
-                .try_into()
-                .map_err(|_| Error::InvalidMessageBody)?;
-            let sub_ism = if amount >= *threshold { upper } else { lower };
-            spec_for_node(sub_ism, message)
-        }
-
-        // Routing requires domain PDA accounts — use spec_for_node_with_pdas instead.
-        IsmNode::Routing { .. } => Err(Error::NoRouteForDomain),
-
-        // Leaf nodes that need no metadata from the relayer.
-        IsmNode::TrustedRelayer { .. }
-        | IsmNode::Test { .. }
-        | IsmNode::Pausable { .. }
-        | IsmNode::RateLimited { .. } => Ok(MetadataSpec::Null),
-    }
-}
-
-/// Like [`spec_for_node`], but resolves `Routing` nodes by reading accounts
-/// from `accounts_iter`.
-///
 /// For each `Routing` encountered during depth-first traversal, the next
 /// account from `accounts_iter` must be the domain PDA for `message.origin`
 /// (or any PDA — the key is verified against the derived address).
@@ -110,12 +49,12 @@ where
     match node {
         IsmNode::Routing { default_ism } => {
             // Expect the domain PDA as the next account.
-            let domain_pda_info = accounts_iter.next().ok_or(Error::AccountOutOfOrder)?;
+            let domain_pda_info = accounts_iter.next().ok_or(Error::InvalidDomainPda)?;
 
             // Verify correct account.
             let (expected_key, _) = derive_domain_pda(program_id, message.origin);
             if *domain_pda_info.key != expected_key {
-                return Err(Error::AccountOutOfOrder);
+                return Err(Error::InvalidDomainPda);
             }
 
             // Load sub-ISM.
@@ -160,14 +99,7 @@ where
             lower,
             upper,
         } => {
-            const AMOUNT_OFFSET: usize = 32;
-            const AMOUNT_END: usize = 64;
-            if message.body.len() < AMOUNT_END {
-                return Err(Error::InvalidMessageBody);
-            }
-            let amount: [u8; 32] = message.body[AMOUNT_OFFSET..AMOUNT_END]
-                .try_into()
-                .map_err(|_| Error::InvalidMessageBody)?;
+            let amount = parse_routing_amount(&message.body).ok_or(Error::InvalidMessageBody)?;
             let sub_ism = if amount >= *threshold { upper } else { lower };
             spec_for_node_with_pdas(sub_ism, message, program_id, accounts_iter)
         }

rust/sealevel/programs/ism/composite-ism/src/processor.rs

@@ -197,7 +197,7 @@ fn initialize(program_id: &Pubkey, accounts: &[AccountInfo], mut root: IsmNode)
     let (storage_pda_key, storage_pda_bump) =
         Pubkey::find_program_address(storage_pda_seeds!(), program_id);
     if *storage_pda_account.key != storage_pda_key {
-        return Err(Error::AccountOutOfOrder.into());
+        return Err(Error::InvalidStoragePda.into());
     }
 
     if let Ok(Some(_)) =
@@ -208,7 +208,7 @@ fn initialize(program_id: &Pubkey, accounts: &[AccountInfo], mut root: IsmNode)
 
     let system_program_account = next_account_info(accounts_iter)?;
     if system_program_account.key != &system_program::ID {
-        return Err(Error::AccountOutOfOrder.into());
+        return Err(Error::InvalidSystemProgram.into());
     }
 
     let storage = CompositeIsmAccount::from(CompositeIsmStorage {
@@ -290,7 +290,11 @@ fn set_domain_ism(
     let (expected_pda, bump) =
         Pubkey::find_program_address(&[DOMAIN_ISM_SEED, &domain_bytes], program_id);
     if *domain_pda_account.key != expected_pda {
-        return Err(Error::AccountOutOfOrder.into());
+        return Err(Error::InvalidDomainPda.into());
+    }
+
+    if system_program_account.key != &system_program::ID {
+        return Err(Error::InvalidSystemProgram.into());
     }
 
     let domain_storage = DomainIsmAccount::from(DomainIsmStorage {
@@ -344,7 +348,7 @@ fn remove_domain_ism(program_id: &Pubkey, accounts: &[AccountInfo], domain: u32)
     // Verify domain PDA address.
     let (expected_pda, _) = derive_domain_pda(program_id, domain);
     if *domain_pda_account.key != expected_pda {
-        return Err(Error::AccountOutOfOrder.into());
+        return Err(Error::InvalidDomainPda.into());
     }
 
     if domain_pda_account.owner != program_id {
@@ -549,7 +553,7 @@ fn load_storage(
     let storage_pda_key =
         Pubkey::create_program_address(storage_pda_seeds!(storage.bump_seed), program_id)?;
     if *storage_pda_account.key != storage_pda_key {
-        return Err(Error::AccountOutOfOrder.into());
+        return Err(Error::InvalidStoragePda.into());
     }
 
     Ok(storage)

rust/sealevel/programs/ism/composite-ism/src/verify.rs

@@ -11,6 +11,7 @@ use crate::{
     account_metas::contains_rate_limited,
     accounts::{derive_domain_pda, load_domain_ism_storage, DomainIsmAccount, IsmNode},
     error::Error,
+    metadata::parse_routing_amount,
     metadata::{parse_aggregation_ranges, sub_metadata},
     multisig_metadata::MultisigIsmMessageIdMetadata,
     rate_limit::calculate_current_level,
@@ -115,15 +116,7 @@ where
             lower,
             upper,
         } => {
-            const AMOUNT_OFFSET: usize = 32;
-            const AMOUNT_END: usize = 64;
-            if message.body.len() < AMOUNT_END {
-                return Err(Error::InvalidMessageBody.into());
-            }
-            let amount: [u8; 32] = message.body[AMOUNT_OFFSET..AMOUNT_END]
-                .try_into()
-                .map_err(|_| Error::InvalidMessageBody)?;
-
+            let amount = parse_routing_amount(&message.body).ok_or(Error::InvalidMessageBody)?;
             let sub_ism: &mut IsmNode = if amount >= *threshold { upper } else { lower };
             verify_node(sub_ism, metadata, message, accounts_iter, program_id)
         }
@@ -135,7 +128,7 @@ where
             // Verify the caller passed the correct domain PDA for this origin.
             let (expected_key, _) = derive_domain_pda(program_id, message.origin);
             if *domain_pda_info.key != expected_key {
-                return Err(Error::AccountOutOfOrder.into());
+                return Err(Error::InvalidDomainPda.into());
             }
 
             // Load the full domain PDA storage (None if not owned by this program).
@@ -147,7 +140,7 @@ where
                     // RateLimited state must be persisted; require a writable domain PDA so a
                     // hand-crafted transaction cannot bypass the rate limit by passing it readonly.
                     if contains_rate_limited(&ism) && !domain_pda_info.is_writable {
-                        return Err(Error::AccountOutOfOrder.into());
+                        return Err(Error::DomainPdaNotWritable.into());
                     }
                     verify_node(&mut ism, metadata, message, accounts_iter, program_id)?;
                     // Write updated state back to the domain PDA (e.g. RateLimited counters).
@@ -196,6 +189,12 @@ where
                 }
             }
 
+            // Warp-route message body layout (TokenMessage): [recipient (32 bytes)][amount (32 bytes, big-endian U256)][metadata].
+            // max_capacity is u64, so we read the amount as the low 8 bytes of the U256
+            // (body[56..64]) and require the high 24 bytes (body[32..56]) to be zero.
+            // A transfer whose amount doesn't fit in u64 would exceed any realistic
+            // capacity and is rejected — AmountRouting reads the full 32 bytes because
+            // it only needs ordering, not arithmetic.
             if message.body.len() < 64 {
                 return Err(Error::InvalidMessageBody.into());
             }

rust/sealevel/programs/ism/composite-ism/tests/functional_nested.rs

@@ -859,7 +859,7 @@ async fn test_rate_limited_in_domain_pda_readonly_bypass_rejected() {
         &result,
         TransactionError::InstructionError(
             0,
-            InstructionError::Custom(Error::AccountOutOfOrder as u32),
+            InstructionError::Custom(Error::DomainPdaNotWritable as u32),
         ),
     );
 }