From a1b9408dda0cb8b12f68d58c22a00431d62fef86 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Thu, 16 Apr 2026 15:27:38 +0200 Subject: [PATCH 1/7] [wip]: derive address of account from hash of credential id and sender --- .../module-implementations/sov-accounts/src/call.rs | 13 +++++++++++-- .../sov-accounts/src/genesis.rs | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/module-system/module-implementations/sov-accounts/src/call.rs b/crates/module-system/module-implementations/sov-accounts/src/call.rs index 6b23a9ece5..73f4f0b256 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/call.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/call.rs @@ -1,8 +1,9 @@ use anyhow::bail; use anyhow::{anyhow, Result}; use schemars::JsonSchema; +use sov_modules_api::digest::Digest; use sov_modules_api::macros::{serialize, UniversalWallet}; -use sov_modules_api::{Context, CredentialId, Spec, StateReader, TxState}; +use sov_modules_api::{Context, CredentialId, CryptoSpec, Spec, StateReader, TxState}; use sov_state::namespaces::User; use crate::{Account, Accounts}; @@ -34,9 +35,17 @@ impl Accounts { self.exit_if_credential_exists(&new_credential_id, state)?; + let mut hasher = ::Hasher::new(); + hasher.update(&new_credential_id.0 .0); + // TODO: Nonce to preserve unique addresses? + hasher.update(context.sender().as_ref()); + let address_source = hasher.finalize(); + + let new_non_controlled_address = S::Address::try_from(address_source.as_slice())?; + // Insert the new credential id -> account mapping let account = Account { - addr: *context.sender(), + addr: new_non_controlled_address, }; self.accounts.set(&new_credential_id, &account, state)?; diff --git a/crates/module-system/module-implementations/sov-accounts/src/genesis.rs b/crates/module-system/module-implementations/sov-accounts/src/genesis.rs index 5bf6eefaec..324020d912 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/genesis.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/genesis.rs @@ -25,7 +25,7 @@ pub struct AccountData
{ pub struct AccountConfig { /// Accounts to initialize the rollup. pub accounts: Vec>, - /// Enable custom `CredentailId` => `Account` mapping. + /// Enable custom `CredentialId` => `Account` mapping. #[serde(default = "default_true")] pub enable_custom_account_mappings: bool, } From 3174682a412290f3f5634c599edcc225e0d344bb Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 11:29:36 +0200 Subject: [PATCH 2/7] Address of new credential is not controlled by sender --- Cargo.lock | 1 + .../sov-accounts/Cargo.toml | 1 + .../sov-accounts/src/call.rs | 24 +++-- .../sov-accounts/tests/integration/main.rs | 89 ++++++++++++++----- .../genesis-schemas/sov-accounts.json | 2 +- 5 files changed, 85 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc3348eab7..96c4ee5aa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11190,6 +11190,7 @@ dependencies = [ "serde", "serde_with", "sov-accounts", + "sov-bank", "sov-modules-api", "sov-state", "sov-test-utils", diff --git a/crates/module-system/module-implementations/sov-accounts/Cargo.toml b/crates/module-system/module-implementations/sov-accounts/Cargo.toml index 2622868de9..fb97d42d7f 100644 --- a/crates/module-system/module-implementations/sov-accounts/Cargo.toml +++ b/crates/module-system/module-implementations/sov-accounts/Cargo.toml @@ -29,6 +29,7 @@ arbitrary = { workspace = true, optional = true } sov-accounts = { path = ".", features = ["native"] } tempfile = { workspace = true } strum = { workspace = true } +sov-bank = { workspace = true, features = ["native"] } sov-modules-api = { workspace = true, features = ["test-utils"] } sov-test-utils = { workspace = true } diff --git a/crates/module-system/module-implementations/sov-accounts/src/call.rs b/crates/module-system/module-implementations/sov-accounts/src/call.rs index 73f4f0b256..c42bc222cb 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/call.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/call.rs @@ -1,5 +1,5 @@ +use anyhow::anyhow; use anyhow::bail; -use anyhow::{anyhow, Result}; use schemars::JsonSchema; use sov_modules_api::digest::Digest; use sov_modules_api::macros::{serialize, UniversalWallet}; @@ -26,7 +26,7 @@ impl Accounts { new_credential_id: CredentialId, context: &Context, state: &mut impl TxState, - ) -> 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.", ) { @@ -36,12 +36,20 @@ impl Accounts { self.exit_if_credential_exists(&new_credential_id, state)?; let mut hasher = ::Hasher::new(); - hasher.update(&new_credential_id.0 .0); - // TODO: Nonce to preserve unique addresses? + hasher.update(new_credential_id.0 .0); + // Mix the sender in so the new credential's address isn't just the sender's own. + // Defense-in-depth: (a) a multisig registered by A shouldn't alias A's address — + // otherwise a later compromise of A's single key also controls the multisig; and + // (b) if credential removal is ever added, this prevents a removed credential + // from being re-registered by a different sender and colliding with the old address. hasher.update(context.sender().as_ref()); - let address_source = hasher.finalize(); + // TODO: also mix in a per-sender nonce so the same (credential, sender) pair + // can't reproduce an address a compromised sender previously controlled. + let address_source: [u8; 32] = hasher.finalize().into(); - let new_non_controlled_address = S::Address::try_from(address_source.as_slice())?; + // Route through CredentialId so each Spec's existing `From` + // handles address-width reduction (28 / 32 / 20 bytes). + let new_non_controlled_address = S::Address::from(CredentialId::from_bytes(address_source)); // Insert the new credential id -> account mapping let account = Account { @@ -56,13 +64,13 @@ impl Accounts { &self, new_credential_id: &CredentialId, state: &mut impl StateReader, - ) -> Result<()> { + ) -> anyhow::Result<()> { anyhow::ensure!( self.accounts .get(new_credential_id, state) .map_err(|err| anyhow!("Error raised while getting account: {err:?}"))? .is_none(), - "New CredentialId already exists" + "New CredentialId already exists: {new_credential_id}" ); Ok(()) } diff --git a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs index c23bf2806a..56547921ad 100644 --- a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs +++ b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs @@ -1,7 +1,10 @@ use sov_accounts::{Accounts, CallMessage, Response}; +use sov_bank::{config_gas_token_id, Amount, Bank, Coins}; +use sov_modules_api::digest::Digest; use sov_modules_api::transaction::{UnsignedTransaction, Version1}; use sov_modules_api::{ - CryptoSpec, PrivateKey, PublicKey, RawTx, Runtime, SkippedTxContents, Spec, TxEffect, + CredentialId, CryptoSpec, PrivateKey, PublicKey, RawTx, Runtime, SkippedTxContents, Spec, + TxEffect, }; use sov_test_utils::runtime::genesis::optimistic::HighLevelOptimisticGenesisConfig; use sov_test_utils::runtime::TestRunner; @@ -19,6 +22,18 @@ generate_optimistic_runtime!(TestAccountsRuntime <=); type RT = TestAccountsRuntime; +/// Mirror of the address derivation in `Accounts::insert_credential_id`. +fn expected_derived_address( + new_credential: CredentialId, + sender: ::Address, +) -> ::Address { + let mut hasher = <::CryptoSpec as CryptoSpec>::Hasher::new(); + hasher.update(new_credential.0 .0); + hasher.update(sender.as_ref()); + let hash: [u8; 32] = hasher.finalize().into(); + ::Address::from(CredentialId::from_bytes(hash)) +} + struct TestData { account_1: TestUser, account_2: TestUser, @@ -107,11 +122,11 @@ fn test_update_account() { let accounts = Accounts::::default(); - // New account with the new public key and an old address is created. + // New credential maps to a freshly derived, non-controlled address. assert_eq!( accounts.get_account(new_credential, state), Response::AccountExists { - addr: user.address() + addr: expected_derived_address(new_credential, user.address()) } ); // Account corresponding to the old credential still exists. @@ -132,23 +147,28 @@ fn test_update_account_fails() { let ( TestData { account_1, - account_2, + non_registered_account, .. }, mut runner, ) = setup(); + // `non_registered_account` has no `custom_credential_id`, so its signing + // credential matches the address that holds its balance — gas reservation + // can succeed and the call handler actually runs. runner.execute_transaction(TransactionTestCase { - input: account_1.create_plain_message::>(CallMessage::InsertCredentialId( - account_2.credential_id(), - )), - assert: Box::new(move |result, _state| { - if let TxEffect::Reverted(contents) = result.tx_receipt { - assert_eq!( - contents.reason.to_string(), - "New CredentialId already exists" + input: non_registered_account.create_plain_message::>( + CallMessage::InsertCredentialId(account_1.credential_id()), + ), + assert: Box::new(move |result, _state| match result.tx_receipt { + TxEffect::Reverted(contents) => { + let reason = contents.reason.to_string(); + assert!( + reason.contains("New CredentialId already exists"), + "Unexpected revert reason: {reason}", ); } + other => panic!("Expected reverted transaction, got {other:?}"), }), }); } @@ -174,6 +194,9 @@ fn test_setup_multisig_and_act() { let multisig = Multisig::new(2, multisig_keys.iter().map(|k| k.pub_key()).collect()); let multisig_credential_id = multisig.credential_id::<<::CryptoSpec as CryptoSpec>::Hasher>(); + let user_address = user.address(); + let user_credential_id = user.credential_id(); + let multisig_address = expected_derived_address(multisig_credential_id, user_address); runner.execute_transaction(TransactionTestCase { input: user.create_plain_message::>(CallMessage::InsertCredentialId( multisig_credential_id, @@ -183,22 +206,38 @@ fn test_setup_multisig_and_act() { let accounts = Accounts::::default(); - // New account with the new public key and an old address is created. + // Multisig credential maps to a freshly derived, non-controlled address — + // it must not alias the creator's address, otherwise a compromise of the + // creator's single key would also control the multisig. assert_eq!( accounts.get_account(multisig_credential_id, state), Response::AccountExists { - addr: user.address() + addr: multisig_address, } ); // Account corresponding to the old credential still exists. assert_eq!( - accounts.get_account(user.credential_id(), state), - Response::AccountExists { - addr: user.address() - } + accounts.get_account(user_credential_id, state), + Response::AccountExists { addr: user_address } ); - assert_ne!(multisig_credential_id, user.credential_id()); + assert_ne!(multisig_credential_id, user_credential_id); + }), + }); + + // The multisig now lives at its own, non-controlled address and has no balance of + // its own — transfer from `user` so subsequent multisig-signed transactions can + // reserve gas. + runner.execute_transaction(TransactionTestCase { + input: user.create_plain_message::>(sov_bank::CallMessage::Transfer { + to: multisig_address, + coins: Coins { + amount: Amount::new(1_000_000_000_000), + token_id: config_gas_token_id(), + }, + }), + assert: Box::new(move |result, _state| { + assert!(result.tx_receipt.is_successful(), "Funding transfer failed"); }), }); @@ -405,11 +444,14 @@ fn test_register_new_account() { let accounts = Accounts::::default(); - // New account with the new public key and an old address is created. + // New credential maps to a freshly derived, non-controlled address. assert_eq!( accounts.get_account(new_credential, state), Response::AccountExists { - addr: non_registered_account.address() + addr: expected_derived_address( + new_credential, + non_registered_account.address(), + ), } ); @@ -507,18 +549,19 @@ fn test_resolve_address_if_more_than_one_credential() { runner.query_visible_state(|state| { let mut accounts = Accounts::::default(); + // Each credential now resolves to its own derived address, not the sender's. assert_eq!( accounts .resolve_sender_address(&default_address_1, &credential_1, state) .unwrap(), - non_registered_account.address() + expected_derived_address(credential_1, non_registered_account.address()), ); assert_eq!( accounts .resolve_sender_address(&default_address_2, &credential_2, state) .unwrap(), - non_registered_account.address() + expected_derived_address(credential_2, non_registered_account.address()), ); }); } diff --git a/crates/module-system/module-schemas/genesis-schemas/sov-accounts.json b/crates/module-system/module-schemas/genesis-schemas/sov-accounts.json index 5b4c04b15a..867858f505 100644 --- a/crates/module-system/module-schemas/genesis-schemas/sov-accounts.json +++ b/crates/module-system/module-schemas/genesis-schemas/sov-accounts.json @@ -15,7 +15,7 @@ } }, "enable_custom_account_mappings": { - "description": "Enable custom `CredentailId` => `Account` mapping.", + "description": "Enable custom `CredentialId` => `Account` mapping.", "default": true, "type": "boolean" } From 7e8b113af179790c1586f3715ac1890aabcfd1f6 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 11:43:23 +0200 Subject: [PATCH 3/7] Clean up 1 --- .../sov-accounts/Cargo.toml | 2 + .../sov-accounts/src/call.rs | 34 ++++------------ .../sov-accounts/src/lib.rs | 2 + .../sov-accounts/src/utils.rs | 22 +++++++++++ .../sov-accounts/tests/integration/main.rs | 39 ++++++++----------- 5 files changed, 50 insertions(+), 49 deletions(-) create mode 100644 crates/module-system/module-implementations/sov-accounts/src/utils.rs diff --git a/crates/module-system/module-implementations/sov-accounts/Cargo.toml b/crates/module-system/module-implementations/sov-accounts/Cargo.toml index fb97d42d7f..b2a5bd3ccc 100644 --- a/crates/module-system/module-implementations/sov-accounts/Cargo.toml +++ b/crates/module-system/module-implementations/sov-accounts/Cargo.toml @@ -41,10 +41,12 @@ arbitrary = [ "sov-modules-api/arbitrary", "sov-state/arbitrary", "sov-test-utils/arbitrary", + "sov-bank/arbitrary" ] native = [ "sov-accounts/native", "sov-modules-api/native", "sov-state/native", + "sov-bank/native" ] test-utils = [] diff --git a/crates/module-system/module-implementations/sov-accounts/src/call.rs b/crates/module-system/module-implementations/sov-accounts/src/call.rs index c42bc222cb..1c27500a23 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/call.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/call.rs @@ -1,12 +1,10 @@ -use anyhow::anyhow; -use anyhow::bail; +use anyhow::{anyhow, bail, Result}; use schemars::JsonSchema; -use sov_modules_api::digest::Digest; use sov_modules_api::macros::{serialize, UniversalWallet}; -use sov_modules_api::{Context, CredentialId, CryptoSpec, Spec, StateReader, TxState}; +use sov_modules_api::{Context, CredentialId, Spec, StateReader, TxState}; use sov_state::namespaces::User; -use crate::{Account, Accounts}; +use crate::{derive_address_for_new_credential, Account, Accounts}; /// Represents the available call messages for interacting with the sov-accounts module. #[derive(Debug, PartialEq, Eq, Clone, JsonSchema, UniversalWallet)] @@ -26,7 +24,7 @@ impl Accounts { new_credential_id: CredentialId, context: &Context, state: &mut impl TxState, - ) -> anyhow::Result<()> { + ) -> Result<()> { if !self.enable_custom_account_mappings.get(state)?.expect( "`enable_custom_account_mappings` should not be None; it must be set at genesis.", ) { @@ -35,27 +33,11 @@ impl Accounts { self.exit_if_credential_exists(&new_credential_id, state)?; - let mut hasher = ::Hasher::new(); - hasher.update(new_credential_id.0 .0); - // Mix the sender in so the new credential's address isn't just the sender's own. - // Defense-in-depth: (a) a multisig registered by A shouldn't alias A's address — - // otherwise a later compromise of A's single key also controls the multisig; and - // (b) if credential removal is ever added, this prevents a removed credential - // from being re-registered by a different sender and colliding with the old address. - hasher.update(context.sender().as_ref()); // TODO: also mix in a per-sender nonce so the same (credential, sender) pair // can't reproduce an address a compromised sender previously controlled. - let address_source: [u8; 32] = hasher.finalize().into(); - - // Route through CredentialId so each Spec's existing `From` - // handles address-width reduction (28 / 32 / 20 bytes). - let new_non_controlled_address = S::Address::from(CredentialId::from_bytes(address_source)); - - // Insert the new credential id -> account mapping - let account = Account { - addr: new_non_controlled_address, - }; - self.accounts.set(&new_credential_id, &account, state)?; + let addr = derive_address_for_new_credential::(&new_credential_id, context.sender()); + self.accounts + .set(&new_credential_id, &Account { addr }, state)?; Ok(()) } @@ -64,7 +46,7 @@ impl Accounts { &self, new_credential_id: &CredentialId, state: &mut impl StateReader, - ) -> anyhow::Result<()> { + ) -> Result<()> { anyhow::ensure!( self.accounts .get(new_credential_id, state) diff --git a/crates/module-system/module-implementations/sov-accounts/src/lib.rs b/crates/module-system/module-implementations/sov-accounts/src/lib.rs index 6bfe8bbda0..0b9dec26c3 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/lib.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/lib.rs @@ -12,11 +12,13 @@ mod query; pub use query::*; #[cfg(test)] mod tests; +mod utils; pub use call::CallMessage; use sov_modules_api::{ Context, CredentialId, DaSpec, GenesisState, Module, ModuleId, ModuleInfo, ModuleRestApi, Spec, StateMap, StateValue, TxState, }; +pub use utils::derive_address_for_new_credential; /// An account on the rollup. #[derive( diff --git a/crates/module-system/module-implementations/sov-accounts/src/utils.rs b/crates/module-system/module-implementations/sov-accounts/src/utils.rs new file mode 100644 index 0000000000..1146cfcc2d --- /dev/null +++ b/crates/module-system/module-implementations/sov-accounts/src/utils.rs @@ -0,0 +1,22 @@ +use sov_modules_api::digest::Digest; +use sov_modules_api::{CredentialId, CryptoSpec, Spec}; + +/// Derives the address assigned to a newly inserted credential. +/// +/// The derivation mixes the transaction sender into the hash so that: +/// - a multisig registered by A does not alias A's own address (a later +/// compromise of A's single key must not also control the multisig); and +/// - if credential removal is ever added, a removed credential cannot be +/// re-registered by a different sender and collide with the old address. +pub fn derive_address_for_new_credential( + new_credential_id: &CredentialId, + sender: &S::Address, +) -> S::Address { + let mut hasher = ::Hasher::new(); + hasher.update(new_credential_id.0 .0); + hasher.update(sender.as_ref()); + let hash: [u8; 32] = hasher.finalize().into(); + // Route through `CredentialId` so each Spec's existing `From` + // handles address-width reduction (28 / 32 / 20 bytes). + S::Address::from(CredentialId::from_bytes(hash)) +} diff --git a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs index 56547921ad..47a6bc31b3 100644 --- a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs +++ b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs @@ -1,10 +1,8 @@ -use sov_accounts::{Accounts, CallMessage, Response}; +use sov_accounts::{derive_address_for_new_credential, Accounts, CallMessage, Response}; use sov_bank::{config_gas_token_id, Amount, Bank, Coins}; -use sov_modules_api::digest::Digest; use sov_modules_api::transaction::{UnsignedTransaction, Version1}; use sov_modules_api::{ - CredentialId, CryptoSpec, PrivateKey, PublicKey, RawTx, Runtime, SkippedTxContents, Spec, - TxEffect, + CryptoSpec, PrivateKey, PublicKey, RawTx, Runtime, SkippedTxContents, Spec, TxEffect, }; use sov_test_utils::runtime::genesis::optimistic::HighLevelOptimisticGenesisConfig; use sov_test_utils::runtime::TestRunner; @@ -22,18 +20,6 @@ generate_optimistic_runtime!(TestAccountsRuntime <=); type RT = TestAccountsRuntime; -/// Mirror of the address derivation in `Accounts::insert_credential_id`. -fn expected_derived_address( - new_credential: CredentialId, - sender: ::Address, -) -> ::Address { - let mut hasher = <::CryptoSpec as CryptoSpec>::Hasher::new(); - hasher.update(new_credential.0 .0); - hasher.update(sender.as_ref()); - let hash: [u8; 32] = hasher.finalize().into(); - ::Address::from(CredentialId::from_bytes(hash)) -} - struct TestData { account_1: TestUser, account_2: TestUser, @@ -126,7 +112,7 @@ fn test_update_account() { assert_eq!( accounts.get_account(new_credential, state), Response::AccountExists { - addr: expected_derived_address(new_credential, user.address()) + addr: derive_address_for_new_credential::(&new_credential, &user.address(),), } ); // Account corresponding to the old credential still exists. @@ -196,7 +182,8 @@ fn test_setup_multisig_and_act() { multisig.credential_id::<<::CryptoSpec as CryptoSpec>::Hasher>(); let user_address = user.address(); let user_credential_id = user.credential_id(); - let multisig_address = expected_derived_address(multisig_credential_id, user_address); + let multisig_address = + derive_address_for_new_credential::(&multisig_credential_id, &user_address); runner.execute_transaction(TransactionTestCase { input: user.create_plain_message::>(CallMessage::InsertCredentialId( multisig_credential_id, @@ -448,9 +435,9 @@ fn test_register_new_account() { assert_eq!( accounts.get_account(new_credential, state), Response::AccountExists { - addr: expected_derived_address( - new_credential, - non_registered_account.address(), + addr: derive_address_for_new_credential::( + &new_credential, + &non_registered_account.address(), ), } ); @@ -554,14 +541,20 @@ fn test_resolve_address_if_more_than_one_credential() { accounts .resolve_sender_address(&default_address_1, &credential_1, state) .unwrap(), - expected_derived_address(credential_1, non_registered_account.address()), + derive_address_for_new_credential::( + &credential_1, + &non_registered_account.address() + ), ); assert_eq!( accounts .resolve_sender_address(&default_address_2, &credential_2, state) .unwrap(), - expected_derived_address(credential_2, non_registered_account.address()), + derive_address_for_new_credential::( + &credential_2, + &non_registered_account.address() + ), ); }); } From 6156e7e875656895689bf77ff208ce6b6717e7e9 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 12:05:30 +0200 Subject: [PATCH 4/7] More fixes --- .../module-implementations/sov-accounts/Cargo.toml | 4 ++-- .../module-implementations/sov-accounts/src/utils.rs | 2 +- .../sov-accounts/tests/integration/main.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/module-system/module-implementations/sov-accounts/Cargo.toml b/crates/module-system/module-implementations/sov-accounts/Cargo.toml index b2a5bd3ccc..55e71820a4 100644 --- a/crates/module-system/module-implementations/sov-accounts/Cargo.toml +++ b/crates/module-system/module-implementations/sov-accounts/Cargo.toml @@ -41,12 +41,12 @@ arbitrary = [ "sov-modules-api/arbitrary", "sov-state/arbitrary", "sov-test-utils/arbitrary", - "sov-bank/arbitrary" + "sov-bank/arbitrary", ] native = [ "sov-accounts/native", "sov-modules-api/native", "sov-state/native", - "sov-bank/native" + "sov-bank/native", ] test-utils = [] diff --git a/crates/module-system/module-implementations/sov-accounts/src/utils.rs b/crates/module-system/module-implementations/sov-accounts/src/utils.rs index 1146cfcc2d..4d624c4e70 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/utils.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/utils.rs @@ -13,7 +13,7 @@ pub fn derive_address_for_new_credential( sender: &S::Address, ) -> S::Address { let mut hasher = ::Hasher::new(); - hasher.update(new_credential_id.0 .0); + hasher.update(&new_credential_id.0 .0); hasher.update(sender.as_ref()); let hash: [u8; 32] = hasher.finalize().into(); // Route through `CredentialId` so each Spec's existing `From` diff --git a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs index 47a6bc31b3..aaf6eaefd9 100644 --- a/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs +++ b/crates/module-system/module-implementations/sov-accounts/tests/integration/main.rs @@ -112,7 +112,7 @@ fn test_update_account() { assert_eq!( accounts.get_account(new_credential, state), Response::AccountExists { - addr: derive_address_for_new_credential::(&new_credential, &user.address(),), + addr: derive_address_for_new_credential::(&new_credential, &user.address()), } ); // Account corresponding to the old credential still exists. From effec969bb9978f3e66e92a7402ad453420ea9a8 Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 12:53:24 +0200 Subject: [PATCH 5/7] Tests update --- Cargo.toml | 1 - .../stf_blueprint/operator/auth_eip712.rs | 45 ++++++++++++++++--- .../sov-accounts/src/call.rs | 3 +- .../sov-accounts/src/utils.rs | 10 ++--- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 19a82b690d..7dc7334ccd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,7 +98,6 @@ members = [ "crates/web3", "python/py_sovereign_web3/rust" , "crates/utils/sov-evm-test-utils"] -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-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/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" diff --git a/crates/module-system/module-implementations/integration-tests/tests/stf_blueprint/operator/auth_eip712.rs b/crates/module-system/module-implementations/integration-tests/tests/stf_blueprint/operator/auth_eip712.rs index c5e4f57621..2f22b03fe2 100644 --- a/crates/module-system/module-implementations/integration-tests/tests/stf_blueprint/operator/auth_eip712.rs +++ b/crates/module-system/module-implementations/integration-tests/tests/stf_blueprint/operator/auth_eip712.rs @@ -1,4 +1,6 @@ -use sov_accounts::{Accounts, CallMessage as AccountsCallMessage}; +use sov_accounts::{ + derive_address_for_new_credential, Accounts, CallMessage as AccountsCallMessage, +}; use sov_address::{EthereumAddress, EvmCryptoSpec}; use sov_eip712_auth::{ Eip712Authenticator, Eip712AuthenticatorInput, Eip712AuthenticatorTrait, SchemaProvider, @@ -229,19 +231,35 @@ 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 + // Pre-generate the multisig so we can configure its (future) derived address + // as the ValueSetter admin at genesis. Without this the multisig-signed txs + // below would be rejected with "Only admin can change the value". 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::<<::CryptoSpec as CryptoSpec>::Hasher>(); + + let genesis_config = + HighLevelOptimisticGenesisConfig::generate().add_accounts_with_default_balance(2); + let admin = genesis_config + .additional_accounts() + .first() + .unwrap() + .clone(); + let multisig_address = + derive_address_for_new_credential::(&multisig_credential_id, &admin.address()); + 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()); + + // Register the multisig credential: inserts `multisig_credential_id -> multisig_address`. runner.execute_transaction(TransactionTestCase { input: admin.create_plain_message::>( AccountsCallMessage::InsertCredentialId(multisig_credential_id), @@ -251,6 +269,23 @@ fn test_multisig_signature_verification() { }), }); + // The multisig now lives at its own, non-controlled address and has no balance — + // fund it from `admin` so subsequent multisig-signed transactions can reserve gas. + runner.execute_transaction(TransactionTestCase { + input: admin.create_plain_message::>( + sov_bank::CallMessage::Transfer { + to: multisig_address, + coins: sov_bank::Coins { + amount: sov_bank::Amount::new(1_000_000_000_000), + token_id: sov_bank::config_gas_token_id(), + }, + }, + ), + assert: Box::new(move |result, _state| { + assert!(result.tx_receipt.is_successful(), "Funding transfer failed"); + }), + }); + // Create a multisig transaction let utx = create_utx::(encode_message::<_, RT>()); let mut signatures = Vec::new(); diff --git a/crates/module-system/module-implementations/sov-accounts/src/call.rs b/crates/module-system/module-implementations/sov-accounts/src/call.rs index 1c27500a23..6bb0770e42 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/call.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/call.rs @@ -31,10 +31,9 @@ impl Accounts { bail!("Custom account mappings are disabled"); } + // Important invariant: credential ids must never be deleted once inserted. self.exit_if_credential_exists(&new_credential_id, state)?; - // TODO: also mix in a per-sender nonce so the same (credential, sender) pair - // can't reproduce an address a compromised sender previously controlled. let addr = derive_address_for_new_credential::(&new_credential_id, context.sender()); self.accounts .set(&new_credential_id, &Account { addr }, state)?; diff --git a/crates/module-system/module-implementations/sov-accounts/src/utils.rs b/crates/module-system/module-implementations/sov-accounts/src/utils.rs index 4d624c4e70..71eb08b11e 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/utils.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/utils.rs @@ -3,17 +3,15 @@ use sov_modules_api::{CredentialId, CryptoSpec, Spec}; /// Derives the address assigned to a newly inserted credential. /// -/// The derivation mixes the transaction sender into the hash so that: -/// - a multisig registered by A does not alias A's own address (a later -/// compromise of A's single key must not also control the multisig); and -/// - if credential removal is ever added, a removed credential cannot be -/// re-registered by a different sender and collide with the old address. +/// The derivation mixes the transaction sender into the hash so that a +/// multisig registered by A does not alias A's own address — a later +/// compromise of A's single key must not also control the multisig. pub fn derive_address_for_new_credential( new_credential_id: &CredentialId, sender: &S::Address, ) -> S::Address { let mut hasher = ::Hasher::new(); - hasher.update(&new_credential_id.0 .0); + hasher.update(new_credential_id.0 .0); hasher.update(sender.as_ref()); let hash: [u8; 32] = hasher.finalize().into(); // Route through `CredentialId` so each Spec's existing `From` From 284aeef7d6fd70f057eaf25b418e1f32e9941dac Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 14:10:00 +0200 Subject: [PATCH 6/7] Revert accidental switcheroo --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 7dc7334ccd..19a82b690d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,7 @@ members = [ "crates/web3", "python/py_sovereign_web3/rust" , "crates/utils/sov-evm-test-utils"] +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-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/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" From 7765d2a7f79df4791274f4c2fd4f4baa06d65b6f Mon Sep 17 00:00:00 2001 From: Nikolai Golub Date: Mon, 20 Apr 2026 14:40:03 +0200 Subject: [PATCH 7/7] Update comments --- .../sov-accounts/src/capabilities.rs | 15 +++++++++++++-- .../sov-accounts/src/utils.rs | 7 +++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/module-system/module-implementations/sov-accounts/src/capabilities.rs b/crates/module-system/module-implementations/sov-accounts/src/capabilities.rs index 8d84fc04d4..fd3fa9217b 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/capabilities.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/capabilities.rs @@ -5,8 +5,19 @@ use crate::{Account, Accounts}; impl Accounts { /// 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. + /// + /// If the credential is already registered (by a prior `InsertCredentialId` + /// or prior auto-registration), returns the stored address. + /// + /// Otherwise immediately auto-registers the credential to `default_address` + /// (typically `Address::from(credential_id)`) and returns that. Note that + /// this auto-registration produces a **different** address than + /// [`CallMessage::InsertCredentialId`](crate::CallMessage::InsertCredentialId) + /// would — the latter derives `hash(credential_id || registering_sender)` + /// via [`derive_address_for_new_credential`](crate::derive_address_for_new_credential). + /// Callers that need a specific on-chain address for a credential (e.g. a + /// multisig) must pre-register via `InsertCredentialId` before the + /// credential's first tx. pub fn resolve_sender_address( &mut self, default_address: &S::Address, diff --git a/crates/module-system/module-implementations/sov-accounts/src/utils.rs b/crates/module-system/module-implementations/sov-accounts/src/utils.rs index 71eb08b11e..cf79c68274 100644 --- a/crates/module-system/module-implementations/sov-accounts/src/utils.rs +++ b/crates/module-system/module-implementations/sov-accounts/src/utils.rs @@ -6,6 +6,13 @@ use sov_modules_api::{CredentialId, CryptoSpec, Spec}; /// The derivation mixes the transaction sender into the hash so that a /// multisig registered by A does not alias A's own address — a later /// compromise of A's single key must not also control the multisig. +/// +/// This derivation is only used by `InsertCredentialId`. A credential that +/// skips explicit registration and first appears as the signer of a tx is +/// auto-registered by `Accounts::resolve_sender_address` to +/// `Address::from(credential_id)` instead — a different address. Callers +/// that need a specific on-chain address for a credential (e.g. a multisig) +/// must pre-register it via `InsertCredentialId` before its first tx. pub fn derive_address_for_new_credential( new_credential_id: &CredentialId, sender: &S::Address,