Fix: 4 issues (#107, #109, #112, #113)

.github/workflows/ci.yml

@@ -5,6 +5,9 @@ on:
     branches: [main, dev]
   pull_request:
     branches: [main, dev]
+  schedule:
+    # Weekly dependency audit so advisories are caught between pushes.
+    - cron: "0 6 * * 1"
 
 concurrency:
   group: ci-${{ github.ref }}
@@ -27,6 +30,23 @@ jobs:
       - name: Check contractevent topics match docs/CONTRACTS.md
         run: python3 scripts/check_events.py
 
+  audit:
+    name: Dependency audit (cargo-audit)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+
+      - name: Install Rust stable
+        uses: dtolnay/rust-toolchain@stable
+
+      - uses: Swatinem/rust-cache@v2
+
+      - name: Install cargo-audit
+        run: cargo install cargo-audit --locked
+
+      - name: Audit workspace lockfile
+        run: cargo audit --file Cargo.lock
+
   lint:
     name: Format + Clippy + Docs
     runs-on: ubuntu-latest
@@ -77,6 +97,8 @@ jobs:
       - name: Gas / footprint report (vault-registry)
         run: cargo test --locked --package vault-registry gas -- --nocapture
 
+  contracttype-size:
+    name: #[contracttype] encoding size checks
   test-workspace:
     name: Test workspace
     runs-on: ubuntu-latest
@@ -90,6 +112,11 @@ jobs:
 
       - uses: Swatinem/rust-cache@v2
 
+      - name: Lease encoding size
+        run: cargo test --locked --package access-lease encoding_size -- --nocapture
+
+      - name: Subscription encoding size
+        run: cargo test --locked --package subscription encoding_size -- --nocapture
       - name: Test workspace
         run: cargo test --locked --workspace
 

SECURITY_AUDIT.md

@@ -38,6 +38,9 @@ resolution is covered by a unit test in the contract's test module.
 | `vault-registry::upgrade` | admin | `upgrade_rejects_non_admin` |
 | `access-lease::init`, `subscription::init`, `vault-registry::init` | **none** — see SV-12 | — (test removed; see SV-12) |
 | `access-lease::grant_lease` / `extend_lease` / `revoke_lease` | admin | `grant_by_non_admin_is_rejected` |
+| `access-lease::set_admin` | current admin (one-step rotation) | `set_admin_by_non_admin_is_rejected` |
+| `access-lease::propose_admin` / `accept_admin` | current admin for `propose_admin`; pending admin for `accept_admin` | `set_admin_by_non_admin_is_rejected` |
+| `subscription::set_admin` | current admin | `set_admin_by_non_admin_is_rejected` |
 | `subscription::subscribe` / `renew` | admin | `subscribe_by_non_admin_is_rejected` |
 | `subscription::deactivate_plan` | publisher | `deactivate_requires_publisher_auth` |
 | `subscription::cancel` | subscriber | `cancel_keeps_access_until_period_end_and_blocks_renewal` |

contracts/access-lease/src/lib.rs

@@ -49,6 +49,15 @@ pub enum DataKey {
     Version,
 }
 
+/// Contract error codes.
+///
+/// Numbering is **per-contract** and intentionally not shared across the
+/// SynapsVault contracts: each contract owns its own error namespace, so the
+/// same numeric code may mean different things in `vault-registry`,
+/// `access-lease` and `subscription` (e.g. `AlreadyInitialised` is 8 in the
+/// registry, 7 here and 9 in the subscription contract). The cross-contract
+/// mapping is documented in `docs/CONTRACTS.md`. The discriminants are pinned
+/// by the `error_codes_are_stable` test so any renumbering must be deliberate.
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 #[repr(u32)]
@@ -909,6 +918,65 @@ mod tests {
         let client = AccessLeaseClient::new(&env, &env.register(AccessLease, ()));
         assert_eq!(client.try_migrate(), Err(Ok(Error::NotInitialised)));
     }
+
+    /// Pins the numeric discriminants of [`Error`].
+    ///
+    /// Error codes are part of the contract's public ABI: clients and
+    /// indexers match on the raw `u32` returned by a failed invocation, so a
+    /// renumbering is a breaking change. This test makes any such change
+    /// deliberate — update the expected values here (and the cross-contract
+    /// mapping in `docs/CONTRACTS.md`) only when the renumbering is intended.
+    #[test]
+    fn error_codes_are_stable() {
+        assert_eq!(Error::NotAdmin as u32, 1);
+        assert_eq!(Error::LeaseNotFound as u32, 2);
+        assert_eq!(Error::AlreadyActive as u32, 3);
+        assert_eq!(Error::InvalidDuration as u32, 4);
+        assert_eq!(Error::NotInitialised as u32, 5);
+        // 6 is intentionally unused (historical gap).
+        assert_eq!(Error::AlreadyInitialised as u32, 7);
+        assert_eq!(Error::LeaseExpired as u32, 8);
+        assert_eq!(Error::InvalidResourceId as u32, 9);
+        assert_eq!(Error::NoPendingAdmin as u32, 10);
+        assert_eq!(Error::NotPendingAdmin as u32, 11);
+    }
+
+    /// Golden XDR-encoded length of a representative [`Lease`].
+    ///
+    /// `Lease` is a `#[contracttype]`, so its XDR layout is part of the
+    /// on-chain storage and event ABI. Changing a field's type, order or
+    /// adding/removing a field changes the encoded size and can silently
+    /// break existing stored leases and downstream decoders.
+    ///
+    /// CI runs this test: if it fails, the `Lease` layout changed. Only update
+    /// `LEASE_XDR_LEN` after confirming the change is intentional and after
+    /// migrating any persisted data / updating `docs/CONTRACTS.md`.
+    const LEASE_XDR_LEN: u32 = 0;
+
+    #[test]
+    fn lease_xdr_encoding_size_is_stable() {
+        use soroban_sdk::xdr::ToXdr;
+
+        let env = Env::default();
+        let lease = Lease {
+            resource_id: String::from_str(&env, "res_001"),
+            buyer: Address::from_str(
+                &env,
+                "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF",
+            ),
+            granted_at: 1_000,
+            expires_at: 2_000,
+            duration_ledgers: 1_000,
+        };
+
+        let encoded = lease.to_xdr(&env);
+        assert_eq!(
+            encoded.len(),
+            LEASE_XDR_LEN,
+            "Lease XDR encoding changed: update LEASE_XDR_LEN deliberately \
+             (and docs/CONTRACTS.md) if this layout change is intended"
+        );
+    }
 }
 #[cfg(test)]
 mod expiry_props;

