fix(router-multicall): resolve #903, #904, #905, #906

#903: remove duplicated/contradictory doc block on admin() — replace
the double-entry (one claiming Panics, one claiming Errors) with a
single clean doc comment that accurately reflects the Result return.

#904: extend module-level Features list to mention simulate (dry-run),
fail_fast (abort on first optional failure), and max_total_gas (pre-
flight cumulative budget cap) — all core execute_batch behaviours that
were documented on the function but omitted from the crate overview.

#905: extract the 'budget_exceeded' and 'invoke_failed' string literals
into named constants FAILURE_REASON_BUDGET_EXCEEDED and
FAILURE_REASON_INVOKE_FAILED in router-common. Replace all inline
occurrences in execute_batch and the budget_failure_count test helper
so there is a single source of truth.

#906: build CallResult once per iteration. Store the value to ledger
before passing it to record_success/record_failure rather than
constructing an identical second struct literal for the store_results
path. CallResult already derives Clone; the second construction was
an unintentional copy-paste duplicate.

Author: Young850

contracts/router-common/src/lib.rs

@@ -224,6 +224,20 @@ pub const EVENT_ROLE_ADMIN_SET: &str = "role_admin_set";
 /// Standard event topic for an address being un-blacklisted
 pub const EVENT_ADDRESS_UNBLACKLISTED: &str = "address_unblacklisted";
 
+/// Failure reason string used when a call fails and an `instruction_budget` was set.
+///
+/// Passed as the payload of [`BatchItemError::Custom`] to distinguish budget-related
+/// failures from generic invocation failures. Use this constant instead of the raw
+/// string literal so that production code and tests share a single source of truth.
+pub const FAILURE_REASON_BUDGET_EXCEEDED: &str = "budget_exceeded";
+
+/// Failure reason string used when a call fails and no `instruction_budget` was set.
+///
+/// Passed as the payload of [`BatchItemError::Custom`] for generic invocation failures.
+/// Use this constant instead of the raw string literal so that production code and
+/// tests share a single source of truth.
+pub const FAILURE_REASON_INVOKE_FAILED: &str = "invoke_failed";
+
 /// Standard event topic for a role grant (pending or direct)
 pub const EVENT_ROLE_GRANT: &str = "role_grant";
 

contracts/router-multicall/src/lib.rs

@@ -10,6 +10,9 @@
 //! - Per-call success/failure tracking (non-atomic mode)
 //! - Atomic mode: revert all if any call fails
 //! - Call result storage for async inspection
+//! - `simulate` — dry-run mode: execute all calls without incrementing the batch counter
+//! - `fail_fast` — abort the batch on the first optional-call failure
+//! - `max_total_gas` — pre-flight cumulative instruction-budget cap to reject over-budget batches before execution
 //!
 //! ## Events (following naming convention: past tense verbs in snake_case)
 //! - `call_result` — Individual call result logged (caller, target, function, success)
@@ -278,34 +281,30 @@ impl RouterMulticall {
                 success,
             };
 
+            if store_results && !simulate {
+                env.storage().instance().set(
+                    &DataKey::BatchResult(batch_id, call_index),
+                    &call_result,
+                );
+            }
+
             if success {
                 result.record_success(call_index, call_result);
             } else {
                 let failure_error = if call.instruction_budget.is_some() {
                     router_common::BatchItemError::Custom(soroban_sdk::String::from_str(
                         &env,
-                        "budget_exceeded",
+                        router_common::FAILURE_REASON_BUDGET_EXCEEDED,
                     ))
                 } else {
                     router_common::BatchItemError::Custom(soroban_sdk::String::from_str(
                         &env,
-                        "invoke_failed",
+                        router_common::FAILURE_REASON_INVOKE_FAILED,
                     ))
                 };
                 result.record_failure(call_index, failure_error);
             }
 
-            if store_results && !simulate {
-                env.storage().instance().set(
-                    &DataKey::BatchResult(batch_id, call_index),
-                    &router_common::CallResult {
-                        target: call.target.clone(),
-                        function: call.function.clone(),
-                        success,
-                    },
-                );
-            }
-
             env.events().publish(
                 (Symbol::new(&env, router_common::EVENT_CALL_RESULT),),
                 (&caller, &call.target, &call.function, success, call_index),
@@ -437,21 +436,16 @@ impl RouterMulticall {
             .ok_or(MulticallError::NotInitialized)
     }
 
-    /// Get current admin.
+    /// Get the current admin address.
     ///
     /// # Arguments
     /// * `env` - The Soroban environment.
     ///
     /// # Returns
     /// The [`Address`] of the current admin.
     ///
-    /// # Panics
-    /// * Panics if the contract has not been initialized.
-    ///
-    /// Get the current admin address.
-    ///
     /// # Errors
-    /// Returns `MulticallError::NotInitialized` if the contract has not been initialized.
+    /// * [`MulticallError::NotInitialized`] — if the contract has not been initialized.
     pub fn admin(env: Env) -> Result<Address, MulticallError> {
         env.storage()
             .instance()
@@ -603,7 +597,7 @@ mod tests {
     }
 
     fn budget_failure_count(env: &Env, result: &router_common::BatchCallResult) -> u32 {
-        let budget_msg = soroban_sdk::String::from_str(env, "budget_exceeded");
+        let budget_msg = soroban_sdk::String::from_str(env, router_common::FAILURE_REASON_BUDGET_EXCEEDED);
         let mut count = 0u32;
         for i in 0..result.failures.len() {
             let failure = result.failures.get(i).unwrap();