Merge pull request #930 from Stevieoche/fix/issues-884-887-886-885

refactor: replace magic literals with named constants and add is_deprecated view

Author: Maki-Zeninn

contracts/router-quote/src/lib.rs

@@ -91,6 +91,15 @@ pub enum QuoteError {
     InvalidFeeTier = 9,
 }
 
+// ── Constants ─────────────────────────────────────────────────────────────────
+
+/// Basis-points denominator: 10000 bps = 100%.
+///
+/// Used in every fee validation (`fee_bps > BPS_DENOMINATOR`) and in the
+/// fee calculation (`fee_amount = amount_in * fee_bps / BPS_DENOMINATOR`).
+/// A single source of truth prevents the literal from diverging across sites.
+const BPS_DENOMINATOR: u32 = 10_000;
+
 // ── Contract ──────────────────────────────────────────────────────────────────
 
 /// Maximum number of routes that can be tracked in the configured-routes index.
@@ -125,7 +134,7 @@ impl RouterQuote {
             return Err(QuoteError::AlreadyInitialized);
         }
 
-        if default_fee_bps > 10000 {
+        if default_fee_bps > BPS_DENOMINATOR {
             return Err(QuoteError::InvalidFeeBps);
         }
 
@@ -168,7 +177,7 @@ impl RouterQuote {
         caller.require_auth();
         router_common::require_admin_simple!(&env, &caller, &DataKey::Admin, QuoteError)?;
 
-        if fee_bps > 10000 {
+        if fee_bps > BPS_DENOMINATOR {
             return Err(QuoteError::InvalidFeeBps);
         }
 
@@ -241,7 +250,7 @@ impl RouterQuote {
             if tier.min_amount < 0 {
                 return Err(QuoteError::InvalidFeeTier);
             }
-            if tier.fee_bps > 10000 {
+            if tier.fee_bps > BPS_DENOMINATOR {
                 return Err(QuoteError::InvalidFeeBps);
             }
 
@@ -346,11 +355,11 @@ impl RouterQuote {
         let fee_bps =
             Self::resolve_route_fee_bps(env.clone(), request.route.clone(), request.amount_in)?;
 
-        // Calculate fee: fee_amount = amount_in * fee_bps / 10000
+        // Calculate fee: fee_amount = amount_in * fee_bps / BPS_DENOMINATOR
         let fee_amount = request
             .amount_in
             .checked_mul(fee_bps as i128)
-            .and_then(|v| v.checked_div(10000))
+            .and_then(|v| v.checked_div(BPS_DENOMINATOR as i128))
             .ok_or(QuoteError::ArithmeticOverflow)?;
 
         // Calculate output: amount_out = amount_in - fee_amount
@@ -509,7 +518,7 @@ impl RouterQuote {
         caller.require_auth();
         router_common::require_admin_simple!(&env, &caller, &DataKey::Admin, QuoteError)?;
 
-        if fee_bps > 10000 {
+        if fee_bps > BPS_DENOMINATOR {
             return Err(QuoteError::InvalidFeeBps);
         }
 

contracts/router-registry/src/lib.rs

@@ -76,6 +76,18 @@ pub enum RegistryError {
     InvalidHealthFn = 12,
 }
 
+// ── Constants ─────────────────────────────────────────────────────────────────
+
+/// Maximum byte length of a semver constraint string accepted by
+/// [`RouterRegistry::get_latest_with_constraint`].
+///
+/// `constraint_str_buf` copies the constraint into a fixed-size stack buffer
+/// of exactly this size. Both the length guard (`if len > MAX_CONSTRAINT_LEN`)
+/// and the buffer declaration (`[0u8; MAX_CONSTRAINT_LEN]`) must use this
+/// constant so they stay in sync — a mismatch would cause a panic on
+/// out-of-bounds indexing at runtime.
+const MAX_CONSTRAINT_LEN: usize = 32;
+
 // ── Contract ──────────────────────────────────────────────────────────────────
 
 #[contract]
@@ -297,6 +309,27 @@ impl RouterRegistry {
             .ok_or(RegistryError::NotFound)
     }
 
+    /// Check whether a specific version of a contract has been deprecated.
+    ///
+    /// This is a lightweight view alternative to calling [`get`] and reading
+    /// `.deprecated` off the full [`ContractEntry`]. Callers who only need to
+    /// know the deprecation status (e.g. a UI rendering a deprecation badge)
+    /// can avoid fetching and discarding the rest of the entry.
+    ///
+    /// # Arguments
+    /// * `env` - The Soroban environment.
+    /// * `name` - The human-readable name of the contract.
+    /// * `version` - The exact version number to query.
+    ///
+    /// # Returns
+    /// `true` if the entry is deprecated, `false` otherwise.
+    ///
+    /// # Errors
+    /// * [`RegistryError::NotFound`] — if no entry exists for `(name, version)`.
+    pub fn is_deprecated(env: Env, name: String, version: u32) -> Result<bool, RegistryError> {
+        Self::get(env, name, version).map(|entry| entry.deprecated)
+    }
+
     /// Get the latest (highest version) non-deprecated entry for a name.
     ///
     /// Iterates registered versions in descending order and returns the first
@@ -794,12 +827,12 @@ impl RouterRegistry {
     /// `soroban_sdk::String` doesn't implement `Display`/`ToString` on the wasm
     /// target, so constraint strings (which are always short) are read via
     /// `copy_into_slice` instead of allocating.
-    fn constraint_str_buf(constraint: &String) -> Result<([u8; 32], usize), RegistryError> {
+    fn constraint_str_buf(constraint: &String) -> Result<([u8; MAX_CONSTRAINT_LEN], usize), RegistryError> {
         let len = constraint.len() as usize;
-        if len > 32 {
+        if len > MAX_CONSTRAINT_LEN {
             return Err(RegistryError::InvalidConstraint);
         }
-        let mut buf = [0u8; 32];
+        let mut buf = [0u8; MAX_CONSTRAINT_LEN];
         constraint.copy_into_slice(&mut buf[..len]);
         Ok((buf, len))
     }
@@ -938,6 +971,36 @@ mod tests {
         assert!(!entry.deprecated);
     }
 
+    #[test]
+    fn test_is_deprecated_returns_false_for_fresh_registration() {
+        let (env, admin, client) = setup();
+        let name = String::from_str(&env, "oracle");
+        let addr = Address::generate(&env);
+        client.register(&admin, &name, &addr, &1);
+        assert!(!client.is_deprecated(&name, &1));
+    }
+
+    #[test]
+    fn test_is_deprecated_returns_true_after_deprecate() {
+        let (env, admin, client) = setup();
+        let name = String::from_str(&env, "oracle");
+        let addr = Address::generate(&env);
+        client.register(&admin, &name, &addr, &1);
+        client.deprecate(&admin, &name, &1, &None::<String>);
+        assert!(client.is_deprecated(&name, &1));
+    }
+
+    #[test]
+    fn test_is_deprecated_returns_not_found_for_unregistered_version() {
+        let (env, admin, client) = setup();
+        let name = String::from_str(&env, "oracle");
+        let addr = Address::generate(&env);
+        client.register(&admin, &name, &addr, &1);
+        // version 99 was never registered
+        let result = client.try_is_deprecated(&name, &99);
+        assert_eq!(result, Err(Ok(RegistryError::NotFound)));
+    }
+
     #[test]
     fn test_get_latest() {
         let (env, admin, client) = setup();