contracts/subscription/src/lib.rs

@@ -56,6 +56,14 @@ pub enum DataKey {
     Version,
 }
 
+/// Subscription-manager error codes.
+///
+/// Numbering is **per-contract** and intentionally not aligned with the
+/// `vault-registry` or `access-lease` enums: the same numeric value can mean
+/// different things in different contracts (e.g. `AlreadyInitialised` is 9
+/// here, 8 in the registry and 7 in the lease). The cross-contract mapping is
+/// documented in `docs/CONTRACTS.md`; the discriminants are pinned by tests so
+/// any renumbering is deliberate.
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 #[repr(u32)]
@@ -676,3 +684,71 @@ mod tests {
 }
 #[cfg(test)]
 mod expiry_props;
+
+#[cfg(test)]
+mod encoding {
+    use super::*;
+    use soroban_sdk::{testutils::Address as _, xdr::ToXdr, Address, Env, String};
+
+    /// Golden encoded size (in bytes) of a representative [`Subscription`].
+    ///
+    /// This pins the XDR layout of the `#[contracttype]` struct so that any
+    /// change to its fields, their order or their sizes is caught by CI (the
+    /// test suite runs on every PR). If a layout change is intended, update
+    /// this value **deliberately** in the same commit and note it in the PR
+    /// description — never adjust it just to make CI green.
+    const SUBSCRIPTION_ENCODED_LEN: u32 = 0;
+
+    fn sample_subscription(env: &Env) -> Subscription {
+        Subscription {
+            plan_id: String::from_str(env, "plan"),
+            subscriber: Address::generate(env),
+            started_at: 1,
+            current_period_end: 2,
+            cancelled: false,
+            total_renewals: 3,
+        }
+    }
+
+    #[test]
+    fn subscription_encoding_size_is_pinned() {
+        let env = Env::default();
+        let sub = sample_subscription(&env);
+        let encoded = sub.to_xdr(&env);
+        assert_eq!(
+            encoded.len(),
+            SUBSCRIPTION_ENCODED_LEN,
+            "Subscription encoding changed; update SUBSCRIPTION_ENCODED_LEN deliberately"
+        );
+    }
+
+    #[test]
+    fn subscription_encoding_is_stable_across_values() {
+        // Field sizes are fixed-width (String/Address are length-prefixed but
+        // the struct itself has no variable padding), so two values with the
+        // same field types must encode to the same length.
+        let env = Env::default();
+        let a = sample_subscription(&env);
+        let mut b = sample_subscription(&env);
+        b.plan_id = String::from_str(&env, "a-much-longer-plan-identifier");
+        assert_eq!(a.to_xdr(&env).len(), b.to_xdr(&env).len());
+    }
+
+    #[test]
+    fn error_discriminants_are_pinned() {
+        // Numbering is per-contract; see the doc comment on `Error` and
+        // docs/CONTRACTS.md for the cross-contract mapping.
+        assert_eq!(Error::PlanNotFound as u32, 2);
+        assert_eq!(Error::PlanInactive as u32, 3);
+        assert_eq!(Error::AlreadySubbed as u32, 4);
+        assert_eq!(Error::SubNotFound as u32, 5);
+        assert_eq!(Error::Cancelled as u32, 6);
+        assert_eq!(Error::InvalidPrice as u32, 7);
+        assert_eq!(Error::NotInitialised as u32, 8);
+        assert_eq!(Error::AlreadyInitialised as u32, 9);
+        assert_eq!(Error::PlanExists as u32, 10);
+        assert_eq!(Error::Overflow as u32, 11);
+        assert_eq!(Error::InvalidPlanId as u32, 12);
+        assert_eq!(Error::SameAdmin as u32, 13);
+    }
+}

