Accounts Refactor PR 3: Addinng more call messages to accounts module

CHANGELOG.md

@@ -50,6 +50,7 @@
 
 # MULTISIG UPGRADE
 Temporary section for maintaining breaking changes from individual PRs, which will be consolidated into a single changelog entry when the feature branch is merged into `dev`.
+- **Breaking change (code, wire, consensus)**: `sov_accounts::CallMessage` gains three variants — `AddCredentialToAddress { address, credential }`, `RemoveCredentialFromAddress { address, credential }`, and `RotateCredentialOnAddress { address, old_credential, new_credential }` — and is now generic in `S: Spec`. The enum's wire format changes, which changes the chain hash. All three new variants require `context.sender() == address` (callers can only modify credentials on the address they are currently signing as); `InsertCredentialId` semantics are unchanged. `Rotate` is functionally equivalent to a `Remove` + `Add` collapsed into a single call for key rotation. There is no orphan guard on remove.
 - #2688 **Breaking change(code, state, consensus)**: The transaction formats have changed in this version. This is a consensus breaking change and requires coordinating an upgrade using `--stop-at-rollup-height`, and using sov-rollup-manager for full resyncs from this point onwards for existing rollups. This fixes Credential ID malleability that allowed the same set of signed bytes to be replayed for different credentials, across V0 and V1 (multisig) transactions and with different V1 multisig parameters.
   * The on-chain transaction data has changed, and multisig transactions now explicitly include the multisig id as part of the signed bytes.
   * Previous signatures are no longer valid. Clients will need to upgrade to our latest SDKs to be able to sign transactions in the new format.

Cargo.toml

@@ -101,6 +101,7 @@ members = [
     "crates/web3",
     "python/py_sovereign_web3/rust",
 ]
+default-members = ["crates/rollup-interface", "crates/adapters/mock-da", "crates/adapters/mock-zkvm", "crates/full-node/sov-blob-sender", "crates/full-node/sov-db", "crates/full-node/sov-sequencer", "crates/full-node/sov-ledger-apis", "crates/full-node/sov-rollup-apis", "crates/full-node/sov-rollup-full-node-interface", "crates/full-node/sov-stf-runner", "crates/full-node/sov-metrics", "crates/full-node/sov-api-spec", "crates/full-node/full-node-configs", "crates/utils/sov-rest-utils", "crates/utils/nearly-linear", "crates/utils/sov-zkvm-utils", "crates/utils/sov-build", "crates/module-system/hyperlane", "crates/module-system/sov-cli", "crates/module-system/sov-modules-stf-blueprint", "crates/module-system/sov-modules-rollup-blueprint", "crates/module-system/sov-modules-macros", "crates/module-system/sov-kernels", "crates/module-system/sov-state", "crates/module-system/sov-modules-api", "crates/module-system/sov-address", "crates/utils/sov-test-utils", "crates/utils/sov-evm-test-utils", "crates/module-system/module-implementations/sov-accounts", "crates/module-system/module-implementations/sov-bank", "crates/module-system/module-implementations/sov-chain-state", "crates/module-system/module-implementations/sov-blob-storage", "crates/module-system/module-implementations/sov-evm", "crates/module-system/module-implementations/sov-paymaster", "crates/module-system/module-implementations/sov-prover-incentives", "crates/module-system/module-implementations/sov-attester-incentives", "crates/module-system/module-implementations/sov-sequencer-registry", "crates/module-system/module-implementations/sov-value-setter", "crates/module-system/module-implementations/sov-revenue-share", "crates/module-system/module-implementations/sov-synthetic-load", "crates/module-system/module-implementations/module-template", "crates/module-system/module-implementations/integration-tests", "crates/module-system/sov-capabilities", "crates/module-system/module-implementations/sov-uniqueness", "crates/module-system/module-implementations/sov-timelock", "crates/utils/sov-node-client", "crates/universal-wallet/schema", "crates/universal-wallet/macros", "crates/universal-wallet/macro-helpers", "crates/utils/workspace-hack", "crates/web3", "python/py_sovereign_web3/rust", "crates/module-system/module-implementations/extern/hyperlane-solana-register"]
 [workspace.package]
 version = "0.3.0"
 edition = "2021"

crates/fuzz/fuzz_targets/accounts_call_random.rs

@@ -9,9 +9,10 @@ use sov_test_utils::storage::SimpleStorageManager;
 use sov_test_utils::TestStorageSpec;
 
 type S = sov_test_utils::TestSpec;
+type FuzzInput<'a> = (&'a [u8], Vec<(Context<S>, CallMessage<S>)>);
 
 // Check arbitrary, random calls
