Accounts Refactor PR 1: Adding account -> [credential_id] mapping

crates/module-system/module-implementations/extern/hyperlane-solana-register/README.md

@@ -132,7 +132,7 @@ pub enum Event<S: Spec> {
 
 The module defines several error types:
 
-- `AlreadyRegistered`: The embedded public key is already linked to a different address
+- `Unauthorized`: The embedded public key is not authorized for the payer address
 - `InvalidBodyLength`: The message body doesn't contain exactly 64 bytes
 - `ExtractPubKey`: Failed to parse public keys from the message body
 - `AdminNotFound`: Admin address not configured
@@ -146,8 +146,8 @@ See `src/lib.rs:196-213` for the `handle` implementation:
 2. Verify the sender matches the trusted Solana program ID
 3. Extract the two 32-byte public keys from the message body
 4. Convert the payer public key to a rollup address
-5. Use `sov-accounts` to resolve or create the address-credential mapping
-6. Reject if the embedded wallet is already linked to a different address
+5. Use `sov-accounts` to verify the embedded credential is authorized for the payer address
+6. Reject if the embedded credential is not authorized for that address
 7. Emit a `UserRegistered` event
 
 ---
@@ -372,4 +372,3 @@ cargo test
 ```
 
 Program tests are located in `solana/program-tests/src/tests.rs`.
-

crates/module-system/module-implementations/extern/hyperlane-solana-register/src/lib.rs

@@ -53,10 +53,10 @@ where
 pub enum SolanaRegistrationError {
     #[error("Core module error: {0}")]
     CoreModuleError(#[from] CoreModuleError),
-    #[error("Embedded pubkey already registered to different address. Attempted: {attempted_address}, Registered: {registered_address}")]
-    AlreadyRegistered {
-        attempted_address: String,
-        registered_address: String,
+    #[error("Embedded pubkey is not authorized for address. Address: {address}, CredentialId: {credential_id}")]
+    Unauthorized {
+        address: String,
+        credential_id: String,
     },
     #[error("Invalid body length. Expected {expected}, found {found}")]
     InvalidBodyLength { expected: usize, found: usize },
@@ -297,15 +297,15 @@ where
         let (user_pubkey, embedded_pubkey) = self.unpack_body(body.as_ref())?;
         let credential_id = CredentialId::from(embedded_pubkey);
         let address = S::Address::try_from(&user_pubkey).map_err(CoreModuleError::from)?;
-        let resolved_address = self
+        let is_authorized = self
             .accounts
-            .resolve_sender_address(&address, &credential_id, state)
-            .map_err(CoreModuleError::state_write)?;
+            .is_authorized_for(&address, &credential_id, state)
+            .map_err(CoreModuleError::state_read)?;
 
-        if address != resolved_address {
-            Err(SolanaRegistrationError::AlreadyRegistered {
-                attempted_address: address.to_string(),
-                registered_address: resolved_address.to_string(),
+        if !is_authorized {
+            Err(SolanaRegistrationError::Unauthorized {
+                address: address.to_string(),
+                credential_id: credential_id.to_string(),
             })
         } else {
             self.emit_event(

crates/module-system/module-implementations/extern/hyperlane-solana-register/tests/integration/registration.rs

@@ -23,13 +23,13 @@ fn test_user_is_registered_correctly() {
     let route_id = register_basic_warp_route(&mut runner, &admin);
 
     let payer = [1u8; 32];
-    let embedded = [2u8; 32];
+    let embedded = payer;
     let body = [payer, embedded].concat();
     let valid_message = make_valid_message(0, route_id, HexString::new(body));
     let message = HexString::new(SafeVec::try_from(valid_message.encode().0).unwrap());
     let credential = CredentialId::from(embedded);
 
-    // Sanity check, ensure account definently doesnt already exist
+    // Sanity check, ensure registration does not rely on an accounts map entry.
     runner.query_state(|state| {
         let account = sov_accounts::Accounts::default().get_account(credential, state);
         assert!(matches!(account, sov_accounts::Response::AccountEmpty));
@@ -60,15 +60,12 @@ fn test_user_is_registered_correctly() {
 
     runner.query_state(|state| {
         let account = sov_accounts::Accounts::default().get_account(credential, state);
-        assert!(matches!(
-            account,
-            sov_accounts::Response::AccountExists { .. }
-        ));
+        assert!(matches!(account, sov_accounts::Response::AccountEmpty));
     });
 }
 
 #[test]
-fn test_errors_if_user_already_registered() {
+fn test_errors_if_embedded_pubkey_is_not_authorized_for_payer() {
     let SetupParams {
         mut runner,
         admin,
@@ -82,26 +79,11 @@ fn test_errors_if_user_already_registered() {
     let body = [payer, embedded].concat();
     let valid_message = make_valid_message(0, route_id, HexString::new(body));
     let message = HexString::new(SafeVec::try_from(valid_message.encode().0).unwrap());
-
-    runner.execute_transaction(TransactionTestCase {
-        input: user.create_plain_message::<RT, Mailbox<S>>(CallMessage::Process {
-            metadata: HexString::new(SafeVec::new()),
-            message: message.clone(),
-        }),
-        assert: Box::new(move |result, _| {
-            assert!(
-                result.tx_receipt.is_successful(),
-                "Recipient was not registered successfully"
-            );
-        }),
-    });
-
-    // payer is different so will try to register to different address
-    let payer = [3u8; 32];
-    let embedded = [2u8; 32];
-    let body = [payer, embedded].concat();
-    let valid_message = make_valid_message(1, route_id, HexString::new(body));
-    let message = HexString::new(SafeVec::try_from(valid_message.encode().0).unwrap());
+    let expected_error = format!(
+        "Embedded pubkey is not authorized for address. Address: {}, CredentialId: {}",
+        <S as Spec>::Address::from(payer),
+        CredentialId::from(embedded)
+    );
 
     runner.execute_transaction(TransactionTestCase {
         input: user.create_plain_message::<RT, Mailbox<S>>(CallMessage::Process {
@@ -110,10 +92,7 @@ fn test_errors_if_user_already_registered() {
         }),
         assert: Box::new(move |result, _| match result.tx_receipt {
             sov_rollup_interface::stf::TxEffect::Reverted(contents) => {
-                assert_eq!(
-                    contents.reason.to_string(),
-                    "Embedded pubkey already registered to different address. Attempted: CktRuQ2mttgRGkXJtyksdKHjUdc2C4TgDzyB98oEzy8, Registered: 4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi".to_string()
-                );
+                assert_eq!(contents.reason.to_string(), expected_error);
             }
             _ => panic!("Registration should have reverted: {:?}", result.tx_receipt),
         }),

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

@@ -1,15 +1,77 @@
 # `sov-accounts` module
 
-The `sov-accounts` module is responsible for managing accounts on the rollup.
+The `sov-accounts` module resolves transaction credentials to rollup
+addresses and records which credentials may act for which addresses.
 
+### The `sov-accounts` module offers the following functionality
 
-### The `sov-accounts` module offers the following functionality:
+1. A credential has a deterministic canonical address, computed as
+   `credential_id.into::<S::Address>()`. This relation is stateless: using the
+   canonical address does not require an account entry to be written.
 
-1. When a sender sends their first message, the `sov-accounts` module will create a new address by deriving it from the sender's credential.
-   The module will then add a mapping between the credential id and the address to its state. For all subsequent messages that include the sender's credential,
-   the module will retrieve the sender's address from the mapping and pass it along with the original message to an intended module.
+1. Legacy state can contain an explicit `credential_id -> address` mapping.
+   This map is used for credential-only resolution and the `get_account` query.
 
-1. It is possible to add new credential to a given address using the `CallMessage::InsertCredentialId(..)` message.
+1. It is possible to authorize another credential for the caller's address using
+   the `CallMessage::InsertCredentialId(..)` message. This writes an
+   `account_owners` authorization, not a credential-indexed account entry.
 
-1. It is possible to query the `sov-accounts` module using the `get_account` method and get the account corresponding to the given credential id.
+1. It is possible to query the `sov-accounts` module using the `get_account`
+   method and get the legacy/custom mapped address corresponding to the given
+   credential id.
 
+## Credential and Address Relations
+
+The module has three credential/address relations. They answer different
+questions and should not be treated as interchangeable.
+
+### Stateless canonical address
+
+```text
+credential_id -> credential_id.into::<S::Address>()
+```
+
+This is the default address for a credential. It is deterministic and requires
+no state write. If a credential has no explicit credential-indexed state
+mapping, this canonical address is the natural fallback for credential-only
+routing.
+
+### Credential-to-account map
+
+```text
+accounts[credential_id] = Account { addr }
+```
+
+This legacy/custom state map records the primary address explicitly associated
+with a credential. A credential has at most one primary address in this map,
+while one address may be the primary address for many credentials.
+
+This map is used when callers only know a `CredentialId` and need an address:
+`resolve_sender_address`, `get_account`, and legacy/custom account mappings all
+depend on this credential-indexed lookup. New `InsertCredentialId` calls do not
+write this map. `get_account` reports entries from this explicit map; it does
+not mean that every possible stateless canonical address or explicit
+authorization has a stored account entry.
+
+### Account-credential authorization map
+
+```text
+account_owners[(address, credential_id)] = true
+```
+
+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 relation answers "may this credential act as this address?" once the target
+address is known. It is not a replacement for the credential-to-account map,
+because it is keyed by `(address, credential_id)` and cannot efficiently answer
+"which address is this credential mapped to?" with a single point lookup.
+New `InsertCredentialId` calls write this relation.
+
+In short: routing from only a credential uses the explicit credential-to-account
+map or the stateless canonical fallback. Callers that need to verify whether a
+known address may be used with a credential should use the combined
+`is_authorized_for` check: legacy/custom mapping, stateless canonical address,
+or `account_owners`.

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

@@ -1,20 +1,21 @@
-use anyhow::bail;
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, bail, Result};
 use schemars::JsonSchema;
 use sov_modules_api::macros::{serialize, UniversalWallet};
 use sov_modules_api::{Context, CredentialId, Spec, StateReader, TxState};
 use sov_state::namespaces::User;
 
-use crate::{Account, Accounts};
+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)]
 #[serde(rename_all = "snake_case")]
 pub enum CallMessage {
-    /// Inserts a new credential id for the corresponding Account.
+    /// Authorizes `credential_id` as a signer for the caller's address.
+    /// Fails if the credential has a legacy/custom account mapping or is
+    /// already authorized for the caller's address.
     InsertCredentialId(
-        /// The new credential id.
+        /// The credential id being authorized.
         CredentialId,
     ),
 }
@@ -32,20 +33,16 @@ impl<S: Spec> Accounts<S> {
             bail!("Custom account mappings are disabled");
         }
 
-        self.exit_if_credential_exists(&new_credential_id, state)?;
-
-        // Insert the new credential id -> account mapping
-        let account = Account {
-            addr: *context.sender(),
-        };
-        self.accounts.set(&new_credential_id, &account, state)?;
+        self.exit_if_credential_exists(&new_credential_id, context.sender(), state)?;
 
+        self.authorize_credential(context.sender(), &new_credential_id, state)?;
         Ok(())
     }
 
     fn exit_if_credential_exists(
         &self,
         new_credential_id: &CredentialId,
+        address: &S::Address,
         state: &mut impl StateReader<User>,
     ) -> Result<()> {
         anyhow::ensure!(
@@ -55,6 +52,13 @@ impl<S: Spec> Accounts<S> {
                 .is_none(),
             "New CredentialId already exists"
         );
+        anyhow::ensure!(
+            self.account_owners
+                .get(&AccountOwnerKey::new(*address, *new_credential_id), state)
+                .map_err(|err| anyhow!("Error raised while getting account owner: {err:?}"))?
+                .is_none(),
+            "CredentialId already authorized for this address"
+        );
         Ok(())
     }
 }

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

@@ -1,45 +1,86 @@
 use sov_modules_api::{CredentialId, Spec, StateAccessor, StateReader, StateWriter};
 use sov_state::User;
 
-use crate::{Account, Accounts};
+use crate::{AccountOwnerKey, Accounts};
 
 impl<S: Spec> Accounts<S> {
-    /// Resolve the sender's public key to an address.
-    /// If the sender is not registered, but a fallback address if provided, immediately registers
-    /// the credential to the fallback and then returns it.
+    /// Authorizes `credential_id` to sign as `address`.
+    pub(crate) fn authorize_credential<ST: StateWriter<User>>(
+        &mut self,
+        address: &S::Address,
+        credential_id: &CredentialId,
+        state: &mut ST,
+    ) -> Result<(), <ST as StateWriter<User>>::Error> {
+        self.account_owners.set(
+            &AccountOwnerKey::new(*address, *credential_id),
+            &true,
+            state,
+        )
+    }
+
+    /// Resolve the sender's credential to an address.
+    /// If `credential_id` has a legacy/custom mapping in `accounts`, return it.
+    /// Otherwise, return the supplied `default_address` without writing account state.
     pub fn resolve_sender_address<ST: StateAccessor>(
         &mut self,
         default_address: &S::Address,
         credential_id: &CredentialId,
         state: &mut ST,
     ) -> Result<S::Address, <ST as StateWriter<User>>::Error> {
-        let maybe_address = self.accounts.get(credential_id, state)?.map(|a| a.addr);
-
-        match maybe_address {
-            Some(address) => Ok(address),
-            None => {
-                // 1. Add the credential -> account mapping
-                let new_account = Account {
-                    addr: *default_address,
-                };
-                self.accounts.set(credential_id, &new_account, state)?;
-
-                Ok(*default_address)
-            }
+        if let Some(account) = self.accounts.get(credential_id, state)? {
+            return Ok(account.addr);
         }
+        Ok(*default_address)
     }
 
-    /// Resolve the sender's public key to an address.
+    /// Read-only variant of [`Self::resolve_sender_address`]: returns
+    /// `default_address` when the credential has no legacy/custom mapping,
+    /// without writing.
     pub fn resolve_sender_address_read_only<ST: StateReader<User>>(
         &self,
         default_address: &S::Address,
         credential_id: &CredentialId,
         state: &mut ST,
     ) -> Result<S::Address, ST::Error> {
-        let maybe_address = self.accounts.get(credential_id, state)?.map(|a| a.addr);
-        match maybe_address {
-            Some(address) => Ok(address),
-            None => Ok(*default_address),
+        if let Some(account) = self.accounts.get(credential_id, state)? {
+            return Ok(account.addr);
         }
+        Ok(*default_address)
+    }
+
+    /// Returns `true` if `credential_id` has an explicit `account_owners`
+    /// authorization for `address`.
+    pub fn is_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)?
+            .is_some())
+    }
+
+    /// Returns `true` if `credential_id` is authorized to act as `address`
+    /// under any supported relation: legacy/custom `accounts` mapping,
+    /// stateless canonical address, or explicit `account_owners`
+    /// authorization.
+    pub fn is_authorized_for<ST: StateReader<User>>(
+        &self,
+        address: &S::Address,
+        credential_id: &CredentialId,
+        state: &mut ST,
+    ) -> Result<bool, ST::Error> {
+        if let Some(account) = self.accounts.get(credential_id, state)? {
+            return Ok(account.addr == *address);
+        }
+
+        let canonical_address: S::Address = (*credential_id).into();
+        if canonical_address == *address {
+            return Ok(true);
+        }
+
+        self.is_authorized(address, credential_id, state)
     }
 }