diff --git a/crates/alloy/src/accounts/store.rs b/crates/alloy/src/accounts/store.rs index 3e4b6b896c..57f7d2a68b 100644 --- a/crates/alloy/src/accounts/store.rs +++ b/crates/alloy/src/accounts/store.rs @@ -31,8 +31,9 @@ use tempo_contracts::precompiles::ITIP20; use tempo_primitives::{ SignatureType, TempoAddressExt, TempoTxEnvelope, transaction::{ - Call, CallScope, KeyAuthorization, KeychainSignature, PrimitiveSignature, SelectorRule, - SignedKeyAuthorization, TempoSignature, TempoTypedTransaction, TokenLimit, + Call, CallScope, KeyAuthorization, KeychainSignature, MultisigSignature, + PrimitiveSignature, SelectorRule, SignedKeyAuthorization, TempoSignature, + TempoTypedTransaction, TokenLimit, tt_signature::{P256SignatureWithPreHash, WebAuthnSignature}, }, }; @@ -1798,7 +1799,15 @@ struct PersistedSignedKeyAuthorization { account: Option
, #[serde(rename = "type")] key_type: PersistedKeyType, - signature: PersistedPrimitiveSignature, + signature: PersistedAuthorizationSignature, +} + +#[derive(Clone, Deserialize)] +#[serde(untagged)] +enum PersistedAuthorizationSignature { + Primitive(PersistedPrimitiveSignature), + Keychain(KeychainSignature), + Multisig(MultisigSignature), } #[derive(Clone, Deserialize)] @@ -1847,7 +1856,8 @@ impl TryFrom for SignedKeyAuthorization { is_admin: false, account: None, }; - Ok(Self::new(authorization, value.signature.try_into()?)) + let signature: PrimitiveSignature = value.signature.try_into()?; + Ok(Self::new(authorization, signature)) } } @@ -1993,7 +2003,18 @@ impl TryFrom for SignedKeyAuthorization { is_admin, account, }; - Ok(Self::new(authorization, signature.try_into()?)) + let signature = match signature { + PersistedAuthorizationSignature::Primitive(signature) => { + TempoSignature::Primitive(signature.try_into()?) + } + PersistedAuthorizationSignature::Keychain(signature) => { + TempoSignature::Keychain(signature) + } + PersistedAuthorizationSignature::Multisig(signature) => { + TempoSignature::Multisig(signature) + } + }; + Ok(Self::new(authorization, signature)) } } @@ -2399,7 +2420,7 @@ struct WritableTokenLimit { period: Option, } -#[derive(Serialize)] +#[derive(Clone, Serialize)] struct WritableScope { address: Address, #[serde(skip_serializing_if = "Option::is_none")] @@ -2427,7 +2448,7 @@ struct WritableSignedKeyAuthorization { account: Option
, #[serde(rename = "type")] key_type: &'static str, - signature: WritablePrimitiveSignature, + signature: WritableAuthorizationSignature, } #[derive(Serialize)] @@ -2451,6 +2472,14 @@ enum WritablePrimitiveSignature { }, } +#[derive(Serialize)] +#[serde(untagged)] +enum WritableAuthorizationSignature { + Primitive(WritablePrimitiveSignature), + Keychain(KeychainSignature), + Multisig(MultisigSignature), +} + #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct WritableSecpSignature { @@ -2592,6 +2621,31 @@ fn writable_access_key( }) .collect() }); + let scopes = writable_scopes(authorization); + let signature = match &authorization.signature { + TempoSignature::Primitive(signature) => { + WritableAuthorizationSignature::Primitive(writable_signature(signature)?) + } + TempoSignature::Keychain(signature) => { + WritableAuthorizationSignature::Keychain(signature.clone()) + } + TempoSignature::Multisig(signature) => { + WritableAuthorizationSignature::Multisig(signature.clone()) + } + }; + let key_authorization = WritableSignedKeyAuthorization { + address: authorization.key_id, + chain_id: writable_bigint(U256::from(authorization.chain_id)), + expiry: authorization.expiry.map(NonZeroU64::get), + limits: limits.clone(), + scopes: scopes.clone(), + witness: authorization.witness, + is_admin: authorization.is_admin, + account: authorization.account, + key_type: "secp256k1", + signature, + }; + Ok(WritableAccessKey { address: signer.address(), access: account, @@ -2599,20 +2653,9 @@ fn writable_access_key( key_type: "secp256k1", private_key: alloy_primitives::hex::encode_prefixed(signer.to_bytes()), expiry: authorization.expiry.map(NonZeroU64::get), - limits: limits.clone(), - scopes: writable_scopes(authorization), - key_authorization: WritableSignedKeyAuthorization { - address: authorization.key_id, - chain_id: writable_bigint(U256::from(authorization.chain_id)), - expiry: authorization.expiry.map(NonZeroU64::get), - limits, - scopes: writable_scopes(authorization), - witness: authorization.witness, - is_admin: authorization.is_admin, - account: authorization.account, - key_type: "secp256k1", - signature: writable_signature(&authorization.signature)?, - }, + limits, + scopes, + key_authorization, }) } @@ -3255,7 +3298,10 @@ mod tests { use alloy_network::{NetworkWallet, TransactionBuilder}; use alloy_provider::{ProviderBuilder, SendableTx, fillers::TxFiller, mock::Asserter}; use alloy_rpc_types_eth::{TransactionInput, TransactionRequest}; - use tempo_primitives::{TempoTxEnvelope, transaction::TempoSignature}; + use tempo_primitives::{ + TempoTxEnvelope, + transaction::{MultisigSignature, TempoSignature}, + }; use super::*; @@ -3555,6 +3601,42 @@ mod tests { fs::remove_dir_all(directory).unwrap(); } + #[test] + fn multisig_key_authorization_roundtrips_through_store() { + let directory = unique_test_directory(); + let path = directory.join("wallet/store.json"); + let account = Address::repeat_byte(0x44); + let signer = PrivateKeySigner::random(); + let authorization = + KeyAuthorization::unrestricted(4217, SignatureType::Secp256k1, signer.address()) + .with_account(account) + .into_signed(TempoSignature::Multisig(MultisigSignature::new( + account, + vec![PrimitiveSignature::default().to_bytes()], + None, + ))); + + TempoAccountsStore::at(&path) + .upsert_secp256k1_access_key(account, &signer, &authorization) + .unwrap(); + + let written: serde_json::Value = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap(); + let persisted = &written["tempo-cli.store"]["state"]["accessKeys"][0]["keyAuthorization"]; + assert_eq!( + persisted["signature"]["account"], + serde_json::to_value(account).unwrap() + ); + assert!(persisted["signature"]["signatures"].is_array()); + let stored = TempoAccountsStore::open(&path) + .unwrap() + .access_keys() + .unwrap() + .remove(0); + assert_eq!(stored.key_authorization(), Some(&authorization)); + + fs::remove_dir_all(directory).unwrap(); + } + #[test] fn persisted_boundary_deserializes_to_strict_types() { let state: PersistedAccountsState = serde_json::from_value(serde_json::json!({ @@ -3634,7 +3716,9 @@ mod tests { .as_slice() ) ); - let PrimitiveSignature::WebAuthn(signature) = &authorization.signature else { + let TempoSignature::Primitive(PrimitiveSignature::WebAuthn(signature)) = + &authorization.signature + else { panic!("expected WebAuthn root signature") }; assert_eq!(signature.webauthn_data.as_ref(), webauthn_data); diff --git a/crates/node/tests/it/tempo_transaction/runners.rs b/crates/node/tests/it/tempo_transaction/runners.rs index d8e3df46e4..094a1c1de2 100644 --- a/crates/node/tests/it/tempo_transaction/runners.rs +++ b/crates/node/tests/it/tempo_transaction/runners.rs @@ -608,11 +608,11 @@ pub(super) async fn run_estimate_gas_matrix( auth.authorization = auth .authorization .with_witness(B256::with_last_byte((i + 1) as u8)); - auth.signature = PrimitiveSignature::Secp256k1( + auth.signature = TempoSignature::Primitive(PrimitiveSignature::Secp256k1( signer .sign_hash_sync(&auth.authorization.signature_hash()) .expect("signing should succeed"), - ); + )); request.key_authorization = Some(auth); } } diff --git a/crates/primitives/src/transaction/key_authorization.rs b/crates/primitives/src/transaction/key_authorization.rs index 80c182a195..8ce32d53c2 100644 --- a/crates/primitives/src/transaction/key_authorization.rs +++ b/crates/primitives/src/transaction/key_authorization.rs @@ -1,5 +1,5 @@ use super::SignatureType; -use crate::transaction::PrimitiveSignature; +use crate::transaction::TempoSignature; use alloc::vec::Vec; use alloy_consensus::crypto::RecoveryError; use alloy_primitives::{Address, B256, U256, keccak256}; @@ -356,7 +356,7 @@ impl KeyAuthorization { } /// Convert the key authorization into a [`SignedKeyAuthorization`] with a signature. - pub fn into_signed(self, signature: PrimitiveSignature) -> SignedKeyAuthorization { + pub fn into_signed(self, signature: impl Into) -> SignedKeyAuthorization { SignedKeyAuthorization::new(self, signature) } @@ -420,50 +420,72 @@ pub struct SignedKeyAuthorization { #[deref] pub authorization: KeyAuthorization, - /// Signature authorizing this key (signed by root key) - pub signature: PrimitiveSignature, + /// Signature authorizing this key. + pub signature: TempoSignature, - /// Cached signer recovered from `signature`. + /// Cached primitive signer and the signed payload fingerprint it was recovered from. /// /// Excluded from encoding, equality, hashing, and arbitrary generation. #[cfg_attr(feature = "serde", serde(skip))] #[cfg_attr(any(test, feature = "arbitrary"), arbitrary(default))] #[rlp(skip, default)] - signer: OnceLock
, + signer: OnceLock<(B256, Address)>, } impl SignedKeyAuthorization { /// Create a signed key authorization with an empty signer cache. - pub fn new(authorization: KeyAuthorization, signature: PrimitiveSignature) -> Self { + pub fn new(authorization: KeyAuthorization, signature: impl Into) -> Self { Self { authorization, - signature, + signature: signature.into(), signer: OnceLock::new(), } } - /// Recover the signer of the [`KeyAuthorization`]. + /// Recovers and cryptographically verifies a primitive signer. pub fn recover_signer(&self) -> Result { - if let Some(signer) = self.signer.get() { + let TempoSignature::Primitive(signature) = &self.signature else { + return Err(RecoveryError::new()); + }; + let signature_hash = self.authorization.signature_hash(); + let mut cache_input = Vec::with_capacity(B256::len_bytes() + signature.encoded_length()); + cache_input.extend_from_slice(signature_hash.as_slice()); + cache_input.extend_from_slice(&signature.to_bytes()); + let cache_key = keccak256(cache_input); + if let Some((cached_key, signer)) = self.signer.get() + && *cached_key == cache_key + { return Ok(*signer); } - let signer = self - .signature - .recover_signer(&self.authorization.signature_hash())?; - self.cache_signer(signer); + let signer = signature.recover_signer(&signature_hash)?; + self.cache_signer(cache_key, signer); Ok(signer) } + /// Returns the account that claims to authorize this key. + /// + /// Primitive signatures are verified here. A multisig account is only shape-checked; callers + /// must verify its owner quorum against native multisig state before granting authority. + pub fn recover_authorizing_account(&self) -> Result { + match &self.signature { + TempoSignature::Primitive(_) => self.recover_signer(), + TempoSignature::Multisig(signature) => signature + .recover_account() + .map_err(|_| RecoveryError::new()), + TempoSignature::Keychain(_) => Err(RecoveryError::new()), + } + } + #[cfg(feature = "std")] - fn cache_signer(&self, signer: Address) { - let _ = self.signer.set(signer); + fn cache_signer(&self, cache_key: B256, signer: Address) { + let _ = self.signer.set((cache_key, signer)); } #[cfg(not(feature = "std"))] - fn cache_signer(&self, signer: Address) { - let _ = self.signer.set(alloc::boxed::Box::new(signer)); + fn cache_signer(&self, cache_key: B256, signer: Address) { + let _ = self.signer.set(alloc::boxed::Box::new((cache_key, signer))); } /// Calculates a heuristic for the in-memory size of the signed key authorization @@ -707,7 +729,7 @@ mod selector_hex_serde { mod tests { use super::*; use crate::transaction::{ - TempoSignature, + PrimitiveSignature, TempoSignature, tt_authorization::tests::{generate_secp256k1_keypair, sign_hash}, }; use alloy_rlp::{Decodable, Encodable}; @@ -928,6 +950,104 @@ mod tests { assert_ne!(bad_recovered.unwrap(), expected_address); } + #[test] + fn signer_cache_is_bound_to_signed_payload() { + let (signing_key, expected_address) = generate_secp256k1_keypair(); + let auth = make_auth(Some(1000), None); + let signature = match sign_hash(&signing_key, &auth.signature_hash()) { + TempoSignature::Primitive(signature) => signature, + _ => unreachable!("secp256k1 signing returns a primitive signature"), + }; + let mut signed = auth.into_signed(signature); + + assert_eq!(signed.recover_signer().unwrap(), expected_address); + + signed.authorization.expiry = NonZeroU64::new(2000); + assert_ne!(signed.recover_signer().unwrap(), expected_address); + + signed.signature = TempoSignature::Multisig(crate::transaction::MultisigSignature::new( + Address::repeat_byte(0x44), + vec![PrimitiveSignature::default().to_bytes()], + None, + )); + assert!(signed.recover_signer().is_err()); + } + + #[test] + fn primitive_signed_authorization_encoding_is_unchanged() { + #[derive(alloy_rlp::RlpEncodable)] + struct LegacySignedKeyAuthorization { + authorization: KeyAuthorization, + signature: PrimitiveSignature, + } + + let auth = make_auth(Some(1000), None); + let signature = PrimitiveSignature::default(); + let signed = auth.clone().into_signed(signature.clone()); + let legacy = LegacySignedKeyAuthorization { + authorization: auth, + signature, + }; + + let mut encoded = Vec::new(); + signed.encode(&mut encoded); + let mut legacy_encoded = Vec::new(); + legacy.encode(&mut legacy_encoded); + assert_eq!(encoded, legacy_encoded); + } + + #[test] + fn multisig_signed_authorization_roundtrip() { + let auth = make_auth(None, None).with_account(Address::repeat_byte(0x44)); + let signature = TempoSignature::Multisig(crate::transaction::MultisigSignature::new( + Address::repeat_byte(0x44), + vec![PrimitiveSignature::default().to_bytes()], + None, + )); + let signed = auth.into_signed(signature); + + let mut encoded = Vec::new(); + signed.encode(&mut encoded); + let decoded = SignedKeyAuthorization::decode(&mut encoded.as_slice()) + .expect("decode multisig-signed key authorization"); + + assert_eq!(decoded, signed); + assert!(decoded.signature.is_multisig()); + assert!(decoded.recover_signer().is_err()); + assert_eq!( + decoded.recover_authorizing_account().unwrap(), + Address::repeat_byte(0x44) + ); + } + + #[cfg(feature = "serde")] + #[test] + fn signed_key_authorization_json_bounds_multisig_nesting() { + let signed = make_auth(None, None).into_signed(PrimitiveSignature::default()); + let primitive_json = serde_json::to_string(&signed.signature).unwrap(); + let account_json = serde_json::to_string(&Address::repeat_byte(0x44)).unwrap(); + let prefix = format!(r#"{{"account":{account_json},"signatures":["#); + let depth = 4_096; + let mut nested_json = + String::with_capacity(depth * (prefix.len() + 2) + primitive_json.len()); + for _ in 0..depth { + nested_json.push_str(&prefix); + } + nested_json.push_str(&primitive_json); + for _ in 0..depth { + nested_json.push_str("]}"); + } + + let json = + serde_json::to_string(&signed) + .unwrap() + .replacen(&primitive_json, &nested_json, 1); + let error = serde_json::from_str::(&json) + .unwrap_err() + .to_string(); + assert!(error.contains("native multisig nesting depth exceeded")); + } + #[test] fn test_spending_expiry_and_size() { // has_unlimited_spending: None = true, Some = false diff --git a/crates/revm/src/handler.rs b/crates/revm/src/handler.rs index bf3a1be45f..faf04b6ad7 100644 --- a/crates/revm/src/handler.rs +++ b/crates/revm/src/handler.rs @@ -64,7 +64,7 @@ use crate::{ error::{FeePaymentError, TempoHaltReason}, evm::TempoContext, gas_credits, - signature_gas::{primitive_signature_verification_gas, tempo_signature_verification_gas}, + signature_gas::tempo_signature_verification_gas, }; /// Base gas for KeyAuthorization (22k storage + 5k buffer), signature gas added at runtime @@ -298,9 +298,9 @@ fn calculate_key_authorization_gas( spec: tempo_chainspec::hardfork::TempoHardfork, ) -> (u64, u64) { // All signature types pay ECRECOVER_GAS (3k) as the baseline since - // primitive_signature_verification_gas assumes ecrecover is already in base 21k. + // tempo_signature_verification_gas assumes ecrecover is already in base 21k. // For KeyAuthorization, we're doing an additional signature verification. - let sig_gas = ECRECOVER_GAS + primitive_signature_verification_gas(&key_auth.signature); + let sig_gas = ECRECOVER_GAS + tempo_signature_verification_gas(&key_auth.signature); let num_limits = key_auth .authorization @@ -1371,7 +1371,11 @@ where .map_err(|_| TempoInvalidTransaction::KeyAuthorizationSignatureRecoveryFailed)?; if auth_signer != tx.caller { - let key_auth_sig_type: u8 = key_auth.signature.signature_type().into(); + let key_auth_sig_type: u8 = key_auth + .signature + .signature_type() + .expect("non-primitive key authorization rejected in validate_env") + .into(); let signer_is_admin = match loaded_tx_access_key { Some(loaded_key) if loaded_key.key_id == auth_signer @@ -1801,6 +1805,10 @@ where .map_err(TempoInvalidTransaction::from)?; if aa_env.signature.is_multisig() + || aa_env + .key_authorization + .as_ref() + .is_some_and(|authorization| authorization.signature.is_multisig()) || aa_env .tempo_authorization_list .iter() @@ -1836,6 +1844,22 @@ where .validate_version(cfg.spec().is_t1c()) .map_err(TempoInvalidTransaction::from)?; } + if let Some(key_auth) = &aa_env.key_authorization { + key_auth + .signature + .validate_version(cfg.spec().is_t1c()) + .map_err(TempoInvalidTransaction::from)?; + if key_auth.signature.is_keychain() { + return Err(TempoInvalidTransaction::KeychainValidationFailed { + reason: "key authorization signatures cannot use keychain encoding" + .to_string(), + } + .into()); + } + if key_auth.signature.is_multisig() { + return Err(TempoInvalidTransaction::NativeMultisigNotActive.into()); + } + } let has_keychain_fields = aa_env.key_authorization.is_some() || aa_env.signature.is_keychain(); @@ -2014,7 +2038,7 @@ where } if key_auth.signature.signature_type() - != keychain_sig.signature.signature_type() + != Some(keychain_sig.signature.signature_type()) { return Err(TempoInvalidTransaction::KeychainValidationFailed { reason: diff --git a/crates/revm/src/handler/tests.rs b/crates/revm/src/handler/tests.rs index c03ceb6241..9d9e59f373 100644 --- a/crates/revm/src/handler/tests.rs +++ b/crates/revm/src/handler/tests.rs @@ -1,7 +1,11 @@ use super::*; use crate::{ FeeTokenResolver, ProtocolFeeManager, TempoBlockEnv, TempoFeeManager, TempoTxEnv, - evm::TempoEvm, gas_params::tempo_gas_params, signature_gas::P256_VERIFY_GAS, + evm::TempoEvm, + gas_params::tempo_gas_params, + signature_gas::{ + P256_VERIFY_GAS, primitive_signature_verification_gas, tempo_signature_verification_gas, + }, tx::TempoBatchCallEnv, }; use alloy_primitives::{Address, B256, Bytes, TxKind, U256}; @@ -24,8 +28,8 @@ use tempo_precompiles::{ tip_fee_manager::TipFeeManager, }; use tempo_primitives::transaction::{ - Call, InitMultisig, MultisigOwner, MultisigSignature, PrimitiveSignature, - RecoveredTempoAuthorization, TempoSignature, TempoSignedAuthorization, + Call, InitMultisig, KeyAuthorization, MultisigOwner, MultisigSignature, PrimitiveSignature, + RecoveredTempoAuthorization, SignatureType, TempoSignature, TempoSignedAuthorization, tt_signature::{P256SignatureWithPreHash, WebAuthnSignature}, }; @@ -1269,7 +1273,7 @@ fn test_t4_key_authorization_matches_tip1016_sstore_regular_cost() { // TIP-1016 is opt-in via amsterdam_eip8037; manually enable for this test. let gas_params = crate::gas_params::tempo_gas_params_with_amsterdam(TempoHardfork::T4, true); - let sig_gas = ECRECOVER_GAS + primitive_signature_verification_gas(&key_auth.signature); + let sig_gas = ECRECOVER_GAS + tempo_signature_verification_gas(&key_auth.signature); let sload = gas_params.warm_storage_read_cost() + gas_params.cold_storage_additional_cost(); let scope_extra_gas = call_scope_extra_gas(&key_auth.authorization); let (regular_gas, state_gas) = @@ -1291,7 +1295,7 @@ fn test_t7_key_authorization_intrinsic_includes_storage_credit_value() { )); let gas_params = crate::gas_params::tempo_gas_params(TempoHardfork::T7); - let sig_gas = ECRECOVER_GAS + primitive_signature_verification_gas(&key_auth.signature); + let sig_gas = ECRECOVER_GAS + tempo_signature_verification_gas(&key_auth.signature); let sload = gas_params.warm_storage_read_cost() + gas_params.cold_storage_additional_cost(); let scope_extra_gas = call_scope_extra_gas(&key_auth.authorization); let (regular_gas, state_gas) = @@ -4513,3 +4517,35 @@ fn native_multisig_execution_remains_inactive() { )) )); } + +#[test] +fn native_multisig_key_authorization_remains_inactive() { + let account = Address::repeat_byte(0x44); + let key_authorization = + KeyAuthorization::unrestricted(1, SignatureType::Secp256k1, Address::repeat_byte(0x55)) + .with_account(account) + .into_signed(TempoSignature::Multisig(MultisigSignature::new( + account, + vec![PrimitiveSignature::default().to_bytes()], + None, + ))); + let aa_env = TempoBatchCallEnv { + key_authorization: Some(key_authorization), + aa_calls: vec![Call { + to: TxKind::Call(Address::random()), + value: U256::ZERO, + input: Bytes::new(), + }], + ..Default::default() + }; + let mut test = TestHandlerEvm::aa(TempoHardfork::T11, aa_env, |tx_env| { + tx_env.inner.caller = account; + }); + + assert!(matches!( + test.validate_env(), + Err(EVMError::Transaction( + TempoInvalidTransaction::NativeMultisigNotActive + )) + )); +}