fix: live protocol fee, EVM chain validation, chain_id bounds, scoped auth

CHANGELOG.md

@@ -11,6 +11,12 @@ first deploys to mainnet.
 
 ### Fixed
 
+- **`fill_intent` now charges the live, admin-configured `protocol_fee_bps`**
+  (via `set_config`) instead of the compile-time `PROTOCOL_FEE_BPS`
+  constant, which previously made fee-rate changes silently have no effect.
+- `validate_src_token` now recognizes `"avalanche"` and `"bsc"` as EVM
+  chains, closing a gap where malformed source-token addresses on those two
+  chains passed with no format validation.
 - `deregister_solver` now refuses to return a solver's bond while they hold
   an `Accepted` intent, closing a path to dodge `slash_solver` by
   withdrawing before the fill window expired.

docs/auth-audit.md

@@ -0,0 +1,44 @@
+# `require_auth()` Call Site Audit
+
+Closes the "Authorization hardening" item in `docs/pre-deploy-security-checklist.md`
+(#45, tracked here as #263). Every `require_auth()` call site in
+`intent_settlement/src/lib.rs` was reviewed for whether upgrading to
+`require_auth_for_args` would meaningfully reduce delegated-execution risk —
+i.e. the risk that a third-party invoker contract calling on a signer's behalf
+could redirect their signature toward unintended arguments.
+
+## Upgraded
+
+| Function | Old | New scope | Rationale |
+|---|---|---|---|
+| `submit_intent` | `user.require_auth()` | `(user, dst_token, min_dst_amount)` | If a composable invoker ever submits on a user's behalf, this prevents it from redirecting the user's signed submission to a different destination token or minimum output. |
+| `accept_intent` | `solver.require_auth()` | `(intent_id,)` | Prevents a delegating invoker contract from having a solver accept a different intent than the one the solver actually signed for. |
+| `fill_intent` | `solver.require_auth()` | `(solver, intent_id, fill_amount)` | Highest-value call site — the auth gates an outgoing token transfer. Prevents a delegating invoker from filling a different intent, or a different amount, than the solver signed for. |
+
+`accept_intent`'s batch wrapper (`accept_intent_batch`) delegates to
+`accept_intent` per element and needed no separate change.
+
+## Kept as `require_auth()`
+
+| Function | Signer | Rationale |
+|---|---|---|
+| `initialize` | `admin` | One-time setup; the signer *is* the value being recorded as admin — no sub-scope to narrow. |
+| `propose_fee_recipient` | stored `admin` | Single global admin capability; no meaningful sub-scope within "being admin". |
+| `accept_fee_recipient` | `new_fee_recipient` | Recipient proves ownership of their own address; the timelock and pending-proposal match (`pending != new_fee_recipient` check) already constrain which proposal can be accepted. |
+| `propose_admin_transfer` | stored `admin` | Same as `propose_fee_recipient`. |
+| `accept_admin_transfer` | `new_admin` | Same as `accept_fee_recipient`. |
+| `register_solver` | `solver` | Solver consents to locking their own bond funds; simple self-action with no delegated-execution surface. |
+| `deregister_solver` | `solver` | Solver-only self-action. |
+| `withdraw_bond` | `solver` | Solver-only self-action on their own bond. |
+| `cancel_intent` | `user` | Simple "cancel my own intent" self-action; an explicit `intent.user != user` ownership check runs immediately after, providing defence-in-depth. |
+| `request_extension` | `solver` | Grants at most one grace-period extension per intent; no funds move and no cross-intent redirection is possible (the intent is loaded and ownership-checked before use). |
+| `require_admin` (helper; gates `unpause`, `set_pauser`, dst-token allowlist admin functions) | `admin` | Single admin address with uniform authority across these functions — no per-argument capability to scope. |
+| `require_admin_or_pauser` (helper; gates `pause`) | `admin` or `pauser` | Same reasoning as `require_admin`; the admin/pauser check already precedes the auth call. |
+
+## Integration impact
+
+`require_auth_for_args` changes the exact signed-payload shape a client must
+build. `submit_intent`, `accept_intent`, and `fill_intent` are called
+respectively by user-facing clients and solver bots — see
+`docs/solver-integration-guide.md` for the updated payload shapes solver bot
+authors must sign.

docs/pre-deploy-security-checklist.md

@@ -18,9 +18,11 @@ checked off as their linked PR is merged into `main`.
 
 ### Authorization hardening
 
-- [ ] **#45 — Audit all `require_auth()` call sites for correctness vs. Soroban's `require_auth_for_args`**
+- [x] **#45 — Audit all `require_auth()` call sites for correctness vs. Soroban's `require_auth_for_args`**
   Review all 12 call sites; document per-function conclusion; upgrade any site
   where scoped authorization meaningfully reduces delegated-execution risk.
+  See `docs/auth-audit.md`. `submit_intent`, `accept_intent`, and `fill_intent`
+  upgraded to `require_auth_for_args`; all other sites kept as-is.
 
 ### Economic / bond sizing
 

docs/solver-integration-guide.md

@@ -215,6 +215,16 @@ Reject the intent immediately if:
 
 ## The Accept → Fill Loop
 
+> **Scoped authorization (2026 update):** `accept_intent` and `fill_intent`
+> now call `require_auth_for_args` instead of `require_auth`, scoped to
+> `(intent_id)` and `(solver, intent_id, fill_amount)` respectively (see
+> `docs/auth-audit.md`). If you invoke directly via `stellar contract invoke`
+> or the standard SDK contract client with your own solver key, this is
+> transparent — the simulated auth entries are signed for you as before. It
+> only matters if you construct and sign `SorobanAuthorizationEntry` values
+> by hand (e.g. for a delegated/invoker-contract flow): the signed payload
+> must now match the specific call's arguments, not just the function name.
+
 ### Step 1 — Accept
 
 Call `accept_intent` to claim the exclusive 5-minute fill window:

intent_settlement/src/lib.rs

@@ -8,7 +8,7 @@
 
 use soroban_sdk::{
     contract, contracterror, contractimpl, contracttype, panic_with_error, token, xdr::ToXdr,
-    Address, Bytes, BytesN, Env, String, Symbol, Vec,
+    Address, Bytes, BytesN, Env, IntoVal, String, Symbol, Vec,
 };
 
 #[cfg(test)]
@@ -1176,13 +1176,18 @@ impl IntentSettlement {
         min_dst_amount: i128,
         deadline: Option<u64>,
     ) -> BytesN<32> {
-        // Auth audit: require_auth() is correct. The user must sign to assert
-        // ownership of the address receiving output tokens (dst). If a third-party
-        // contract were ever to call submit_intent on a user's behalf, switching to
-        // require_auth_for_args scoped to (user, dst_token, min_dst_amount) would
-        // limit the scope of delegated authorisation — noted as a future hardening
-        // opportunity if composable intent submission is added.
-        user.require_auth();
+        // Auth audit (see docs/auth-audit.md): upgraded from require_auth() to
+        // require_auth_for_args, scoped to (user, dst_token, min_dst_amount),
+        // so a delegating invoker contract cannot redirect a user's signed
+        // submission toward a different destination token or minimum output.
+        user.require_auth_for_args(Vec::from_array(
+            &env,
+            [
+                user.into_val(&env),
+                dst_token.into_val(&env),
+                min_dst_amount.into_val(&env),
+            ],
+        ));
         Self::require_not_paused(&env);
         Self::bump_instance_ttl(&env);
 
@@ -1324,12 +1329,10 @@ impl IntentSettlement {
 
     /// Solver claims an intent (exclusive fill right for FILL_WINDOW seconds)
     pub fn accept_intent(env: Env, solver: Address, intent_id: BytesN<32>) {
-        // Auth audit: require_auth() is correct. The solver must sign to
-        // voluntarily take on the fill obligation and bond risk associated with
-        // this intent. require_auth_for_args scoped to intent_id could prevent a
-        // malicious invoker contract from accepting an unintended intent on the
-        // solver's behalf; noted as a future hardening opportunity.
-        solver.require_auth();
+        // Auth audit (see docs/auth-audit.md): upgraded from require_auth() to
+        // require_auth_for_args, scoped to intent_id, so a delegating invoker
+        // contract cannot accept an unintended intent on the solver's behalf.
+        solver.require_auth_for_args(Vec::from_array(&env, [intent_id.into_val(&env)]));
         Self::require_not_paused(&env);
         Self::bump_instance_ttl(&env);
 
@@ -1418,14 +1421,20 @@ impl IntentSettlement {
     /// The protocol fee is taken on each individual fill so the fee accounting
     /// stays consistent regardless of how many fills it takes.
     pub fn fill_intent(env: Env, solver: Address, intent_id: BytesN<32>, fill_amount: i128) {
-        // Auth audit: require_auth() is correct. The solver must sign to
-        // authorise the token transfer from their address to the user and fee
-        // recipient. This is the highest-value call site: the solver authorises
-        // a token transfer, so the auth is load-bearing. require_auth_for_args
-        // scoped to (solver, intent_id, fill_amount) would meaningfully tighten
-        // the scope if a delegated-execution pattern is ever introduced — noted
-        // as the strongest candidate for future hardening.
-        solver.require_auth();
+        // Auth audit (see docs/auth-audit.md): upgraded from require_auth() to
+        // require_auth_for_args, scoped to (solver, intent_id, fill_amount) —
+        // the highest-value call site, since it authorises a token transfer.
+        // This closes the gap where a delegating invoker contract could
+        // authorise a fill of different size or against a different intent
+        // than the solver actually signed for.
+        solver.require_auth_for_args(Vec::from_array(
+            &env,
+            [
+                solver.into_val(&env),
+                intent_id.into_val(&env),
+                fill_amount.into_val(&env),
+            ],
+        ));
         Self::require_not_paused(&env);
         Self::bump_instance_ttl(&env);
 
@@ -1458,12 +1467,14 @@ impl IntentSettlement {
             panic_with_error!(&env, Error::ZeroAmount);
         }
 
+        let protocol_fee_bps = Self::load_config(&env).protocol_fee_bps;
+
         // Deliver this fill's tokens to the user.
         let dst_client = token::Client::new(&env, &intent.dst_token);
         dst_client.transfer(&solver, &intent.user, &fill_amount);
 
         // Solver also pays the protocol fee on each fill.
-        let fee = fill_amount * PROTOCOL_FEE_BPS / 10_000;
+        let fee = fill_amount * protocol_fee_bps / 10_000;
         // ── Effects first (CEI) ──────────────────────────────────────────────
         // Mark the intent Filled and write every state change to storage
         // *before* any external token transfer executes. A hostile SEP-41
@@ -1482,7 +1493,7 @@ impl IntentSettlement {
         // visible in code, rather than relying solely on the Cargo.toml
         // overflow-checks = true release-profile setting (issue #31).
         let fee = fill_amount
-            .checked_mul(PROTOCOL_FEE_BPS)
+            .checked_mul(protocol_fee_bps)
             .unwrap_or_else(|| panic_with_error!(&env, Error::FeeOverflow))
             .checked_div(10_000)
             .unwrap_or_else(|| panic_with_error!(&env, Error::FeeOverflow));
@@ -1567,7 +1578,7 @@ impl IntentSettlement {
         // fee from the solver — rather than clawing it back from the user — keeps
         // the user's received amount at or above `min_dst_amount`, and keeps every
         // token transfer authorized by the solver who signed this call.
-        let fee = fill_amount * PROTOCOL_FEE_BPS / 10_000;
+        let fee = fill_amount * protocol_fee_bps / 10_000;
         if fee > 0 {
             let fee_recipient: Address = env
                 .storage()
@@ -2164,7 +2175,9 @@ impl IntentSettlement {
             || chain_is(b"base")
             || chain_is(b"polygon")
             || chain_is(b"arbitrum")
-            || chain_is(b"optimism");
+            || chain_is(b"optimism")
+            || chain_is(b"avalanche")
+            || chain_is(b"bsc");
 
         if is_evm {
             // EVM token address: exactly "0x" + 40 hex chars = 42 characters.

intent_settlement/src/test.rs

@@ -10,8 +10,8 @@ use crate::{
     FILL_WINDOW, INTENT_EXPIRY, MIN_BOND, ADMIN_TIMELOCK_DELAY,
 };
 use soroban_sdk::{
-    testutils::{Address as _, Ledger},
-    token, Address, BytesN, Env, String, Symbol,
+    testutils::{Address as _, Ledger, MockAuth, MockAuthInvoke},
+    token, Address, BytesN, Env, IntoVal, String, Symbol,
 };
 
 // ─── Test fixture ───────────────────────────────────────────────────────────────
@@ -1687,6 +1687,37 @@ fn solver_record_consistent_with_token_balances_after_register() {
     assert_eq!(ctx.bond().balance(&ctx.solver), 0);
 }
 
+// #263 — fill_intent now scopes its auth to (solver, intent_id, fill_amount)
+// via require_auth_for_args. A signed auth entry bound to a different
+// intent_id than the one actually being filled — the shape a delegating
+// invoker contract could otherwise exploit — must be rejected.
+#[test]
+fn fill_intent_delegated_auth_wrong_intent_id_rejected() {
+    let ctx = setup();
+    let c = ctx.client();
+
+    ctx.register_solver();
+    let id = ctx.submit();
+    c.accept_intent(&ctx.solver, &id);
+
+    let wrong_id = BytesN::from_array(&ctx.env, &[0u8; 32]);
+    let fee = FILL * 5 / 10_000;
+    ctx.dst_admin().mint(&ctx.solver, &(FILL + fee));
+
+    ctx.env.mock_auths(&[MockAuth {
+        address: &ctx.solver,
+        invoke: &MockAuthInvoke {
+            contract: &ctx.contract_id,
+            fn_name: "fill_intent",
+            args: (ctx.solver.clone(), wrong_id, FILL).into_val(&ctx.env),
+            sub_invokes: &[],
+        },
+    }]);
+
+    let res = c.try_fill_intent(&ctx.solver, &id, &FILL);
+    assert!(res.is_err());
+}
+
 // #26 — CEI ordering in fill_intent: state is committed before transfers.
 //
 // We verify two complementary properties:
@@ -2122,6 +2153,32 @@ fn fill_intent_fee_at_boundary_does_not_overflow() {
     assert!(c.get_intent(&id).unwrap().state == IntentState::Filled);
 }
 
+// #260 — fill_intent must charge the live, admin-configured protocol fee
+// rate, not the compile-time PROTOCOL_FEE_BPS constant.
+#[test]
+fn fill_intent_honors_set_config_protocol_fee() {
+    let ctx = setup();
+    let c = ctx.client();
+
+    // Set a non-default fee rate (1% = 100 bps) via set_config, keeping the
+    // other three parameters at their existing defaults.
+    let new_fee_bps: i128 = 100;
+    c.set_config(&MIN_BOND, &FILL_WINDOW, &INTENT_EXPIRY, &new_fee_bps);
+
+    ctx.register_solver();
+    let id = ctx.submit();
+    c.accept_intent(&ctx.solver, &id);
+
+    let fee = FILL * new_fee_bps / 10_000;
+    ctx.dst_admin().mint(&ctx.solver, &(FILL + fee));
+
+    c.fill_intent(&ctx.solver, &id, &FILL);
+
+    assert_eq!(ctx.dst().balance(&ctx.user), FILL);
+    assert_eq!(ctx.dst().balance(&ctx.fee_recipient), fee);
+    assert_eq!(ctx.dst().balance(&ctx.solver), 0);
+}
+
 // ─── Issue #32: tiny bond slash floor ────────────────────────────────────────────
 
 /// #32: When a solver's bond has been whittled to a very small value (< 10 in
@@ -2655,6 +2712,47 @@ fn valid_evm_token_lowercase_accepted() {
     );
 }
 
+/// Well-formed EVM addresses on "avalanche" and "bsc" are accepted — these
+/// two chains were previously missing from the `is_evm` check (#261).
+#[test]
+fn valid_evm_token_avalanche_and_bsc_accepted() {
+    let ctx = setup();
+    for chain_str in ["avalanche", "bsc"] {
+        let deadline: Option<u64> = None;
+        ctx.client().submit_intent(
+            &ctx.user,
+            &String::from_str(&ctx.env, chain_str),
+            &String::from_str(&ctx.env, "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"),
+            &SRC_AMT,
+            &ctx.dst_token,
+            &MIN_DST,
+            &deadline,
+        );
+    }
+}
+
+/// A malformed token address on "avalanche" or "bsc" must be rejected, just
+/// like the other five EVM chains (#261 regression test).
+#[test]
+fn malformed_evm_token_avalanche_and_bsc_rejected() {
+    let ctx = setup();
+    for chain_str in ["avalanche", "bsc"] {
+        let deadline: Option<u64> = None;
+        let res = ctx.client().try_submit_intent(
+            &ctx.user,
+            &String::from_str(&ctx.env, chain_str),
+            // No "0x" prefix — would previously fall through as "unknown
+            // chain: skip validation" and pass.
+            &String::from_str(&ctx.env, "A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"),
+            &SRC_AMT,
+            &ctx.dst_token,
+            &MIN_DST,
+            &deadline,
+        );
+        assert_eq!(res, Err(Ok(Error::InvalidSrcToken.into())));
+    }
+}
+
 /// Missing "0x" prefix on an EVM chain is rejected with InvalidSrcToken.
 #[test]
 fn evm_token_without_0x_prefix_rejected() {

proof_registry/src/lib.rs

@@ -97,6 +97,10 @@ pub enum Error {
     InvalidPayload = 6,
     /// Contract not initialized (`Admin` key absent).
     NotInitialized = 7,
+    /// `set_authorized_emitter` called with a `chain_id` that exceeds
+    /// `u16::MAX` — the real Wormhole chain-ID space is 16 bits, so such a
+    /// value can never appear in a decoded VAA.
+    ChainIdOutOfRange = 8,
 }
 
 // ─── Contract ─────────────────────────────────────────────────────────────────
@@ -128,6 +132,9 @@ impl ProofRegistry {
     /// accepted by `receive_message`.
     pub fn set_authorized_emitter(env: Env, chain_id: u32, emitter: BytesN<32>) {
         Self::require_admin(&env);
+        if chain_id > u16::MAX as u32 {
+            panic_with_error!(&env, Error::ChainIdOutOfRange);
+        }
         env.storage()
             .instance()
             .set(&ProofKey::AuthorizedEmitter(chain_id), &emitter);

proof_registry/src/test.rs

@@ -128,6 +128,28 @@ fn get_authorized_emitter_returns_none_if_unset() {
     assert_eq!(ctx.client().get_authorized_emitter(&2), None);
 }
 
+// #262 — the real Wormhole chain-ID space is 16 bits; chain_id must be
+// rejected once it exceeds u16::MAX.
+#[test]
+fn set_authorized_emitter_accepts_u16_max_boundary() {
+    let ctx = setup();
+    let c = ctx.client();
+    let emitter: BytesN<32> = BytesN::from_array(&ctx.env, &[0xde; 32]);
+
+    c.set_authorized_emitter(&(u16::MAX as u32), &emitter);
+    assert_eq!(c.get_authorized_emitter(&(u16::MAX as u32)), Some(emitter));
+}
+
+#[test]
+fn set_authorized_emitter_rejects_above_u16_max() {
+    let ctx = setup();
+    let c = ctx.client();
+    let emitter: BytesN<32> = BytesN::from_array(&ctx.env, &[0xde; 32]);
+
+    let res = c.try_set_authorized_emitter(&(u16::MAX as u32 + 1), &emitter);
+    assert_eq!(res, Err(Ok(Error::ChainIdOutOfRange.into())));
+}
+
 #[test]
 fn remove_authorized_emitter_clears_entry() {
     let ctx = setup();