Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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::<<<S as Spec>::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::<S>(&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::<RT, Accounts<S>>(
AccountsCallMessage::InsertCredentialId(multisig_credential_id),
Expand All @@ -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::<RT, sov_bank::Bank<S>>(
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::<S, RT>(encode_message::<_, RT>());
let mut signatures = Vec::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -40,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 = []
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
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::{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)]
Expand All @@ -32,13 +31,12 @@ impl<S: Spec> Accounts<S> {
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)?;

// Insert the new credential id -> account mapping
let account = Account {
addr: *context.sender(),
};
self.accounts.set(&new_credential_id, &account, state)?;
let addr = derive_address_for_new_credential::<S>(&new_credential_id, context.sender());
self.accounts
.set(&new_credential_id, &Account { addr }, state)?;

Ok(())
}
Expand All @@ -53,7 +51,7 @@ impl<S: Spec> Accounts<S> {
.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(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,19 @@ use crate::{Account, 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.
///
/// 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<ST: StateAccessor>(
&mut self,
default_address: &S::Address,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct AccountData<Address> {
pub struct AccountConfig<S: Spec> {
/// Accounts to initialize the rollup.
pub accounts: Vec<AccountData<S::Address>>,
/// Enable custom `CredentailId` => `Account` mapping.
/// Enable custom `CredentialId` => `Account` mapping.
#[serde(default = "default_true")]
pub enable_custom_account_mappings: bool,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
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.
///
/// 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<S: Spec>(
new_credential_id: &CredentialId,
sender: &S::Address,
) -> S::Address {
let mut hasher = <S::CryptoSpec as CryptoSpec>::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<CredentialId>`
// handles address-width reduction (28 / 32 / 20 bytes).
S::Address::from(CredentialId::from_bytes(hash))
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
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::transaction::{UnsignedTransaction, Version1};
use sov_modules_api::{
CryptoSpec, PrivateKey, PublicKey, RawTx, Runtime, SkippedTxContents, Spec, TxEffect,
Expand Down Expand Up @@ -107,11 +108,11 @@ fn test_update_account() {

let accounts = Accounts::<S>::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: derive_address_for_new_credential::<S>(&new_credential, &user.address()),
}
);
// Account corresponding to the old credential still exists.
Expand All @@ -132,23 +133,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::<RT, Accounts<S>>(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::<RT, Accounts<S>>(
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:?}"),
}),
});
}
Expand All @@ -174,6 +180,10 @@ 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::<<<S as Spec>::CryptoSpec as CryptoSpec>::Hasher>();
let user_address = user.address();
let user_credential_id = user.credential_id();
let multisig_address =
derive_address_for_new_credential::<S>(&multisig_credential_id, &user_address);
runner.execute_transaction(TransactionTestCase {
input: user.create_plain_message::<RT, Accounts<S>>(CallMessage::InsertCredentialId(
multisig_credential_id,
Expand All @@ -183,22 +193,38 @@ fn test_setup_multisig_and_act() {

let accounts = Accounts::<S>::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::<RT, Bank<S>>(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");
}),
});

Expand Down Expand Up @@ -405,11 +431,14 @@ fn test_register_new_account() {

let accounts = Accounts::<S>::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: derive_address_for_new_credential::<S>(
&new_credential,
&non_registered_account.address(),
),
}
);

Expand Down Expand Up @@ -507,18 +536,25 @@ fn test_resolve_address_if_more_than_one_credential() {
runner.query_visible_state(|state| {
let mut accounts = Accounts::<S>::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()
derive_address_for_new_credential::<S>(
&credential_1,
&non_registered_account.address()
),
);

assert_eq!(
accounts
.resolve_sender_address(&default_address_2, &credential_2, state)
.unwrap(),
non_registered_account.address()
derive_address_for_new_credential::<S>(
&credential_2,
&non_registered_account.address()
),
);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
}
},
"enable_custom_account_mappings": {
"description": "Enable custom `CredentailId` => `Account` mapping.",
"description": "Enable custom `CredentialId` => `Account` mapping.",
"default": true,
"type": "boolean"
}
Expand Down
Loading