Skip to content
Draft
Show file tree
Hide file tree
Changes from 6 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 @@ -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,20 @@
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.
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