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,17 @@ fn test_user_is_registered_correctly() {
     let route_id = register_basic_warp_route(&mut runner, &admin);
 
     let payer = [1u8; 32];
-    let embedded = [2u8; 32];
+    // When `embedded == payer`, the embedded credential's canonical address equals
+    // the payer address, so `is_authorized_for` succeeds via the canonical-address
+    // arm without needing any stored `accounts` or `account_owners` entry. This
+    // exercises the stateless happy path.
+    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: no legacy accounts entry is required for registration.
     runner.query_state(|state| {
         let account = sov_accounts::Accounts::default().get_account(credential, state);
         assert!(matches!(account, sov_accounts::Response::AccountEmpty));
@@ -60,15 +64,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 +83,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 +96,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/integration-tests/tests/stf_blueprint/operator/auth_eip712.rs

@@ -1,4 +1,3 @@
-use sov_accounts::{Accounts, CallMessage as AccountsCallMessage};
 use sov_address::{EthereumAddress, EvmCryptoSpec};
 use sov_eip712_auth::{
     Eip712Authenticator, Eip712AuthenticatorInput, Eip712AuthenticatorTrait, SchemaProvider,
@@ -22,7 +21,6 @@ use sov_rollup_interface::da::RelevantBlobs;
 use sov_rollup_interface::stf::{TxEffect, TxReceiptContents};
 use sov_test_utils::runtime::genesis::optimistic::HighLevelOptimisticGenesisConfig;
 use sov_test_utils::runtime::{TestRunner, ValueSetter};
-use sov_test_utils::TransactionTestCase;
 use sov_test_utils::{generate_runtime, EncodeCall, TestStorage, TestUser, TEST_DEFAULT_MAX_FEE};
 use sov_value_setter::CallMessage;
 
@@ -274,28 +272,34 @@ fn correct_signature_is_accepted() {
 
 #[test]
 fn test_multisig_signature_verification() {
-    use sov_test_utils::AsUser;
-    let (mut runner, admin) = setup();
-
-    // First, create and register a multisig
+    // Build the multisig first so we can seed genesis with its canonical address
+    // funded. After the accounts refactor, `resolve_sender_address` no longer
+    // writes an `accounts` entry, so the multisig must either be a canonical-
+    // address owner with gas, or be covered by a legacy mapping. Here we pick
+    // the canonical path.
     let multisig_keys = [
         TestPrivateKey::generate(),
         TestPrivateKey::generate(),
         TestPrivateKey::generate(),
     ];
-
-    // Create the multisig and register it
     let multisig = Multisig::new(2, multisig_keys.iter().map(|k| k.pub_key()).collect());
     let multisig_credential_id =
         multisig.credential_id::<<<S as Spec>::CryptoSpec as CryptoSpec>::Hasher>();
-    runner.execute_transaction(TransactionTestCase {
-        input: admin.create_plain_message::<RT, Accounts<S>>(
-            AccountsCallMessage::InsertCredentialId(multisig_credential_id),
-        ),
-        assert: Box::new(move |result, _state| {
-            assert!(result.tx_receipt.is_successful());
-        }),
-    });
+    let multisig_user =
+        TestUser::generate_with_default_balance().add_credential_id(multisig_credential_id);
+
+    // The multisig is the sender of every `SetValue` tx below, so ValueSetter's
+    // admin must be the multisig canonical address for the successful-path
+    // assertions to actually reach ValueSetter.
+    let multisig_address = multisig_user.address();
+    let genesis_config = HighLevelOptimisticGenesisConfig::generate()
+        .add_accounts_with_default_balance(2)
+        .add_accounts(vec![multisig_user]);
+    let module_config = sov_value_setter::ValueSetterConfig {
+        admin: multisig_address,
+    };
+    let genesis = GenesisConfig::from_minimal_config(genesis_config.clone().into(), module_config);
+    let mut runner = TestRunner::new_with_genesis(genesis.into_genesis_params(), RT::default());
 
     // Generate a signature from a random private key that's not part of the multisig. We'll use this in some of the test cases.
     let random_private_key = TestPrivateKey::generate();

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::{bail, Context as _};
 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,
     ),
 }
@@ -25,36 +26,39 @@ impl<S: Spec> Accounts<S> {
         new_credential_id: CredentialId,
         context: &Context<S>,
         state: &mut impl TxState<S>,
-    ) -> Result<()> {
+    ) -> 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.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::Result<()> {
         anyhow::ensure!(
             self.accounts
                 .get(new_credential_id, state)
-                .map_err(|err| anyhow!("Error raised while getting account: {err:?}"))?
+                .context("Failed to read legacy account mapping")?
                 .is_none(),
             "New CredentialId already exists"
         );
+        anyhow::ensure!(
+            self.account_owners
+                .get(&AccountOwnerKey::new(*address, *new_credential_id), state)
+                .context("Failed to read account owner")?
+                .is_none(),
+            "CredentialId already authorized for this address"
+        );
         Ok(())
     }
 }