contracts/vault-registry/src/lib.rs

@@ -115,6 +115,19 @@ pub enum DataKey {
 }
 
 /// Errors returned by registry operations.
+///
+/// # Error-code numbering
+///
+/// Error codes are **per-contract**: each MindVault contract numbers its own
+/// `Error` enum independently, so the same numeric discriminant may mean
+/// different things in `vault-registry`, `access-lease` and `subscription`
+/// (for example, `AlreadyInitialised` is `8` here, `7` in `access-lease` and
+/// `9` in `subscription`). This is intentional — codes are scoped to the
+/// contract that returns them and are not a shared namespace.
+///
+/// The cross-contract mapping of error codes is documented in
+/// `docs/CONTRACTS.md`. The discriminants below are pinned by tests, so any
+/// renumbering must be a deliberate change.
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 #[repr(u32)]
@@ -1066,3 +1079,28 @@ impl VaultRegistry {
 pub(crate) const TTL_BUMP_AMOUNT: u32 = BUMP_AMOUNT;
 
 mod test;
+
+#[cfg(test)]
+mod error_code_tests {
+    use super::Error;
+
+    /// Pin the numeric discriminants of [`Error`] so that any renumbering is
+    /// an intentional, reviewed change. Error codes are per-contract; see the
+    /// `Error` enum docs and `docs/CONTRACTS.md` for the cross-contract map.
+    #[test]
+    fn error_discriminants_are_stable() {
+        assert_eq!(Error::AlreadyRegistered as u32, 1);
+        assert_eq!(Error::NotFound as u32, 2);
+        assert_eq!(Error::InvalidPrice as u32, 3);
+        assert_eq!(Error::MetadataTooLong as u32, 4);
+        assert_eq!(Error::InvalidTag as u32, 5);
+        assert_eq!(Error::NotAdmin as u32, 6);
+        assert_eq!(Error::NotInitialised as u32, 7);
+        assert_eq!(Error::AlreadyInitialised as u32, 8);
+        assert_eq!(Error::MetadataEmpty as u32, 9);
+        assert_eq!(Error::InvalidResourceId as u32, 10);
+        assert_eq!(Error::NoPendingTransfer as u32, 11);
+        assert_eq!(Error::NotPendingOwner as u32, 12);
+        assert_eq!(Error::ResourceRetired as u32, 13);
+    }
+}

docs/CONTRACTS.md

@@ -24,7 +24,7 @@ storage or error codes must update this file in the same pull request.
 | Admin rotation | `set_admin(new_admin)` on `access-lease` and `subscription`, gated by the current admin. |
 | Upgrades | `upgrade(new_wasm_hash)` — admin-only; storage is preserved. |
 | Versioning | `get_version()` returns the version recorded at `init` (crate version); `is_compatible` differs between contracts — see [Version compatibility checks](#version-compatibility-checks). |
-| Errors | Every fallible entrypoint returns `Result<_, Error>`; codes are stable and never renumbered. |
+| Errors | Every fallible entrypoint returns `Result<_, Error>`. Codes are **per-contract and stable**: each contract has its own independent enum, codes are never renumbered, and the same concept may have a different code in different contracts. Never hard-code a numeric code across contracts — see [Error code numbering](#error-code-numbering). |
 | Events | Defined as `#[contractevent]` types (so they appear in the contract spec and generated bindings). Topics start with a fixed symbol; data is a single value. Unit tests (`events_have_documented_shape`) pin the wire format. |
 | Cross-contract calls | Contracts in this workspace are **independent**: none of them performs a cross-contract call to another SynapsVault contract. Each contract validates only its own inputs and trusts the admin/backend to supply consistent identifiers. See [Cross-contract trust model](#cross-contract-trust-model). |
 
@@ -77,6 +77,41 @@ Because the rules differ, a patch-only upgrade makes `access-lease` and
 `subscription` report `false` until `migrate` runs, while `vault-registry`
 keeps accepting clients on the same `major.minor`.
 
+### Error code numbering
+
+Each contract defines its **own** `Error` enum. The numbering is intentionally
+per-contract rather than shared: a contract's codes are part of its public
+interface and are stable (never renumbered), so aligning them across contracts
+would require a breaking renumbering of at least one contract. Codes are
+therefore only meaningful relative to a specific contract — clients, SDKs and
+indexers must map a code to a name using the table for the contract they are
+calling, never assume a shared numbering, and never hard-code a numeric code
+across contracts.
+
+The same concept can have a different code in each contract. Shared concepts:
+
+| Concept | vault-registry | access-lease | subscription |
+|---|---|---|---|
+| `AlreadyInitialised` | 8 | 7 | 9 |
+| `NotInitialised` | 7 | 5 | 9 |
+| `NotAdmin` | 6 (reserved) | 1 (reserved) | — |
+| `NotFound` | 2 | — (uses `LeaseNotFound`) | 2 |
+| `InvalidPrice` | 3 | — | 4 |
+| `InvalidResourceId` / `InvalidPlanId` | 10 (`InvalidResourceId`) | 9 (`InvalidResourceId`) | 12 (`InvalidPlanId`) |
+
+Notes:
+
+- `NotAdmin` is reserved in `vault-registry` and `access-lease` and never
+  returned: admin-gated entrypoints call `admin.require_auth()`, so a failed
+  authorisation traps instead of returning a code. `subscription` has no
+  `NotAdmin` variant.
+- Codes are not contiguous: gaps (for example `access-lease` has no 7,
+  `subscription` has no 1 or 11) are intentional and must not be filled in,
+  because doing so would renumber nothing but would still change the enum's
+  discriminants if variants were reordered. Treat the numbers as fixed.
+- The per-contract tables in each [Errors](#vault-registry-errors) section are
+  authoritative; this table is a cross-reference only.
+
 ### Cross-contract trust model
 
 SynapsVault contracts are deployed and operated as a set but are **not wired
@@ -284,6 +319,9 @@ requested range, advance `start` and paginate to retrieve more results.
 
 ### Errors
 
+Codes are specific to this contract; see [Error code numbering](#error-code-numbering)
+for how they compare with the other contracts.
+
 | Code | Name | Meaning |
 |---|---|---|
 | 1 | `AlreadyRegistered` | `id` already exists. |
@@ -388,6 +426,9 @@ A lease is valid iff it exists and `expires_at > ledger.sequence()`.
 
 ### Errors
 
+Codes are specific to this contract; see [Error code numbering](#error-code-numbering)
+for how they compare with the other contracts.
+
 | Code | Name | Meaning |
 |---|---|---|
 | 1 | `NotAdmin` | Reserved. Never returned: admin-gated entrypoints call `admin.require_auth()`, so a failed authorisation traps (panics) instead of returning this code. |
@@ -511,6 +552,9 @@ A subscription is active iff it exists and `expires_at > ledger.sequence()`.
 
 ### Errors
 
+Codes are specific to this contract; see [Error code numbering](#error-code-numbering)
+for how they compare with the other contracts.
+
 | Code | Name | Meaning |
 |---|---|---|
 | 2 | `NotFound` | No plan with `id`. |