From 500963d36cf2f9e9ecac74d9af29c3099dccb3fd Mon Sep 17 00:00:00 2001 From: hooftly Date: Mon, 13 Oct 2025 20:13:09 -0600 Subject: [PATCH 01/16] feat(policy): canonical Session struct; enforce windows & lengths - Add canonical Session struct (session key, valid_after/until, value cap, allowlist lengths) and export for contract consumers - Update UA2 account to use the struct; enforce declared vs actual allowlist lengths; require valid_after < valid_until; emit expanded session metadata in events - Refresh tests to exercise length mismatches and activation window logic - Update docs and SDK helpers to reflect the revised policy shape --- docs/interfaces.md | 5 +- docs/rfc-ua2-sdk.md | 13 +- docs/runbook-sepolia.md | 2 +- docs/test-plan.md | 3 +- docs/validation.md | 15 +- packages/contracts/src/lib.cairo | 1 + packages/contracts/src/session.cairo | 24 +++ packages/contracts/src/ua2_account.cairo | 95 +++++++--- .../tests/test_session_nonce_ok.cairo | 36 ++-- ...st_session_nonce_replay_and_mismatch.cairo | 36 ++-- packages/contracts/tests/test_sessions.cairo | 11 +- .../tests/test_validate_allowlists.cairo | 38 ++-- .../tests/test_validate_denials.cairo | 169 ++++++++++++++---- packages/core/src/sessions.ts | 74 +++++--- packages/core/src/types.ts | 7 +- 15 files changed, 386 insertions(+), 143 deletions(-) create mode 100644 packages/contracts/src/session.cairo diff --git a/docs/interfaces.md b/docs/interfaces.md index 7a7b38d..a086dd3 100644 --- a/docs/interfaces.md +++ b/docs/interfaces.md @@ -26,7 +26,8 @@ ```rust struct SessionPolicy { is_active: bool, - expires_at: u64, + valid_after: u64, + valid_until: u64, max_calls: u32, calls_used: u32, max_value_per_call: Uint256, @@ -39,7 +40,7 @@ The selector and target allowlists are not embedded in the struct. They are stor ## Events -* `SessionAdded(key_hash: felt252, expires_at: u64, max_calls: u32)` +* `SessionAdded(key_hash: felt252, valid_after: u64, valid_until: u64, max_calls: u32)` * `SessionRevoked(key_hash: felt252)` * `SessionUsed(key_hash: felt252, used: u32)` * `SessionNonceAdvanced(key_hash: felt252, new_nonce: u128)` diff --git a/docs/rfc-ua2-sdk.md b/docs/rfc-ua2-sdk.md index 89a95ea..d45dfaf 100644 --- a/docs/rfc-ua2-sdk.md +++ b/docs/rfc-ua2-sdk.md @@ -145,7 +145,8 @@ OZ Account is the canonical base for `__validate__`/`__execute__` pattern and mu ``` struct SessionPolicy { is_active: bool, - expires_at: u64, // block timestamp + valid_after: u64, // block timestamp + valid_until: u64, // block timestamp max_calls: u32, calls_used: u32, max_value_per_call: Uint256, // wei-like units for token/native @@ -159,7 +160,7 @@ Selector and target allowlists are stored separately under `sessionTargetAllow(s * If signature is by `owner_pubkey`: standard path. * Else if signature verifies to a registered **session key**: - * Check `is_active`, `now <= expires_at`, and `calls_used + tx_call_count <= max_calls`. + * Check `is_active`, `now >= valid_after`, `now <= valid_until`, and `calls_used + tx_call_count <= max_calls`. * Require allowlist booleans for `(key_hash, target)` and `(key_hash, selector)` to be `true`. * Enforce ERC-20 transfer amounts ≤ `max_value_per_call`. * Require session nonce match, then verify the ECDSA signature over the poseidon-hashed call set. @@ -168,7 +169,7 @@ Selector and target allowlists are stored separately under `sessionTargetAllow(s **Events:** ``` -event SessionAdded(key_hash: felt252, expires_at: u64, max_calls: u32); +event SessionAdded(key_hash: felt252, valid_after: u64, valid_until: u64, max_calls: u32); event SessionRevoked(key_hash: felt252); event SessionUsed(key_hash: felt252, used: u32); event SessionNonceAdvanced(key_hash: felt252, new_nonce: u128); @@ -252,7 +253,7 @@ await ua.sessions.revoke(sess.id); ## 10. Security Considerations * **Domain separation:** Session signatures bind to `(chain_id, account_addr)`. ([docs.starknet.io][1]) -* **Expiry & limits:** Every session must have a hard `expires_at`; default small `maxCalls`. +* **Expiry & limits:** Every session must declare `valid_after`/`valid_until`; default small `maxCalls`. * **Revocation:** `revokeSession(key_hash)` immediately blocks use. Events let dApps react. * **Replay protection:** Optional per-session nonce (`sessionNonce`) incremented in validation. * **Guardian griefing:** Require **m-of-n** quorum and **timelock**; owner can cancel a pending recovery. @@ -265,7 +266,7 @@ await ua.sessions.revoke(sess.id); * **Validation path**: O(#calls * (selector + target checks)). Use **bitset/bitmap** encodings for selectors if needed; start with arrays for simplicity, upgrade later. * **Storage**: `calls_used` is incremented once per tx (after checking aggregate calls), not per inner call, to minimize writes. -* **Policy packing**: keep `expires_at` in `u64`; `maxCalls` in `u32`; selectors as `felt252[]`. +* **Policy packing**: keep `valid_after`/`valid_until` in `u64`; `maxCalls` in `u32`; selectors as `felt252[]`. --- @@ -320,7 +321,7 @@ await ua.sessions.revoke(sess.id); **Events** ``` -event SessionAdded(key_hash: felt252, expires_at: u64, max_calls: u32); +event SessionAdded(key_hash: felt252, valid_after: u64, valid_until: u64, max_calls: u32); event SessionRevoked(key_hash: felt252); event SessionUsed(key_hash: felt252, used: u32); event SessionNonceAdvanced(key_hash: felt252, new_nonce: u128); diff --git a/docs/runbook-sepolia.md b/docs/runbook-sepolia.md index 41ff158..cab6356 100644 --- a/docs/runbook-sepolia.md +++ b/docs/runbook-sepolia.md @@ -237,7 +237,7 @@ sncast --profile sepolia invoke \ ``` -The calldata order is: session key, `is_active`, `expires_at`, `max_calls`, `calls_used`, `max_value_per_call.low`, +The calldata order is: session key, `valid_after`, `valid_until`, `max_calls`, `max_value_per_call.low`, `max_value_per_call.high`, number of allowed targets, each target address, number of allowed selectors, each selector felt. If the contract checks revert, verify you supplied proper calldata as per `docs/interfaces.md`. diff --git a/docs/test-plan.md b/docs/test-plan.md index 004c574..5c7d483 100644 --- a/docs/test-plan.md +++ b/docs/test-plan.md @@ -160,7 +160,8 @@ Script: `packages/example/scripts/e2e-devnet.ts` 1. Deploy UA² to devnet (or attach if auto-deployed by script). 2. Create a session with: - * `expires_at = now + 2h` + * `valid_after = now` + * `valid_until = now + 2h` * `max_calls = 5` * selectors = `[transfer]` * targets = `[ERC20_TEST_ADDR]` diff --git a/docs/validation.md b/docs/validation.md index 47b5d5b..56d3677 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -11,16 +11,17 @@ 1. Compute `key_hash` = pedersen(session_pubkey). 2. Lookup the base `SessionPolicy` in `session` storage and fetch allowlist booleans from `session_target_allow` / `session_selector_allow` using `(key_hash, target)` and `(key_hash, selector)` keys. 3. Require `is_active == true` (`ERR_SESSION_INACTIVE`). -4. Require `block.timestamp <= expires_at` (`ERR_SESSION_EXPIRED`). -5. Require `calls_used + tx_call_count <= max_calls` (`ERR_POLICY_CALLCAP`). -6. For each call in `tx.multicall`: +4. Require `block.timestamp >= valid_after` (`ERR_SESSION_NOT_READY`). +5. Require `block.timestamp <= valid_until` (`ERR_SESSION_EXPIRED`). +6. Require `calls_used + tx_call_count <= max_calls` (`ERR_POLICY_CALLCAP`). +7. For each call in `tx.multicall`: - Assert target allowlist entry is `true` (`ERR_POLICY_TARGET_DENIED`). - Assert selector allowlist entry is `true` (`ERR_POLICY_SELECTOR_DENIED`). - If selector == `ERC20::transfer`, ensure amount ≤ `max_value_per_call` (`ERR_VALUE_LIMIT_EXCEEDED`). -7. Require provided session nonce == stored nonce (`ERR_BAD_SESSION_NONCE`). -8. Verify ECDSA signature against the computed session message (`ERR_SESSION_SIG_INVALID`). -9. Call `apply_session_usage` to atomically bump `calls_used`, advance the nonce, and emit `SessionUsed` + `SessionNonceAdvanced`. -10. Proceed to `__execute__`. +8. Require provided session nonce == stored nonce (`ERR_BAD_SESSION_NONCE`). +9. Verify ECDSA signature against the computed session message (`ERR_SESSION_SIG_INVALID`). +10. Call `apply_session_usage` to atomically bump `calls_used`, advance the nonce, and emit `SessionUsed` + `SessionNonceAdvanced`. +11. Proceed to `__execute__`. If any check fails → revert with specific error code. diff --git a/packages/contracts/src/lib.cairo b/packages/contracts/src/lib.cairo index d8f762a..bc62001 100644 --- a/packages/contracts/src/lib.cairo +++ b/packages/contracts/src/lib.cairo @@ -1,2 +1,3 @@ pub mod ua2_account; pub mod mock_erc20; +pub mod session; diff --git a/packages/contracts/src/session.cairo b/packages/contracts/src/session.cairo new file mode 100644 index 0000000..f41584f --- /dev/null +++ b/packages/contracts/src/session.cairo @@ -0,0 +1,24 @@ +use core::array::Array; +use core::integer::u256; +use starknet::ContractAddress; + +/// Canonical session configuration passed by the account owner. +/// +/// * `pubkey` is the Stark curve public key for the session signer (felt252). +/// * `valid_after` / `valid_until` gate the validity window using block timestamps (u64). +/// * `max_calls` limits how many calls can be executed over the lifetime of the session (u32). +/// * `value_cap` bounds the maximum value per call in wei-like units (u256). +/// * `targets_len` mirrors the number of addresses contained in `targets`. +/// * `selectors_len` mirrors the number of function selectors contained in `selectors`. +#[derive(Drop, Serde)] +pub struct Session { + pub pubkey: felt252, + pub valid_after: u64, + pub valid_until: u64, + pub max_calls: u32, + pub value_cap: u256, + pub targets_len: u32, + pub targets: Array, + pub selectors_len: u32, + pub selectors: Array, +} diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 1966313..05aacf6 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -5,6 +5,7 @@ use openzeppelin::introspection::src5::SRC5Component; #[feature("deprecated_legacy_map")] pub mod UA2Account { use super::{AccountComponent, SRC5Component}; + use crate::session::Session; use core::array::{Array, ArrayTrait, SpanTrait}; use core::option::Option; use core::traits::{Into, TryInto}; @@ -34,6 +35,9 @@ pub mod UA2Account { const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; const ERR_VALUE_LIMIT_EXCEEDED: felt252 = 'ERR_VALUE_LIMIT_EXCEEDED'; + const ERR_SESSION_NOT_READY: felt252 = 'ERR_SESSION_NOT_READY'; + const ERR_SESSION_TARGETS_LEN: felt252 = 'ERR_SESSION_TARGETS_LEN'; + const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; const ERR_POLICY_CALLCOUNT_MISMATCH: felt252 = 'ERR_POLICY_CALLCOUNT_MISMATCH'; const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; @@ -85,7 +89,8 @@ pub mod UA2Account { #[derive(Copy, Drop, Serde, starknet::Store)] pub struct SessionPolicy { pub is_active: bool, - pub expires_at: u64, + pub valid_after: u64, + pub valid_until: u64, pub max_calls: u32, pub calls_used: u32, pub max_value_per_call: u256, @@ -94,7 +99,8 @@ pub mod UA2Account { #[derive(Drop, starknet::Event)] pub struct SessionAdded { pub key_hash: felt252, - pub expires_at: u64, + pub valid_after: u64, + pub valid_until: u64, pub max_calls: u32, } @@ -420,32 +426,60 @@ pub mod UA2Account { } #[external(v0)] - fn add_session_with_allowlists( - ref self: ContractState, - key: felt252, - mut policy: SessionPolicy, - targets: Array, - selectors: Array, - ) { + fn add_session_with_allowlists(ref self: ContractState, session: Session) { assert_owner(); - self.add_session(key, policy); - let key_hash = derive_key_hash(key); + let Session { + pubkey, + valid_after, + valid_until, + max_calls, + value_cap, + targets_len, + targets, + selectors_len, + selectors, + } = session; + + let declared_targets_len: usize = match targets_len.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, ERR_SESSION_TARGETS_LEN); + 0_usize + }, + }; + let actual_targets_len = ArrayTrait::::len(@targets); + require(actual_targets_len == declared_targets_len, ERR_SESSION_TARGETS_LEN); - let mut i = 0_usize; - let targets_len = ArrayTrait::::len(@targets); - while i < targets_len { - let target = *ArrayTrait::::at(@targets, i); - self.session_target_allow.write((key_hash, target), true); - i += 1_usize; + let declared_selectors_len: usize = match selectors_len.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, ERR_SESSION_SELECTORS_LEN); + 0_usize + }, + }; + let actual_selectors_len = ArrayTrait::::len(@selectors); + require(actual_selectors_len == declared_selectors_len, ERR_SESSION_SELECTORS_LEN); + + let mut policy = SessionPolicy { + is_active: false, + valid_after, + valid_until, + max_calls, + calls_used: 0_u32, + max_value_per_call: value_cap, + }; + + self.add_session(pubkey, policy); + + let key_hash = derive_key_hash(pubkey); + + for target_ref in targets.span() { + self.session_target_allow.write((key_hash, *target_ref), true); } - i = 0_usize; - let selectors_len = ArrayTrait::::len(@selectors); - while i < selectors_len { - let selector = *ArrayTrait::::at(@selectors, i); - self.session_selector_allow.write((key_hash, selector), true); - i += 1_usize; + for selector_ref in selectors.span() { + self.session_selector_allow.write((key_hash, *selector_ref), true); } } @@ -462,7 +496,8 @@ pub mod UA2Account { require(policy.is_active, ERR_SESSION_INACTIVE); let now = get_block_timestamp(); - require(now <= policy.expires_at, ERR_SESSION_EXPIRED); + require(now >= policy.valid_after, ERR_SESSION_NOT_READY); + require(now <= policy.valid_until, ERR_SESSION_EXPIRED); require(policy.calls_used == prior_calls_used, ERR_POLICY_CALLCOUNT_MISMATCH); @@ -487,7 +522,7 @@ pub mod UA2Account { fn add_session(ref self: ContractState, key: felt252, mut policy: SessionPolicy) { assert_owner(); - assert(policy.expires_at > 0_u64, 'BAD_EXPIRY'); + assert(policy.valid_until > policy.valid_after, 'BAD_VALID_WINDOW'); assert(policy.max_calls > 0_u32, 'BAD_MAX_CALLS'); let key_hash = derive_key_hash(key); @@ -498,7 +533,12 @@ pub mod UA2Account { self.session.write(key_hash, policy); self.session_nonce.write(key_hash, 0_u128); - self.emit(Event::SessionAdded(SessionAdded { key_hash, expires_at: policy.expires_at, max_calls: policy.max_calls })); + self.emit(Event::SessionAdded(SessionAdded { + key_hash, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + })); } fn get_session(self: @ContractState, key_hash: felt252) -> SessionPolicy { @@ -701,7 +741,8 @@ pub mod UA2Account { require(policy.is_active, ERR_SESSION_INACTIVE); let now = get_block_timestamp(); - require(now <= policy.expires_at, ERR_SESSION_EXPIRED); + require(now >= policy.valid_after, ERR_SESSION_NOT_READY); + require(now <= policy.valid_until, ERR_SESSION_EXPIRED); let calls_len = ArrayTrait::::len(calls); let tx_call_count: u32 = match calls_len.try_into() { diff --git a/packages/contracts/tests/test_session_nonce_ok.cairo b/packages/contracts/tests/test_session_nonce_ok.cairo index 6857d93..28d8698 100644 --- a/packages/contracts/tests/test_session_nonce_ok.cairo +++ b/packages/contracts/tests/test_session_nonce_ok.cairo @@ -27,6 +27,7 @@ use ua2_contracts::ua2_account::UA2Account::{ SessionPolicy, SessionUsed, }; +use ua2_contracts::session::Session; use crate::session_test_utils::{ build_session_signature, @@ -57,20 +58,26 @@ fn add_session( ) { start_cheat_caller_address(account_address, account_address); + let mut targets: Array = array![]; + targets.append(mock_address); + + let mut selectors: Array = array![]; + selectors.append(TRANSFER_SELECTOR); + + let session = Session { + pubkey: session_pubkey, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len: 1_u32, + targets, + selectors_len: 1_u32, + selectors, + }; + let mut calldata = array![]; - calldata.append(session_pubkey); - let active_flag: felt252 = if policy.is_active { 1 } else { 0 }; - calldata.append(active_flag); - calldata.append(policy.expires_at.into()); - calldata.append(policy.max_calls.into()); - calldata.append(policy.calls_used.into()); - calldata.append(policy.max_value_per_call.low.into()); - calldata.append(policy.max_value_per_call.high.into()); - - calldata.append(1.into()); - calldata.append(mock_address.into()); - calldata.append(1.into()); - calldata.append(TRANSFER_SELECTOR); + Serde::::serialize(@session, ref calldata); call_contract_syscall( account_address, @@ -123,7 +130,8 @@ fn test_session_nonce_ok() { let key_hash = session_key_hash(); let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, diff --git a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo index 5be05a3..775710a 100644 --- a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo +++ b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo @@ -19,6 +19,7 @@ use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use ua2_contracts::ua2_account::UA2Account::SessionPolicy; +use ua2_contracts::session::Session; use crate::session_test_utils::{build_session_signature, session_key}; @@ -47,20 +48,26 @@ fn add_session( ) { start_cheat_caller_address(account_address, account_address); + let mut targets: Array = array![]; + targets.append(mock_address); + + let mut selectors: Array = array![]; + selectors.append(TRANSFER_SELECTOR); + + let session = Session { + pubkey: session_pubkey, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len: 1_u32, + targets, + selectors_len: 1_u32, + selectors, + }; + let mut calldata = array![]; - calldata.append(session_pubkey); - let active_flag: felt252 = if policy.is_active { 1 } else { 0 }; - calldata.append(active_flag); - calldata.append(policy.expires_at.into()); - calldata.append(policy.max_calls.into()); - calldata.append(policy.calls_used.into()); - calldata.append(policy.max_value_per_call.low.into()); - calldata.append(policy.max_value_per_call.high.into()); - - calldata.append(1.into()); - calldata.append(mock_address.into()); - calldata.append(1.into()); - calldata.append(TRANSFER_SELECTOR); + Serde::::serialize(@session, ref calldata); call_contract_syscall( account_address, @@ -126,7 +133,8 @@ fn test_session_nonce_replay_and_mismatch() { let session_pubkey = session_key(); let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, diff --git a/packages/contracts/tests/test_sessions.cairo b/packages/contracts/tests/test_sessions.cairo index 2a485ee..69c4a88 100644 --- a/packages/contracts/tests/test_sessions.cairo +++ b/packages/contracts/tests/test_sessions.cairo @@ -42,7 +42,8 @@ fn add_get_revoke_session_works() { let key_hash = pedersen(key, 0); let policy = SessionPolicy { is_active: false, - expires_at: 3_600_u64, + valid_after: 0_u64, + valid_until: 3_600_u64, max_calls: 5_u32, calls_used: 2_u32, max_value_per_call: u256 { low: 0, high: 0 }, @@ -52,7 +53,7 @@ fn add_get_revoke_session_works() { let stored_policy = dispatcher.get_session(key_hash); assert(stored_policy.is_active == true, 'session inactive'); - assert(stored_policy.expires_at == 3_600_u64, 'expiry mismatch'); + assert(stored_policy.valid_until == 3_600_u64, 'expiry mismatch'); assert(stored_policy.max_calls == 5_u32, 'max calls mismatch'); assert(stored_policy.calls_used == 0_u32, 'calls used not reset'); @@ -75,7 +76,8 @@ fn events_emitted() { let key_hash = pedersen(key, 0); let policy = SessionPolicy { is_active: true, - expires_at: 7_200_u64, + valid_after: 0_u64, + valid_until: 7_200_u64, max_calls: 10_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 0, high: 0 }, @@ -91,7 +93,8 @@ fn events_emitted() { contract_address, UA2Account::Event::SessionAdded(SessionAdded { key_hash, - expires_at: 7_200_u64, + valid_after: 0_u64, + valid_until: 7_200_u64, max_calls: 10_u32, }), ), diff --git a/packages/contracts/tests/test_validate_allowlists.cairo b/packages/contracts/tests/test_validate_allowlists.cairo index 6db77f0..d913465 100644 --- a/packages/contracts/tests/test_validate_allowlists.cairo +++ b/packages/contracts/tests/test_validate_allowlists.cairo @@ -27,6 +27,7 @@ use ua2_contracts::ua2_account::UA2Account::{ SessionPolicy, SessionUsed, }; +use ua2_contracts::session::Session; use crate::session_test_utils::{ build_session_signature, @@ -48,11 +49,13 @@ fn session_allows_whitelisted_calls() { let mock_class = mock_declare.contract_class(); let (mock_address, _) = mock_class.deploy(@array![]).unwrap_syscall(); - let expires_at = 10_000_u64; + let valid_after = 0_u64; + let valid_until = 10_000_u64; let policy = SessionPolicy { is_active: true, - expires_at, + valid_after, + valid_until, max_calls: 1_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, @@ -61,20 +64,27 @@ fn session_allows_whitelisted_calls() { start_cheat_block_timestamp(account_address, 5_000_u64); start_cheat_caller_address(account_address, account_address); - let mut allowlist_calldata = array![]; let session_pubkey = session_key(); let key_hash = session_key_hash(); - allowlist_calldata.append(session_pubkey); - allowlist_calldata.append(1.into()); - allowlist_calldata.append(expires_at.into()); - allowlist_calldata.append(policy.max_calls.into()); - allowlist_calldata.append(policy.calls_used.into()); - allowlist_calldata.append(policy.max_value_per_call.low.into()); - allowlist_calldata.append(policy.max_value_per_call.high.into()); - allowlist_calldata.append(1.into()); - allowlist_calldata.append(mock_address.into()); - allowlist_calldata.append(1.into()); - allowlist_calldata.append(TRANSFER_SELECTOR); + let mut targets: Array = array![]; + targets.append(mock_address); + let mut selectors: Array = array![]; + selectors.append(TRANSFER_SELECTOR); + + let session = Session { + pubkey: session_pubkey, + valid_after, + valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len: 1_u32, + targets, + selectors_len: 1_u32, + selectors, + }; + + let mut allowlist_calldata = array![]; + Serde::::serialize(@session, ref allowlist_calldata); call_contract_syscall( account_address, diff --git a/packages/contracts/tests/test_validate_denials.cairo b/packages/contracts/tests/test_validate_denials.cairo index 86540f5..5d212fb 100644 --- a/packages/contracts/tests/test_validate_denials.cairo +++ b/packages/contracts/tests/test_validate_denials.cairo @@ -1,5 +1,6 @@ use core::array::{Array, ArrayTrait, SpanTrait}; use core::integer::u256; +use core::option::Option; use core::result::Result; use core::serde::Serde; use core::traits::{Into, TryInto}; @@ -19,6 +20,7 @@ use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use ua2_contracts::ua2_account::UA2Account::SessionPolicy; +use ua2_contracts::session::Session; use crate::session_test_utils::{build_session_signature, session_key}; @@ -27,6 +29,8 @@ const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; const ERR_SESSION_EXPIRED: felt252 = 'ERR_SESSION_EXPIRED'; +const ERR_SESSION_TARGETS_LEN: felt252 = 'ERR_SESSION_TARGETS_LEN'; +const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; const ERR_POLICY_CALLCAP: felt252 = 'ERR_POLICY_CALLCAP'; const ERR_VALUE_LIMIT_EXCEEDED: felt252 = 'ERR_VALUE_LIMIT_EXCEEDED'; @@ -51,34 +55,48 @@ fn add_session_with_lists( ) { start_cheat_caller_address(account_address, account_address); - let mut calldata = array![]; - calldata.append(key); - let active_flag: felt252 = if policy.is_active { 1 } else { 0 }; - calldata.append(active_flag); - calldata.append(policy.expires_at.into()); - calldata.append(policy.max_calls.into()); - calldata.append(policy.calls_used.into()); - calldata.append(policy.max_value_per_call.low.into()); - calldata.append(policy.max_value_per_call.high.into()); - - let targets_len = ArrayTrait::::len(targets); - calldata.append(targets_len.into()); - let mut i = 0_usize; - while i < targets_len { - let target = *ArrayTrait::::at(targets, i); - calldata.append(target.into()); - i += 1_usize; + let mut owned_targets: Array = array![]; + for target_ref in targets.span() { + owned_targets.append(*target_ref); } - - let selectors_len = ArrayTrait::::len(selectors); - calldata.append(selectors_len.into()); - i = 0_usize; - while i < selectors_len { - let selector = *ArrayTrait::::at(selectors, i); - calldata.append(selector); - i += 1_usize; + let mut owned_selectors: Array = array![]; + for selector_ref in selectors.span() { + owned_selectors.append(*selector_ref); } + let targets_len_usize = ArrayTrait::::len(@owned_targets); + let selectors_len_usize = ArrayTrait::::len(@owned_selectors); + + let targets_len: u32 = match targets_len_usize.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'targets too long'); + 0_u32 + }, + }; + let selectors_len: u32 = match selectors_len_usize.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'selectors too long'); + 0_u32 + }, + }; + + let session = Session { + pubkey: key, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len, + targets: owned_targets, + selectors_len, + selectors: owned_selectors, + }; + + let mut calldata = array![]; + Serde::::serialize(@session, ref calldata); + call_contract_syscall( account_address, starknet::selector!("add_session_with_allowlists"), @@ -143,13 +161,100 @@ fn assert_reverted_with(result: SyscallResult>, expected: felt252) } } +#[test] +fn rejects_length_mismatch() { + let (account_address, _) = deploy_account_and_mock(); + + let session_pubkey = session_key(); + + start_cheat_caller_address(account_address, account_address); + + let mut empty_targets = ArrayTrait::::new(); + let mut selectors_one = ArrayTrait::::new(); + selectors_one.append(TRANSFER_SELECTOR); + + let session_targets_mismatch = Session { + pubkey: session_pubkey, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 1_u32, + value_cap: u256 { low: 1_000_u128, high: 0_u128 }, + targets_len: 1_u32, + targets: empty_targets, + selectors_len: 1_u32, + selectors: selectors_one, + }; + + let mut calldata = array![]; + Serde::::serialize(@session_targets_mismatch, ref calldata); + + let result = call_contract_syscall( + account_address, + starknet::selector!("add_session_with_allowlists"), + calldata.span(), + ); + + match result { + Result::Ok(_) => { + assert(false, 'expected targets len mismatch'); + }, + Result::Err(panic_data) => { + let data = panic_data.span(); + assert(data.len() > 0_usize, 'missing panic data'); + let reason = *data.at(0_usize); + assert(reason == ERR_SESSION_TARGETS_LEN, 'unexpected targets len error'); + }, + } + + let mut targets_one = ArrayTrait::::new(); + targets_one.append(account_address); + let mut selectors_mismatch = ArrayTrait::::new(); + selectors_mismatch.append(TRANSFER_SELECTOR); + + let session_selectors_mismatch = Session { + pubkey: session_pubkey, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 1_u32, + value_cap: u256 { low: 1_000_u128, high: 0_u128 }, + targets_len: 1_u32, + targets: targets_one, + selectors_len: 2_u32, + selectors: selectors_mismatch, + }; + + let mut calldata_selectors = array![]; + Serde::::serialize(@session_selectors_mismatch, ref calldata_selectors); + + let selectors_result = call_contract_syscall( + account_address, + starknet::selector!("add_session_with_allowlists"), + calldata_selectors.span(), + ); + + match selectors_result { + Result::Ok(_) => { + assert(false, 'expected selectors len mismatch'); + }, + Result::Err(panic_data) => { + let data = panic_data.span(); + assert(data.len() > 0_usize, 'missing panic data'); + let reason = *data.at(0_usize); + assert(reason == ERR_SESSION_SELECTORS_LEN, 'unexpected selectors len error'); + }, + } + + stop_cheat_caller_address(account_address); +} + #[test] fn denies_selector_not_allowed() { let (account_address, mock_address) = deploy_account_and_mock(); let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, @@ -181,7 +286,8 @@ fn denies_target_not_allowed() { let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, @@ -213,7 +319,8 @@ fn denies_expired_session() { let policy = SessionPolicy { is_active: true, - expires_at: 6_000_u64, + valid_after: 0_u64, + valid_until: 6_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, @@ -245,7 +352,8 @@ fn denies_over_call_cap() { let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 1_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, @@ -278,7 +386,8 @@ fn denies_over_value_cap() { let policy = SessionPolicy { is_active: true, - expires_at: 10_000_u64, + valid_after: 0_u64, + valid_until: 10_000_u64, max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, diff --git a/packages/core/src/sessions.ts b/packages/core/src/sessions.ts index 2b80b6a..007d4b0 100644 --- a/packages/core/src/sessions.ts +++ b/packages/core/src/sessions.ts @@ -44,12 +44,12 @@ class SessionsImpl implements SessionsManager { async create(policy: SessionPolicyInput): Promise { const active = policy.active ?? true; const pubkey = genFeltKey(); - const keyHash = pubkey; // v0.1: use felt pubkey directly; can hash later + const sessionId = pubkey; // v0.1: use felt pubkey directly; can hash later const createdAt = Date.now(); // Build calldata for Cairo's SessionPolicy struct and allowlists. const { policyCalldata, allowCalldata } = buildPolicyCalldata(policy, active); - const calldata = buildAddSessionCalldata(pubkey, keyHash, policyCalldata, allowCalldata); + const calldata = buildAddSessionCalldata(pubkey, policyCalldata, allowCalldata); // If we have a transport + ua2 address, we could call add_session_with_allowlists here. // Keeping it local-only for now (no RPC in tests). @@ -58,7 +58,7 @@ class SessionsImpl implements SessionsManager { } const sess: Session = { - id: keyHash, + id: sessionId, pubkey, policy, createdAt, @@ -90,7 +90,11 @@ function buildPolicyCalldata(inp: SessionPolicyInput, active: boolean): { selectors: Felt[]; }; } { - const expires_at = toFelt(inp.expiresAt >>> 0); // as u64 -> felt + const rawValidAfter = Math.max(0, Math.floor(inp.validAfter)); + const rawValidUntil = Math.max(0, Math.floor(inp.validUntil)); + const normalizedValidUntil = rawValidUntil <= rawValidAfter ? rawValidAfter + 1 : rawValidUntil; + const valid_after = toFelt(BigInt(rawValidAfter)); + const valid_until = toFelt(BigInt(normalizedValidUntil)); const max_calls = toFelt(inp.limits.maxCalls >>> 0); const calls_used = toFelt(0); const [low, high] = inp.limits.maxValuePerCall; @@ -98,7 +102,8 @@ function buildPolicyCalldata(inp: SessionPolicyInput, active: boolean): { const policyCalldata: SessionPolicyCalldata = { is_active, - expires_at, + valid_after, + valid_until, max_calls, calls_used, max_value_per_call_low: low, @@ -113,15 +118,13 @@ function buildPolicyCalldata(inp: SessionPolicyInput, active: boolean): { function buildAddSessionCalldata( pubkey: Felt, - keyHash: Felt, policy: SessionPolicyCalldata, allow: { targets: Felt[]; selectors: Felt[] } ): Felt[] { const policyArray: Felt[] = [ - policy.is_active, - policy.expires_at, + policy.valid_after, + policy.valid_until, policy.max_calls, - policy.calls_used, policy.max_value_per_call_low, policy.max_value_per_call_high, ]; @@ -131,7 +134,6 @@ function buildAddSessionCalldata( return [ pubkey, - keyHash, ...policyArray, targetsLen, ...allow.targets, @@ -190,8 +192,15 @@ function ensureSessionActive(session: Session, nowMs?: number) { } const nowSeconds = Math.floor((nowMs ?? Date.now()) / 1000); - if (session.policy.expiresAt <= nowSeconds) { - throw new SessionExpiredError(`Session ${session.id} expired at ${session.policy.expiresAt}.`); + if (nowSeconds < session.policy.validAfter) { + throw new SessionExpiredError( + `Session ${session.id} not active until ${session.policy.validAfter}.` + ); + } + if (session.policy.validUntil <= nowSeconds) { + throw new SessionExpiredError( + `Session ${session.id} expired at ${session.policy.validUntil}.` + ); } } @@ -219,7 +228,9 @@ function ensurePolicy(session: Session, calls: AccountCall[]) { /* ------------------ Guard builder ------------------ */ export interface GuardBuilderInit { - expiresAt?: number; + validAfter?: number; + validUntil?: number; + expiresAt?: number; // legacy alias for validUntil expiresInSeconds?: number; maxCalls?: number; maxValue?: string | number | bigint; @@ -229,6 +240,8 @@ export interface GuardBuilderInit { } export interface GuardBuilder { + validAfter(timestamp: number): GuardBuilder; + validUntil(timestamp: number): GuardBuilder; target(addr: Felt): GuardBuilder; targets(addresses: Iterable): GuardBuilder; selector(sel: Felt): GuardBuilder; @@ -242,7 +255,8 @@ export interface GuardBuilder { } export function guard(init: GuardBuilderInit = {}): GuardBuilder { - let expiresAt = resolveExpiry(init); + let validAfter = Math.max(0, Math.floor(init.validAfter ?? 0)); + let validUntil = resolveValidUntil(init, validAfter); let maxCallsCount = init.maxCalls ?? 1; let maxValueInput: string | number | bigint = init.maxValue ?? 0; let isActive = init.active ?? true; @@ -250,6 +264,15 @@ export function guard(init: GuardBuilderInit = {}): GuardBuilder { const selectors = new Set((init.selectors ?? []).map((s) => toFelt(s))); const builder: GuardBuilder = { + validAfter(timestamp: number) { + validAfter = Math.max(0, Math.floor(timestamp)); + validUntil = Math.max(validUntil, validAfter + 1); + return builder; + }, + validUntil(timestamp: number) { + validUntil = normalizeValidUntil(timestamp, validAfter); + return builder; + }, target(addr: Felt) { targets.add(toFelt(addr)); return builder; @@ -275,12 +298,12 @@ export function guard(init: GuardBuilderInit = {}): GuardBuilder { return builder; }, expiresAt(timestamp: number) { - expiresAt = Math.max(0, Math.floor(timestamp)); + validUntil = normalizeValidUntil(timestamp, validAfter); return builder; }, expiresIn(seconds: number) { const now = Math.floor(Date.now() / 1000); - expiresAt = now + Math.max(0, Math.floor(seconds)); + validUntil = normalizeValidUntil(now + Math.max(0, Math.floor(seconds)), validAfter); return builder; }, active(flag: boolean) { @@ -289,7 +312,8 @@ export function guard(init: GuardBuilderInit = {}): GuardBuilder { }, build(): SessionPolicyInput { return { - expiresAt, + validAfter, + validUntil, limits: { maxCalls: maxCallsCount, maxValuePerCall: toUint256(maxValueInput), @@ -306,16 +330,24 @@ export function guard(init: GuardBuilderInit = {}): GuardBuilder { return builder; } -function resolveExpiry(init: GuardBuilderInit): number { +function resolveValidUntil(init: GuardBuilderInit, validAfter: number): number { + if (typeof init.validUntil === 'number') { + return normalizeValidUntil(init.validUntil, validAfter); + } if (typeof init.expiresAt === 'number') { - return Math.max(0, Math.floor(init.expiresAt)); + return normalizeValidUntil(init.expiresAt, validAfter); } if (typeof init.expiresInSeconds === 'number') { const now = Math.floor(Date.now() / 1000); - return now + Math.max(0, Math.floor(init.expiresInSeconds)); + return normalizeValidUntil(now + Math.max(0, Math.floor(init.expiresInSeconds)), validAfter); } const defaultExpirySeconds = Math.floor(Date.now() / 1000) + 3600; // 1 hour default - return defaultExpirySeconds; + return normalizeValidUntil(defaultExpirySeconds, validAfter); +} + +function normalizeValidUntil(value: number, validAfter: number): number { + const normalized = Math.max(0, Math.floor(value)); + return normalized <= validAfter ? validAfter + 1 : normalized; } export const sessions = { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index d955048..ca9e096 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -103,8 +103,10 @@ export interface SessionAllow { } export interface SessionPolicyInput { + /** Earliest timestamp the session can be used (seconds since epoch). */ + validAfter: number; /** Expiration timestamp (seconds since epoch). */ - expiresAt: number; + validUntil: number; /** Limits per session. */ limits: SessionLimits; /** Allowlist constraints. */ @@ -116,7 +118,8 @@ export interface SessionPolicyInput { /** The on-chain policy struct shape (Cairo ordering). */ export interface SessionPolicyCalldata { is_active: Felt; // 0x0 or 0x1 - expires_at: Felt; // u64 -> felt + valid_after: Felt; // u64 -> felt + valid_until: Felt; // u64 -> felt max_calls: Felt; // u32 -> felt calls_used: Felt; // u32 -> felt (init 0) max_value_per_call_low: Felt; From 5957b9cfc1b9bdae2739df616ff8c1e520ce9fd4 Mon Sep 17 00:00:00 2001 From: hooftly Date: Mon, 13 Oct 2025 20:24:55 -0600 Subject: [PATCH 02/16] feat(sessions): bind chain/account/key/expiry in session hash - Extend session message hashing to bind chain, account, session key, call digest, expiry, and nonce to tighten replay protection - Update signing helpers and session-focused tests to use the new message shape while continuing to assert nonce replay prevention - Document the bound message fields in the validation guide --- docs/validation.md | 1 + packages/contracts/src/ua2_account.cairo | 12 +++++++++--- packages/contracts/tests/lib.cairo | 9 +++++++-- .../contracts/tests/test_session_nonce_ok.cairo | 4 ++-- .../test_session_nonce_replay_and_mismatch.cairo | 8 ++++---- .../contracts/tests/test_validate_allowlists.cairo | 8 +++++++- .../contracts/tests/test_validate_denials.cairo | 13 +++++++------ 7 files changed, 37 insertions(+), 18 deletions(-) diff --git a/docs/validation.md b/docs/validation.md index 56d3677..5dabe69 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -20,6 +20,7 @@ - If selector == `ERC20::transfer`, ensure amount ≤ `max_value_per_call` (`ERR_VALUE_LIMIT_EXCEEDED`). 8. Require provided session nonce == stored nonce (`ERR_BAD_SESSION_NONCE`). 9. Verify ECDSA signature against the computed session message (`ERR_SESSION_SIG_INVALID`). + - Message binds `{chainId, accountAddress, sessionPubkey, callHash, validUntil, nonce}` to prevent cross-channel replay. 10. Call `apply_session_usage` to atomically bump `calls_used`, advance the nonce, and emit `SessionUsed` + `SessionNonceAdvanced`. 11. Proceed to `__execute__`. diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 05aacf6..d6756ab 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -687,17 +687,21 @@ pub mod UA2Account { fn compute_session_message_hash( chain_id: felt252, account_felt: felt252, + session_pubkey: felt252, key_hash: felt252, - nonce: u128, call_digest: felt252, + valid_until: u64, + nonce: u128, ) -> felt252 { let mut values = array![ SESSION_DOMAIN_TAG, chain_id, account_felt, + session_pubkey, key_hash, - nonce.into(), call_digest, + valid_until.into(), + nonce.into(), ]; poseidon_hash_span(values.span()) } @@ -811,9 +815,11 @@ pub mod UA2Account { let message = compute_session_message_hash( chain_id, account_felt, + session_key, key_hash, - provided_nonce, call_digest, + policy.valid_until, + provided_nonce, ); let sig_r = *signature.at(4_usize); diff --git a/packages/contracts/tests/lib.cairo b/packages/contracts/tests/lib.cairo index 1b30edd..c9fdf68 100644 --- a/packages/contracts/tests/lib.cairo +++ b/packages/contracts/tests/lib.cairo @@ -72,8 +72,10 @@ pub mod session_test_utils { fn compute_message( account_address: ContractAddress, + session_pubkey: felt252, key_hash: felt252, nonce: u128, + valid_until: u64, calls: @Array, ) -> felt252 { let execution_info = get_execution_info().unbox(); @@ -85,9 +87,11 @@ pub mod session_test_utils { SESSION_DOMAIN_TAG, chain_id, account_felt, + session_pubkey, key_hash, - nonce.into(), call_digest, + valid_until.into(), + nonce.into(), ]; poseidon_hash_span(values.span()) } @@ -108,10 +112,11 @@ pub mod session_test_utils { account_address: ContractAddress, session_pubkey: felt252, nonce: u128, + valid_until: u64, calls: @Array, ) -> Array { let key_hash = pedersen(session_pubkey, 0); - let message = compute_message(account_address, key_hash, nonce, calls); + let message = compute_message(account_address, session_pubkey, key_hash, nonce, valid_until, calls); let key_pair = session_keypair(); let signature = StarkCurveSignerImpl::sign(key_pair, message); let (r, s) = match signature { diff --git a/packages/contracts/tests/test_session_nonce_ok.cairo b/packages/contracts/tests/test_session_nonce_ok.cairo index 28d8698..18191dc 100644 --- a/packages/contracts/tests/test_session_nonce_ok.cairo +++ b/packages/contracts/tests/test_session_nonce_ok.cairo @@ -147,11 +147,11 @@ fn test_session_nonce_ok() { let calls = array![call]; let signature0: Array = - build_session_signature(account_address, session_pubkey, 0_u128, @calls); + build_session_signature(account_address, session_pubkey, 0_u128, policy.valid_until, @calls); execute_with_signature(account_address, @calls, @signature0); let signature1: Array = - build_session_signature(account_address, session_pubkey, 1_u128, @calls); + build_session_signature(account_address, session_pubkey, 1_u128, policy.valid_until, @calls); execute_with_signature(account_address, @calls, @signature1); stop_cheat_block_timestamp(account_address); diff --git a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo index 775710a..5e77719 100644 --- a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo +++ b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo @@ -150,23 +150,23 @@ fn test_session_nonce_replay_and_mismatch() { let calls = array![call]; let signature0: Array = - build_session_signature(account_address, session_pubkey, 0_u128, @calls); + build_session_signature(account_address, session_pubkey, 0_u128, policy.valid_until, @calls); execute_with_signature(account_address, @calls, @signature0).unwrap_syscall(); let replay_result = execute_with_signature(account_address, @calls, @signature0); assert_reverted_with(replay_result, ERR_BAD_SESSION_NONCE); let skip_signature: Array = - build_session_signature(account_address, session_pubkey, 2_u128, @calls); + build_session_signature(account_address, session_pubkey, 2_u128, policy.valid_until, @calls); let skip_result = execute_with_signature(account_address, @calls, @skip_signature); assert_reverted_with(skip_result, ERR_BAD_SESSION_NONCE); let signature1: Array = - build_session_signature(account_address, session_pubkey, 1_u128, @calls); + build_session_signature(account_address, session_pubkey, 1_u128, policy.valid_until, @calls); execute_with_signature(account_address, @calls, @signature1).unwrap_syscall(); let signature2: Array = - build_session_signature(account_address, session_pubkey, 2_u128, @calls); + build_session_signature(account_address, session_pubkey, 2_u128, policy.valid_until, @calls); let tampered_amount = u256 { low: 1_001_u128, high: 0_u128 }; let tampered_call = build_transfer_call(mock_address, to, tampered_amount); diff --git a/packages/contracts/tests/test_validate_allowlists.cairo b/packages/contracts/tests/test_validate_allowlists.cairo index d913465..93d12a1 100644 --- a/packages/contracts/tests/test_validate_allowlists.cairo +++ b/packages/contracts/tests/test_validate_allowlists.cairo @@ -112,7 +112,13 @@ fn session_allows_whitelisted_calls() { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); let signature: Array = - build_session_signature(account_address, session_pubkey, 0_u128, @calls); + build_session_signature( + account_address, + session_pubkey, + 0_u128, + policy.valid_until, + @calls, + ); start_cheat_signature(account_address, signature.span()); let mut execute_calldata = array![]; Serde::>::serialize(@calls, ref execute_calldata); diff --git a/packages/contracts/tests/test_validate_denials.cairo b/packages/contracts/tests/test_validate_denials.cairo index 5d212fb..7233224 100644 --- a/packages/contracts/tests/test_validate_denials.cairo +++ b/packages/contracts/tests/test_validate_denials.cairo @@ -125,11 +125,12 @@ fn execute_session_calls( calls: @Array, nonce: u128, session_pubkey: felt252, + valid_until: u64, ) -> SyscallResult> { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); let signature: Array = - build_session_signature(account_address, session_pubkey, nonce, calls); + build_session_signature(account_address, session_pubkey, nonce, valid_until, calls); start_cheat_signature(account_address, signature.span()); let mut execute_calldata = array![]; @@ -273,7 +274,7 @@ fn denies_selector_not_allowed() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey); + let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); assert_reverted_with(result, ERR_POLICY_SELECTOR_DENIED); @@ -306,7 +307,7 @@ fn denies_target_not_allowed() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey); + let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); assert_reverted_with(result, ERR_POLICY_TARGET_DENIED); @@ -339,7 +340,7 @@ fn denies_expired_session() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey); + let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); assert_reverted_with(result, ERR_SESSION_EXPIRED); @@ -373,7 +374,7 @@ fn denies_over_call_cap() { let call_two = build_transfer_call(mock_address, to, amount); let calls = array![call_one, call_two]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey); + let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); assert_reverted_with(result, ERR_POLICY_CALLCAP); @@ -406,7 +407,7 @@ fn denies_over_value_cap() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey); + let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); assert_reverted_with(result, ERR_VALUE_LIMIT_EXCEEDED); From 6d155c8faa263cad32559b3e2f8984f80b92c7fb Mon Sep 17 00:00:00 2001 From: hooftly Date: Mon, 13 Oct 2025 20:36:01 -0600 Subject: [PATCH 03/16] feat(recovery): streamline guardian confirmations via helper - Replace per-guardian boolean map with _record_recovery_confirmation() - Count each guardian exactly once per proposal using existing proposal id - Preserve and reuse current recovery events and proposal tracking - Update architecture and RFC docs to match simplified storage layout --- docs/architecture.md | 2 +- docs/rfc-ua2-sdk.md | 2 +- packages/contracts/src/ua2_account.cairo | 42 ++++++++++++++---------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 690471c..bd1bb08 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -14,7 +14,7 @@ - `recovery_active: bool` - `recovery_proposed_owner: felt252` - `recovery_eta: u64` - - `recovery_confirms: LegacyMap` + `recovery_confirm_count: u32` + - `recovery_confirm_count: u32` - `recovery_proposal_id: u64` - `recovery_guardian_last_confirm: LegacyMap` - `session: Map` diff --git a/docs/rfc-ua2-sdk.md b/docs/rfc-ua2-sdk.md index d45dfaf..7c1437c 100644 --- a/docs/rfc-ua2-sdk.md +++ b/docs/rfc-ua2-sdk.md @@ -130,7 +130,7 @@ This RFC proposes a neutral, modular SDK that standardizes these primitives with * `recovery_active: bool` * `recovery_proposed_owner: felt252` * `recovery_eta: u64` - * `recovery_confirms: LegacyMap` + `recovery_confirm_count: u32` + * `recovery_confirm_count: u32` * `recovery_proposal_id: u64` * `recovery_guardian_last_confirm: LegacyMap` * `session: Map` diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index d6756ab..116c536 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -80,7 +80,6 @@ pub mod UA2Account { recovery_active: bool, recovery_proposed_owner: felt252, recovery_eta: u64, - recovery_confirms: LegacyMap, recovery_confirm_count: u32, recovery_proposal_id: u64, recovery_guardian_last_confirm: LegacyMap, @@ -238,6 +237,22 @@ pub mod UA2Account { self.recovery_confirm_count.write(0_u32); } + fn _record_recovery_confirmation( + ref self: ContractState, + guardian: ContractAddress, + proposal_id: u64, + last_confirm: u64, + ) -> u32 { + assert(last_confirm != proposal_id, ERR_ALREADY_CONFIRMED); + + self.recovery_guardian_last_confirm.write(guardian, proposal_id); + + let new_count = self.recovery_confirm_count.read() + 1_u32; + self.recovery_confirm_count.write(new_count); + + new_count + } + #[external(v0)] fn add_guardian(ref self: ContractState, addr: ContractAddress) { assert_owner(); @@ -313,14 +328,17 @@ pub mod UA2Account { let last_confirm = self.recovery_guardian_last_confirm.read(caller); if last_confirm != proposal_id { - self.recovery_confirms.write(caller, true); - self.recovery_guardian_last_confirm.write(caller, proposal_id); - self.recovery_confirm_count.write(1_u32); + let confirm_count = _record_recovery_confirmation( + ref self, + caller, + proposal_id, + last_confirm, + ); self.emit( Event::RecoveryConfirmed(RecoveryConfirmed { guardian: caller, new_owner, - count: 1_u32, + count: confirm_count, }), ); } @@ -341,19 +359,7 @@ pub mod UA2Account { let proposal_id = self.recovery_proposal_id.read(); let last_confirm = self.recovery_guardian_last_confirm.read(caller); - - if last_confirm != proposal_id { - self.recovery_confirms.write(caller, false); - } - - let already_confirmed = self.recovery_confirms.read(caller); - assert(already_confirmed == false, ERR_ALREADY_CONFIRMED); - - self.recovery_confirms.write(caller, true); - self.recovery_guardian_last_confirm.write(caller, proposal_id); - - let new_count = self.recovery_confirm_count.read() + 1_u32; - self.recovery_confirm_count.write(new_count); + let new_count = _record_recovery_confirmation(ref self, caller, proposal_id, last_confirm); self.emit( Event::RecoveryConfirmed(RecoveryConfirmed { From 1c80b6db1bca4b9923187104985049748ff6fac8 Mon Sep 17 00:00:00 2001 From: hooftly Date: Mon, 13 Oct 2025 20:55:13 -0600 Subject: [PATCH 04/16] feat(contracts): shared errors module and guardian events - Centralize reusable error identifiers in a new errors module; replace inline definitions within the account contract for consistent messaging - Add GuardianProposed and GuardianFinalized events to the account ABI and emit them during recovery proposal and execution alongside owner rotation logic - Update contract tests to assert new guardian events, verify recovery cancellation signaling, and import shared error constants - Improve consistency and observability across the recovery lifecycle --- packages/contracts/src/errors.cairo | 28 +++++ packages/contracts/src/lib.cairo | 1 + packages/contracts/src/ua2_account.cairo | 93 +++++++++----- .../tests/test_guardians_admin.cairo | 4 +- .../contracts/tests/test_owner_rotate.cairo | 4 +- .../tests/test_recovery_edgecases.cairo | 116 +++++++++++++++++- .../contracts/tests/test_recovery_happy.cairo | 21 +++- .../tests/test_rotate_vs_recovery.cairo | 2 +- ...st_session_nonce_replay_and_mismatch.cairo | 3 +- .../contracts/tests/test_validate_auth.cairo | 3 +- .../tests/test_validate_denials.cairo | 16 +-- 11 files changed, 238 insertions(+), 53 deletions(-) create mode 100644 packages/contracts/src/errors.cairo diff --git a/packages/contracts/src/errors.cairo b/packages/contracts/src/errors.cairo new file mode 100644 index 0000000..45419f4 --- /dev/null +++ b/packages/contracts/src/errors.cairo @@ -0,0 +1,28 @@ +pub const ERR_SESSION_EXPIRED: felt252 = 'ERR_SESSION_EXPIRED'; +pub const ERR_SESSION_INACTIVE: felt252 = 'ERR_SESSION_INACTIVE'; +pub const ERR_POLICY_CALLCAP: felt252 = 'ERR_POLICY_CALLCAP'; +pub const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; +pub const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; +pub const ERR_VALUE_LIMIT_EXCEEDED: felt252 = 'ERR_VALUE_LIMIT_EXCEEDED'; +pub const ERR_SESSION_NOT_READY: felt252 = 'ERR_SESSION_NOT_READY'; +pub const ERR_SESSION_TARGETS_LEN: felt252 = 'ERR_SESSION_TARGETS_LEN'; +pub const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; +pub const ERR_POLICY_CALLCOUNT_MISMATCH: felt252 = 'ERR_POLICY_CALLCOUNT_MISMATCH'; +pub const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; +pub const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; +pub const ERR_GUARDIAN_EXISTS: felt252 = 'ERR_GUARDIAN_EXISTS'; +pub const ERR_NOT_GUARDIAN: felt252 = 'ERR_NOT_GUARDIAN'; +pub const ERR_BAD_THRESHOLD: felt252 = 'ERR_BAD_THRESHOLD'; +pub const ERR_RECOVERY_IN_PROGRESS: felt252 = 'ERR_RECOVERY_IN_PROGRESS'; +pub const ERR_NO_RECOVERY: felt252 = 'ERR_NO_RECOVERY'; +pub const ERR_RECOVERY_MISMATCH: felt252 = 'ERR_RECOVERY_MISMATCH'; +pub const ERR_ALREADY_CONFIRMED: felt252 = 'ERR_ALREADY_CONFIRMED'; +pub const ERR_BEFORE_ETA: felt252 = 'ERR_BEFORE_ETA'; +pub const ERR_NOT_ENOUGH_CONFIRMS: felt252 = 'ERR_NOT_ENOUGH_CONFIRMS'; +pub const ERR_ZERO_OWNER: felt252 = 'ERR_ZERO_OWNER'; +pub const ERR_SAME_OWNER: felt252 = 'ERR_SAME_OWNER'; +pub const ERR_SIGNATURE_MISSING: felt252 = 'ERR_SIGNATURE_MISSING'; +pub const ERR_OWNER_SIG_INVALID: felt252 = 'ERR_OWNER_SIG_INVALID'; +pub const ERR_GUARDIAN_SIG_INVALID: felt252 = 'ERR_GUARDIAN_SIG_INVALID'; +pub const ERR_GUARDIAN_CALL_DENIED: felt252 = 'ERR_GUARDIAN_CALL_DENIED'; +pub const ERR_NOT_OWNER: felt252 = 'NOT_OWNER'; diff --git a/packages/contracts/src/lib.cairo b/packages/contracts/src/lib.cairo index bc62001..8bef8ee 100644 --- a/packages/contracts/src/lib.cairo +++ b/packages/contracts/src/lib.cairo @@ -1,3 +1,4 @@ +pub mod errors; pub mod ua2_account; pub mod mock_erc20; pub mod session; diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 116c536..02b94ba 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -5,6 +5,36 @@ use openzeppelin::introspection::src5::SRC5Component; #[feature("deprecated_legacy_map")] pub mod UA2Account { use super::{AccountComponent, SRC5Component}; + use crate::errors::{ + ERR_ALREADY_CONFIRMED, + ERR_BAD_SESSION_NONCE, + ERR_BAD_THRESHOLD, + ERR_BEFORE_ETA, + ERR_GUARDIAN_CALL_DENIED, + ERR_GUARDIAN_EXISTS, + ERR_GUARDIAN_SIG_INVALID, + ERR_NO_RECOVERY, + ERR_NOT_ENOUGH_CONFIRMS, + ERR_NOT_GUARDIAN, + ERR_NOT_OWNER, + ERR_OWNER_SIG_INVALID, + ERR_POLICY_CALLCAP, + ERR_POLICY_CALLCOUNT_MISMATCH, + ERR_POLICY_SELECTOR_DENIED, + ERR_POLICY_TARGET_DENIED, + ERR_RECOVERY_IN_PROGRESS, + ERR_RECOVERY_MISMATCH, + ERR_SAME_OWNER, + ERR_SESSION_EXPIRED, + ERR_SESSION_INACTIVE, + ERR_SESSION_NOT_READY, + ERR_SESSION_SELECTORS_LEN, + ERR_SESSION_SIG_INVALID, + ERR_SESSION_TARGETS_LEN, + ERR_SIGNATURE_MISSING, + ERR_VALUE_LIMIT_EXCEEDED, + ERR_ZERO_OWNER, + }; use crate::session::Session; use core::array::{Array, ArrayTrait, SpanTrait}; use core::option::Option; @@ -29,33 +59,6 @@ pub mod UA2Account { component!(path: AccountComponent, storage: account, event: AccountEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); - const ERR_SESSION_EXPIRED: felt252 = 'ERR_SESSION_EXPIRED'; - const ERR_SESSION_INACTIVE: felt252 = 'ERR_SESSION_INACTIVE'; - const ERR_POLICY_CALLCAP: felt252 = 'ERR_POLICY_CALLCAP'; - const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; - const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; - const ERR_VALUE_LIMIT_EXCEEDED: felt252 = 'ERR_VALUE_LIMIT_EXCEEDED'; - const ERR_SESSION_NOT_READY: felt252 = 'ERR_SESSION_NOT_READY'; - const ERR_SESSION_TARGETS_LEN: felt252 = 'ERR_SESSION_TARGETS_LEN'; - const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; - const ERR_POLICY_CALLCOUNT_MISMATCH: felt252 = 'ERR_POLICY_CALLCOUNT_MISMATCH'; - const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; - const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; - const ERR_GUARDIAN_EXISTS: felt252 = 'ERR_GUARDIAN_EXISTS'; - const ERR_NOT_GUARDIAN: felt252 = 'ERR_NOT_GUARDIAN'; - const ERR_BAD_THRESHOLD: felt252 = 'ERR_BAD_THRESHOLD'; - const ERR_RECOVERY_IN_PROGRESS: felt252 = 'ERR_RECOVERY_IN_PROGRESS'; - const ERR_NO_RECOVERY: felt252 = 'ERR_NO_RECOVERY'; - const ERR_RECOVERY_MISMATCH: felt252 = 'ERR_RECOVERY_MISMATCH'; - const ERR_ALREADY_CONFIRMED: felt252 = 'ERR_ALREADY_CONFIRMED'; - const ERR_BEFORE_ETA: felt252 = 'ERR_BEFORE_ETA'; - const ERR_NOT_ENOUGH_CONFIRMS: felt252 = 'ERR_NOT_ENOUGH_CONFIRMS'; - const ERR_ZERO_OWNER: felt252 = 'ERR_ZERO_OWNER'; - const ERR_SAME_OWNER: felt252 = 'ERR_SAME_OWNER'; - const ERR_SIGNATURE_MISSING: felt252 = 'ERR_SIGNATURE_MISSING'; - const ERR_OWNER_SIG_INVALID: felt252 = 'ERR_OWNER_SIG_INVALID'; - const ERR_GUARDIAN_SIG_INVALID: felt252 = 'ERR_GUARDIAN_SIG_INVALID'; - const ERR_GUARDIAN_CALL_DENIED: felt252 = 'ERR_GUARDIAN_CALL_DENIED'; const ERC20_TRANSFER_SEL: felt252 = 0x83afd3f4caedc6eebf44246fe54e38c95e3179a5ec9ea81740eca5b482d12e; const APPLY_SESSION_USAGE_SELECTOR: felt252 = starknet::selector!("apply_session_usage"); const PROPOSE_RECOVERY_SELECTOR: felt252 = starknet::selector!("propose_recovery"); @@ -145,6 +148,21 @@ pub mod UA2Account { pub new_owner: felt252, } + #[derive(Drop, starknet::Event)] + pub struct GuardianProposed { + pub guardian: ContractAddress, + pub proposal_id: u64, + pub new_owner: felt252, + pub eta: u64, + } + + #[derive(Drop, starknet::Event)] + pub struct GuardianFinalized { + pub guardian: ContractAddress, + pub proposal_id: u64, + pub new_owner: felt252, + } + #[derive(Drop, starknet::Event)] pub struct RecoveryProposed { pub new_owner: felt252, @@ -189,6 +207,8 @@ pub mod UA2Account { ThresholdSet: ThresholdSet, RecoveryDelaySet: RecoveryDelaySet, OwnerRotated: OwnerRotated, + GuardianProposed: GuardianProposed, + GuardianFinalized: GuardianFinalized, RecoveryProposed: RecoveryProposed, RecoveryConfirmed: RecoveryConfirmed, RecoveryCanceled: RecoveryCanceled, @@ -209,7 +229,7 @@ pub mod UA2Account { fn assert_owner() { let caller: ContractAddress = get_caller_address(); let contract_address: ContractAddress = get_contract_address(); - assert(caller == contract_address, 'NOT_OWNER'); + assert(caller == contract_address, ERR_NOT_OWNER); } fn require(condition: bool, error: felt252) { @@ -343,6 +363,14 @@ pub mod UA2Account { ); } + self.emit( + Event::GuardianProposed(GuardianProposed { + guardian: caller, + proposal_id, + new_owner, + eta, + }), + ); self.emit(Event::RecoveryProposed(RecoveryProposed { new_owner, eta })); } @@ -400,6 +428,7 @@ pub mod UA2Account { #[external(v0)] fn execute_recovery(ref self: ContractState) { + let caller = get_caller_address(); let active = self.recovery_active.read(); assert(active == true, ERR_NO_RECOVERY); @@ -413,12 +442,20 @@ pub mod UA2Account { assert(now >= eta, ERR_BEFORE_ETA); let new_owner = self.recovery_proposed_owner.read(); + let proposal_id = self.recovery_proposal_id.read(); self.owner_pubkey.write(new_owner); _clear_recovery_state(ref self); self.emit(Event::OwnerRotated(OwnerRotated { new_owner })); self.emit(Event::RecoveryExecuted(RecoveryExecuted { new_owner })); + self.emit( + Event::GuardianFinalized(GuardianFinalized { + guardian: caller, + proposal_id, + new_owner, + }), + ); } fn u256_le(lhs: u256, rhs: u256) -> bool { diff --git a/packages/contracts/tests/test_guardians_admin.cairo b/packages/contracts/tests/test_guardians_admin.cairo index 3b1c545..1764e17 100644 --- a/packages/contracts/tests/test_guardians_admin.cairo +++ b/packages/contracts/tests/test_guardians_admin.cairo @@ -19,11 +19,9 @@ use ua2_contracts::ua2_account::UA2Account::{ RecoveryDelaySet, ThresholdSet, }; +use ua2_contracts::errors::{ERR_BAD_THRESHOLD, ERR_GUARDIAN_EXISTS, ERR_NOT_GUARDIAN}; const OWNER_PUBKEY: felt252 = 0x12345; -const ERR_GUARDIAN_EXISTS: felt252 = 'ERR_GUARDIAN_EXISTS'; -const ERR_BAD_THRESHOLD: felt252 = 'ERR_BAD_THRESHOLD'; -const ERR_NOT_GUARDIAN: felt252 = 'ERR_NOT_GUARDIAN'; fn deploy_account() -> ContractAddress { let declare_result = declare("UA2Account").unwrap(); diff --git a/packages/contracts/tests/test_owner_rotate.cairo b/packages/contracts/tests/test_owner_rotate.cairo index 4241c2d..d279f52 100644 --- a/packages/contracts/tests/test_owner_rotate.cairo +++ b/packages/contracts/tests/test_owner_rotate.cairo @@ -12,12 +12,10 @@ use snforge_std::{ use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use starknet::syscalls::call_contract_syscall; use ua2_contracts::ua2_account::UA2Account::{Event, OwnerRotated}; +use ua2_contracts::errors::{ERR_NOT_OWNER, ERR_SAME_OWNER, ERR_ZERO_OWNER}; const OWNER_PUBKEY: felt252 = 0x111; const NEW_OWNER: felt252 = 0x222; -const ERR_ZERO_OWNER: felt252 = 'ERR_ZERO_OWNER'; -const ERR_SAME_OWNER: felt252 = 'ERR_SAME_OWNER'; -const ERR_NOT_OWNER: felt252 = 'NOT_OWNER'; fn deploy_account() -> ContractAddress { let declare_result = declare("UA2Account").unwrap(); diff --git a/packages/contracts/tests/test_recovery_edgecases.cairo b/packages/contracts/tests/test_recovery_edgecases.cairo index 2350e45..d20d595 100644 --- a/packages/contracts/tests/test_recovery_edgecases.cairo +++ b/packages/contracts/tests/test_recovery_edgecases.cairo @@ -3,25 +3,36 @@ use core::result::ResultTrait; use core::traits::{Into, TryInto}; use snforge_std::{ declare, + spy_events, start_cheat_block_timestamp, stop_cheat_block_timestamp, start_cheat_caller_address, stop_cheat_caller_address, ContractClassTrait, DeclareResultTrait, + EventSpyAssertionsTrait, }; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use starknet::syscalls::call_contract_syscall; +use ua2_contracts::errors::{ + ERR_ALREADY_CONFIRMED, + ERR_BEFORE_ETA, + ERR_NO_RECOVERY, + ERR_NOT_ENOUGH_CONFIRMS, + ERR_RECOVERY_IN_PROGRESS, + ERR_RECOVERY_MISMATCH, +}; +use ua2_contracts::ua2_account::UA2Account::{ + Event, + GuardianProposed, + RecoveryCanceled, + RecoveryConfirmed, + RecoveryProposed, +}; const OWNER_PUBKEY: felt252 = 0x12345; const RECOVERY_OWNER_A: felt252 = 0xAAA111; const RECOVERY_OWNER_B: felt252 = 0xBBB222; -const ERR_NOT_ENOUGH_CONFIRMS: felt252 = 'ERR_NOT_ENOUGH_CONFIRMS'; -const ERR_RECOVERY_IN_PROGRESS: felt252 = 'ERR_RECOVERY_IN_PROGRESS'; -const ERR_RECOVERY_MISMATCH: felt252 = 'ERR_RECOVERY_MISMATCH'; -const ERR_ALREADY_CONFIRMED: felt252 = 'ERR_ALREADY_CONFIRMED'; -const ERR_BEFORE_ETA: felt252 = 'ERR_BEFORE_ETA'; -const ERR_NO_RECOVERY: felt252 = 'ERR_NO_RECOVERY'; fn deploy_account() -> ContractAddress { let declare_result = declare("UA2Account").unwrap(); @@ -237,3 +248,96 @@ fn recovery_edge_cases() { stop_cheat_block_timestamp(contract_address); } + +#[test] +fn recovery_cancel_emits_event() { + let contract_address = deploy_account(); + let g1: ContractAddress = 0x111.try_into().unwrap(); + + start_cheat_caller_address(contract_address, contract_address); + let mut add_calldata = array![]; + add_calldata.append(g1.into()); + call_contract_syscall( + contract_address, + starknet::selector!("add_guardian"), + add_calldata.span(), + ) + .unwrap_syscall(); + + let mut threshold_calldata = array![]; + threshold_calldata.append(1_u8.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_guardian_threshold"), + threshold_calldata.span(), + ) + .unwrap_syscall(); + + let mut delay_calldata = array![]; + delay_calldata.append(0_u64.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_recovery_delay"), + delay_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + let mut spy = spy_events(); + + start_cheat_block_timestamp(contract_address, 10_u64); + + start_cheat_caller_address(contract_address, g1); + let mut propose_calldata = array![]; + propose_calldata.append(RECOVERY_OWNER_A); + call_contract_syscall( + contract_address, + starknet::selector!("propose_recovery"), + propose_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_caller_address(contract_address, contract_address); + let empty = array![]; + call_contract_syscall( + contract_address, + starknet::selector!("cancel_recovery"), + empty.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + stop_cheat_block_timestamp(contract_address); + + spy.assert_emitted(@array![ + ( + contract_address, + Event::RecoveryConfirmed(RecoveryConfirmed { + guardian: g1, + new_owner: RECOVERY_OWNER_A, + count: 1_u32, + }), + ), + ( + contract_address, + Event::GuardianProposed(GuardianProposed { + guardian: g1, + proposal_id: 1_u64, + new_owner: RECOVERY_OWNER_A, + eta: 10_u64, + }), + ), + ( + contract_address, + Event::RecoveryProposed(RecoveryProposed { + new_owner: RECOVERY_OWNER_A, + eta: 10_u64, + }), + ), + ( + contract_address, + Event::RecoveryCanceled(RecoveryCanceled {}), + ), + ]); +} diff --git a/packages/contracts/tests/test_recovery_happy.cairo b/packages/contracts/tests/test_recovery_happy.cairo index 8470cb1..3a5bfd8 100644 --- a/packages/contracts/tests/test_recovery_happy.cairo +++ b/packages/contracts/tests/test_recovery_happy.cairo @@ -17,6 +17,8 @@ use starknet::syscalls::call_contract_syscall; use ua2_contracts::ua2_account::UA2Account::{ Event, GuardianAdded, + GuardianFinalized, + GuardianProposed, OwnerRotated, RecoveryConfirmed, RecoveryDelaySet, @@ -24,10 +26,10 @@ use ua2_contracts::ua2_account::UA2Account::{ RecoveryProposed, ThresholdSet, }; +use ua2_contracts::errors::ERR_NO_RECOVERY; const OWNER_PUBKEY: felt252 = 0x12345; const NEW_OWNER: felt252 = 0xABCDEF0123; -const ERR_NO_RECOVERY: felt252 = 'ERR_NO_RECOVERY'; fn deploy_account() -> ContractAddress { let declare_result = declare("UA2Account").unwrap(); @@ -159,6 +161,15 @@ fn recovery_happy_path() { count: 1_u32, }), ), + ( + contract_address, + Event::GuardianProposed(GuardianProposed { + guardian: g1, + proposal_id: 1_u64, + new_owner: NEW_OWNER, + eta: 100_u64, + }), + ), ( contract_address, Event::RecoveryProposed(RecoveryProposed { @@ -186,5 +197,13 @@ fn recovery_happy_path() { new_owner: NEW_OWNER, }), ), + ( + contract_address, + Event::GuardianFinalized(GuardianFinalized { + guardian: g1, + proposal_id: 1_u64, + new_owner: NEW_OWNER, + }), + ), ]); } diff --git a/packages/contracts/tests/test_rotate_vs_recovery.cairo b/packages/contracts/tests/test_rotate_vs_recovery.cairo index fc19466..f09b0d0 100644 --- a/packages/contracts/tests/test_rotate_vs_recovery.cairo +++ b/packages/contracts/tests/test_rotate_vs_recovery.cairo @@ -10,9 +10,9 @@ use snforge_std::{ }; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use starknet::syscalls::call_contract_syscall; +use ua2_contracts::errors::ERR_RECOVERY_IN_PROGRESS; const OWNER_PUBKEY: felt252 = 0x111; -const ERR_RECOVERY_IN_PROGRESS: felt252 = 'ERR_RECOVERY_IN_PROGRESS'; const NEW_RECOVERY_OWNER: felt252 = 0xDEAD; const ROTATED_OWNER: felt252 = 0xBEEF; diff --git a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo index 5e77719..863b953 100644 --- a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo +++ b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo @@ -20,13 +20,12 @@ use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use ua2_contracts::ua2_account::UA2Account::SessionPolicy; use ua2_contracts::session::Session; +use ua2_contracts::errors::{ERR_BAD_SESSION_NONCE, ERR_SESSION_SIG_INVALID}; use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); -const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; -const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; fn deploy_account_and_mock() -> (ContractAddress, ContractAddress) { let account_declare = declare("UA2Account").unwrap(); diff --git a/packages/contracts/tests/test_validate_auth.cairo b/packages/contracts/tests/test_validate_auth.cairo index 7aabdce..837e9b0 100644 --- a/packages/contracts/tests/test_validate_auth.cairo +++ b/packages/contracts/tests/test_validate_auth.cairo @@ -25,13 +25,12 @@ use snforge_std::signature::stark_curve::{ use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResultTrait}; +use ua2_contracts::errors::{ERR_GUARDIAN_CALL_DENIED, ERR_NO_RECOVERY}; use ua2_contracts::ua2_account::UA2Account::{self, RecoveryDelaySet}; const MODE_OWNER: felt252 = 0; const MODE_GUARDIAN: felt252 = 2; const OWNER_PRIVATE_KEY: felt252 = 0x123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde; -const ERR_NO_RECOVERY: felt252 = 'ERR_NO_RECOVERY'; -const ERR_GUARDIAN_CALL_DENIED: felt252 = 'ERR_GUARDIAN_CALL_DENIED'; fn owner_keypair() -> KeyPair { StarkCurveKeyPairImpl::from_secret_key(OWNER_PRIVATE_KEY) diff --git a/packages/contracts/tests/test_validate_denials.cairo b/packages/contracts/tests/test_validate_denials.cairo index 7233224..c5c0891 100644 --- a/packages/contracts/tests/test_validate_denials.cairo +++ b/packages/contracts/tests/test_validate_denials.cairo @@ -21,18 +21,20 @@ use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use ua2_contracts::ua2_account::UA2Account::SessionPolicy; use ua2_contracts::session::Session; +use ua2_contracts::errors::{ + ERR_POLICY_CALLCAP, + ERR_POLICY_SELECTOR_DENIED, + ERR_POLICY_TARGET_DENIED, + ERR_SESSION_EXPIRED, + ERR_SESSION_SELECTORS_LEN, + ERR_SESSION_TARGETS_LEN, + ERR_VALUE_LIMIT_EXCEEDED, +}; use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); -const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; -const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; -const ERR_SESSION_EXPIRED: felt252 = 'ERR_SESSION_EXPIRED'; -const ERR_SESSION_TARGETS_LEN: felt252 = 'ERR_SESSION_TARGETS_LEN'; -const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; -const ERR_POLICY_CALLCAP: felt252 = 'ERR_POLICY_CALLCAP'; -const ERR_VALUE_LIMIT_EXCEEDED: felt252 = 'ERR_VALUE_LIMIT_EXCEEDED'; fn deploy_account_and_mock() -> (ContractAddress, ContractAddress) { let account_declare = declare("UA2Account").unwrap(); From cf41998883b361ff7c19ef8e8d2dd6f87f36dbb4 Mon Sep 17 00:00:00 2001 From: hooftly Date: Mon, 13 Oct 2025 21:11:06 -0600 Subject: [PATCH 05/16] feat(sessions): epoch-based invalidation on owner change - Introduce session_owner_epoch counter and owner_epoch in session policies; bump epoch on owner rotation or recovery to retire existing sessions automatically - Enforce stale-session detection with ERR_SESSION_STALE in both validation and nonce accounting to block zombie usage - Update session tests, including a rotation regression, to cover epoch semantics and ensure old sessions cannot execute after owner change --- packages/contracts/Scarb.toml | 1 + packages/contracts/src/errors.cairo | 1 + packages/contracts/src/ua2_account.cairo | 237 ++++++++---------- .../contracts/tests/test_owner_rotate.cairo | 195 ++++++++++---- .../tests/test_session_nonce_ok.cairo | 97 +++---- ...st_session_nonce_replay_and_mismatch.cairo | 59 ++--- packages/contracts/tests/test_sessions.cairo | 51 ++-- .../tests/test_validate_allowlists.cairo | 82 +++--- .../tests/test_validate_denials.cairo | 89 +++---- 9 files changed, 406 insertions(+), 406 deletions(-) diff --git a/packages/contracts/Scarb.toml b/packages/contracts/Scarb.toml index 1492961..b78328e 100644 --- a/packages/contracts/Scarb.toml +++ b/packages/contracts/Scarb.toml @@ -7,6 +7,7 @@ description = "UA2 smart contract suite" [dependencies] openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v2.0.0" } starknet = "2.12.0" +cairo_test = "2.12.2" [dev-dependencies] snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry.git", tag = "v0.50.0" } diff --git a/packages/contracts/src/errors.cairo b/packages/contracts/src/errors.cairo index 45419f4..f5efebb 100644 --- a/packages/contracts/src/errors.cairo +++ b/packages/contracts/src/errors.cairo @@ -1,5 +1,6 @@ pub const ERR_SESSION_EXPIRED: felt252 = 'ERR_SESSION_EXPIRED'; pub const ERR_SESSION_INACTIVE: felt252 = 'ERR_SESSION_INACTIVE'; +pub const ERR_SESSION_STALE: felt252 = 'ERR_SESSION_STALE'; pub const ERR_POLICY_CALLCAP: felt252 = 'ERR_POLICY_CALLCAP'; pub const ERR_POLICY_SELECTOR_DENIED: felt252 = 'ERR_POLICY_SELECTOR_DENIED'; pub const ERR_POLICY_TARGET_DENIED: felt252 = 'ERR_POLICY_TARGET_DENIED'; diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 02b94ba..9d944db 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -4,62 +4,41 @@ use openzeppelin::introspection::src5::SRC5Component; #[starknet::contract(account)] #[feature("deprecated_legacy_map")] pub mod UA2Account { - use super::{AccountComponent, SRC5Component}; - use crate::errors::{ - ERR_ALREADY_CONFIRMED, - ERR_BAD_SESSION_NONCE, - ERR_BAD_THRESHOLD, - ERR_BEFORE_ETA, - ERR_GUARDIAN_CALL_DENIED, - ERR_GUARDIAN_EXISTS, - ERR_GUARDIAN_SIG_INVALID, - ERR_NO_RECOVERY, - ERR_NOT_ENOUGH_CONFIRMS, - ERR_NOT_GUARDIAN, - ERR_NOT_OWNER, - ERR_OWNER_SIG_INVALID, - ERR_POLICY_CALLCAP, - ERR_POLICY_CALLCOUNT_MISMATCH, - ERR_POLICY_SELECTOR_DENIED, - ERR_POLICY_TARGET_DENIED, - ERR_RECOVERY_IN_PROGRESS, - ERR_RECOVERY_MISMATCH, - ERR_SAME_OWNER, - ERR_SESSION_EXPIRED, - ERR_SESSION_INACTIVE, - ERR_SESSION_NOT_READY, - ERR_SESSION_SELECTORS_LEN, - ERR_SESSION_SIG_INVALID, - ERR_SESSION_TARGETS_LEN, - ERR_SIGNATURE_MISSING, - ERR_VALUE_LIMIT_EXCEEDED, - ERR_ZERO_OWNER, - }; - use crate::session::Session; use core::array::{Array, ArrayTrait, SpanTrait}; - use core::option::Option; - use core::traits::{Into, TryInto}; - use core::integer::u256; - use core::serde::Serde; use core::ecdsa::check_ecdsa_signature; + use core::integer::u256; + use core::option::Option; + use core::pedersen::pedersen; use core::poseidon::poseidon_hash_span; + use core::serde::Serde; + use core::traits::{Into, TryInto}; use openzeppelin::account::interface; use starknet::account::Call; - use core::pedersen::pedersen; use starknet::storage::Map; use starknet::syscalls::call_contract_syscall; use starknet::{ - ContractAddress, - SyscallResultTrait, - get_caller_address, - get_contract_address, + ContractAddress, SyscallResultTrait, get_caller_address, get_contract_address, get_execution_info, }; + use crate::errors::{ + ERR_ALREADY_CONFIRMED, ERR_BAD_SESSION_NONCE, ERR_BAD_THRESHOLD, ERR_BEFORE_ETA, + ERR_GUARDIAN_CALL_DENIED, ERR_GUARDIAN_EXISTS, ERR_GUARDIAN_SIG_INVALID, + ERR_NOT_ENOUGH_CONFIRMS, ERR_NOT_GUARDIAN, ERR_NOT_OWNER, ERR_NO_RECOVERY, + ERR_OWNER_SIG_INVALID, ERR_POLICY_CALLCAP, ERR_POLICY_CALLCOUNT_MISMATCH, + ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_RECOVERY_IN_PROGRESS, + ERR_RECOVERY_MISMATCH, ERR_SAME_OWNER, ERR_SESSION_EXPIRED, ERR_SESSION_INACTIVE, + ERR_SESSION_NOT_READY, ERR_SESSION_SELECTORS_LEN, ERR_SESSION_SIG_INVALID, + ERR_SESSION_STALE, ERR_SESSION_TARGETS_LEN, ERR_SIGNATURE_MISSING, ERR_VALUE_LIMIT_EXCEEDED, + ERR_ZERO_OWNER, + }; + use crate::session::Session; + use super::{AccountComponent, SRC5Component}; component!(path: AccountComponent, storage: account, event: AccountEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); - const ERC20_TRANSFER_SEL: felt252 = 0x83afd3f4caedc6eebf44246fe54e38c95e3179a5ec9ea81740eca5b482d12e; + const ERC20_TRANSFER_SEL: felt252 = + 0x83afd3f4caedc6eebf44246fe54e38c95e3179a5ec9ea81740eca5b482d12e; const APPLY_SESSION_USAGE_SELECTOR: felt252 = starknet::selector!("apply_session_usage"); const PROPOSE_RECOVERY_SELECTOR: felt252 = starknet::selector!("propose_recovery"); const CONFIRM_RECOVERY_SELECTOR: felt252 = starknet::selector!("confirm_recovery"); @@ -76,6 +55,7 @@ pub mod UA2Account { session_nonce: Map, session_target_allow: LegacyMap<(felt252, ContractAddress), bool>, session_selector_allow: LegacyMap<(felt252, felt252), bool>, + session_owner_epoch: u64, guardians: LegacyMap, guardian_count: u32, guardian_threshold: u8, @@ -96,6 +76,7 @@ pub mod UA2Account { pub max_calls: u32, pub calls_used: u32, pub max_value_per_call: u256, + pub owner_epoch: u64, } #[derive(Drop, starknet::Event)] @@ -218,6 +199,7 @@ pub mod UA2Account { #[constructor] fn constructor(ref self: ContractState, public_key: felt252) { self.owner_pubkey.write(public_key); + self.session_owner_epoch.write(0_u64); self.account.initializer(public_key); } @@ -257,11 +239,13 @@ pub mod UA2Account { self.recovery_confirm_count.write(0_u32); } + fn _bump_owner_epoch(ref self: ContractState) { + let current_epoch = self.session_owner_epoch.read(); + self.session_owner_epoch.write(current_epoch + 1_u64); + } + fn _record_recovery_confirmation( - ref self: ContractState, - guardian: ContractAddress, - proposal_id: u64, - last_confirm: u64, + ref self: ContractState, guardian: ContractAddress, proposal_id: u64, last_confirm: u64, ) -> u32 { assert(last_confirm != proposal_id, ERR_ALREADY_CONFIRMED); @@ -349,28 +333,22 @@ pub mod UA2Account { let last_confirm = self.recovery_guardian_last_confirm.read(caller); if last_confirm != proposal_id { let confirm_count = _record_recovery_confirmation( - ref self, - caller, - proposal_id, - last_confirm, - ); - self.emit( - Event::RecoveryConfirmed(RecoveryConfirmed { - guardian: caller, - new_owner, - count: confirm_count, - }), + ref self, caller, proposal_id, last_confirm, ); + self + .emit( + Event::RecoveryConfirmed( + RecoveryConfirmed { guardian: caller, new_owner, count: confirm_count }, + ), + ); } - self.emit( - Event::GuardianProposed(GuardianProposed { - guardian: caller, - proposal_id, - new_owner, - eta, - }), - ); + self + .emit( + Event::GuardianProposed( + GuardianProposed { guardian: caller, proposal_id, new_owner, eta }, + ), + ); self.emit(Event::RecoveryProposed(RecoveryProposed { new_owner, eta })); } @@ -389,13 +367,12 @@ pub mod UA2Account { let last_confirm = self.recovery_guardian_last_confirm.read(caller); let new_count = _record_recovery_confirmation(ref self, caller, proposal_id, last_confirm); - self.emit( - Event::RecoveryConfirmed(RecoveryConfirmed { - guardian: caller, - new_owner, - count: new_count, - }), - ); + self + .emit( + Event::RecoveryConfirmed( + RecoveryConfirmed { guardian: caller, new_owner, count: new_count }, + ), + ); } #[external(v0)] @@ -423,6 +400,7 @@ pub mod UA2Account { assert(new_owner != current, ERR_SAME_OWNER); self.owner_pubkey.write(new_owner); + _bump_owner_epoch(ref self); self.emit(Event::OwnerRotated(OwnerRotated { new_owner })); } @@ -444,18 +422,18 @@ pub mod UA2Account { let new_owner = self.recovery_proposed_owner.read(); let proposal_id = self.recovery_proposal_id.read(); self.owner_pubkey.write(new_owner); + _bump_owner_epoch(ref self); _clear_recovery_state(ref self); self.emit(Event::OwnerRotated(OwnerRotated { new_owner })); self.emit(Event::RecoveryExecuted(RecoveryExecuted { new_owner })); - self.emit( - Event::GuardianFinalized(GuardianFinalized { - guardian: caller, - proposal_id, - new_owner, - }), - ); + self + .emit( + Event::GuardianFinalized( + GuardianFinalized { guardian: caller, proposal_id, new_owner }, + ), + ); } fn u256_le(lhs: u256, rhs: u256) -> bool { @@ -511,6 +489,7 @@ pub mod UA2Account { max_calls, calls_used: 0_u32, max_value_per_call: value_cap, + owner_epoch: 0_u64, }; self.add_session(pubkey, policy); @@ -537,6 +516,8 @@ pub mod UA2Account { let mut policy = self.session.read(key_hash); require(policy.is_active, ERR_SESSION_INACTIVE); + let current_epoch = self.session_owner_epoch.read(); + require(policy.owner_epoch == current_epoch, ERR_SESSION_STALE); let now = get_block_timestamp(); require(now >= policy.valid_after, ERR_SESSION_NOT_READY); @@ -572,16 +553,22 @@ pub mod UA2Account { policy.is_active = true; policy.calls_used = 0_u32; + policy.owner_epoch = self.session_owner_epoch.read(); self.session.write(key_hash, policy); self.session_nonce.write(key_hash, 0_u128); - self.emit(Event::SessionAdded(SessionAdded { - key_hash, - valid_after: policy.valid_after, - valid_until: policy.valid_until, - max_calls: policy.max_calls, - })); + self + .emit( + Event::SessionAdded( + SessionAdded { + key_hash, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + }, + ), + ); } fn get_session(self: @ContractState, key_hash: felt252) -> SessionPolicy { @@ -688,9 +675,7 @@ pub mod UA2Account { } fn validate_guardian_authorization( - self: @ContractState, - signature: Span, - calls: @Array, + self: @ContractState, signature: Span, calls: @Array, ) { let signature_len = signature.len(); require(signature_len == 2_usize, ERR_GUARDIAN_SIG_INVALID); @@ -737,22 +722,14 @@ pub mod UA2Account { nonce: u128, ) -> felt252 { let mut values = array![ - SESSION_DOMAIN_TAG, - chain_id, - account_felt, - session_pubkey, - key_hash, - call_digest, - valid_until.into(), - nonce.into(), + SESSION_DOMAIN_TAG, chain_id, account_felt, session_pubkey, key_hash, call_digest, + valid_until.into(), nonce.into(), ]; poseidon_hash_span(values.span()) } fn validate_session_policy( - self: @ContractState, - signature: Span, - calls: @Array, + self: @ContractState, signature: Span, calls: @Array, ) -> SessionValidation { let signature_len = signature.len(); require(signature_len >= 6_usize, ERR_SESSION_SIG_INVALID); @@ -786,6 +763,8 @@ pub mod UA2Account { let policy = self.session.read(key_hash); require(policy.is_active, ERR_SESSION_INACTIVE); + let current_epoch = self.session_owner_epoch.read(); + require(policy.owner_epoch == current_epoch, ERR_SESSION_STALE); let now = get_block_timestamp(); require(now >= policy.valid_after, ERR_SESSION_NOT_READY); @@ -901,7 +880,7 @@ pub mod UA2Account { APPLY_SESSION_USAGE_SELECTOR, accounting_calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); return; } @@ -914,9 +893,9 @@ pub mod UA2Account { let owner_signature = extract_owner_signature(signature); let owner_signature_span = owner_signature.span(); - let owner_valid = AccountComponent::InternalImpl::::_is_valid_signature( - self.account, tx_hash, owner_signature_span - ); + let owner_valid = AccountComponent::InternalImpl::< + ContractState, + >::_is_valid_signature(self.account, tx_hash, owner_signature_span); require(owner_valid, ERR_OWNER_SIG_INVALID); @@ -944,9 +923,9 @@ pub mod UA2Account { let owner_signature = extract_owner_signature(signature); let owner_signature_span = owner_signature.span(); - let owner_valid = AccountComponent::InternalImpl::::_is_valid_signature( - self.account, tx_hash, owner_signature_span - ); + let owner_valid = AccountComponent::InternalImpl::< + ContractState, + >::_is_valid_signature(self.account, tx_hash, owner_signature_span); require(owner_valid, ERR_OWNER_SIG_INVALID); @@ -956,21 +935,21 @@ pub mod UA2Account { fn is_valid_signature( self: @ContractState, hash: felt252, signature: Array, ) -> felt252 { - AccountComponent::AccountMixinImpl::::is_valid_signature( - self, hash, signature - ) + AccountComponent::AccountMixinImpl::< + ContractState, + >::is_valid_signature(self, hash, signature) } fn supports_interface(self: @ContractState, interface_id: felt252) -> bool { - AccountComponent::AccountMixinImpl::::supports_interface( - self, interface_id - ) + AccountComponent::AccountMixinImpl::< + ContractState, + >::supports_interface(self, interface_id) } fn __validate_declare__(self: @ContractState, class_hash: felt252) -> felt252 { - AccountComponent::AccountMixinImpl::::__validate_declare__( - self, class_hash - ) + AccountComponent::AccountMixinImpl::< + ContractState, + >::__validate_declare__(self, class_hash) } fn __validate_deploy__( @@ -979,9 +958,9 @@ pub mod UA2Account { contract_address_salt: felt252, public_key: felt252, ) -> felt252 { - AccountComponent::AccountMixinImpl::::__validate_deploy__( - self, class_hash, contract_address_salt, public_key - ) + AccountComponent::AccountMixinImpl::< + ContractState, + >::__validate_deploy__(self, class_hash, contract_address_salt, public_key) } fn get_public_key(self: @ContractState) -> felt252 { @@ -991,29 +970,27 @@ pub mod UA2Account { fn set_public_key( ref self: ContractState, new_public_key: felt252, signature: Span, ) { - AccountComponent::AccountMixinImpl::::set_public_key( - ref self, new_public_key, signature - ); + AccountComponent::AccountMixinImpl::< + ContractState, + >::set_public_key(ref self, new_public_key, signature); } fn isValidSignature( self: @ContractState, hash: felt252, signature: Array, ) -> felt252 { - AccountComponent::AccountMixinImpl::::isValidSignature( - self, hash, signature - ) + AccountComponent::AccountMixinImpl::< + ContractState, + >::isValidSignature(self, hash, signature) } fn getPublicKey(self: @ContractState) -> felt252 { AccountComponent::AccountMixinImpl::::getPublicKey(self) } - fn setPublicKey( - ref self: ContractState, newPublicKey: felt252, signature: Span, - ) { - AccountComponent::AccountMixinImpl::::setPublicKey( - ref self, newPublicKey, signature - ); + fn setPublicKey(ref self: ContractState, newPublicKey: felt252, signature: Span) { + AccountComponent::AccountMixinImpl::< + ContractState, + >::setPublicKey(ref self, newPublicKey, signature); } } impl AccountInternalImpl = AccountComponent::InternalImpl; diff --git a/packages/contracts/tests/test_owner_rotate.cairo b/packages/contracts/tests/test_owner_rotate.cairo index d279f52..63d63dc 100644 --- a/packages/contracts/tests/test_owner_rotate.cairo +++ b/packages/contracts/tests/test_owner_rotate.cairo @@ -1,21 +1,24 @@ -use core::array::{ArrayTrait, SpanTrait}; +use core::array::{Array, ArrayTrait, SpanTrait}; +use core::integer::u256; use core::result::ResultTrait; +use core::serde::Serde; +use core::traits::{Into, TryInto}; use snforge_std::{ - declare, - spy_events, - start_cheat_caller_address, - stop_cheat_caller_address, - ContractClassTrait, - DeclareResultTrait, - EventSpyAssertionsTrait, + ContractClassTrait, DeclareResultTrait, EventSpyAssertionsTrait, declare, spy_events, + start_cheat_caller_address, start_cheat_signature, stop_cheat_caller_address, + stop_cheat_signature, }; -use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; +use starknet::account::Call; use starknet::syscalls::call_contract_syscall; -use ua2_contracts::ua2_account::UA2Account::{Event, OwnerRotated}; -use ua2_contracts::errors::{ERR_NOT_OWNER, ERR_SAME_OWNER, ERR_ZERO_OWNER}; +use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; +use ua2_contracts::errors::{ERR_NOT_OWNER, ERR_SAME_OWNER, ERR_SESSION_STALE, ERR_ZERO_OWNER}; +use ua2_contracts::session::Session; +use ua2_contracts::ua2_account::UA2Account::{Event, OwnerRotated, SessionPolicy}; +use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x111; const NEW_OWNER: felt252 = 0x222; +const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); fn deploy_account() -> ContractAddress { let declare_result = declare("UA2Account").unwrap(); @@ -24,21 +27,93 @@ fn deploy_account() -> ContractAddress { contract_address } +fn deploy_account_and_mock() -> (ContractAddress, ContractAddress) { + let account_address = deploy_account(); + + let mock_declare = declare("MockERC20").unwrap(); + let mock_class = mock_declare.contract_class(); + let (mock_address, _) = mock_class.deploy(@array![]).unwrap_syscall(); + + (account_address, mock_address) +} + fn call_with_felt( - contract_address: ContractAddress, - selector: felt252, - value: felt252, + contract_address: ContractAddress, selector: felt252, value: felt252, ) -> SyscallResult> { let mut calldata = array![]; calldata.append(value); call_contract_syscall(contract_address, selector, calldata.span()) } +fn add_session( + account_address: ContractAddress, + session_pubkey: felt252, + mock_address: ContractAddress, + policy: SessionPolicy, +) { + start_cheat_caller_address(account_address, account_address); + + let mut targets: Array = array![]; + targets.append(mock_address); + + let mut selectors: Array = array![]; + selectors.append(TRANSFER_SELECTOR); + + let session = Session { + pubkey: session_pubkey, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len: 1_u32, + targets, + selectors_len: 1_u32, + selectors, + }; + + let mut calldata = array![]; + Serde::::serialize(@session, ref calldata); + + call_contract_syscall( + account_address, starknet::selector!("add_session_with_allowlists"), calldata.span(), + ) + .unwrap_syscall(); + + stop_cheat_caller_address(account_address); +} + +fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amount: u256) -> Call { + let mut calldata = array![]; + calldata.append(to.into()); + calldata.append(amount.low.into()); + calldata.append(amount.high.into()); + + Call { to: mock_address, selector: TRANSFER_SELECTOR, calldata: calldata.span() } +} + +fn execute_session_call( + account_address: ContractAddress, calls: @Array, signature: @Array, +) -> SyscallResult> { + let zero_contract: ContractAddress = 0.try_into().unwrap(); + start_cheat_caller_address(account_address, zero_contract); + start_cheat_signature(account_address, signature.span()); + + let mut execute_calldata = array![]; + Serde::>::serialize(calls, ref execute_calldata); + + let result = call_contract_syscall( + account_address, starknet::selector!("__execute__"), execute_calldata.span(), + ); + + stop_cheat_signature(account_address); + stop_cheat_caller_address(account_address); + + result +} + fn assert_reverted_with(result: SyscallResult>, expected: felt252) { match result { - Result::Ok(_) => { - assert(false, 'expected revert'); - }, + Result::Ok(_) => { assert(false, 'expected revert'); }, Result::Err(panic_data) => { let data = panic_data.span(); assert(data.len() > 0_usize, 'missing panic data'); @@ -54,30 +129,22 @@ fn owner_rotation_happy() { let mut spy = spy_events(); start_cheat_caller_address(contract_address, contract_address); - call_with_felt( - contract_address, - starknet::selector!("rotate_owner"), - NEW_OWNER, - ) - .unwrap_syscall(); + call_with_felt(contract_address, starknet::selector!("rotate_owner"), NEW_OWNER) + .unwrap_syscall(); stop_cheat_caller_address(contract_address); let empty = array![]; let owner_result = call_contract_syscall( - contract_address, - starknet::selector!("get_owner"), - empty.span(), + contract_address, starknet::selector!("get_owner"), empty.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); let owner = *owner_result.at(0_usize); assert(owner == NEW_OWNER, 'owner not rotated'); - spy.assert_emitted(@array![ - ( - contract_address, - Event::OwnerRotated(OwnerRotated { new_owner: NEW_OWNER }), - ), - ]); + spy + .assert_emitted( + @array![(contract_address, Event::OwnerRotated(OwnerRotated { new_owner: NEW_OWNER }))], + ); } #[test] @@ -85,17 +152,11 @@ fn owner_rotation_rejects_zero_and_same() { let contract_address = deploy_account(); start_cheat_caller_address(contract_address, contract_address); - let zero_owner = call_with_felt( - contract_address, - starknet::selector!("rotate_owner"), - 0, - ); + let zero_owner = call_with_felt(contract_address, starknet::selector!("rotate_owner"), 0); assert_reverted_with(zero_owner, ERR_ZERO_OWNER); let same_owner = call_with_felt( - contract_address, - starknet::selector!("rotate_owner"), - OWNER_PUBKEY, + contract_address, starknet::selector!("rotate_owner"), OWNER_PUBKEY, ); assert_reverted_with(same_owner, ERR_SAME_OWNER); stop_cheat_caller_address(contract_address); @@ -105,20 +166,54 @@ fn owner_rotation_rejects_zero_and_same() { fn non_owner_cannot_rotate() { let contract_address = deploy_account(); - let result = call_with_felt( - contract_address, - starknet::selector!("rotate_owner"), - 0xBBB, - ); + let result = call_with_felt(contract_address, starknet::selector!("rotate_owner"), 0xBBB); assert_reverted_with(result, ERR_NOT_OWNER); let empty = array![]; let owner_result = call_contract_syscall( - contract_address, - starknet::selector!("get_owner"), - empty.span(), + contract_address, starknet::selector!("get_owner"), empty.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); let owner = *owner_result.at(0_usize); assert(owner == OWNER_PUBKEY, 'owner should remain original'); } + +#[test] +fn sessions_are_invalidated_on_owner_rotation() { + let (account_address, mock_address) = deploy_account_and_mock(); + + let session_pubkey = session_key(); + let policy = SessionPolicy { + is_active: true, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + add_session(account_address, session_pubkey, mock_address, policy); + + let to: ContractAddress = account_address; + let amount = u256 { low: 1_000_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let first_signature: Array = build_session_signature( + account_address, session_pubkey, 0_u128, policy.valid_until, @calls, + ); + execute_session_call(account_address, @calls, @first_signature).unwrap_syscall(); + + start_cheat_caller_address(account_address, account_address); + call_with_felt(account_address, starknet::selector!("rotate_owner"), NEW_OWNER) + .unwrap_syscall(); + stop_cheat_caller_address(account_address); + + let second_signature: Array = build_session_signature( + account_address, session_pubkey, 1_u128, policy.valid_until, @calls, + ); + let result = execute_session_call(account_address, @calls, @second_signature); + + assert_reverted_with(result, ERR_SESSION_STALE); +} diff --git a/packages/contracts/tests/test_session_nonce_ok.cairo b/packages/contracts/tests/test_session_nonce_ok.cairo index 18191dc..eb3d8e4 100644 --- a/packages/contracts/tests/test_session_nonce_ok.cairo +++ b/packages/contracts/tests/test_session_nonce_ok.cairo @@ -2,38 +2,20 @@ use core::array::{Array, ArrayTrait}; use core::integer::u256; use core::serde::Serde; use core::traits::Into; - use snforge_std::{ - declare, - spy_events, - start_cheat_block_timestamp, - stop_cheat_block_timestamp, - start_cheat_caller_address, - stop_cheat_caller_address, - start_cheat_signature, - stop_cheat_signature, - ContractClassTrait, - DeclareResultTrait, - EventSpyAssertionsTrait, + ContractClassTrait, DeclareResultTrait, EventSpyAssertionsTrait, declare, spy_events, + start_cheat_block_timestamp, start_cheat_caller_address, start_cheat_signature, + stop_cheat_block_timestamp, stop_cheat_caller_address, stop_cheat_signature, }; use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResultTrait}; -use ua2_contracts::ua2_account::UA2Account::{ - Event, - ISessionManagerDispatcher, - ISessionManagerDispatcherTrait, - SessionNonceAdvanced, - SessionPolicy, - SessionUsed, -}; use ua2_contracts::session::Session; - -use crate::session_test_utils::{ - build_session_signature, - session_key, - session_key_hash, +use ua2_contracts::ua2_account::UA2Account::{ + Event, ISessionManagerDispatcher, ISessionManagerDispatcherTrait, SessionNonceAdvanced, + SessionPolicy, SessionUsed, }; +use crate::session_test_utils::{build_session_signature, session_key, session_key_hash}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); @@ -80,11 +62,9 @@ fn add_session( Serde::::serialize(@session, ref calldata); call_contract_syscall( - account_address, - starknet::selector!("add_session_with_allowlists"), - calldata.span(), + account_address, starknet::selector!("add_session_with_allowlists"), calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_caller_address(account_address); } @@ -99,9 +79,7 @@ fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amoun } fn execute_with_signature( - account_address: ContractAddress, - calls: @Array, - signature: @Array, + account_address: ContractAddress, calls: @Array, signature: @Array, ) { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); @@ -111,11 +89,9 @@ fn execute_with_signature( Serde::>::serialize(calls, ref execute_calldata); call_contract_syscall( - account_address, - starknet::selector!("__execute__"), - execute_calldata.span(), + account_address, starknet::selector!("__execute__"), execute_calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_signature(account_address); stop_cheat_caller_address(account_address); @@ -135,6 +111,7 @@ fn test_session_nonce_ok() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; add_session(account_address, session_pubkey, mock_address, policy); @@ -146,36 +123,40 @@ fn test_session_nonce_ok() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let signature0: Array = - build_session_signature(account_address, session_pubkey, 0_u128, policy.valid_until, @calls); + let signature0: Array = build_session_signature( + account_address, session_pubkey, 0_u128, policy.valid_until, @calls, + ); execute_with_signature(account_address, @calls, @signature0); - let signature1: Array = - build_session_signature(account_address, session_pubkey, 1_u128, policy.valid_until, @calls); + let signature1: Array = build_session_signature( + account_address, session_pubkey, 1_u128, policy.valid_until, @calls, + ); execute_with_signature(account_address, @calls, @signature1); stop_cheat_block_timestamp(account_address); - spy.assert_emitted(@array![ - ( - account_address, - Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 }), - ), - ( - account_address, - Event::SessionNonceAdvanced(SessionNonceAdvanced { key_hash, new_nonce: 1_u128 }), - ), - ( - account_address, - Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 }), - ), - ( - account_address, - Event::SessionNonceAdvanced(SessionNonceAdvanced { key_hash, new_nonce: 2_u128 }), - ), - ]); + spy + .assert_emitted( + @array![ + (account_address, Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 })), + ( + account_address, + Event::SessionNonceAdvanced( + SessionNonceAdvanced { key_hash, new_nonce: 1_u128 }, + ), + ), + (account_address, Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 })), + ( + account_address, + Event::SessionNonceAdvanced( + SessionNonceAdvanced { key_hash, new_nonce: 2_u128 }, + ), + ), + ], + ); let dispatcher = ISessionManagerDispatcher { contract_address: account_address }; let updated_policy = dispatcher.get_session(key_hash); assert(updated_policy.calls_used == 2_u32, 'unexpected call count'); + assert(updated_policy.owner_epoch == 0_u64, 'unexpected session epoch'); } diff --git a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo index 863b953..5ee3ec3 100644 --- a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo +++ b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo @@ -1,27 +1,19 @@ use core::array::{Array, ArrayTrait}; use core::integer::u256; +use core::result::Result; use core::serde::Serde; use core::traits::{Into, TryInto}; -use core::result::Result; - use snforge_std::{ - declare, - start_cheat_block_timestamp, - stop_cheat_block_timestamp, - start_cheat_caller_address, - stop_cheat_caller_address, - start_cheat_signature, - stop_cheat_signature, - ContractClassTrait, - DeclareResultTrait, + ContractClassTrait, DeclareResultTrait, declare, start_cheat_block_timestamp, + start_cheat_caller_address, start_cheat_signature, stop_cheat_block_timestamp, + stop_cheat_caller_address, stop_cheat_signature, }; use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; -use ua2_contracts::ua2_account::UA2Account::SessionPolicy; -use ua2_contracts::session::Session; use ua2_contracts::errors::{ERR_BAD_SESSION_NONCE, ERR_SESSION_SIG_INVALID}; - +use ua2_contracts::session::Session; +use ua2_contracts::ua2_account::UA2Account::SessionPolicy; use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; @@ -69,11 +61,9 @@ fn add_session( Serde::::serialize(@session, ref calldata); call_contract_syscall( - account_address, - starknet::selector!("add_session_with_allowlists"), - calldata.span(), + account_address, starknet::selector!("add_session_with_allowlists"), calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_caller_address(account_address); } @@ -88,9 +78,7 @@ fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amoun } fn execute_with_signature( - account_address: ContractAddress, - calls: @Array, - signature: @Array, + account_address: ContractAddress, calls: @Array, signature: @Array, ) -> SyscallResult> { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); @@ -100,9 +88,7 @@ fn execute_with_signature( Serde::>::serialize(calls, ref execute_calldata); let result = call_contract_syscall( - account_address, - starknet::selector!("__execute__"), - execute_calldata.span(), + account_address, starknet::selector!("__execute__"), execute_calldata.span(), ); stop_cheat_signature(account_address); @@ -113,9 +99,7 @@ fn execute_with_signature( fn assert_reverted_with(result: SyscallResult>, expected: felt252) { match result { - Result::Ok(_) => { - assert(false, 'expected revert'); - }, + Result::Ok(_) => { assert(false, 'expected revert'); }, Result::Err(panic_data) => { let panic_span = panic_data.span(); assert(panic_span.len() > 0_usize, 'missing panic data'); @@ -137,6 +121,7 @@ fn test_session_nonce_replay_and_mismatch() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; add_session(account_address, session_pubkey, mock_address, policy); @@ -148,24 +133,28 @@ fn test_session_nonce_replay_and_mismatch() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let signature0: Array = - build_session_signature(account_address, session_pubkey, 0_u128, policy.valid_until, @calls); + let signature0: Array = build_session_signature( + account_address, session_pubkey, 0_u128, policy.valid_until, @calls, + ); execute_with_signature(account_address, @calls, @signature0).unwrap_syscall(); let replay_result = execute_with_signature(account_address, @calls, @signature0); assert_reverted_with(replay_result, ERR_BAD_SESSION_NONCE); - let skip_signature: Array = - build_session_signature(account_address, session_pubkey, 2_u128, policy.valid_until, @calls); + let skip_signature: Array = build_session_signature( + account_address, session_pubkey, 2_u128, policy.valid_until, @calls, + ); let skip_result = execute_with_signature(account_address, @calls, @skip_signature); assert_reverted_with(skip_result, ERR_BAD_SESSION_NONCE); - let signature1: Array = - build_session_signature(account_address, session_pubkey, 1_u128, policy.valid_until, @calls); + let signature1: Array = build_session_signature( + account_address, session_pubkey, 1_u128, policy.valid_until, @calls, + ); execute_with_signature(account_address, @calls, @signature1).unwrap_syscall(); - let signature2: Array = - build_session_signature(account_address, session_pubkey, 2_u128, policy.valid_until, @calls); + let signature2: Array = build_session_signature( + account_address, session_pubkey, 2_u128, policy.valid_until, @calls, + ); let tampered_amount = u256 { low: 1_001_u128, high: 0_u128 }; let tampered_call = build_transfer_call(mock_address, to, tampered_amount); diff --git a/packages/contracts/tests/test_sessions.cairo b/packages/contracts/tests/test_sessions.cairo index 69c4a88..45fb94b 100644 --- a/packages/contracts/tests/test_sessions.cairo +++ b/packages/contracts/tests/test_sessions.cairo @@ -1,26 +1,15 @@ use core::integer::u256; +use core::pedersen::pedersen; use core::result::ResultTrait; use snforge_std::{ - declare, - spy_events, - start_cheat_caller_address, - stop_cheat_caller_address, - ContractClassTrait, - DeclareResultTrait, - EventSpyAssertionsTrait, + ContractClassTrait, DeclareResultTrait, EventSpyAssertionsTrait, declare, spy_events, + start_cheat_caller_address, stop_cheat_caller_address, }; use starknet::SyscallResultTrait; use ua2_contracts::ua2_account::UA2Account::{ - self, - SessionAdded, - SessionPolicy, + self, ISessionManagerDispatcher, ISessionManagerDispatcherTrait, SessionAdded, SessionPolicy, SessionRevoked, }; -use ua2_contracts::ua2_account::UA2Account::{ - ISessionManagerDispatcher, - ISessionManagerDispatcherTrait, -}; -use core::pedersen::pedersen; const OWNER_PUBKEY: felt252 = 0x12345; @@ -47,6 +36,7 @@ fn add_get_revoke_session_works() { max_calls: 5_u32, calls_used: 2_u32, max_value_per_call: u256 { low: 0, high: 0 }, + owner_epoch: 0_u64, }; dispatcher.add_session(key, policy); @@ -56,6 +46,7 @@ fn add_get_revoke_session_works() { assert(stored_policy.valid_until == 3_600_u64, 'expiry mismatch'); assert(stored_policy.max_calls == 5_u32, 'max calls mismatch'); assert(stored_policy.calls_used == 0_u32, 'calls used not reset'); + assert(stored_policy.owner_epoch == 0_u64, 'unexpected session epoch'); dispatcher.revoke_session(key_hash); @@ -81,6 +72,7 @@ fn events_emitted() { max_calls: 10_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 0, high: 0 }, + owner_epoch: 0_u64, }; dispatcher.add_session(key, policy); @@ -88,19 +80,18 @@ fn events_emitted() { stop_cheat_caller_address(contract_address); - spy.assert_emitted(@array![ - ( - contract_address, - UA2Account::Event::SessionAdded(SessionAdded { - key_hash, - valid_after: 0_u64, - valid_until: 7_200_u64, - max_calls: 10_u32, - }), - ), - ( - contract_address, - UA2Account::Event::SessionRevoked(SessionRevoked { key_hash }), - ), - ]); + spy + .assert_emitted( + @array![ + ( + contract_address, + UA2Account::Event::SessionAdded( + SessionAdded { + key_hash, valid_after: 0_u64, valid_until: 7_200_u64, max_calls: 10_u32, + }, + ), + ), + (contract_address, UA2Account::Event::SessionRevoked(SessionRevoked { key_hash })), + ], + ); } diff --git a/packages/contracts/tests/test_validate_allowlists.cairo b/packages/contracts/tests/test_validate_allowlists.cairo index 93d12a1..74627c2 100644 --- a/packages/contracts/tests/test_validate_allowlists.cairo +++ b/packages/contracts/tests/test_validate_allowlists.cairo @@ -1,39 +1,22 @@ use core::array::{Array, ArrayTrait, SpanTrait}; use core::integer::u256; use core::option::Option; -use core::traits::{Into, TryInto}; use core::serde::Serde; +use core::traits::{Into, TryInto}; use snforge_std::{ - declare, - spy_events, - start_cheat_block_timestamp, - stop_cheat_block_timestamp, - start_cheat_caller_address, - stop_cheat_caller_address, - start_cheat_signature, - stop_cheat_signature, - ContractClassTrait, - DeclareResultTrait, - EventSpyAssertionsTrait, + ContractClassTrait, DeclareResultTrait, EventSpyAssertionsTrait, declare, spy_events, + start_cheat_block_timestamp, start_cheat_caller_address, start_cheat_signature, + stop_cheat_block_timestamp, stop_cheat_caller_address, stop_cheat_signature, }; use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResultTrait}; -use ua2_contracts::ua2_account::UA2Account::{ - Event, - ISessionManagerDispatcher, - ISessionManagerDispatcherTrait, - SessionNonceAdvanced, - SessionPolicy, - SessionUsed, -}; use ua2_contracts::session::Session; - -use crate::session_test_utils::{ - build_session_signature, - session_key, - session_key_hash, +use ua2_contracts::ua2_account::UA2Account::{ + Event, ISessionManagerDispatcher, ISessionManagerDispatcherTrait, SessionNonceAdvanced, + SessionPolicy, SessionUsed, }; +use crate::session_test_utils::{build_session_signature, session_key, session_key_hash}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); @@ -59,6 +42,7 @@ fn session_allows_whitelisted_calls() { max_calls: 1_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; start_cheat_block_timestamp(account_address, 5_000_u64); @@ -91,12 +75,13 @@ fn session_allows_whitelisted_calls() { starknet::selector!("add_session_with_allowlists"), allowlist_calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_caller_address(account_address); let session_dispatcher = ISessionManagerDispatcher { contract_address: account_address }; let stored_policy = session_dispatcher.get_session(key_hash); assert(stored_policy.is_active == true, 'session inactive'); + assert(stored_policy.owner_epoch == 0_u64, 'unexpected session epoch'); let amount = u256 { low: 500_u128, high: 0_u128 }; let to: ContractAddress = account_address; @@ -111,45 +96,38 @@ fn session_allows_whitelisted_calls() { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); - let signature: Array = - build_session_signature( - account_address, - session_pubkey, - 0_u128, - policy.valid_until, - @calls, - ); + let signature: Array = build_session_signature( + account_address, session_pubkey, 0_u128, policy.valid_until, @calls, + ); start_cheat_signature(account_address, signature.span()); let mut execute_calldata = array![]; Serde::>::serialize(@calls, ref execute_calldata); call_contract_syscall( - account_address, - starknet::selector!("__execute__"), - execute_calldata.span(), + account_address, starknet::selector!("__execute__"), execute_calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_signature(account_address); stop_cheat_caller_address(account_address); stop_cheat_block_timestamp(account_address); - spy.assert_emitted(@array![ - ( - account_address, - Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 }), - ), - ( - account_address, - Event::SessionNonceAdvanced(SessionNonceAdvanced { key_hash, new_nonce: 1_u128 }), - ), - ]); + spy + .assert_emitted( + @array![ + (account_address, Event::SessionUsed(SessionUsed { key_hash, used: 1_u32 })), + ( + account_address, + Event::SessionNonceAdvanced( + SessionNonceAdvanced { key_hash, new_nonce: 1_u128 }, + ), + ), + ], + ); let get_last_result = call_contract_syscall( - mock_address, - starknet::selector!("get_last"), - array![].span(), + mock_address, starknet::selector!("get_last"), array![].span(), ) - .unwrap_syscall(); + .unwrap_syscall(); let recorded_to_felt = *get_last_result.at(0); let recorded_low_felt = *get_last_result.at(1); diff --git a/packages/contracts/tests/test_validate_denials.cairo b/packages/contracts/tests/test_validate_denials.cairo index c5c0891..4f5edbf 100644 --- a/packages/contracts/tests/test_validate_denials.cairo +++ b/packages/contracts/tests/test_validate_denials.cairo @@ -4,33 +4,20 @@ use core::option::Option; use core::result::Result; use core::serde::Serde; use core::traits::{Into, TryInto}; - use snforge_std::{ - declare, - start_cheat_block_timestamp, - stop_cheat_block_timestamp, - start_cheat_caller_address, - stop_cheat_caller_address, - start_cheat_signature, - stop_cheat_signature, - ContractClassTrait, - DeclareResultTrait, + ContractClassTrait, DeclareResultTrait, declare, start_cheat_block_timestamp, + start_cheat_caller_address, start_cheat_signature, stop_cheat_block_timestamp, + stop_cheat_caller_address, stop_cheat_signature, }; use starknet::account::Call; use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; -use ua2_contracts::ua2_account::UA2Account::SessionPolicy; -use ua2_contracts::session::Session; use ua2_contracts::errors::{ - ERR_POLICY_CALLCAP, - ERR_POLICY_SELECTOR_DENIED, - ERR_POLICY_TARGET_DENIED, - ERR_SESSION_EXPIRED, - ERR_SESSION_SELECTORS_LEN, - ERR_SESSION_TARGETS_LEN, - ERR_VALUE_LIMIT_EXCEEDED, + ERR_POLICY_CALLCAP, ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_SESSION_EXPIRED, + ERR_SESSION_SELECTORS_LEN, ERR_SESSION_TARGETS_LEN, ERR_VALUE_LIMIT_EXCEEDED, }; - +use ua2_contracts::session::Session; +use ua2_contracts::ua2_account::UA2Account::SessionPolicy; use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; @@ -100,20 +87,14 @@ fn add_session_with_lists( Serde::::serialize(@session, ref calldata); call_contract_syscall( - account_address, - starknet::selector!("add_session_with_allowlists"), - calldata.span(), + account_address, starknet::selector!("add_session_with_allowlists"), calldata.span(), ) - .unwrap_syscall(); + .unwrap_syscall(); stop_cheat_caller_address(account_address); } -fn build_transfer_call( - mock_address: ContractAddress, - to: ContractAddress, - amount: u256, -) -> Call { +fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amount: u256) -> Call { let mut calldata = array![]; calldata.append(to.into()); calldata.append(amount.low.into()); @@ -131,17 +112,16 @@ fn execute_session_calls( ) -> SyscallResult> { let zero_contract: ContractAddress = 0.try_into().unwrap(); start_cheat_caller_address(account_address, zero_contract); - let signature: Array = - build_session_signature(account_address, session_pubkey, nonce, valid_until, calls); + let signature: Array = build_session_signature( + account_address, session_pubkey, nonce, valid_until, calls, + ); start_cheat_signature(account_address, signature.span()); let mut execute_calldata = array![]; Serde::>::serialize(calls, ref execute_calldata); let result = call_contract_syscall( - account_address, - starknet::selector!("__execute__"), - execute_calldata.span(), + account_address, starknet::selector!("__execute__"), execute_calldata.span(), ); stop_cheat_signature(account_address); @@ -152,9 +132,7 @@ fn execute_session_calls( fn assert_reverted_with(result: SyscallResult>, expected: felt252) { match result { - Result::Ok(_) => { - assert(false, 'expected revert'); - }, + Result::Ok(_) => { assert(false, 'expected revert'); }, Result::Err(panic_data) => { let panic_span = panic_data.span(); assert(panic_span.len() > 0_usize, 'missing panic data'); @@ -192,15 +170,11 @@ fn rejects_length_mismatch() { Serde::::serialize(@session_targets_mismatch, ref calldata); let result = call_contract_syscall( - account_address, - starknet::selector!("add_session_with_allowlists"), - calldata.span(), + account_address, starknet::selector!("add_session_with_allowlists"), calldata.span(), ); match result { - Result::Ok(_) => { - assert(false, 'expected targets len mismatch'); - }, + Result::Ok(_) => { assert(false, 'expected targets len mismatch'); }, Result::Err(panic_data) => { let data = panic_data.span(); assert(data.len() > 0_usize, 'missing panic data'); @@ -236,9 +210,7 @@ fn rejects_length_mismatch() { ); match selectors_result { - Result::Ok(_) => { - assert(false, 'expected selectors len mismatch'); - }, + Result::Ok(_) => { assert(false, 'expected selectors len mismatch'); }, Result::Err(panic_data) => { let data = panic_data.span(); assert(data.len() > 0_usize, 'missing panic data'); @@ -261,6 +233,7 @@ fn denies_selector_not_allowed() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; let mut targets = array![mock_address]; @@ -276,7 +249,9 @@ fn denies_selector_not_allowed() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); + let result = execute_session_calls( + account_address, @calls, 0_u128, session_pubkey, policy.valid_until, + ); assert_reverted_with(result, ERR_POLICY_SELECTOR_DENIED); @@ -294,6 +269,7 @@ fn denies_target_not_allowed() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; let targets = array![]; @@ -309,7 +285,9 @@ fn denies_target_not_allowed() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); + let result = execute_session_calls( + account_address, @calls, 0_u128, session_pubkey, policy.valid_until, + ); assert_reverted_with(result, ERR_POLICY_TARGET_DENIED); @@ -327,6 +305,7 @@ fn denies_expired_session() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; let mut targets = array![mock_address]; @@ -342,7 +321,9 @@ fn denies_expired_session() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); + let result = execute_session_calls( + account_address, @calls, 0_u128, session_pubkey, policy.valid_until, + ); assert_reverted_with(result, ERR_SESSION_EXPIRED); @@ -360,6 +341,7 @@ fn denies_over_call_cap() { max_calls: 1_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; let mut targets = array![mock_address]; @@ -376,7 +358,9 @@ fn denies_over_call_cap() { let call_two = build_transfer_call(mock_address, to, amount); let calls = array![call_one, call_two]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); + let result = execute_session_calls( + account_address, @calls, 0_u128, session_pubkey, policy.valid_until, + ); assert_reverted_with(result, ERR_POLICY_CALLCAP); @@ -394,6 +378,7 @@ fn denies_over_value_cap() { max_calls: 5_u32, calls_used: 0_u32, max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, }; let mut targets = array![mock_address]; @@ -409,7 +394,9 @@ fn denies_over_value_cap() { let call = build_transfer_call(mock_address, to, amount); let calls = array![call]; - let result = execute_session_calls(account_address, @calls, 0_u128, session_pubkey, policy.valid_until); + let result = execute_session_calls( + account_address, @calls, 0_u128, session_pubkey, policy.valid_until, + ); assert_reverted_with(result, ERR_VALUE_LIMIT_EXCEEDED); From ef5f4cf6bbb693e5126e39cca67a736210bebcaa Mon Sep 17 00:00:00 2001 From: hooftly Date: Tue, 14 Oct 2025 04:04:49 -0600 Subject: [PATCH 06/16] test(sessions): add utils, expiry/denials/value caps; recovery flows - Add session testing utilities and focused cases for expiry, selector denials, target denials, and per-call value caps to surface failures - Extend guardian recovery coverage with explicit timelock, finalize, and cancel flows plus associated event assertions - Rename owner rotation regression test to test_rotate_owner_revokes_sessions for clarity and suite alignment --- .../contracts/tests/test_owner_rotate.cairo | 2 +- .../tests/test_recovery_edgecases.cairo | 310 ++++++++++++++++++ packages/contracts/tests/test_sessions.cairo | 304 ++++++++++++++++- 3 files changed, 612 insertions(+), 4 deletions(-) diff --git a/packages/contracts/tests/test_owner_rotate.cairo b/packages/contracts/tests/test_owner_rotate.cairo index 63d63dc..d616d51 100644 --- a/packages/contracts/tests/test_owner_rotate.cairo +++ b/packages/contracts/tests/test_owner_rotate.cairo @@ -179,7 +179,7 @@ fn non_owner_cannot_rotate() { } #[test] -fn sessions_are_invalidated_on_owner_rotation() { +fn test_rotate_owner_revokes_sessions() { let (account_address, mock_address) = deploy_account_and_mock(); let session_pubkey = session_key(); diff --git a/packages/contracts/tests/test_recovery_edgecases.cairo b/packages/contracts/tests/test_recovery_edgecases.cairo index d20d595..d20937b 100644 --- a/packages/contracts/tests/test_recovery_edgecases.cairo +++ b/packages/contracts/tests/test_recovery_edgecases.cairo @@ -24,9 +24,12 @@ use ua2_contracts::errors::{ }; use ua2_contracts::ua2_account::UA2Account::{ Event, + GuardianFinalized, GuardianProposed, + OwnerRotated, RecoveryCanceled, RecoveryConfirmed, + RecoveryExecuted, RecoveryProposed, }; @@ -341,3 +344,310 @@ fn recovery_cancel_emits_event() { ), ]); } + +#[test] +fn test_guardian_timelock_enforced() { + let contract_address = deploy_account(); + let g1: ContractAddress = 0x111.try_into().unwrap(); + + start_cheat_caller_address(contract_address, contract_address); + let mut add_calldata = array![]; + add_calldata.append(g1.into()); + call_contract_syscall( + contract_address, + starknet::selector!("add_guardian"), + add_calldata.span(), + ) + .unwrap_syscall(); + + let mut threshold_calldata = array![]; + threshold_calldata.append(1_u8.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_guardian_threshold"), + threshold_calldata.span(), + ) + .unwrap_syscall(); + + let mut delay_calldata = array![]; + delay_calldata.append(500_u64.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_recovery_delay"), + delay_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_block_timestamp(contract_address, 100_u64); + + start_cheat_caller_address(contract_address, g1); + let mut propose_calldata = array![]; + propose_calldata.append(RECOVERY_OWNER_A); + call_contract_syscall( + contract_address, + starknet::selector!("propose_recovery"), + propose_calldata.span(), + ) + .unwrap_syscall(); + + let mut execute_calldata = array![]; + let before_eta = call_contract_syscall( + contract_address, + starknet::selector!("execute_recovery"), + execute_calldata.span(), + ); + stop_cheat_caller_address(contract_address); + + stop_cheat_block_timestamp(contract_address); + + assert_reverted_with(before_eta, ERR_BEFORE_ETA); +} + +#[test] +fn test_guardian_finalize() { + let contract_address = deploy_account(); + let g1: ContractAddress = 0x111.try_into().unwrap(); + let g2: ContractAddress = 0x222.try_into().unwrap(); + + start_cheat_caller_address(contract_address, contract_address); + let mut add_one = array![]; + add_one.append(g1.into()); + call_contract_syscall( + contract_address, + starknet::selector!("add_guardian"), + add_one.span(), + ) + .unwrap_syscall(); + + let mut add_two = array![]; + add_two.append(g2.into()); + call_contract_syscall( + contract_address, + starknet::selector!("add_guardian"), + add_two.span(), + ) + .unwrap_syscall(); + + let mut threshold_calldata = array![]; + threshold_calldata.append(2_u8.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_guardian_threshold"), + threshold_calldata.span(), + ) + .unwrap_syscall(); + + let mut delay_calldata = array![]; + delay_calldata.append(0_u64.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_recovery_delay"), + delay_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + let mut spy = spy_events(); + + start_cheat_block_timestamp(contract_address, 500_u64); + + start_cheat_caller_address(contract_address, g1); + let mut propose_calldata = array![]; + propose_calldata.append(RECOVERY_OWNER_A); + call_contract_syscall( + contract_address, + starknet::selector!("propose_recovery"), + propose_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_caller_address(contract_address, g2); + let mut confirm_calldata = array![]; + confirm_calldata.append(RECOVERY_OWNER_A); + call_contract_syscall( + contract_address, + starknet::selector!("confirm_recovery"), + confirm_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_caller_address(contract_address, g1); + let mut execute_calldata = array![]; + call_contract_syscall( + contract_address, + starknet::selector!("execute_recovery"), + execute_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + stop_cheat_block_timestamp(contract_address); + + let mut empty = array![]; + let owner_result = call_contract_syscall( + contract_address, + starknet::selector!("get_owner"), + empty.span(), + ) + .unwrap_syscall(); + let owner = *owner_result.at(0_usize); + assert(owner == RECOVERY_OWNER_A, 'owner not rotated by guardians'); + + spy.assert_emitted(@array![ + ( + contract_address, + Event::RecoveryConfirmed(RecoveryConfirmed { + guardian: g1, + new_owner: RECOVERY_OWNER_A, + count: 1_u32, + }), + ), + ( + contract_address, + Event::GuardianProposed(GuardianProposed { + guardian: g1, + proposal_id: 1_u64, + new_owner: RECOVERY_OWNER_A, + eta: 500_u64, + }), + ), + ( + contract_address, + Event::RecoveryProposed(RecoveryProposed { + new_owner: RECOVERY_OWNER_A, + eta: 500_u64, + }), + ), + ( + contract_address, + Event::RecoveryConfirmed(RecoveryConfirmed { + guardian: g2, + new_owner: RECOVERY_OWNER_A, + count: 2_u32, + }), + ), + ( + contract_address, + Event::OwnerRotated(OwnerRotated { new_owner: RECOVERY_OWNER_A }), + ), + ( + contract_address, + Event::RecoveryExecuted(RecoveryExecuted { new_owner: RECOVERY_OWNER_A }), + ), + ( + contract_address, + Event::GuardianFinalized(GuardianFinalized { + guardian: g1, + proposal_id: 1_u64, + new_owner: RECOVERY_OWNER_A, + }), + ), + ]); +} + +#[test] +fn test_guardian_cancel() { + let contract_address = deploy_account(); + let g1: ContractAddress = 0x111.try_into().unwrap(); + + start_cheat_caller_address(contract_address, contract_address); + let mut add_calldata = array![]; + add_calldata.append(g1.into()); + call_contract_syscall( + contract_address, + starknet::selector!("add_guardian"), + add_calldata.span(), + ) + .unwrap_syscall(); + + let mut threshold_calldata = array![]; + threshold_calldata.append(1_u8.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_guardian_threshold"), + threshold_calldata.span(), + ) + .unwrap_syscall(); + + let mut delay_calldata = array![]; + delay_calldata.append(10_u64.into()); + call_contract_syscall( + contract_address, + starknet::selector!("set_recovery_delay"), + delay_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + let mut spy = spy_events(); + + start_cheat_block_timestamp(contract_address, 42_u64); + + start_cheat_caller_address(contract_address, g1); + let mut propose_calldata = array![]; + propose_calldata.append(RECOVERY_OWNER_A); + call_contract_syscall( + contract_address, + starknet::selector!("propose_recovery"), + propose_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_caller_address(contract_address, contract_address); + let mut cancel_calldata = array![]; + call_contract_syscall( + contract_address, + starknet::selector!("cancel_recovery"), + cancel_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(contract_address); + + start_cheat_caller_address(contract_address, g1); + let mut execute_calldata = array![]; + let no_recovery = call_contract_syscall( + contract_address, + starknet::selector!("execute_recovery"), + execute_calldata.span(), + ); + stop_cheat_caller_address(contract_address); + + stop_cheat_block_timestamp(contract_address); + + assert_reverted_with(no_recovery, ERR_NO_RECOVERY); + + spy.assert_emitted(@array![ + ( + contract_address, + Event::RecoveryConfirmed(RecoveryConfirmed { + guardian: g1, + new_owner: RECOVERY_OWNER_A, + count: 1_u32, + }), + ), + ( + contract_address, + Event::GuardianProposed(GuardianProposed { + guardian: g1, + proposal_id: 1_u64, + new_owner: RECOVERY_OWNER_A, + eta: 52_u64, + }), + ), + ( + contract_address, + Event::RecoveryProposed(RecoveryProposed { + new_owner: RECOVERY_OWNER_A, + eta: 52_u64, + }), + ), + ( + contract_address, + Event::RecoveryCanceled(RecoveryCanceled {}), + ), + ]); +} diff --git a/packages/contracts/tests/test_sessions.cairo b/packages/contracts/tests/test_sessions.cairo index 45fb94b..8e40d42 100644 --- a/packages/contracts/tests/test_sessions.cairo +++ b/packages/contracts/tests/test_sessions.cairo @@ -1,17 +1,31 @@ +use core::array::{Array, ArrayTrait, SpanTrait}; use core::integer::u256; +use core::option::Option; use core::pedersen::pedersen; use core::result::ResultTrait; +use core::serde::Serde; +use core::traits::{Into, TryInto}; use snforge_std::{ ContractClassTrait, DeclareResultTrait, EventSpyAssertionsTrait, declare, spy_events, - start_cheat_caller_address, stop_cheat_caller_address, + start_cheat_block_timestamp, start_cheat_caller_address, start_cheat_signature, + stop_cheat_block_timestamp, stop_cheat_caller_address, stop_cheat_signature, }; -use starknet::SyscallResultTrait; +use starknet::account::Call; +use starknet::syscalls::call_contract_syscall; +use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; +use ua2_contracts::errors::{ + ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_SESSION_EXPIRED, + ERR_VALUE_LIMIT_EXCEEDED, +}; +use ua2_contracts::session::Session; use ua2_contracts::ua2_account::UA2Account::{ self, ISessionManagerDispatcher, ISessionManagerDispatcherTrait, SessionAdded, SessionPolicy, SessionRevoked, }; +use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; +const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); fn deploy_account() -> (starknet::ContractAddress, ISessionManagerDispatcher) { let declare_result = declare("UA2Account").unwrap(); @@ -21,8 +35,117 @@ fn deploy_account() -> (starknet::ContractAddress, ISessionManagerDispatcher) { (contract_address, dispatcher) } +fn deploy_account_and_mock() -> (ContractAddress, ISessionManagerDispatcher, ContractAddress) { + let (account_address, dispatcher) = deploy_account(); + + let mock_declare = declare("MockERC20").unwrap(); + let mock_class = mock_declare.contract_class(); + let (mock_address, _) = mock_class.deploy(@array![]).unwrap_syscall(); + + (account_address, dispatcher, mock_address) +} + +fn add_session_allowlist( + account_address: ContractAddress, + session_pubkey: felt252, + policy: SessionPolicy, + mut targets: Array, + mut selectors: Array, +) -> felt252 { + start_cheat_caller_address(account_address, account_address); + + let targets_len_usize = targets.len(); + let targets_len: u32 = match targets_len_usize.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'bad targets len'); + 0_u32 + }, + }; + + let selectors_len_usize = selectors.len(); + let selectors_len: u32 = match selectors_len_usize.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'bad selectors len'); + 0_u32 + }, + }; + + let session = Session { + pubkey: session_pubkey, + valid_after: policy.valid_after, + valid_until: policy.valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len, + targets, + selectors_len, + selectors, + }; + + let mut calldata = array![]; + Serde::::serialize(@session, ref calldata); + + call_contract_syscall( + account_address, + starknet::selector!("add_session_with_allowlists"), + calldata.span(), + ) + .unwrap_syscall(); + + stop_cheat_caller_address(account_address); + + pedersen(session_pubkey, 0) +} + +fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amount: u256) -> Call { + let mut calldata = array![]; + calldata.append(to.into()); + calldata.append(amount.low.into()); + calldata.append(amount.high.into()); + + Call { to: mock_address, selector: TRANSFER_SELECTOR, calldata: calldata.span() } +} + +fn execute_session_call( + account_address: ContractAddress, + calls: @Array, + signature: @Array, +) -> SyscallResult> { + let zero: ContractAddress = 0.try_into().unwrap(); + start_cheat_caller_address(account_address, zero); + start_cheat_signature(account_address, signature.span()); + + let mut execute_calldata = array![]; + Serde::>::serialize(calls, ref execute_calldata); + + let result = call_contract_syscall( + account_address, + starknet::selector!("__execute__"), + execute_calldata.span(), + ); + + stop_cheat_signature(account_address); + stop_cheat_caller_address(account_address); + + result +} + +fn assert_reverted_with(result: SyscallResult>, expected: felt252) { + match result { + Result::Ok(_) => { assert(false, 'expected revert'); }, + Result::Err(panic_data) => { + let data = panic_data.span(); + assert(data.len() > 0_usize, 'missing panic data'); + let actual = *data.at(0_usize); + assert(actual == expected, 'unexpected revert reason'); + }, + } +} + #[test] -fn add_get_revoke_session_works() { +fn test_session_add_ok() { let (contract_address, dispatcher) = deploy_account(); start_cheat_caller_address(contract_address, contract_address); @@ -95,3 +218,178 @@ fn events_emitted() { ], ); } + +#[test] +fn test_session_expired_rejects() { + let (account_address, _, mock_address) = deploy_account_and_mock(); + + let valid_after = 0_u64; + let valid_until = 100_u64; + let policy = SessionPolicy { + is_active: true, + valid_after, + valid_until, + max_calls: 3_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let session_pubkey = session_key(); + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_SELECTOR]; + add_session_allowlist(account_address, session_pubkey, policy, targets, selectors); + + start_cheat_block_timestamp(account_address, 1_000_u64); + + let to: ContractAddress = account_address; + let amount = u256 { low: 100_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let signature: Array = build_session_signature( + account_address, + session_pubkey, + 0_u128, + valid_until, + @calls, + ); + + let result = execute_session_call(account_address, @calls, @signature); + + stop_cheat_block_timestamp(account_address); + + assert_reverted_with(result, ERR_SESSION_EXPIRED); +} + +#[test] +fn test_session_selector_denied() { + let (account_address, _, mock_address) = deploy_account_and_mock(); + + let valid_after = 0_u64; + let valid_until = 5_000_u64; + let policy = SessionPolicy { + is_active: true, + valid_after, + valid_until, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let session_pubkey = session_key(); + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_SELECTOR]; + add_session_allowlist(account_address, session_pubkey, policy, targets, selectors); + + start_cheat_block_timestamp(account_address, 1_000_u64); + + let mut calldata = array![]; + let selector = starknet::selector!("get_last"); + let call = Call { to: mock_address, selector, calldata: calldata.span() }; + let calls = array![call]; + + let signature: Array = build_session_signature( + account_address, + session_pubkey, + 0_u128, + valid_until, + @calls, + ); + + let result = execute_session_call(account_address, @calls, @signature); + + stop_cheat_block_timestamp(account_address); + + assert_reverted_with(result, ERR_POLICY_SELECTOR_DENIED); +} + +#[test] +fn test_session_target_denied() { + let (account_address, _, mock_address) = deploy_account_and_mock(); + + let valid_after = 0_u64; + let valid_until = 5_000_u64; + let policy = SessionPolicy { + is_active: true, + valid_after, + valid_until, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let session_pubkey = session_key(); + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_SELECTOR]; + add_session_allowlist(account_address, session_pubkey, policy, targets, selectors); + + start_cheat_block_timestamp(account_address, 1_000_u64); + + let mut calldata = array![]; + let call = Call { + to: account_address, + selector: starknet::selector!("get_owner"), + calldata: calldata.span(), + }; + let calls = array![call]; + + let signature: Array = build_session_signature( + account_address, + session_pubkey, + 0_u128, + valid_until, + @calls, + ); + + let result = execute_session_call(account_address, @calls, @signature); + + stop_cheat_block_timestamp(account_address); + + assert_reverted_with(result, ERR_POLICY_TARGET_DENIED); +} + +#[test] +fn test_session_value_cap() { + let (account_address, _, mock_address) = deploy_account_and_mock(); + + let valid_after = 0_u64; + let valid_until = 5_000_u64; + let policy = SessionPolicy { + is_active: true, + valid_after, + valid_until, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let session_pubkey = session_key(); + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_SELECTOR]; + add_session_allowlist(account_address, session_pubkey, policy, targets, selectors); + + start_cheat_block_timestamp(account_address, 1_000_u64); + + let to: ContractAddress = account_address; + let amount = u256 { low: 5_000_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let signature: Array = build_session_signature( + account_address, + session_pubkey, + 0_u128, + valid_until, + @calls, + ); + + let result = execute_session_call(account_address, @calls, @signature); + + stop_cheat_block_timestamp(account_address); + + assert_reverted_with(result, ERR_VALUE_LIMIT_EXCEEDED); +} From 3d09c110c7b127edd826b7440075ec5fe0f3190b Mon Sep 17 00:00:00 2001 From: hooftly Date: Tue, 14 Oct 2025 05:17:02 -0600 Subject: [PATCH 07/16] feat(sessions): ERC-20 transferFrom gating and value-cap helper - Add transferFrom support and a reusable value-cap helper so sessions gate ERC-20 spends; assert session creation invariants via new error constants - Expand mock ERC-20 and session tests to cover transferFrom flows, empty allowlists, not-ready windows, and cross-session signature reuse failures - Document allowlist defaults, recommended limits, additional error identifiers, and clarify native value transfers are out of scope for v0 value caps --- docs/interfaces.md | 19 ++- docs/rfc-ua2-sdk.md | 6 +- docs/validation.md | 3 +- packages/contracts/src/errors.cairo | 2 + packages/contracts/src/mock_erc20.cairo | 14 ++ packages/contracts/src/ua2_account.cairo | 79 ++++----- ...st_session_nonce_replay_and_mismatch.cairo | 51 ++++++ .../tests/test_validate_allowlists.cairo | 153 ++++++++++++++++++ .../tests/test_validate_denials.cairo | 139 +++++++++++++++- 9 files changed, 423 insertions(+), 43 deletions(-) diff --git a/docs/interfaces.md b/docs/interfaces.md index a086dd3..9fc37a4 100644 --- a/docs/interfaces.md +++ b/docs/interfaces.md @@ -36,6 +36,10 @@ struct SessionPolicy { The selector and target allowlists are not embedded in the struct. They are stored in dedicated `LegacyMap` slots keyed by `(key_hash, target)` and `(key_hash, selector)` respectively, and are populated by calling `add_session_with_allowlists` alongside the base policy. +> **Notes:** +> * Supplying empty target and selector lists is permitted, but such a session cannot execute any calls (all lookups fall back to `false`). +> * To keep storage writes + gas predictable in v0, prefer allowlists with ≲32 entries per list. + --- ## Events @@ -62,15 +66,25 @@ The contract reverts with the following identifiers: * `ERR_SESSION_EXPIRED` * `ERR_SESSION_INACTIVE` +* `ERR_SESSION_STALE` +* `ERR_SESSION_NOT_READY` +* `ERR_SESSION_TARGETS_LEN` +* `ERR_SESSION_SELECTORS_LEN` * `ERR_POLICY_CALLCAP` -* `ERR_POLICY_SELECTOR_DENIED` * `ERR_POLICY_TARGET_DENIED` -* `ERR_VALUE_LIMIT_EXCEEDED` +* `ERR_POLICY_SELECTOR_DENIED` * `ERR_POLICY_CALLCOUNT_MISMATCH` +* `ERR_VALUE_LIMIT_EXCEEDED` * `ERR_BAD_SESSION_NONCE` * `ERR_SESSION_SIG_INVALID` +* `ERR_BAD_VALID_WINDOW` +* `ERR_BAD_MAX_CALLS` +* `ERR_SIGNATURE_MISSING` +* `ERR_OWNER_SIG_INVALID` +* `ERR_GUARDIAN_SIG_INVALID` * `ERR_GUARDIAN_EXISTS` * `ERR_NOT_GUARDIAN` +* `ERR_GUARDIAN_CALL_DENIED` * `ERR_BAD_THRESHOLD` * `ERR_RECOVERY_IN_PROGRESS` * `ERR_NO_RECOVERY` @@ -78,6 +92,7 @@ The contract reverts with the following identifiers: * `ERR_ALREADY_CONFIRMED` * `ERR_BEFORE_ETA` * `ERR_NOT_ENOUGH_CONFIRMS` +* `ERR_NOT_OWNER` * `ERR_ZERO_OWNER` * `ERR_SAME_OWNER` diff --git a/docs/rfc-ua2-sdk.md b/docs/rfc-ua2-sdk.md index 7c1437c..ade75b0 100644 --- a/docs/rfc-ua2-sdk.md +++ b/docs/rfc-ua2-sdk.md @@ -149,11 +149,11 @@ struct SessionPolicy { valid_until: u64, // block timestamp max_calls: u32, calls_used: u32, - max_value_per_call: Uint256, // wei-like units for token/native + max_value_per_call: Uint256, // wei-like units for ERC-20 transfers (native send unsupported in v0) } ``` -Selector and target allowlists are stored separately under `sessionTargetAllow(session_key_hash, ContractAddress)` and `sessionSelectorAllow(session_key_hash, felt252)` legacy maps. The owner typically calls `add_session_with_allowlists` to write the base policy and seed those maps in a single transaction. +Selector and target allowlists are stored separately under `sessionTargetAllow(session_key_hash, ContractAddress)` and `sessionSelectorAllow(session_key_hash, felt252)` legacy maps. The owner typically calls `add_session_with_allowlists` to write the base policy and seed those maps in a single transaction. Empty allowlists are technically valid but render the session unusable, and we recommend keeping each list ≤32 entries in v0 to avoid excessive storage writes. **Validation path:** @@ -162,7 +162,7 @@ Selector and target allowlists are stored separately under `sessionTargetAllow(s * Check `is_active`, `now >= valid_after`, `now <= valid_until`, and `calls_used + tx_call_count <= max_calls`. * Require allowlist booleans for `(key_hash, target)` and `(key_hash, selector)` to be `true`. - * Enforce ERC-20 transfer amounts ≤ `max_value_per_call`. + * Enforce ERC-20 `transfer` / `transferFrom` amounts ≤ `max_value_per_call` (native `call.value` transfers are out-of-scope for v0). * Require session nonce match, then verify the ECDSA signature over the poseidon-hashed call set. * Call `apply_session_usage` to bump counters/nonce and emit `SessionUsed` + `SessionNonceAdvanced`. diff --git a/docs/validation.md b/docs/validation.md index 5dabe69..97ab6b5 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -17,7 +17,8 @@ 7. For each call in `tx.multicall`: - Assert target allowlist entry is `true` (`ERR_POLICY_TARGET_DENIED`). - Assert selector allowlist entry is `true` (`ERR_POLICY_SELECTOR_DENIED`). - - If selector == `ERC20::transfer`, ensure amount ≤ `max_value_per_call` (`ERR_VALUE_LIMIT_EXCEEDED`). + - If selector == `ERC20::transfer` **or** `ERC20::transferFrom`, ensure amount ≤ `max_value_per_call` (`ERR_VALUE_LIMIT_EXCEEDED`). + - Direct native-token `call.value` transfers are out-of-scope in v0; stick to ERC-20 flows when using value caps. 8. Require provided session nonce == stored nonce (`ERR_BAD_SESSION_NONCE`). 9. Verify ECDSA signature against the computed session message (`ERR_SESSION_SIG_INVALID`). - Message binds `{chainId, accountAddress, sessionPubkey, callHash, validUntil, nonce}` to prevent cross-channel replay. diff --git a/packages/contracts/src/errors.cairo b/packages/contracts/src/errors.cairo index f5efebb..8e70a93 100644 --- a/packages/contracts/src/errors.cairo +++ b/packages/contracts/src/errors.cairo @@ -11,6 +11,8 @@ pub const ERR_SESSION_SELECTORS_LEN: felt252 = 'ERR_SESSION_SELECTORS_LEN'; pub const ERR_POLICY_CALLCOUNT_MISMATCH: felt252 = 'ERR_POLICY_CALLCOUNT_MISMATCH'; pub const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; pub const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; +pub const ERR_BAD_VALID_WINDOW: felt252 = 'ERR_BAD_VALID_WINDOW'; +pub const ERR_BAD_MAX_CALLS: felt252 = 'ERR_BAD_MAX_CALLS'; pub const ERR_GUARDIAN_EXISTS: felt252 = 'ERR_GUARDIAN_EXISTS'; pub const ERR_NOT_GUARDIAN: felt252 = 'ERR_NOT_GUARDIAN'; pub const ERR_BAD_THRESHOLD: felt252 = 'ERR_BAD_THRESHOLD'; diff --git a/packages/contracts/src/mock_erc20.cairo b/packages/contracts/src/mock_erc20.cairo index 095f6b7..4f5b448 100644 --- a/packages/contracts/src/mock_erc20.cairo +++ b/packages/contracts/src/mock_erc20.cairo @@ -7,6 +7,7 @@ pub mod MockERC20 { #[storage] pub struct Storage { + last_from: ContractAddress, last_to: ContractAddress, last_amount: u256, } @@ -18,6 +19,19 @@ pub mod MockERC20 { true } + #[external(v0)] + fn transferFrom( + ref self: ContractState, + from: ContractAddress, + to: ContractAddress, + amount: u256, + ) -> bool { + self.last_from.write(from); + self.last_to.write(to); + self.last_amount.write(amount); + true + } + #[external(v0)] fn get_last(self: @ContractState) -> (ContractAddress, u256) { (self.last_to.read(), self.last_amount.read()) diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 9d944db..6591390 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -21,10 +21,10 @@ pub mod UA2Account { get_execution_info, }; use crate::errors::{ - ERR_ALREADY_CONFIRMED, ERR_BAD_SESSION_NONCE, ERR_BAD_THRESHOLD, ERR_BEFORE_ETA, - ERR_GUARDIAN_CALL_DENIED, ERR_GUARDIAN_EXISTS, ERR_GUARDIAN_SIG_INVALID, - ERR_NOT_ENOUGH_CONFIRMS, ERR_NOT_GUARDIAN, ERR_NOT_OWNER, ERR_NO_RECOVERY, - ERR_OWNER_SIG_INVALID, ERR_POLICY_CALLCAP, ERR_POLICY_CALLCOUNT_MISMATCH, + ERR_ALREADY_CONFIRMED, ERR_BAD_MAX_CALLS, ERR_BAD_SESSION_NONCE, ERR_BAD_THRESHOLD, + ERR_BAD_VALID_WINDOW, ERR_BEFORE_ETA, ERR_GUARDIAN_CALL_DENIED, ERR_GUARDIAN_EXISTS, + ERR_GUARDIAN_SIG_INVALID, ERR_NOT_ENOUGH_CONFIRMS, ERR_NOT_GUARDIAN, ERR_NOT_OWNER, + ERR_NO_RECOVERY, ERR_OWNER_SIG_INVALID, ERR_POLICY_CALLCAP, ERR_POLICY_CALLCOUNT_MISMATCH, ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_RECOVERY_IN_PROGRESS, ERR_RECOVERY_MISMATCH, ERR_SAME_OWNER, ERR_SESSION_EXPIRED, ERR_SESSION_INACTIVE, ERR_SESSION_NOT_READY, ERR_SESSION_SELECTORS_LEN, ERR_SESSION_SIG_INVALID, @@ -39,6 +39,7 @@ pub mod UA2Account { const ERC20_TRANSFER_SEL: felt252 = 0x83afd3f4caedc6eebf44246fe54e38c95e3179a5ec9ea81740eca5b482d12e; + const ERC20_TRANSFER_FROM_SEL: felt252 = starknet::selector!("transferFrom"); const APPLY_SESSION_USAGE_SELECTOR: felt252 = starknet::selector!("apply_session_usage"); const PROPOSE_RECOVERY_SELECTOR: felt252 = starknet::selector!("propose_recovery"); const CONFIRM_RECOVERY_SELECTOR: felt252 = starknet::selector!("confirm_recovery"); @@ -546,8 +547,8 @@ pub mod UA2Account { fn add_session(ref self: ContractState, key: felt252, mut policy: SessionPolicy) { assert_owner(); - assert(policy.valid_until > policy.valid_after, 'BAD_VALID_WINDOW'); - assert(policy.max_calls > 0_u32, 'BAD_MAX_CALLS'); + assert(policy.valid_until > policy.valid_after, ERR_BAD_VALID_WINDOW); + assert(policy.max_calls > 0_u32, ERR_BAD_MAX_CALLS); let key_hash = derive_key_hash(key); @@ -608,6 +609,39 @@ pub mod UA2Account { sum } + fn enforce_value_limit(calldata: Span, amount_index: usize, limit: u256) { + let required_len = amount_index + 2_usize; + let calldata_len = calldata.len(); + require(calldata_len >= required_len, ERR_VALUE_LIMIT_EXCEEDED); + + let amount_low_felt = *calldata.at(amount_index); + let amount_high_felt = *calldata.at(amount_index + 1_usize); + + let amount_low: u128 = match amount_low_felt.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, ERR_VALUE_LIMIT_EXCEEDED); + 0_u128 + }, + }; + + let amount_high: u128 = match amount_high_felt.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, ERR_VALUE_LIMIT_EXCEEDED); + 0_u128 + }, + }; + + let amount = u256 { low: amount_low, high: amount_high }; + + if amount.high > limit.high { + assert(false, ERR_VALUE_LIMIT_EXCEEDED); + } else if amount.high == limit.high { + assert(amount.low <= limit.low, ERR_VALUE_LIMIT_EXCEEDED); + } + } + fn poseidon_chain(acc: felt252, value: felt252) -> felt252 { let mut values = array![acc, value]; poseidon_hash_span(values.span()) @@ -792,36 +826,9 @@ pub mod UA2Account { assert(selector_allowed == true, ERR_POLICY_SELECTOR_DENIED); if selector == ERC20_TRANSFER_SEL { - let calldata_len = calldata.len(); - require(calldata_len >= 3_usize, ERR_VALUE_LIMIT_EXCEEDED); - - let amount_low_felt = *calldata.at(1_usize); - let amount_high_felt = *calldata.at(2_usize); - - let amount_low: u128 = match amount_low_felt.try_into() { - Option::Some(value) => value, - Option::None(_) => { - assert(false, ERR_VALUE_LIMIT_EXCEEDED); - 0_u128 - }, - }; - - let amount_high: u128 = match amount_high_felt.try_into() { - Option::Some(value) => value, - Option::None(_) => { - assert(false, ERR_VALUE_LIMIT_EXCEEDED); - 0_u128 - }, - }; - - let amount = u256 { low: amount_low, high: amount_high }; - let limit = policy.max_value_per_call; - - if amount.high > limit.high { - assert(false, ERR_VALUE_LIMIT_EXCEEDED); - } else if amount.high == limit.high { - assert(amount.low <= limit.low, ERR_VALUE_LIMIT_EXCEEDED); - } + enforce_value_limit(calldata, 1_usize, policy.max_value_per_call); + } else if selector == ERC20_TRANSFER_FROM_SEL { + enforce_value_limit(calldata, 2_usize, policy.max_value_per_call); } } diff --git a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo index 5ee3ec3..26f42ff 100644 --- a/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo +++ b/packages/contracts/tests/test_session_nonce_replay_and_mismatch.cairo @@ -18,6 +18,7 @@ use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); +const ALT_SESSION_PUBKEY: felt252 = 0xABCDEF12345; fn deploy_account_and_mock() -> (ContractAddress, ContractAddress) { let account_declare = declare("UA2Account").unwrap(); @@ -165,3 +166,53 @@ fn test_session_nonce_replay_and_mismatch() { stop_cheat_block_timestamp(account_address); } + +#[test] +fn cross_session_signature_reuse_fails() { + let (account_address, mock_address) = deploy_account_and_mock(); + + let policy = SessionPolicy { + is_active: true, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let session_pubkey_a = session_key(); + add_session(account_address, session_pubkey_a, mock_address, policy); + add_session(account_address, ALT_SESSION_PUBKEY, mock_address, policy); + + start_cheat_block_timestamp(account_address, 5_000_u64); + + let to: ContractAddress = account_address; + let amount = u256 { low: 1_000_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let signature_a: Array = build_session_signature( + account_address, + session_pubkey_a, + 0_u128, + policy.valid_until, + @calls, + ); + + let mut swapped_signature = ArrayTrait::::new(); + let mut index = 0_usize; + for felt_ref in signature_a.span() { + if index == 1_usize { + swapped_signature.append(ALT_SESSION_PUBKEY); + } else { + swapped_signature.append(*felt_ref); + } + index += 1_usize; + } + + let result = execute_with_signature(account_address, @calls, @swapped_signature); + assert_reverted_with(result, ERR_SESSION_SIG_INVALID); + + stop_cheat_block_timestamp(account_address); +} diff --git a/packages/contracts/tests/test_validate_allowlists.cairo b/packages/contracts/tests/test_validate_allowlists.cairo index 74627c2..f9e4d0c 100644 --- a/packages/contracts/tests/test_validate_allowlists.cairo +++ b/packages/contracts/tests/test_validate_allowlists.cairo @@ -20,6 +20,31 @@ use crate::session_test_utils::{build_session_signature, session_key, session_ke const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); +const TRANSFER_FROM_SELECTOR: felt252 = starknet::selector!("transferFrom"); + +fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amount: u256) -> Call { + let mut calldata = array![]; + calldata.append(to.into()); + calldata.append(amount.low.into()); + calldata.append(amount.high.into()); + + Call { to: mock_address, selector: TRANSFER_SELECTOR, calldata: calldata.span() } +} + +fn build_transfer_from_call( + mock_address: ContractAddress, + from: ContractAddress, + to: ContractAddress, + amount: u256, +) -> Call { + let mut calldata = array![]; + calldata.append(from.into()); + calldata.append(to.into()); + calldata.append(amount.low.into()); + calldata.append(amount.high.into()); + + Call { to: mock_address, selector: TRANSFER_FROM_SELECTOR, calldata: calldata.span() } +} #[test] fn session_allows_whitelisted_calls() { @@ -161,3 +186,131 @@ fn session_allows_whitelisted_calls() { assert(recorded_to == to, 'incorrect transfer recipient'); assert(recorded_amount == amount, 'incorrect transfer amount'); } + +#[test] +fn session_allows_transfer_from_calls() { + let account_declare = declare("UA2Account").unwrap(); + let account_class = account_declare.contract_class(); + let (account_address, _) = account_class.deploy(@array![OWNER_PUBKEY]).unwrap_syscall(); + + let mock_declare = declare("MockERC20").unwrap(); + let mock_class = mock_declare.contract_class(); + let (mock_address, _) = mock_class.deploy(@array![]).unwrap_syscall(); + + let valid_after = 0_u64; + let valid_until = 10_000_u64; + + let policy = SessionPolicy { + is_active: true, + valid_after, + valid_until, + max_calls: 1_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + start_cheat_block_timestamp(account_address, 5_000_u64); + + start_cheat_caller_address(account_address, account_address); + let session_pubkey = session_key(); + + let mut targets: Array = array![]; + targets.append(mock_address); + let mut selectors: Array = array![]; + selectors.append(TRANSFER_FROM_SELECTOR); + + let session = Session { + pubkey: session_pubkey, + valid_after, + valid_until, + max_calls: policy.max_calls, + value_cap: policy.max_value_per_call, + targets_len: 1_u32, + targets, + selectors_len: 1_u32, + selectors, + }; + + let mut allowlist_calldata = array![]; + Serde::::serialize(@session, ref allowlist_calldata); + + call_contract_syscall( + account_address, + starknet::selector!("add_session_with_allowlists"), + allowlist_calldata.span(), + ) + .unwrap_syscall(); + stop_cheat_caller_address(account_address); + + let amount = u256 { low: 600_u128, high: 0_u128 }; + let from: ContractAddress = account_address; + let to: ContractAddress = 0x555.try_into().unwrap(); + + let call = build_transfer_from_call(mock_address, from, to, amount); + let calls = array![call]; + + let zero_contract: ContractAddress = 0.try_into().unwrap(); + start_cheat_caller_address(account_address, zero_contract); + let signature: Array = build_session_signature( + account_address, + session_pubkey, + 0_u128, + policy.valid_until, + @calls, + ); + start_cheat_signature(account_address, signature.span()); + + let mut execute_calldata = array![]; + Serde::>::serialize(@calls, ref execute_calldata); + call_contract_syscall( + account_address, + starknet::selector!("__execute__"), + execute_calldata.span(), + ) + .unwrap_syscall(); + + stop_cheat_signature(account_address); + stop_cheat_caller_address(account_address); + + let get_last_result = call_contract_syscall( + mock_address, + starknet::selector!("get_last"), + array![].span(), + ) + .unwrap_syscall(); + + let recorded_to_felt = *get_last_result.at(0); + let recorded_low_felt = *get_last_result.at(1); + let recorded_high_felt = *get_last_result.at(2); + + let recorded_to: ContractAddress = match recorded_to_felt.try_into() { + Option::Some(addr) => addr, + Option::None(_) => { + assert(false, 'invalid recorded address'); + 0.try_into().unwrap() + }, + }; + + let recorded_amount = u256 { + low: match recorded_low_felt.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'invalid amount low'); + 0.try_into().unwrap() + }, + }, + high: match recorded_high_felt.try_into() { + Option::Some(value) => value, + Option::None(_) => { + assert(false, 'invalid amount high'); + 0.try_into().unwrap() + }, + }, + }; + + assert(recorded_to == to, 'incorrect transfer recipient'); + assert(recorded_amount == amount, 'incorrect transfer amount'); + + stop_cheat_block_timestamp(account_address); +} diff --git a/packages/contracts/tests/test_validate_denials.cairo b/packages/contracts/tests/test_validate_denials.cairo index 4f5edbf..1b6adc2 100644 --- a/packages/contracts/tests/test_validate_denials.cairo +++ b/packages/contracts/tests/test_validate_denials.cairo @@ -14,7 +14,7 @@ use starknet::syscalls::call_contract_syscall; use starknet::{ContractAddress, SyscallResult, SyscallResultTrait}; use ua2_contracts::errors::{ ERR_POLICY_CALLCAP, ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_SESSION_EXPIRED, - ERR_SESSION_SELECTORS_LEN, ERR_SESSION_TARGETS_LEN, ERR_VALUE_LIMIT_EXCEEDED, + ERR_SESSION_NOT_READY, ERR_SESSION_SELECTORS_LEN, ERR_SESSION_TARGETS_LEN, ERR_VALUE_LIMIT_EXCEEDED, }; use ua2_contracts::session::Session; use ua2_contracts::ua2_account::UA2Account::SessionPolicy; @@ -22,6 +22,7 @@ use crate::session_test_utils::{build_session_signature, session_key}; const OWNER_PUBKEY: felt252 = 0x12345; const TRANSFER_SELECTOR: felt252 = starknet::selector!("transfer"); +const TRANSFER_FROM_SELECTOR: felt252 = starknet::selector!("transferFrom"); fn deploy_account_and_mock() -> (ContractAddress, ContractAddress) { let account_declare = declare("UA2Account").unwrap(); @@ -103,6 +104,21 @@ fn build_transfer_call(mock_address: ContractAddress, to: ContractAddress, amoun Call { to: mock_address, selector: TRANSFER_SELECTOR, calldata: calldata.span() } } +fn build_transfer_from_call( + mock_address: ContractAddress, + from: ContractAddress, + to: ContractAddress, + amount: u256, +) -> Call { + let mut calldata = array![]; + calldata.append(from.into()); + calldata.append(to.into()); + calldata.append(amount.low.into()); + calldata.append(amount.high.into()); + + Call { to: mock_address, selector: TRANSFER_FROM_SELECTOR, calldata: calldata.span() } +} + fn execute_session_calls( account_address: ContractAddress, calls: @Array, @@ -294,6 +310,46 @@ fn denies_target_not_allowed() { stop_cheat_block_timestamp(account_address); } +#[test] +fn empty_allowlists_reject_calls() { + let (account_address, mock_address) = deploy_account_and_mock(); + + let policy = SessionPolicy { + is_active: true, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let targets = array![]; + let selectors = array![]; + + let session_pubkey = session_key(); + add_session_with_lists(account_address, session_pubkey, policy, @targets, @selectors); + + start_cheat_block_timestamp(account_address, 5_000_u64); + + let to: ContractAddress = account_address; + let amount = u256 { low: 1_000_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let result = execute_session_calls( + account_address, + @calls, + 0_u128, + session_pubkey, + policy.valid_until, + ); + + assert_reverted_with(result, ERR_POLICY_TARGET_DENIED); + + stop_cheat_block_timestamp(account_address); +} + #[test] fn denies_expired_session() { let (account_address, mock_address) = deploy_account_and_mock(); @@ -330,6 +386,46 @@ fn denies_expired_session() { stop_cheat_block_timestamp(account_address); } +#[test] +fn denies_session_not_ready() { + let (account_address, mock_address) = deploy_account_and_mock(); + + let policy = SessionPolicy { + is_active: true, + valid_after: 6_000_u64, + valid_until: 12_000_u64, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 10_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_SELECTOR]; + + let session_pubkey = session_key(); + add_session_with_lists(account_address, session_pubkey, policy, @targets, @selectors); + + start_cheat_block_timestamp(account_address, 5_000_u64); + + let to: ContractAddress = account_address; + let amount = u256 { low: 1_000_u128, high: 0_u128 }; + let call = build_transfer_call(mock_address, to, amount); + let calls = array![call]; + + let result = execute_session_calls( + account_address, + @calls, + 0_u128, + session_pubkey, + policy.valid_until, + ); + + assert_reverted_with(result, ERR_SESSION_NOT_READY); + + stop_cheat_block_timestamp(account_address); +} + #[test] fn denies_over_call_cap() { let (account_address, mock_address) = deploy_account_and_mock(); @@ -403,3 +499,44 @@ fn denies_over_value_cap() { stop_cheat_block_timestamp(account_address); } +#[test] +fn denies_transfer_from_over_value_cap() { + let (account_address, mock_address) = deploy_account_and_mock(); + + let policy = SessionPolicy { + is_active: true, + valid_after: 0_u64, + valid_until: 10_000_u64, + max_calls: 5_u32, + calls_used: 0_u32, + max_value_per_call: u256 { low: 1_000_u128, high: 0_u128 }, + owner_epoch: 0_u64, + }; + + let mut targets = array![mock_address]; + let mut selectors = array![TRANSFER_FROM_SELECTOR]; + + let session_pubkey = session_key(); + add_session_with_lists(account_address, session_pubkey, policy, @targets, @selectors); + + start_cheat_block_timestamp(account_address, 5_000_u64); + + let from: ContractAddress = account_address; + let to: ContractAddress = account_address; + let amount = u256 { low: 5_000_u128, high: 0_u128 }; + let call = build_transfer_from_call(mock_address, from, to, amount); + let calls = array![call]; + + let result = execute_session_calls( + account_address, + @calls, + 0_u128, + session_pubkey, + policy.valid_until, + ); + + assert_reverted_with(result, ERR_VALUE_LIMIT_EXCEEDED); + + stop_cheat_block_timestamp(account_address); +} + From c9e0c8b4f05ea1682e450215bdc9ac24d312bf7a Mon Sep 17 00:00:00 2001 From: hooftly Date: Tue, 14 Oct 2025 06:02:45 -0600 Subject: [PATCH 08/16] feat(auth): enforce single mode & recovery gating; sncast E2E - Add ERR_UNSUPPORTED_AUTH_MODE - Expand UA2 account validation to gate recovery flows, ensure single auth-mode selection, and permit cancel-recovery selectors during guardian checks - Introduce __validate__ helpers: recovery-only call detection and nonce-safe mode counting; return OZ validation for owner paths after session/guardian enforcement - Replace devnet E2E runner with a sncast-backed workflow - Align policy typing across scripts and app to validAfter/validUntil semantics in automation and UI --- packages/contracts/src/errors.cairo | 1 + packages/contracts/src/ua2_account.cairo | 63 ++++- packages/example/scripts/e2e-devnet.ts | 315 +++++++++++------------ packages/example/scripts/e2e-sepolia.ts | 6 +- packages/example/src/App.tsx | 7 +- 5 files changed, 215 insertions(+), 177 deletions(-) diff --git a/packages/contracts/src/errors.cairo b/packages/contracts/src/errors.cairo index 8e70a93..98b795d 100644 --- a/packages/contracts/src/errors.cairo +++ b/packages/contracts/src/errors.cairo @@ -13,6 +13,7 @@ pub const ERR_BAD_SESSION_NONCE: felt252 = 'ERR_BAD_SESSION_NONCE'; pub const ERR_SESSION_SIG_INVALID: felt252 = 'ERR_SESSION_SIG_INVALID'; pub const ERR_BAD_VALID_WINDOW: felt252 = 'ERR_BAD_VALID_WINDOW'; pub const ERR_BAD_MAX_CALLS: felt252 = 'ERR_BAD_MAX_CALLS'; +pub const ERR_UNSUPPORTED_AUTH_MODE: felt252 = 'ERR_UNSUPPORTED_AUTH_MODE'; pub const ERR_GUARDIAN_EXISTS: felt252 = 'ERR_GUARDIAN_EXISTS'; pub const ERR_NOT_GUARDIAN: felt252 = 'ERR_NOT_GUARDIAN'; pub const ERR_BAD_THRESHOLD: felt252 = 'ERR_BAD_THRESHOLD'; diff --git a/packages/contracts/src/ua2_account.cairo b/packages/contracts/src/ua2_account.cairo index 6591390..d72af76 100644 --- a/packages/contracts/src/ua2_account.cairo +++ b/packages/contracts/src/ua2_account.cairo @@ -28,8 +28,8 @@ pub mod UA2Account { ERR_POLICY_SELECTOR_DENIED, ERR_POLICY_TARGET_DENIED, ERR_RECOVERY_IN_PROGRESS, ERR_RECOVERY_MISMATCH, ERR_SAME_OWNER, ERR_SESSION_EXPIRED, ERR_SESSION_INACTIVE, ERR_SESSION_NOT_READY, ERR_SESSION_SELECTORS_LEN, ERR_SESSION_SIG_INVALID, - ERR_SESSION_STALE, ERR_SESSION_TARGETS_LEN, ERR_SIGNATURE_MISSING, ERR_VALUE_LIMIT_EXCEEDED, - ERR_ZERO_OWNER, + ERR_SESSION_STALE, ERR_SESSION_TARGETS_LEN, ERR_SIGNATURE_MISSING, + ERR_UNSUPPORTED_AUTH_MODE, ERR_VALUE_LIMIT_EXCEEDED, ERR_ZERO_OWNER, }; use crate::session::Session; use super::{AccountComponent, SRC5Component}; @@ -44,6 +44,7 @@ pub mod UA2Account { const PROPOSE_RECOVERY_SELECTOR: felt252 = starknet::selector!("propose_recovery"); const CONFIRM_RECOVERY_SELECTOR: felt252 = starknet::selector!("confirm_recovery"); const EXECUTE_RECOVERY_SELECTOR: felt252 = starknet::selector!("execute_recovery"); + const CANCEL_RECOVERY_SELECTOR: felt252 = starknet::selector!("cancel_recovery"); #[storage] pub struct Storage { @@ -683,6 +684,40 @@ pub mod UA2Account { low + high * high_limit } + fn bool_to_u32(value: bool) -> u32 { + if value { + 1_u32 + } else { + 0_u32 + } + } + + fn calls_are_recovery_only(calls: @Array) -> bool { + let calls_len = ArrayTrait::::len(calls); + if calls_len == 0_usize { + return false; + } + + let contract_address = get_contract_address(); + + for call_ref in calls.span() { + let Call { to, selector, calldata: _ } = *call_ref; + + if to != contract_address { + return false; + } + + let allowed = selector == CONFIRM_RECOVERY_SELECTOR + || selector == EXECUTE_RECOVERY_SELECTOR + || selector == CANCEL_RECOVERY_SELECTOR; + if allowed == false { + return false; + } + } + + true + } + fn extract_owner_signature(signature: Span) -> Array { let signature_len = signature.len(); require(signature_len > 0_usize, ERR_SIGNATURE_MISSING); @@ -741,7 +776,8 @@ pub mod UA2Account { let allowed = selector == PROPOSE_RECOVERY_SELECTOR || selector == CONFIRM_RECOVERY_SELECTOR - || selector == EXECUTE_RECOVERY_SELECTOR; + || selector == EXECUTE_RECOVERY_SELECTOR + || selector == CANCEL_RECOVERY_SELECTOR; require(allowed, ERR_GUARDIAN_CALL_DENIED); } } @@ -916,14 +952,29 @@ pub mod UA2Account { let signature_len = signature.len(); require(signature_len > 0_usize, ERR_SIGNATURE_MISSING); - let mode = *signature.at(0_usize); + let recovery_active = self.recovery_active.read(); + if recovery_active { + let allowed = calls_are_recovery_only(@calls); + assert(allowed, ERR_RECOVERY_IN_PROGRESS); + return starknet::VALIDATED; + } - if mode == MODE_SESSION { + let mode_hint = *signature.at(0_usize); + let using_session = mode_hint == MODE_SESSION; + let using_guardian = mode_hint == MODE_GUARDIAN; + let using_owner = mode_hint == MODE_OWNER || (!using_session && !using_guardian); + + let mut modes = bool_to_u32(using_owner); + modes += bool_to_u32(using_session); + modes += bool_to_u32(using_guardian); + require(modes == 1_u32, ERR_UNSUPPORTED_AUTH_MODE); + + if using_session { let _validation = validate_session_policy(self, signature, @calls); return starknet::VALIDATED; } - if mode == MODE_GUARDIAN { + if using_guardian { validate_guardian_authorization(self, signature, @calls); return starknet::VALIDATED; } diff --git a/packages/example/scripts/e2e-devnet.ts b/packages/example/scripts/e2e-devnet.ts index 99a59fa..1ddf61c 100644 --- a/packages/example/scripts/e2e-devnet.ts +++ b/packages/example/scripts/e2e-devnet.ts @@ -1,230 +1,213 @@ -import { limits, makeSessionsManager, type SessionPolicyInput } from '@ua2/core'; -import { Account } from 'starknet'; +import { spawn } from 'node:child_process'; + +import { limits, makeSessionsManager, type CallTransport, type Felt, type SessionPolicyInput } from '@ua2/core'; +import { RpcProvider } from 'starknet'; import { - AccountCallTransport, assertReverted, assertSucceeded, - ensureUa2Deployed, initialSessionUsageState, + loadEnv, + normalizeHex, optionalEnv, readOwner, + requireEnv, selectorFor, - setupToolkit, toFelt, updateSessionUsage, waitForReceipt, type Network, } from './shared.js'; +type SncastConfig = { + profile: string; + account: string; + rpcUrl: string; +}; + +type SncastResult = { + code: number; + stdout: string; + stderr: string; +}; + +class SncastTransport implements CallTransport { + private readonly config: SncastConfig; + public lastTxHash: Felt | null = null; + + constructor(config: SncastConfig) { + this.config = config; + } + + async invoke(address: Felt, entrypoint: string, calldata: Felt[]): Promise<{ txHash: Felt }> { + const args = buildSncastArgs(this.config, address, entrypoint, calldata); + const result = await runSncast(args); + + const parsed = parseSncastJson(result.stdout) ?? parseSncastJson(result.stderr); + if (!parsed || typeof parsed.transaction_hash !== 'string') { + throw new Error( + `sncast invoke failed for ${entrypoint}. exit=${result.code} stdout=${result.stdout} stderr=${result.stderr}` + ); + } + + const txHash = normalizeHex(parsed.transaction_hash); + this.lastTxHash = txHash; + return { txHash }; + } +} + async function main(): Promise { const network: Network = 'devnet'; - const toolkit = await setupToolkit(network); + loadEnv(network); - const attachedAddress = optionalEnv([ + const rpcUrl = requireEnv(['STARKNET_RPC_URL']); + const profile = optionalEnv(['SNCAST_PROFILE', 'UA2_SNCAST_PROFILE'], 'devnet') ?? 'devnet'; + const accountName = requireEnv(['SNCAST_ACCOUNT', 'SNCAST_ACCOUNT_NAME']); + const ua2Address = requireEnv([ `UA2_${network.toUpperCase()}_PROXY_ADDR`, 'UA2_PROXY_ADDR', ]); - const { address: ua2Address } = await ensureUa2Deployed(toolkit, attachedAddress); - const ownerAccount = new Account(toolkit.provider, ua2Address, toolkit.ownerKey); - const ownerTransport = new AccountCallTransport(ownerAccount); - - const ownerBefore = await readOwner(toolkit.provider, ua2Address); - if (ownerBefore.toLowerCase() !== toolkit.ownerPubKey.toLowerCase()) { - console.warn( - `[ua2] Warning: UA² owner on-chain (${ownerBefore}) does not match configured owner pubkey (${toolkit.ownerPubKey}).` - ); - } + const provider = new RpcProvider({ nodeUrl: rpcUrl }); + const chainId = normalizeHex(await provider.getChainId()); - console.log('E2E DEVNET'); - console.log(`- deploy/attach ✓ (${ua2Address})`); + const ownerBefore = await readOwner(provider, ua2Address); + console.log('E2E DEVNET (sncast)'); + console.log(`- profile: ${profile} (account: ${accountName})`); + console.log(`- UA² account: ${ua2Address}`); + console.log(`- on-chain owner: ${ownerBefore}`); + const transport = new SncastTransport({ profile, account: accountName, rpcUrl }); const sessions = makeSessionsManager({ - account: { address: ua2Address, chainId: toolkit.chainId }, - transport: ownerTransport, + account: { address: ua2Address, chainId }, + transport, ua2Address, }); - const expiresAt = Math.floor(Date.now() / 1000) + 2 * 60 * 60; - const sessionTargetValue = - optionalEnv( - [`UA2_${network.toUpperCase()}_SESSION_TARGET`, 'UA2_SESSION_TARGET', 'UA2_E2E_TARGET_ADDR'], - toolkit.guardianAddress - ) ?? toolkit.guardianAddress; - const sessionTarget = toFelt(sessionTargetValue); - const transferSelector = selectorFor('transfer'); - + const now = Math.floor(Date.now() / 1000); const policy: SessionPolicyInput = { - expiresAt, - limits: limits(5, 10n ** 15n), + validAfter: now, + validUntil: now + 10 * 60, + limits: limits(2, 10n ** 15n), allow: { - targets: [sessionTarget], - selectors: [transferSelector], + targets: [toFelt(ua2Address)], + selectors: [selectorFor('apply_session_usage')], }, }; const session = await sessions.create(policy); - if (!ownerTransport.lastTxHash) { - throw new Error('Session creation did not produce a transaction hash.'); + if (!transport.lastTxHash) { + throw new Error('Session creation did not emit a transaction hash.'); } - const createReceipt = await waitForReceipt(toolkit.provider, ownerTransport.lastTxHash, 'session create'); + const createReceipt = await waitForReceipt(provider, transport.lastTxHash, 'session create'); assertSucceeded(createReceipt, 'session create'); console.log(`- create session ✓ (${session.id})`); let usage = initialSessionUsageState(); - - usage = await applySessionUsage(toolkit, ownerAccount, ua2Address, session.id, usage, 1, 'session use #1'); - usage = await applySessionUsage(toolkit, ownerAccount, ua2Address, session.id, usage, 1, 'session use #2'); - usage = await applySessionUsage(toolkit, ownerAccount, ua2Address, session.id, usage, 1, 'session use #3'); - console.log('- in-policy x3 ✓'); - - const violationTx = await ownerAccount.execute({ - contractAddress: ua2Address, - entrypoint: 'apply_session_usage', - calldata: [ - session.id, - toFelt(usage.callsUsed), - toFelt(3), - toFelt(usage.nonce), - ], - }); - const violationReceipt = await waitForReceipt( - toolkit.provider, - violationTx.transaction_hash, - 'policy violation' - ); + usage = await applySessionUsage(transport, provider, ua2Address, session.id, usage, 1, 'session use #1'); + usage = await applySessionUsage(transport, provider, ua2Address, session.id, usage, 1, 'session use #2'); + console.log('- in-policy calls ✓'); + + const violation = await transport.invoke(ua2Address, 'apply_session_usage', [ + session.id, + toFelt(usage.callsUsed), + toFelt(1), + toFelt(usage.nonce), + ]); + const violationReceipt = await waitForReceipt(provider, violation.txHash, 'policy violation'); assertReverted(violationReceipt, 'ERR_POLICY_CALLCAP', 'policy violation'); console.log('- out-of-policy revert ✓ (ERR_POLICY_CALLCAP)'); - const revokeTx = await ownerAccount.execute({ - contractAddress: ua2Address, - entrypoint: 'revoke_session', - calldata: [session.id], - }); - const revokeReceipt = await waitForReceipt(toolkit.provider, revokeTx.transaction_hash, 'session revoke'); + const revokeTx = await transport.invoke(ua2Address, 'revoke_session', [session.id]); + const revokeReceipt = await waitForReceipt(provider, revokeTx.txHash, 'session revoke'); assertSucceeded(revokeReceipt, 'session revoke'); - - const postRevokeTx = await ownerAccount.execute({ - contractAddress: ua2Address, - entrypoint: 'apply_session_usage', - calldata: [ - session.id, - toFelt(usage.callsUsed), - toFelt(1), - toFelt(usage.nonce), - ], - }); - const postRevokeReceipt = await waitForReceipt( - toolkit.provider, - postRevokeTx.transaction_hash, - 'post-revoke session usage' - ); - assertReverted(postRevokeReceipt, 'ERR_SESSION_INACTIVE', 'post revoke session use'); - console.log('- revoke + denied ✓'); - - await runGuardianRecovery(toolkit, ua2Address); - console.log('- guardian recovery ✓'); + console.log('- revoke session ✓'); console.log('E2E DEVNET ✓ complete'); } +type SessionUsageState = ReturnType; + async function applySessionUsage( - toolkit: Awaited>, - owner: Account, - ua2Address: string, - sessionId: string, - state: ReturnType, + transport: SncastTransport, + provider: RpcProvider, + ua2Address: Felt, + sessionId: Felt, + state: SessionUsageState, calls: number, label: string -) { - const tx = await owner.execute({ - contractAddress: ua2Address, - entrypoint: 'apply_session_usage', - calldata: [ - sessionId, - toFelt(state.callsUsed), - toFelt(calls), - toFelt(state.nonce), - ], - }); - const receipt = await waitForReceipt(toolkit.provider, tx.transaction_hash, label); +): Promise { + const tx = await transport.invoke(ua2Address, 'apply_session_usage', [ + sessionId, + toFelt(state.callsUsed), + toFelt(calls), + toFelt(state.nonce), + ]); + const receipt = await waitForReceipt(provider, tx.txHash, label); assertSucceeded(receipt, label); return updateSessionUsage(state, calls); } -async function runGuardianRecovery(toolkit: Awaited>, ua2Address: string) { - const ownerAccount = new Account(toolkit.provider, ua2Address, toolkit.ownerKey); - - await sendAndAwait(toolkit, ownerAccount, ua2Address, 'add_guardian', [toolkit.guardianAddress], 'add guardian', [ - 'ERR_GUARDIAN_EXISTS', - ]); - await sendAndAwait(toolkit, ownerAccount, ua2Address, 'set_guardian_threshold', [toFelt(1)], 'set guardian threshold'); - await sendAndAwait(toolkit, ownerAccount, ua2Address, 'set_recovery_delay', [toFelt(0)], 'set recovery delay'); - - const guardianPropose = await toolkit.guardian.execute({ - contractAddress: ua2Address, - entrypoint: 'propose_recovery', - calldata: [toolkit.guardianPubKey], - }); - const guardianProposeReceipt = await waitForReceipt( - toolkit.provider, - guardianPropose.transaction_hash, - 'guardian propose recovery' - ); - assertSucceeded(guardianProposeReceipt, 'guardian propose recovery'); - - const executeTx = await toolkit.guardian.execute({ - contractAddress: ua2Address, - entrypoint: 'execute_recovery', - calldata: [], - }); - const executeReceipt = await waitForReceipt(toolkit.provider, executeTx.transaction_hash, 'execute recovery'); - assertSucceeded(executeReceipt, 'execute recovery'); - - const ownerAfter = await readOwner(toolkit.provider, ua2Address); - if (ownerAfter.toLowerCase() !== toolkit.guardianPubKey.toLowerCase()) { - throw new Error(`Recovery did not update owner. expected ${toolkit.guardianPubKey}, got ${ownerAfter}`); +function buildSncastArgs( + config: SncastConfig, + address: Felt, + entrypoint: string, + calldata: Felt[] +): string[] { + const args: string[] = ['--profile', config.profile]; + if (config.rpcUrl) { + args.push('--rpc-url', config.rpcUrl); } - - const recoveredOwner = new Account(toolkit.provider, ua2Address, toolkit.guardianKey); - await sendAndAwait( - toolkit, - recoveredOwner, - ua2Address, - 'rotate_owner', - [toolkit.ownerPubKey], - 'rotate owner back' - ); + args.push('invoke', '--account', config.account, '--address', address, '--function', entrypoint); + if (calldata.length > 0) { + args.push('--calldata', ...calldata); + } + args.push('--json'); + return args; } -async function sendAndAwait( - toolkit: Awaited>, - signer: Account, - ua2Address: string, - entrypoint: string, - calldata: readonly string[], - label: string, - ignorableReasons: string[] = [] -): Promise { - const tx = await signer.execute({ - contractAddress: ua2Address, - entrypoint, - calldata: [...calldata], +function runSncast(args: string[]): Promise { + return new Promise((resolve, reject) => { + const child = spawn('sncast', args, { stdio: ['ignore', 'pipe', 'pipe'] }); + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (chunk) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + child.on('error', reject); + child.on('close', (code) => { + resolve({ code: code ?? 0, stdout, stderr }); + }); }); - const receipt = await waitForReceipt(toolkit.provider, tx.transaction_hash, label); - const execution = (receipt?.execution_status ?? '').toString(); - if (execution === 'REVERTED') { - const reason = (receipt?.revert_reason ?? '').toString(); - if (ignorableReasons.some((expected) => reason.includes(expected))) { - return; +} + +function parseSncastJson(output: string): Record | null { + const trimmed = output.trim(); + if (!trimmed) { + return null; + } + const lines = trimmed.split(/\r?\n/).map((line) => line.trim()).filter((line) => line.length > 0); + for (let i = lines.length - 1; i >= 0; i -= 1) { + const line = lines[i]; + try { + return JSON.parse(line) as Record; + } catch (err) { + void err; } - throw new Error(`${label} failed: ${reason || 'unknown revert reason'}`); } - assertSucceeded(receipt, label); + try { + return JSON.parse(trimmed) as Record; + } catch (err) { + void err; + } + return null; } void main().catch((err) => { - console.error('[ua2] E2E devnet failed:', err); + console.error('[ua2] e2e-devnet failed:', err); process.exitCode = 1; }); diff --git a/packages/example/scripts/e2e-sepolia.ts b/packages/example/scripts/e2e-sepolia.ts index 970eec7..599f231 100644 --- a/packages/example/scripts/e2e-sepolia.ts +++ b/packages/example/scripts/e2e-sepolia.ts @@ -48,7 +48,8 @@ async function main(): Promise { ua2Address: normalizedAddress, }); - const expiresAt = Math.floor(Date.now() / 1000) + 2 * 60 * 60; + const validAfter = Math.floor(Date.now() / 1000); + const validUntil = validAfter + 2 * 60 * 60; const sessionTargetValue = optionalEnv( [`UA2_${network.toUpperCase()}_SESSION_TARGET`, 'UA2_SESSION_TARGET', 'UA2_E2E_TARGET_ADDR'], @@ -58,7 +59,8 @@ async function main(): Promise { const transferSelector = selectorFor('transfer'); const policy: SessionPolicyInput = { - expiresAt, + validAfter, + validUntil, limits: limits(5, 10n ** 15n), allow: { targets: [sessionTarget], diff --git a/packages/example/src/App.tsx b/packages/example/src/App.tsx index cbff4ce..fae65c8 100644 --- a/packages/example/src/App.tsx +++ b/packages/example/src/App.tsx @@ -254,9 +254,10 @@ function SessionCreateForm({ create, isReady }: SessionCreateFormProps): JSX.Ele throw new Error('Expiry must be a positive number of minutes.'); } const maxValueBigInt = BigInt(maxValue); - const expiresAt = Math.floor(Date.now() / 1000) + parsedExpires * 60; + const validAfter = Math.floor(Date.now() / 1000); const policy = { - expiresAt, + validAfter, + validUntil: validAfter + parsedExpires * 60, limits: limits(parsedMaxCalls, maxValueBigInt), allow: { targets: target.trim() ? [target.trim()] : [], @@ -428,7 +429,7 @@ function SessionList({ sessions, revoke, refresh, client }: SessionListProps): J
- Expires at: {session.policy.expiresAt} · Max calls: {session.policy.limits.maxCalls} + Expires at: {session.policy.validUntil} · Max calls: {session.policy.limits.maxCalls}