-fuzz_target!(|input: (&[u8], Vec<(Context<S>, CallMessage)>)| {
+fuzz_target!(|input: FuzzInput| {
     let storage_manager = SimpleStorageManager::<TestStorageSpec>::new();
     let storage = storage_manager.create_storage();
     let mut state = StateCheckpoint::new(storage, &MockKernel::<S>::default(), None);

crates/fuzz/fuzz_targets/accounts_parse_call_message.rs

@@ -3,8 +3,10 @@
 use libfuzzer_sys::fuzz_target;
 use sov_accounts::CallMessage;
 
-fuzz_target!(|input: CallMessage| {
+type S = sov_test_utils::TestSpec;
+
+fuzz_target!(|input: CallMessage<S>| {
     let json = serde_json::to_vec(&input).unwrap();
-    let msg = serde_json::from_slice::<CallMessage>(&json).unwrap();
+    let msg = serde_json::from_slice::<CallMessage<S>>(&json).unwrap();
     assert_eq!(input, msg);
 });

crates/fuzz/fuzz_targets/accounts_parse_call_message_random.rs

@@ -3,6 +3,8 @@
 use libfuzzer_sys::fuzz_target;
 use sov_accounts::CallMessage;
 
+type S = sov_test_utils::TestSpec;
+
 fuzz_target!(|input: &[u8]| {
-    serde_json::from_slice::<CallMessage>(input).ok();
+    serde_json::from_slice::<CallMessage<S>>(input).ok();
 });

crates/module-system/module-implementations/sov-accounts/README.md

@@ -12,6 +12,20 @@ addresses and records which credentials may act for which addresses.
    the `CallMessage::InsertCredentialId(..)` message.
    This writes an `account_owners` authorization.
 
+5. It is possible to explicitly authorize a credential for the caller's own
+   address with `CallMessage::AddCredentialToAddress { address, credential }`,
+   revoke such an authorization with
+   `CallMessage::RemoveCredentialFromAddress { address, credential }`, and
+   atomically swap one credential for another with
+   `CallMessage::RotateCredentialOnAddress { address, old_credential, new_credential }`.
+   All three calls require `message.address == context.sender()`: callers can
+   only modify credentials on the address they are currently signing as. The
+   V1 signing path's `target_address` field lets a caller signing with a
+   credential authorized for multiple addresses select which one to act as.
+   There is no orphan guard on remove — revoking the last credential leaves
+   the address unspendable via `account_owners`.
+
+
 ## Credential and Address Relations
 
 ### Stateless canonical address
@@ -26,12 +40,14 @@ If a credential has no explicit authorization, this canonical address is the nat
 ### Account-credential authorization map
 
 ```text
-account_owners[(address, credential_id)] = true
+account_owners[(address, credential_id)] = true | false
 ```
 
-This state map records authorization.
-A present entry means the credential is authorized to sign transactions that execute as the given address.
-The key is the exact `(address, credential_id)` pair, so this relation does not provide a credential-only lookup by itself.
+This state map records authorization overrides. `true` means the credential is authorized to sign transactions that execute as the given address.
+`false`
+explicitly revokes fallback authorization for that pair, including stateless
+canonical fallback. The key is the exact `(address, credential_id)` pair, so
+this relation does not provide a credential-only lookup by itself.
 
 This relation answers "may this credential act as this address?" once the target address is known.
 New `InsertCredentialId` calls write this relation.

crates/module-system/module-implementations/sov-accounts/src/call.rs

@@ -9,14 +9,54 @@ use crate::{AccountOwnerKey, Accounts};
 /// Represents the available call messages for interacting with the sov-accounts module.
 #[derive(Debug, PartialEq, Eq, Clone, JsonSchema, UniversalWallet)]
 #[serialize(Borsh, Serde)]
+#[schemars(bound = "S::Address: ::schemars::JsonSchema", rename = "CallMessage")]
 #[serde(rename_all = "snake_case")]
-pub enum CallMessage {
+pub enum CallMessage<S: Spec> {
     /// Authorizes `credential_id` as a signer for the caller's address.
     /// Fails if the credential is already authorized for the caller's address.
     InsertCredentialId(
         /// The credential id being authorized.
         CredentialId,
     ),
+
+    /// Authorizes `credential` to sign transactions that execute as `address`.
+    /// The caller must currently be signing as `address`. Fails if the tuple
+    /// is already authorized.
+    AddCredentialToAddress {
+        /// The address whose credential set is being extended. Must equal
+        /// `context.sender()`.
+        address: S::Address,
+        /// The credential being authorized for `address`.
+        credential: CredentialId,
+    },
+
+    /// Revokes `credential` from `address`. The caller must currently be
+    /// signing as `address`. No orphan guard: revoking the last credential
+    /// succeeds and leaves `address` unspendable via this map.
+    RemoveCredentialFromAddress {
+        /// The address whose credential set is being reduced. Must equal
+        /// `context.sender()`.
+        address: S::Address,
+        /// The credential being revoked from `address`.
+        credential: CredentialId,
+    },
+
+    /// Atomically swaps `old_credential` for `new_credential` on `address`.
+    /// Functionally equivalent to a `RemoveCredentialFromAddress` followed by
+    /// an `AddCredentialToAddress`, collapsed into a single call so the
+    /// caller does not have to authorize two transactions during a rotation.
+    /// The caller must currently be signing as `address`.
+    RotateCredentialOnAddress {
+        /// The address whose credential set is being rotated. Must equal
+        /// `context.sender()`.
+        address: S::Address,
+        /// The credential being revoked from `address`. Must be currently
+        /// authorized.
+        old_credential: CredentialId,
+        /// The credential being authorized for `address`. Must not already
+        /// be authorized.
+        new_credential: CredentialId,
+    },
 }
 
 impl<S: Spec> Accounts<S> {
@@ -26,29 +66,124 @@ impl<S: Spec> Accounts<S> {
         context: &Context<S>,
         state: &mut impl TxState<S>,
     ) -> anyhow::Result<()> {
-        if !self.enable_custom_account_mappings.get(state)?.expect(
-            "`enable_custom_account_mappings` should not be None; it must be set at genesis.",
-        ) {
-            bail!("Custom account mappings are disabled");
-        }
+        self.ensure_custom_account_mappings_enabled(state)?;
 
-        self.exit_if_credential_exists(&new_credential_id, context.sender(), state)?;
+        self.ensure_credential_not_authorized(context.sender(), &new_credential_id, state)?;
 
         self.authorize_credential(context.sender(), &new_credential_id, state)?;
         Ok(())
     }
 
-    fn exit_if_credential_exists(
+    pub(crate) fn add_credential_to_address(
+        &mut self,
+        address: S::Address,
+        credential: CredentialId,
+        context: &Context<S>,
+        state: &mut impl TxState<S>,
+    ) -> anyhow::Result<()> {
+        self.ensure_custom_account_mappings_enabled(state)?;
+        self.ensure_caller_owns(&address, context)?;
+        self.ensure_credential_not_authorized(&address, &credential, state)?;
+
+        self.authorize_credential(&address, &credential, state)?;
+        Ok(())
+    }
+
+    pub(crate) fn remove_credential_from_address(
+        &mut self,
+        address: S::Address,
+        credential: CredentialId,
+        context: &Context<S>,
+        state: &mut impl TxState<S>,
+    ) -> anyhow::Result<()> {
+        self.ensure_custom_account_mappings_enabled(state)?;
+        self.ensure_caller_owns(&address, context)?;
+
+        anyhow::ensure!(
+            self.is_authorized_for(&address, &credential, state)
+                .context("Failed to check credential authorization")?,
+            "CredentialId is not authorized for this address"
+        );
+
+        self.revoke_credential(&address, &credential, state)?;
+        Ok(())
+    }
+
+    pub(crate) fn rotate_credential_on_address(
+        &mut self,
+        address: S::Address,
+        old_credential: CredentialId,
+        new_credential: CredentialId,
+        context: &Context<S>,
+        state: &mut impl TxState<S>,
+    ) -> anyhow::Result<()> {
+        self.ensure_custom_account_mappings_enabled(state)?;
+        self.ensure_caller_owns(&address, context)?;
+
+        anyhow::ensure!(
+            self.is_authorized_for(&address, &old_credential, state)
+                .context("Failed to check old credential authorization")?,
+            "old_credential is not authorized for this address"
+        );
+        self.ensure_credential_not_authorized(&address, &new_credential, state)?;
+
+        self.revoke_credential(&address, &old_credential, state)?;
+        self.authorize_credential(&address, &new_credential, state)?;
+        Ok(())
+    }
+
+    fn revoke_credential(
+        &mut self,
+        address: &S::Address,
+        credential: &CredentialId,
+        state: &mut impl TxState<S>,
+    ) -> anyhow::Result<()> {
+        let key = AccountOwnerKey::new(*address, *credential);
+        // Write `false` explicitly so the canonical-address fallback in
+        // `is_authorized_for` cannot re-authorize the tuple.
+        self.account_owners.set(&key, &false, state)?;
+        Ok(())
+    }
+
+    fn ensure_custom_account_mappings_enabled(
+        &self,
+        state: &mut impl StateReader<User>,
+    ) -> anyhow::Result<()> {
+        if !self
+            .enable_custom_account_mappings
+            .get(state)
+            .context("Failed to read enable_custom_account_mappings")?
+            .expect(
+                "`enable_custom_account_mappings` should not be None; it must be set at genesis.",
+            )
+        {
+            bail!("Custom account mappings are disabled");
+        }
+        Ok(())
+    }
+
+    /// Enforces that the caller is signing as `address`. The upstream
+    /// authorization path has already verified the caller controls a
+    /// credential authorized for `context.sender()`.
+    fn ensure_caller_owns(&self, address: &S::Address, context: &Context<S>) -> anyhow::Result<()> {
+        let sender = context.sender();
+        anyhow::ensure!(
+            sender == address,
+            "Caller {sender} is not authorized to modify credentials for address {address}"
+        );
+        Ok(())
+    }
+
+    fn ensure_credential_not_authorized(
         &self,
-        new_credential_id: &CredentialId,
         address: &S::Address,
+        credential: &CredentialId,
         state: &mut impl StateReader<User>,
     ) -> anyhow::Result<()> {
         anyhow::ensure!(
-            self.account_owners
-                .get(&AccountOwnerKey::new(*address, *new_credential_id), state)
-                .context("Failed to read account owner")?
-                .is_none(),
+            !self
+                .is_authorized_for(address, credential, state)
+                .context("Failed to check credential authorization")?,
             "CredentialId already authorized for this address"
         );
         Ok(())

crates/module-system/module-implementations/sov-accounts/src/capabilities.rs

@@ -30,25 +30,52 @@ impl<S: Spec> Accounts<S> {
         Ok(self
             .account_owners
             .get(&AccountOwnerKey::new(*address, *credential_id), state)?
-            .is_some())
+            .unwrap_or(false))
+    }
+
+    /// Returns `true` if the authenticator-selected default address may be used
+    /// with `credential_id`.
+    ///
+    /// Lookup order:
+    /// 1. If `account_owners[(address, credential_id)]` has an explicit
+    ///    entry, that value is authoritative — including `false`, which
+    ///    is how `revoke_credential` denies the default path.
+    /// 2. Otherwise, returns `true`: the authenticator's declared default
+    ///    address is trusted for the `address_override = None` path.
+    pub fn is_default_address_authorized<ST: StateReader<User>>(
+        &self,
+        address: &S::Address,
+        credential_id: &CredentialId,
+        state: &mut ST,
+    ) -> Result<bool, ST::Error> {
+        Ok(self
+            .account_owners
+            .get(&AccountOwnerKey::new(*address, *credential_id), state)?
+            .unwrap_or(true))
     }
 
     /// Returns `true` if `credential_id` is authorized to act as `address`.
     ///
-    /// Returns `true` when `address` is the canonical address of
-    /// `credential_id` (i.e. `credential_id.into() == address`) or when an
-    /// explicit `account_owners` authorization exists.
+    /// Lookup order:
+    /// 1. If `account_owners[(address, credential_id)]` has an explicit
+    ///    entry, that value is authoritative — including `false`, which
+    ///    is how `revoke_credential` denies the canonical fallback.
+    /// 2. Otherwise, returns `true` iff `address` is the canonical address
+    ///    of `credential_id` (i.e. `credential_id.into() == address`).
     pub fn is_authorized_for<ST: StateReader<User>>(
         &self,
         address: &S::Address,
         credential_id: &CredentialId,
         state: &mut ST,
     ) -> Result<bool, ST::Error> {
-        let canonical_address: S::Address = (*credential_id).into();
-        if canonical_address == *address {
-            return Ok(true);
+        if let Some(is_authorized) = self
+            .account_owners
+            .get(&AccountOwnerKey::new(*address, *credential_id), state)?
+        {
+            return Ok(is_authorized);
         }
 
-        self.is_explicitly_authorized(address, credential_id, state)
+        let canonical_address: S::Address = (*credential_id).into();
+        Ok(canonical_address == *address)
     }
 }

crates/module-system/module-implementations/sov-accounts/src/fuzz.rs

@@ -3,9 +3,28 @@ use sov_modules_api::{CryptoSpec, DaSpec, Module, Spec, StateCheckpoint};
 
 use crate::{Account, AccountConfig, AccountData, Accounts, CallMessage};
 
-impl<'a> Arbitrary<'a> for CallMessage {
+impl<'a, S> Arbitrary<'a> for CallMessage<S>
+where
+    S: Spec,
+    S::Address: Arbitrary<'a>,
+{
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
-        Ok(Self::InsertCredentialId(u.arbitrary()?))
+        match u.int_in_range(0..=3)? {
+            0 => Ok(Self::InsertCredentialId(u.arbitrary()?)),
+            1 => Ok(Self::AddCredentialToAddress {
+                address: u.arbitrary()?,
+                credential: u.arbitrary()?,
+            }),
+            2 => Ok(Self::RemoveCredentialFromAddress {
+                address: u.arbitrary()?,
+                credential: u.arbitrary()?,
+            }),
+            _ => Ok(Self::RotateCredentialOnAddress {
+                address: u.arbitrary()?,
+                old_credential: u.arbitrary()?,
+                new_credential: u.arbitrary()?,
+            }),
+        }
     }
 }