From 89918ffe8837efa5eb9d19e330cfdfa3e264028f Mon Sep 17 00:00:00 2001 From: Darlington02 Date: Wed, 28 Aug 2024 22:25:19 +0100 Subject: [PATCH 01/11] chore: add signatory component --- src/components.cairo | 1 + src/components/account/account.cairo | 43 ++++------- src/components/lockable/lockable.cairo | 5 -- src/components/presets/account_preset.cairo | 38 +++++++++- src/components/signatory.cairo | 1 + src/components/signatory/signatory.cairo | 80 ++++++++++++++++++++ src/components/upgradeable/upgradeable.cairo | 6 -- src/interfaces.cairo | 1 + src/interfaces/IAccount.cairo | 2 - src/interfaces/ISignatory.cairo | 9 +++ 10 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 src/components/signatory.cairo create mode 100644 src/components/signatory/signatory.cairo create mode 100644 src/interfaces/ISignatory.cairo diff --git a/src/components.cairo b/src/components.cairo index 54838ad..18d258c 100644 --- a/src/components.cairo +++ b/src/components.cairo @@ -3,3 +3,4 @@ pub mod lockable; pub mod permissionable; pub mod upgradeable; pub mod presets; +pub mod signatory; \ No newline at end of file diff --git a/src/components/account/account.cairo b/src/components/account/account.cairo index b7a123e..9d676fd 100644 --- a/src/components/account/account.cairo +++ b/src/components/account/account.cairo @@ -91,18 +91,6 @@ pub mod AccountComponent { self._is_valid_signature(hash, signature) } - /// @notice used to validate signer - /// @param signer address to be validated - fn is_valid_signer(self: @ComponentState, signer: ContractAddress) -> bool { - self._is_valid_signer(signer) - } - - fn __validate_declare__( - self: @ComponentState, class_hash: felt252 - ) -> felt252 { - self._validate_transaction() - } - /// @notice gets the NFT owner /// @param token_contract the contract address of the NFT /// @param token_id the token ID of the NFT @@ -170,10 +158,6 @@ pub mod AccountComponent { fn _execute( ref self: ComponentState, mut calls: Array ) -> Array> { - // validate signer - let caller = get_caller_address(); - assert(self._is_valid_signer(caller), Errors::UNAUTHORIZED); - // update state self._update_state(); @@ -225,6 +209,20 @@ pub mod AccountComponent { Serde::::deserialize(ref address).unwrap() } + /// @notice internal function for getting the root NFT owner + /// @param token_contract contract address of NFT + // @param token_id token ID of NFT + // NB: This function aims for compatibility with all contracts (snake or camel case) but do + // not work as expected on mainnet as low level calls do not return err at the moment. + // Should work for contracts which implements CamelCase but not snake_case until starknet + // v0.15. + fn _get_root_owner( + self: @ComponentState, token_contract: ContractAddress, token_id: u256 + ) -> ContractAddress { + // TODO: implement logic to get root owner + 123.try_into().unwrap() + } + /// @notice internal transaction for returning the contract address and token ID of the NFT fn _get_token(self: @ComponentState) -> (ContractAddress, u256, felt252) { let contract = self.account_token_contract.read(); @@ -234,19 +232,6 @@ pub mod AccountComponent { (contract, token_id, chain_id) } - // @notice internal function for validating signer - fn _is_valid_signer( - self: @ComponentState, signer: ContractAddress - ) -> bool { - let owner = self - ._get_owner(self.account_token_contract.read(), self.account_token_id.read()); - if (signer == owner) { - return true; - } else { - return false; - } - } - /// @notice internal function for signature validation fn _is_valid_signature( self: @ComponentState, hash: felt252, signature: Span diff --git a/src/components/lockable/lockable.cairo b/src/components/lockable/lockable.cairo index 2ffda9c..ff05b27 100644 --- a/src/components/lockable/lockable.cairo +++ b/src/components/lockable/lockable.cairo @@ -74,11 +74,6 @@ pub mod LockableComponent { > of ILockable> { fn lock(ref self: ComponentState, lock_until: u64) { let current_timestamp = get_block_timestamp(); - let account_comp = get_dep_component!(@self, Account); - - let is_valid = account_comp._is_valid_signer(get_caller_address()); - assert(is_valid, Errors::UNAUTHORIZED); - assert( lock_until <= current_timestamp + YEAR_DAYS_SECONDS, Errors::EXCEEDS_MAX_LOCK_TIME ); diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index 7a87a86..875b292 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -7,13 +7,15 @@ pub mod AccountPreset { use token_bound_accounts::components::account::account::AccountComponent; use token_bound_accounts::components::upgradeable::upgradeable::UpgradeableComponent; use token_bound_accounts::components::lockable::lockable::LockableComponent; + use token_bound_accounts::components::signatory::signatory::SignatoryComponent; use token_bound_accounts::interfaces::{ - IUpgradeable::IUpgradeable, IExecutable::IExecutable, ILockable::ILockable + IUpgradeable::IUpgradeable, IExecutable::IExecutable, ILockable::ILockable, ISignatory::ISignatory }; component!(path: AccountComponent, storage: account, event: AccountEvent); component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); component!(path: LockableComponent, storage: lockable, event: LockableEvent); + component!(path: SignatoryComponent, storage: signatory, event: SignatoryEvent); // Account #[abi(embed_v0)] @@ -22,6 +24,7 @@ pub mod AccountPreset { impl AccountInternalImpl = AccountComponent::InternalImpl; impl UpgradeableInternalImpl = UpgradeableComponent::Private; impl LockableImpl = LockableComponent::LockableImpl; + impl SignerImpl = SignatoryComponent::Private; // ************************************************************************* // STORAGE @@ -34,6 +37,8 @@ pub mod AccountPreset { upgradeable: UpgradeableComponent::Storage, #[substorage(v0)] lockable: LockableComponent::Storage, + #[substorage(v0)] + signatory: SignatoryComponent::Storage } // ************************************************************************* @@ -47,7 +52,9 @@ pub mod AccountPreset { #[flat] UpgradeableEvent: UpgradeableComponent::Event, #[flat] - LockableEvent: LockableComponent::Event + LockableEvent: LockableComponent::Event, + #[flat] + SignatoryEvent: SignatoryComponent::Event } // ************************************************************************* @@ -58,15 +65,31 @@ pub mod AccountPreset { self.account.initializer(token_contract, token_id); } + // ************************************************************************* + // SIGNATORY IMPL + // ************************************************************************* + #[abi(embed_v0)] + impl Signatory of ISignatory { + fn is_valid_signer(self: @ContractState, signer: ContractAddress) -> bool { + self.signatory._permissioned_signer_validation(signer) + } + } + // ************************************************************************* // EXECUTABLE IMPL // ************************************************************************* #[abi(embed_v0)] impl Executable of IExecutable { fn execute(ref self: ContractState, mut calls: Array) -> Array> { + // validate signer + let caller = get_caller_address(); + assert(self.is_valid_signer(caller), 'Account: unauthorized'); + // cannot make this call when the account is lock let (is_locked, _) = self.lockable.is_locked(); assert(is_locked != true, 'Account: locked'); + + // execute calls self.account._execute(calls) } } @@ -77,9 +100,15 @@ pub mod AccountPreset { #[abi(embed_v0)] impl Upgradeable of IUpgradeable { fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { + // validate signer + let caller = get_caller_address(); + assert(self.is_valid_signer(caller), 'Account: unauthorized'); + // cannot make this call when the account is lock let (is_locked, _) = self.lockable.is_locked(); assert(is_locked != true, 'Account: locked'); + + // upgrade account self.upgradeable._upgrade(new_class_hash); } } @@ -90,6 +119,11 @@ pub mod AccountPreset { #[abi(embed_v0)] impl Lockable of ILockable { fn lock(ref self: ContractState, lock_until: u64) { + // validate signer + let caller = get_caller_address(); + assert(self.is_valid_signer(caller), 'Account: unauthorized'); + + // lock account self.lockable.lock(lock_until); } fn is_locked(self: @ContractState) -> (bool, u64) { diff --git a/src/components/signatory.cairo b/src/components/signatory.cairo new file mode 100644 index 0000000..5c7b7a2 --- /dev/null +++ b/src/components/signatory.cairo @@ -0,0 +1 @@ +pub mod signatory; \ No newline at end of file diff --git a/src/components/signatory/signatory.cairo b/src/components/signatory/signatory.cairo new file mode 100644 index 0000000..cdb53c0 --- /dev/null +++ b/src/components/signatory/signatory.cairo @@ -0,0 +1,80 @@ +// ************************************************************************* +// SIGNATORY COMPONENT +// ************************************************************************* +#[starknet::component] +pub mod SignatoryComponent { + // ************************************************************************* + // IMPORTS + // ************************************************************************* + use starknet::{ + get_caller_address, get_contract_address, ContractAddress + }; + use token_bound_accounts::components::account::account::AccountComponent; + use token_bound_accounts::components::account::account::AccountComponent::InternalImpl; + + // ************************************************************************* + // STORAGE + // ************************************************************************* + #[storage] + pub struct Storage {} + + // ************************************************************************* + // PRIVATE FUNCTIONS + // ************************************************************************* + #[generate_trait] + pub impl Private< + TContractState, + +HasComponent, + +Drop, + impl Account: AccountComponent::HasComponent + > of PrivateTrait { + /// @notice implements a simple signer validation where only NFT owner is a valid signer. + /// @param signer the address to be validated + fn _base_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + + // get owner + let owner = account + ._get_owner(contract_address, token_id); + + // validate + if (signer == owner) { + return true; + } else { + return false; + } + } + + /// @notice implements a signer validation where both NFT owner and the root owner (for nested accounts) are valid signers. + /// @param signer the address to be validated + fn _base_and_root_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + + // get owner + let owner = account + ._get_owner(contract_address, token_id); + // get root owner + let root_owner = account + ._get_root_owner(contract_address, token_id); + + // validate + if (signer == owner) { + return true; + } + else if(signer == root_owner) { + return true; + } + else { + return false; + } + } + + /// @notice implements a more complex signer validation where NFT owner, root owner, and permissioned addresses are valid signers. + /// @param signer the address to be validated + fn _permissioned_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { + true + } + } +} diff --git a/src/components/upgradeable/upgradeable.cairo b/src/components/upgradeable/upgradeable.cairo index acf5056..994e8bd 100644 --- a/src/components/upgradeable/upgradeable.cairo +++ b/src/components/upgradeable/upgradeable.cairo @@ -41,7 +41,6 @@ pub mod UpgradeableComponent { // ************************************************************************* pub mod Errors { pub const INVALID_CLASS: felt252 = 'Class hash cannot be zero'; - pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; } // ************************************************************************* @@ -57,11 +56,6 @@ pub mod UpgradeableComponent { /// @notice replaces the contract's class hash with `new_class_hash`. /// Emits an `Upgraded` event. fn _upgrade(ref self: ComponentState, new_class_hash: ClassHash) { - // validate new signer - let account_comp = get_dep_component!(@self, Account); - let is_valid = account_comp._is_valid_signer(get_caller_address()); - assert(is_valid, Errors::UNAUTHORIZED); - // update state let mut account_comp_mut = get_dep_component_mut!(ref self, Account); account_comp_mut._update_state(); diff --git a/src/interfaces.cairo b/src/interfaces.cairo index 56551f2..de01f5a 100644 --- a/src/interfaces.cairo +++ b/src/interfaces.cairo @@ -4,3 +4,4 @@ pub mod IRegistry; pub mod IUpgradeable; pub mod IExecutable; pub mod ILockable; +pub mod ISignatory; \ No newline at end of file diff --git a/src/interfaces/IAccount.cairo b/src/interfaces/IAccount.cairo index 6f6f7e0..cc31e14 100644 --- a/src/interfaces/IAccount.cairo +++ b/src/interfaces/IAccount.cairo @@ -14,8 +14,6 @@ pub trait IAccount { fn is_valid_signature( self: @TContractState, hash: felt252, signature: Span ) -> felt252; - fn is_valid_signer(self: @TContractState, signer: ContractAddress) -> bool; - fn __validate_declare__(self: @TContractState, class_hash: felt252) -> felt252; fn token(self: @TContractState) -> (ContractAddress, u256, felt252); fn owner(self: @TContractState) -> ContractAddress; fn state(self: @TContractState) -> u256; diff --git a/src/interfaces/ISignatory.cairo b/src/interfaces/ISignatory.cairo new file mode 100644 index 0000000..b41b27c --- /dev/null +++ b/src/interfaces/ISignatory.cairo @@ -0,0 +1,9 @@ +// ************************************************************************* +// SIGNER VALIDATION INTERFACE +// ************************************************************************* +use starknet::ContractAddress; + +#[starknet::interface] +pub trait ISignatory { + fn is_valid_signer(self: @TContractState, signer: ContractAddress) -> bool; +} From 981262fb3a22d2425556962aa523ab6192e0a524 Mon Sep 17 00:00:00 2001 From: Darlington02 Date: Wed, 28 Aug 2024 22:56:28 +0100 Subject: [PATCH 02/11] chore: add permissionable component, update signatory comp. --- src/components.cairo | 2 +- src/components/lockable/lockable.cairo | 4 +- .../permissionable/permissionable.cairo | 111 +++++++++++++++++- src/components/presets/account_preset.cairo | 41 ++++++- src/components/signatory.cairo | 2 +- src/components/signatory/signatory.cairo | 65 ++++++---- src/interfaces.cairo | 3 +- src/interfaces/ILockable.cairo | 3 + src/interfaces/IPermissionable.cairo | 16 +++ tests/test_account_component.cairo | 14 --- 10 files changed, 218 insertions(+), 43 deletions(-) create mode 100644 src/interfaces/IPermissionable.cairo diff --git a/src/components.cairo b/src/components.cairo index 18d258c..7554689 100644 --- a/src/components.cairo +++ b/src/components.cairo @@ -3,4 +3,4 @@ pub mod lockable; pub mod permissionable; pub mod upgradeable; pub mod presets; -pub mod signatory; \ No newline at end of file +pub mod signatory; diff --git a/src/components/lockable/lockable.cairo b/src/components/lockable/lockable.cairo index ff05b27..6fbac1e 100644 --- a/src/components/lockable/lockable.cairo +++ b/src/components/lockable/lockable.cairo @@ -1,4 +1,3 @@ -// lockable component // ************************************************************************* // LOCKABLE COMPONENT // ************************************************************************* @@ -72,6 +71,8 @@ pub mod LockableComponent { +Drop, impl Account: AccountComponent::HasComponent > of ILockable> { + // @notice locks an account + // @param lock_until duration for which account should be locked fn lock(ref self: ComponentState, lock_until: u64) { let current_timestamp = get_block_timestamp(); assert( @@ -98,6 +99,7 @@ pub mod LockableComponent { ); } + // @notice returns the lock status of an account fn is_locked(self: @ComponentState) -> (bool, u64) { let unlock_timestamp = self.lock_until.read(); let current_time = get_block_timestamp(); diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index b111e5e..795a3b1 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -1,3 +1,112 @@ -// permissionable component +// ************************************************************************* +// PERMISSIONABLE COMPONENT +// ************************************************************************* +#[starknet::component] +pub mod PermissionableComponent { + // ************************************************************************* + // IMPORTS + // ************************************************************************* + use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess}; + use starknet::{ContractAddress, get_caller_address, get_block_timestamp}; + use token_bound_accounts::components::account::account::AccountComponent; + use token_bound_accounts::interfaces::IAccount::{IAccount, IAccountDispatcherTrait}; + use token_bound_accounts::components::account::account::AccountComponent::InternalImpl; + use token_bound_accounts::interfaces::IPermissionable::{ + IPermissionable, IPermissionableDispatcher, IPermissionableDispatcherTrait + }; + // ************************************************************************* + // STORAGE + // ************************************************************************* + #[storage] + pub struct Storage { + permissions: Map< + (ContractAddress, ContractAddress), bool + > // <, bool> + } + // ************************************************************************* + // EVENTS + // ************************************************************************* + #[event] + #[derive(Drop, starknet::Event)] + pub enum Event { + PermissionUpdated: PermissionUpdated + } + + // @notice emitted when permissions are updated for an account + // @param owner tokenbound account owner + // @param permissioned_address address to be given/revoked permission + // @param has_permission returns true if user has permission else false + #[derive(Drop, starknet::Event)] + pub struct PermissionUpdated { + #[key] + pub owner: ContractAddress, + pub permissioned_address: ContractAddress, + pub has_permission: bool, + } + + // ************************************************************************* + // ERRORS + // ************************************************************************* + pub mod Errors { + pub const UNAUTHORIZED: felt252 = 'Permission: unauthorized'; + pub const NOT_OWNER: felt252 = 'Permission: not account owner'; + pub const INVALID_LENGTH: felt252 = 'Permission: invalid length'; + pub const NOT_PERMITTED: felt252 = 'Permisson: not permitted'; + } + + + // ************************************************************************* + // EXTERNAL FUNCTIONS + // ************************************************************************* + #[embeddable_as(PermissionableImpl)] + pub impl Permissionable< + TContractState, + +HasComponent, + +Drop, + impl Account: AccountComponent::HasComponent + > of IPermissionable> { + // @notice sets permission for an account + // @permissioned_addresses array of addresses who's permission is to be updated + // @param permssions permission value + fn set_permission( + ref self: ComponentState, + permissioned_addresses: Array, + permissions: Array + ) { + assert(permissioned_addresses.len() == permissions.len(), Errors::INVALID_LENGTH); + + let account_comp = get_dep_component!(@self, Account); + let owner = account_comp.owner(); + let length = permissioned_addresses.len(); + let mut index: u32 = 0; + while index < length { + self + .permissions + .write((owner, *permissioned_addresses[index]), *permissions[index]); + self + .emit( + PermissionUpdated { + owner: owner, + permissioned_address: *permissioned_addresses[index], + has_permission: *permissions[index] + } + ); + index += 1 + } + } + + // @notice returns if a user has permission or not + // @param owner tokenbound account owner + // @param permissioned_address address to check permission for + fn has_permission( + self: @ComponentState, + owner: ContractAddress, + permissioned_address: ContractAddress + ) -> bool { + let permission = self.permissions.read((owner, permissioned_address)); + permission + } + } +} diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index 875b292..e472756 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -8,14 +8,17 @@ pub mod AccountPreset { use token_bound_accounts::components::upgradeable::upgradeable::UpgradeableComponent; use token_bound_accounts::components::lockable::lockable::LockableComponent; use token_bound_accounts::components::signatory::signatory::SignatoryComponent; + use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent; use token_bound_accounts::interfaces::{ - IUpgradeable::IUpgradeable, IExecutable::IExecutable, ILockable::ILockable, ISignatory::ISignatory + IUpgradeable::IUpgradeable, IExecutable::IExecutable, ILockable::ILockable, + ISignatory::ISignatory, IPermissionable::IPermissionable }; component!(path: AccountComponent, storage: account, event: AccountEvent); component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); component!(path: LockableComponent, storage: lockable, event: LockableEvent); component!(path: SignatoryComponent, storage: signatory, event: SignatoryEvent); + component!(path: PermissionableComponent, storage: permissionable, event: PermissionableEvent); // Account #[abi(embed_v0)] @@ -38,7 +41,9 @@ pub mod AccountPreset { #[substorage(v0)] lockable: LockableComponent::Storage, #[substorage(v0)] - signatory: SignatoryComponent::Storage + signatory: SignatoryComponent::Storage, + #[substorage(v0)] + permissionable: PermissionableComponent::Storage, } // ************************************************************************* @@ -54,7 +59,9 @@ pub mod AccountPreset { #[flat] LockableEvent: LockableComponent::Event, #[flat] - SignatoryEvent: SignatoryComponent::Event + SignatoryEvent: SignatoryComponent::Event, + #[flat] + PermissionableEvent: PermissionableComponent::Event } // ************************************************************************* @@ -65,7 +72,7 @@ pub mod AccountPreset { self.account.initializer(token_contract, token_id); } - // ************************************************************************* + // ************************************************************************* // SIGNATORY IMPL // ************************************************************************* #[abi(embed_v0)] @@ -126,8 +133,34 @@ pub mod AccountPreset { // lock account self.lockable.lock(lock_until); } + fn is_locked(self: @ContractState) -> (bool, u64) { self.lockable.is_locked() } } + + // ************************************************************************* + // PERMISSIONABLE IMPL + // ************************************************************************* + #[abi(embed_v0)] + impl Permissionable of IPermissionable { + fn set_permission( + ref self: ContractState, + permissioned_addresses: Array, + permissions: Array + ) { + // validate signer + let caller = get_caller_address(); + assert(self.is_valid_signer(caller), 'Account: unauthorized'); + + // set permissions + self.permissionable.set_permission(permissioned_addresses, permissions) + } + + fn has_permission( + self: @ContractState, owner: ContractAddress, permissioned_address: ContractAddress + ) -> bool { + self.permissionable.has_permission(owner, permissioned_address) + } + } } diff --git a/src/components/signatory.cairo b/src/components/signatory.cairo index 5c7b7a2..fa38d7b 100644 --- a/src/components/signatory.cairo +++ b/src/components/signatory.cairo @@ -1 +1 @@ -pub mod signatory; \ No newline at end of file +pub mod signatory; diff --git a/src/components/signatory/signatory.cairo b/src/components/signatory/signatory.cairo index cdb53c0..efdcee5 100644 --- a/src/components/signatory/signatory.cairo +++ b/src/components/signatory/signatory.cairo @@ -6,11 +6,11 @@ pub mod SignatoryComponent { // ************************************************************************* // IMPORTS // ************************************************************************* - use starknet::{ - get_caller_address, get_contract_address, ContractAddress - }; + use starknet::{get_caller_address, get_contract_address, ContractAddress}; use token_bound_accounts::components::account::account::AccountComponent; use token_bound_accounts::components::account::account::AccountComponent::InternalImpl; + use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent; + use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent::PermissionableImpl; // ************************************************************************* // STORAGE @@ -26,17 +26,19 @@ pub mod SignatoryComponent { TContractState, +HasComponent, +Drop, - impl Account: AccountComponent::HasComponent + impl Account: AccountComponent::HasComponent, + impl Permissionable: PermissionableComponent::HasComponent > of PrivateTrait { /// @notice implements a simple signer validation where only NFT owner is a valid signer. /// @param signer the address to be validated - fn _base_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { + fn _base_signer_validation( + self: @ComponentState, signer: ContractAddress + ) -> bool { let account = get_dep_component!(self, Account); let (contract_address, token_id, _) = account._get_token(); // get owner - let owner = account - ._get_owner(contract_address, token_id); + let owner = account._get_owner(contract_address, token_id); // validate if (signer == owner) { @@ -46,35 +48,58 @@ pub mod SignatoryComponent { } } - /// @notice implements a signer validation where both NFT owner and the root owner (for nested accounts) are valid signers. + /// @notice implements a signer validation where both NFT owner and the root owner (for + /// nested accounts) are valid signers. /// @param signer the address to be validated - fn _base_and_root_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { + fn _base_and_root_signer_validation( + self: @ComponentState, signer: ContractAddress + ) -> bool { let account = get_dep_component!(self, Account); let (contract_address, token_id, _) = account._get_token(); // get owner - let owner = account - ._get_owner(contract_address, token_id); + let owner = account._get_owner(contract_address, token_id); // get root owner - let root_owner = account - ._get_root_owner(contract_address, token_id); + let root_owner = account._get_root_owner(contract_address, token_id); // validate if (signer == owner) { return true; - } - else if(signer == root_owner) { + } else if (signer == root_owner) { return true; - } - else { + } else { return false; } } - /// @notice implements a more complex signer validation where NFT owner, root owner, and permissioned addresses are valid signers. + /// @notice implements a more complex signer validation where NFT owner, root owner, and + /// permissioned addresses are valid signers. /// @param signer the address to be validated - fn _permissioned_signer_validation(self: @ComponentState, signer: ContractAddress) -> bool { - true + fn _permissioned_signer_validation( + self: @ComponentState, signer: ContractAddress + ) -> bool { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + + // get owner + let owner = account._get_owner(contract_address, token_id); + // get root owner + let root_owner = account._get_root_owner(contract_address, token_id); + + // check if signer has permissions + let permission = get_dep_component!(self, Permissionable); + let is_permissioned = permission.has_permission(owner, signer); + + // validate + if (signer == owner) { + return true; + } else if (signer == root_owner) { + return true; + } else if (is_permissioned) { + return true; + } else { + return false; + } } } } diff --git a/src/interfaces.cairo b/src/interfaces.cairo index de01f5a..8048787 100644 --- a/src/interfaces.cairo +++ b/src/interfaces.cairo @@ -4,4 +4,5 @@ pub mod IRegistry; pub mod IUpgradeable; pub mod IExecutable; pub mod ILockable; -pub mod ISignatory; \ No newline at end of file +pub mod ISignatory; +pub mod IPermissionable; diff --git a/src/interfaces/ILockable.cairo b/src/interfaces/ILockable.cairo index d075dd2..d893e39 100644 --- a/src/interfaces/ILockable.cairo +++ b/src/interfaces/ILockable.cairo @@ -1,3 +1,6 @@ +// ************************************************************************* +// LOCKABLE INTERFACE +// ************************************************************************* use starknet::ContractAddress; #[starknet::interface] diff --git a/src/interfaces/IPermissionable.cairo b/src/interfaces/IPermissionable.cairo new file mode 100644 index 0000000..438a477 --- /dev/null +++ b/src/interfaces/IPermissionable.cairo @@ -0,0 +1,16 @@ +// ************************************************************************* +// PERMISSIONABLE INTERFACE +// ************************************************************************* +use starknet::ContractAddress; + +#[starknet::interface] +pub trait IPermissionable { + fn set_permission( + ref self: TContractState, + permissioned_addresses: Array, + permissions: Array + ); + fn has_permission( + self: @TContractState, owner: ContractAddress, permissioned_address: ContractAddress + ) -> bool; +} diff --git a/tests/test_account_component.cairo b/tests/test_account_component.cairo index 33c217c..58c84b0 100644 --- a/tests/test_account_component.cairo +++ b/tests/test_account_component.cairo @@ -159,20 +159,6 @@ fn test_is_valid_signature() { stop_cheat_caller_address(contract_address); } -#[test] -fn test_is_valid_signer() { - let (contract_address, erc721_contract_address) = __setup__(); - let dispatcher = IAccountDispatcher { contract_address }; - let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; - let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); - - // check for valid signer - let valid_signer = dispatcher.is_valid_signer(token_owner); - let invalid_signer = dispatcher.is_valid_signer(ACCOUNT.try_into().unwrap()); - assert(valid_signer == true, 'signer is meant to be valid!'); - assert(invalid_signer == false, 'signer is meant to be invalid!'); -} - #[test] fn test_execute() { let (contract_address, erc721_contract_address) = __setup__(); From 860c2e248366962d52f4cbadd253418ef592da19 Mon Sep 17 00:00:00 2001 From: Darlington02 Date: Wed, 28 Aug 2024 23:49:56 +0100 Subject: [PATCH 03/11] chore: abstract `is_valid_signature` into Signatory --- src/components/account/account.cairo | 39 -------- .../permissionable/permissionable.cairo | 8 +- src/components/presets/account_preset.cairo | 6 ++ src/components/signatory/signatory.cairo | 96 +++++++++++++++++++ src/interfaces.cairo | 1 + src/interfaces/IAccount.cairo | 3 - src/interfaces/ISRC6.cairo | 16 ++++ src/interfaces/ISignatory.cairo | 3 + tests/test_account_component.cairo | 44 ++++----- 9 files changed, 148 insertions(+), 68 deletions(-) create mode 100644 src/interfaces/ISRC6.cairo diff --git a/src/components/account/account.cairo b/src/components/account/account.cairo index 9d676fd..a5120c1 100644 --- a/src/components/account/account.cairo +++ b/src/components/account/account.cairo @@ -71,7 +71,6 @@ pub mod AccountComponent { pub mod Errors { pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; pub const INV_SIG_LEN: felt252 = 'Account: invalid sig length'; - pub const INV_SIGNATURE: felt252 = 'Account: invalid signature'; pub const INV_TX_VERSION: felt252 = 'Account: invalid tx version'; } @@ -82,15 +81,6 @@ pub mod AccountComponent { pub impl Account< TContractState, +HasComponent, +Drop > of IAccount> { - /// @notice used for signature validation - /// @param hash The message hash - /// @param signature The signature to be validated - fn is_valid_signature( - self: @ComponentState, hash: felt252, signature: Span - ) -> felt252 { - self._is_valid_signature(hash, signature) - } - /// @notice gets the NFT owner /// @param token_contract the contract address of the NFT /// @param token_id the token ID of the NFT @@ -232,35 +222,6 @@ pub mod AccountComponent { (contract, token_id, chain_id) } - /// @notice internal function for signature validation - fn _is_valid_signature( - self: @ComponentState, hash: felt252, signature: Span - ) -> felt252 { - let signature_length = signature.len(); - assert(signature_length == 2_u32, Errors::INV_SIG_LEN); - - let owner = self - ._get_owner(self.account_token_contract.read(), self.account_token_id.read()); - let account = IAccountDispatcher { contract_address: owner }; - if (account.is_valid_signature(hash, signature) == starknet::VALIDATED) { - return starknet::VALIDATED; - } else { - return 0; - } - } - - /// @notice internal function for tx validation - fn _validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); - let tx_hash = tx_info.transaction_hash; - let signature = tx_info.signature; - assert( - self._is_valid_signature(tx_hash, signature) == starknet::VALIDATED, - Errors::INV_SIGNATURE - ); - starknet::VALIDATED - } - /// @notice internal function for executing transactions /// @param calls An array of transactions to be executed fn _execute_calls( diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index 795a3b1..30cd5ad 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -50,10 +50,10 @@ pub mod PermissionableComponent { // ERRORS // ************************************************************************* pub mod Errors { - pub const UNAUTHORIZED: felt252 = 'Permission: unauthorized'; - pub const NOT_OWNER: felt252 = 'Permission: not account owner'; - pub const INVALID_LENGTH: felt252 = 'Permission: invalid length'; - pub const NOT_PERMITTED: felt252 = 'Permisson: not permitted'; + pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; + pub const NOT_OWNER: felt252 = 'Account: not account owner'; + pub const INVALID_LENGTH: felt252 = 'Account: invalid length'; + pub const NOT_PERMITTED: felt252 = 'Account: not permitted'; } diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index e472756..d4204c6 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -80,6 +80,12 @@ pub mod AccountPreset { fn is_valid_signer(self: @ContractState, signer: ContractAddress) -> bool { self.signatory._permissioned_signer_validation(signer) } + + fn is_valid_signature( + self: @ContractState, hash: felt252, signature: Span + ) -> felt252 { + self.signatory._permissioned_signature_validation(hash, signature) + } } // ************************************************************************* diff --git a/src/components/signatory/signatory.cairo b/src/components/signatory/signatory.cairo index efdcee5..a6ac21d 100644 --- a/src/components/signatory/signatory.cairo +++ b/src/components/signatory/signatory.cairo @@ -11,6 +11,7 @@ pub mod SignatoryComponent { use token_bound_accounts::components::account::account::AccountComponent::InternalImpl; use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent; use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent::PermissionableImpl; + use token_bound_accounts::interfaces::ISRC6::{ISRC6Dispatcher, ISRC6DispatcherTrait}; // ************************************************************************* // STORAGE @@ -18,6 +19,15 @@ pub mod SignatoryComponent { #[storage] pub struct Storage {} + // ************************************************************************* + // ERRORS + // ************************************************************************* + pub mod Errors { + pub const INV_SIG_LEN: felt252 = 'Account: invalid sig length'; + pub const UNAUTHORIZED: felt252 = 'Account: invalid signer'; + pub const INVALID_SIGNATURE: felt252 = 'Account: invalid signature'; + } + // ************************************************************************* // PRIVATE FUNCTIONS // ************************************************************************* @@ -48,6 +58,27 @@ pub mod SignatoryComponent { } } + /// @notice used for signature validation where only NFT owner is a valid signer. + /// @param hash The message hash + /// @param signature The signature to be validated + fn _base_signature_validation( + self: @ComponentState, hash: felt252, signature: Span + ) -> felt252 { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + let owner = account._get_owner(contract_address, token_id); + + let signature_length = signature.len(); + assert(signature_length == 2_u32, Errors::INV_SIG_LEN); + + let account = ISRC6Dispatcher { contract_address: owner }; + if (account.is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else { + return Errors::INVALID_SIGNATURE; + } + } + /// @notice implements a signer validation where both NFT owner and the root owner (for /// nested accounts) are valid signers. /// @param signer the address to be validated @@ -72,6 +103,35 @@ pub mod SignatoryComponent { } } + /// @notice used for signature validation where both NFT owner and the root owner (for + /// nested accounts) are valid signers. + /// @param hash The message hash + /// @param signature The signature to be validated + fn _base_and_root_signature_validation( + self: @ComponentState, hash: felt252, signature: Span + ) -> felt252 { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + let owner = account._get_owner(contract_address, token_id); + let root_owner = account._get_root_owner(contract_address, token_id); + + let signature_length = signature.len(); + assert(signature_length == 2_u32, Errors::INV_SIG_LEN); + + let owner_account = ISRC6Dispatcher { contract_address: owner }; + let root_owner_account = ISRC6Dispatcher { contract_address: root_owner }; + + // validate + if (owner_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else if (root_owner_account + .is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else { + return Errors::INVALID_SIGNATURE; + } + } + /// @notice implements a more complex signer validation where NFT owner, root owner, and /// permissioned addresses are valid signers. /// @param signer the address to be validated @@ -101,5 +161,41 @@ pub mod SignatoryComponent { return false; } } + + /// @notice used for signature validation where NFT owner, root owner, and + /// permissioned addresses are valid signers. + /// @param hash The message hash + /// @param signature The signature to be validated + fn _permissioned_signature_validation( + self: @ComponentState, hash: felt252, signature: Span + ) -> felt252 { + let account = get_dep_component!(self, Account); + let (contract_address, token_id, _) = account._get_token(); + let owner = account._get_owner(contract_address, token_id); + let root_owner = account._get_root_owner(contract_address, token_id); + + let signature_length = signature.len(); + assert(signature_length == 2_u32, Errors::INV_SIG_LEN); + + let owner_account = ISRC6Dispatcher { contract_address: owner }; + let root_owner_account = ISRC6Dispatcher { contract_address: root_owner }; + + // check if caller has permissions + let permission = get_dep_component!(self, Permissionable); + assert(permission.has_permission(owner, get_caller_address()), Errors::UNAUTHORIZED); + let caller_account = ISRC6Dispatcher { contract_address: get_caller_address() }; + + // validate + if (owner_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else if (root_owner_account + .is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else if (caller_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { + return starknet::VALIDATED; + } else { + return Errors::INVALID_SIGNATURE; + } + } } } diff --git a/src/interfaces.cairo b/src/interfaces.cairo index 8048787..1e3f2b4 100644 --- a/src/interfaces.cairo +++ b/src/interfaces.cairo @@ -1,4 +1,5 @@ pub mod IAccount; +pub mod ISRC6; pub mod IERC721; pub mod IRegistry; pub mod IUpgradeable; diff --git a/src/interfaces/IAccount.cairo b/src/interfaces/IAccount.cairo index cc31e14..8f5e26d 100644 --- a/src/interfaces/IAccount.cairo +++ b/src/interfaces/IAccount.cairo @@ -11,9 +11,6 @@ pub const TBA_INTERFACE_ID: felt252 = #[starknet::interface] pub trait IAccount { - fn is_valid_signature( - self: @TContractState, hash: felt252, signature: Span - ) -> felt252; fn token(self: @TContractState) -> (ContractAddress, u256, felt252); fn owner(self: @TContractState) -> ContractAddress; fn state(self: @TContractState) -> u256; diff --git a/src/interfaces/ISRC6.cairo b/src/interfaces/ISRC6.cairo new file mode 100644 index 0000000..49e0982 --- /dev/null +++ b/src/interfaces/ISRC6.cairo @@ -0,0 +1,16 @@ +use starknet::ContractAddress; +use starknet::ClassHash; +use starknet::account::Call; + +#[starknet::interface] +pub trait ISRC6 { + fn is_valid_signature( + self: @TContractState, hash: felt252, signature: Span + ) -> felt252; + fn __validate__(ref self: TContractState, calls: Array) -> felt252; + fn __validate_declare__(self: @TContractState, class_hash: felt252) -> felt252; + fn __validate_deploy__( + self: @TContractState, class_hash: felt252, contract_address_salt: felt252 + ) -> felt252; + fn __execute__(ref self: TContractState, calls: Array) -> Array>; +} diff --git a/src/interfaces/ISignatory.cairo b/src/interfaces/ISignatory.cairo index b41b27c..72b33a6 100644 --- a/src/interfaces/ISignatory.cairo +++ b/src/interfaces/ISignatory.cairo @@ -6,4 +6,7 @@ use starknet::ContractAddress; #[starknet::interface] pub trait ISignatory { fn is_valid_signer(self: @TContractState, signer: ContractAddress) -> bool; + fn is_valid_signature( + self: @TContractState, hash: felt252, signature: Span + ) -> felt252; } diff --git a/tests/test_account_component.cairo b/tests/test_account_component.cairo index 58c84b0..a9b5f02 100644 --- a/tests/test_account_component.cairo +++ b/tests/test_account_component.cairo @@ -136,28 +136,28 @@ fn test_event_is_emitted_on_initialization() { ); } -#[test] -fn test_is_valid_signature() { - let (contract_address, erc721_contract_address) = __setup__(); - let dispatcher = IAccountDispatcher { contract_address }; - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; - - let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; - let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); - - start_cheat_caller_address(contract_address, token_owner); - let mut good_signature = array![data.r, data.s]; - let is_valid = dispatcher.is_valid_signature(hash, good_signature.span()); - assert(is_valid == 'VALID', 'should accept valid signature'); - stop_cheat_caller_address(contract_address); - - start_cheat_caller_address(contract_address, ACCOUNT2.try_into().unwrap()); - let mut bad_signature = array![0x284, 0x492]; - let is_valid = dispatcher.is_valid_signature(hash, bad_signature.span()); - assert(is_valid == 0, 'should reject invalid signature'); - stop_cheat_caller_address(contract_address); -} +// #[test] +// fn test_is_valid_signature() { +// let (contract_address, erc721_contract_address) = __setup__(); +// let dispatcher = IAccountDispatcher { contract_address }; +// let data = SIGNED_TX_DATA(); +// let hash = data.transaction_hash; + +// let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; +// let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + +// start_cheat_caller_address(contract_address, token_owner); +// let mut good_signature = array![data.r, data.s]; +// let is_valid = dispatcher.is_valid_signature(hash, good_signature.span()); +// assert(is_valid == 'VALID', 'should accept valid signature'); +// stop_cheat_caller_address(contract_address); + +// start_cheat_caller_address(contract_address, ACCOUNT2.try_into().unwrap()); +// let mut bad_signature = array![0x284, 0x492]; +// let is_valid = dispatcher.is_valid_signature(hash, bad_signature.span()); +// assert(is_valid == 0, 'should reject invalid signature'); +// stop_cheat_caller_address(contract_address); +// } #[test] fn test_execute() { From 975bc01b404a9b2484ac1d9517eb88a4f65f9480 Mon Sep 17 00:00:00 2001 From: Darlington02 Date: Thu, 29 Aug 2024 00:08:17 +0100 Subject: [PATCH 04/11] chore: update signatory component --- src/components/presets/account_preset.cairo | 2 +- src/components/signatory/signatory.cairo | 61 +-------------------- 2 files changed, 3 insertions(+), 60 deletions(-) diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index d4204c6..f31f428 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -84,7 +84,7 @@ pub mod AccountPreset { fn is_valid_signature( self: @ContractState, hash: felt252, signature: Span ) -> felt252 { - self.signatory._permissioned_signature_validation(hash, signature) + self.signatory._is_valid_signature(hash, signature) } } diff --git a/src/components/signatory/signatory.cairo b/src/components/signatory/signatory.cairo index a6ac21d..bbc3f6c 100644 --- a/src/components/signatory/signatory.cairo +++ b/src/components/signatory/signatory.cairo @@ -58,27 +58,6 @@ pub mod SignatoryComponent { } } - /// @notice used for signature validation where only NFT owner is a valid signer. - /// @param hash The message hash - /// @param signature The signature to be validated - fn _base_signature_validation( - self: @ComponentState, hash: felt252, signature: Span - ) -> felt252 { - let account = get_dep_component!(self, Account); - let (contract_address, token_id, _) = account._get_token(); - let owner = account._get_owner(contract_address, token_id); - - let signature_length = signature.len(); - assert(signature_length == 2_u32, Errors::INV_SIG_LEN); - - let account = ISRC6Dispatcher { contract_address: owner }; - if (account.is_valid_signature(hash, signature) == starknet::VALIDATED) { - return starknet::VALIDATED; - } else { - return Errors::INVALID_SIGNATURE; - } - } - /// @notice implements a signer validation where both NFT owner and the root owner (for /// nested accounts) are valid signers. /// @param signer the address to be validated @@ -103,34 +82,6 @@ pub mod SignatoryComponent { } } - /// @notice used for signature validation where both NFT owner and the root owner (for - /// nested accounts) are valid signers. - /// @param hash The message hash - /// @param signature The signature to be validated - fn _base_and_root_signature_validation( - self: @ComponentState, hash: felt252, signature: Span - ) -> felt252 { - let account = get_dep_component!(self, Account); - let (contract_address, token_id, _) = account._get_token(); - let owner = account._get_owner(contract_address, token_id); - let root_owner = account._get_root_owner(contract_address, token_id); - - let signature_length = signature.len(); - assert(signature_length == 2_u32, Errors::INV_SIG_LEN); - - let owner_account = ISRC6Dispatcher { contract_address: owner }; - let root_owner_account = ISRC6Dispatcher { contract_address: root_owner }; - - // validate - if (owner_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { - return starknet::VALIDATED; - } else if (root_owner_account - .is_valid_signature(hash, signature) == starknet::VALIDATED) { - return starknet::VALIDATED; - } else { - return Errors::INVALID_SIGNATURE; - } - } /// @notice implements a more complex signer validation where NFT owner, root owner, and /// permissioned addresses are valid signers. @@ -162,11 +113,10 @@ pub mod SignatoryComponent { } } - /// @notice used for signature validation where NFT owner, root owner, and - /// permissioned addresses are valid signers. + /// @notice used for signature validation /// @param hash The message hash /// @param signature The signature to be validated - fn _permissioned_signature_validation( + fn _is_valid_signature( self: @ComponentState, hash: felt252, signature: Span ) -> felt252 { let account = get_dep_component!(self, Account); @@ -180,19 +130,12 @@ pub mod SignatoryComponent { let owner_account = ISRC6Dispatcher { contract_address: owner }; let root_owner_account = ISRC6Dispatcher { contract_address: root_owner }; - // check if caller has permissions - let permission = get_dep_component!(self, Permissionable); - assert(permission.has_permission(owner, get_caller_address()), Errors::UNAUTHORIZED); - let caller_account = ISRC6Dispatcher { contract_address: get_caller_address() }; - // validate if (owner_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { return starknet::VALIDATED; } else if (root_owner_account .is_valid_signature(hash, signature) == starknet::VALIDATED) { return starknet::VALIDATED; - } else if (caller_account.is_valid_signature(hash, signature) == starknet::VALIDATED) { - return starknet::VALIDATED; } else { return Errors::INVALID_SIGNATURE; } From 229d078d10c9b634a4e962160e42a19578ef6fa3 Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Fri, 30 Aug 2024 12:07:23 +0100 Subject: [PATCH 05/11] unit test for permissionable docs --- tests/test_permissionable_component.cairo | 200 ++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 tests/test_permissionable_component.cairo diff --git a/tests/test_permissionable_component.cairo b/tests/test_permissionable_component.cairo new file mode 100644 index 0000000..8ad0db7 --- /dev/null +++ b/tests/test_permissionable_component.cairo @@ -0,0 +1,200 @@ +// ************************************************************************* +// COMPONENT COMPONENT TEST +// ************************************************************************* +use starknet::{ContractAddress, account::Call, get_block_timestamp}; +use snforge_std::{ + declare, start_cheat_caller_address, stop_cheat_caller_address, start_cheat_transaction_hash, + start_cheat_nonce, spy_events, EventSpyAssertionsTrait, ContractClassTrait, ContractClass, + start_cheat_block_timestamp, stop_cheat_block_timestamp +}; +use core::hash::HashStateTrait; +use core::pedersen::PedersenTrait; + +use token_bound_accounts::interfaces::IAccount::{ + IAccountDispatcher, IAccountDispatcherTrait, IAccountSafeDispatcher, IAccountSafeDispatcherTrait +}; + +use token_bound_accounts::interfaces::IPermissionable::{ + IPermissionableDispatcher, IPermissionableDispatcherTrait +}; + +use token_bound_accounts::interfaces::IExecutable::{ + IExecutableDispatcher, IExecutableDispatcherTrait +}; +use token_bound_accounts::interfaces::IUpgradeable::{ + IUpgradeableDispatcher, IUpgradeableDispatcherTrait +}; +use token_bound_accounts::components::presets::account_preset::AccountPreset; +use token_bound_accounts::components::account::account::AccountComponent; + +use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent; + +use token_bound_accounts::test_helper::{ + hello_starknet::{IHelloStarknetDispatcher, IHelloStarknetDispatcherTrait, HelloStarknet}, + erc721_helper::{IERC721Dispatcher, IERC721DispatcherTrait, ERC721}, + simple_account::{ISimpleAccountDispatcher, ISimpleAccountDispatcherTrait, SimpleAccount} +}; + +const ACCOUNT1: felt252 = 5729; +const ACCOUNT2: felt252 = 1234; +const ACCOUNT3: felt252 = 6908; +const ACCOUNT4: felt252 = 4697; + +// ************************************************************************* +// SETUP +// ************************************************************************* +fn __setup__() -> (ContractAddress, ContractAddress) { + // deploy erc721 helper contract + let erc721_contract = declare("ERC721").unwrap(); + let mut erc721_constructor_calldata = array!['tokenbound', 'TBA']; + let (erc721_contract_address, _) = erc721_contract + .deploy(@erc721_constructor_calldata) + .unwrap(); + + // deploy recipient contract + let account_contract = declare("SimpleAccount").unwrap(); + let (recipient, _) = account_contract + .deploy( + @array![883045738439352841478194533192765345509759306772397516907181243450667673002] + ) + .unwrap(); + + // mint a new token + let dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + dispatcher.mint(recipient, 1.try_into().unwrap()); + + // deploy account contract + let account_contract = declare("AccountPreset").unwrap(); + let mut acct_constructor_calldata = array![erc721_contract_address.try_into().unwrap(), 1, 0]; + let (account_contract_address, _) = account_contract + .deploy(@acct_constructor_calldata) + .unwrap(); + + (account_contract_address, erc721_contract_address) +} + +#[test] +#[should_panic(expected: ('Account: invalid length',))] +fn test_when_permissioned_addresses_and_permissions_not_equal() { + let (contract_address, _) = __setup__(); + let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; + + let owner = acct_dispatcher.owner(); + + let mut permission_addresses = ArrayTrait::new(); + permission_addresses.append(ACCOUNT2.try_into().unwrap()); + permission_addresses.append(ACCOUNT3.try_into().unwrap()); + permission_addresses.append(ACCOUNT4.try_into().unwrap()); + + let mut permissions = ArrayTrait::new(); + permissions.append(true); + permissions.append(true); + + let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; + + start_cheat_caller_address(contract_address, owner); + permissionable_dispatcher.set_permission(permission_addresses, permissions) +} + + +#[test] +fn test_permissionable() { + let (contract_address, _) = __setup__(); + let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; + + let owner = acct_dispatcher.owner(); + + let mut permission_addresses = ArrayTrait::new(); + permission_addresses.append(ACCOUNT2.try_into().unwrap()); + permission_addresses.append(ACCOUNT3.try_into().unwrap()); + permission_addresses.append(ACCOUNT4.try_into().unwrap()); + + let mut permissions = ArrayTrait::new(); + permissions.append(true); + permissions.append(true); + permissions.append(true); + + start_cheat_caller_address(contract_address, owner); + + let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; + permissionable_dispatcher.set_permission(permission_addresses, permissions); + + let has_permission = permissionable_dispatcher + .has_permission(owner, ACCOUNT2.try_into().unwrap()); + + assert(has_permission == true, 'Account: not permitted'); + stop_cheat_caller_address(contract_address); +} + +#[test] +fn test_not_permitted() { + let (contract_address, _) = __setup__(); + let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; + + let owner = acct_dispatcher.owner(); + + let mut permission_addresses = ArrayTrait::new(); + permission_addresses.append(ACCOUNT2.try_into().unwrap()); + permission_addresses.append(ACCOUNT3.try_into().unwrap()); + permission_addresses.append(ACCOUNT4.try_into().unwrap()); + + let mut permissions = ArrayTrait::new(); + permissions.append(true); + permissions.append(true); + permissions.append(false); + + start_cheat_caller_address(contract_address, owner); + + let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; + permissionable_dispatcher.set_permission(permission_addresses, permissions); + + let has_permission = permissionable_dispatcher + .has_permission(owner, ACCOUNT4.try_into().unwrap()); + + assert(has_permission == false, 'Account: permitted'); + stop_cheat_caller_address(contract_address); +} + + +#[test] +fn test_permissionable_emit_event() { + let (contract_address, _) = __setup__(); + let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; + + let owner = acct_dispatcher.owner(); + // spy on emitted events + let mut spy = spy_events(); + + let mut permission_addresses = ArrayTrait::new(); + permission_addresses.append(ACCOUNT2.try_into().unwrap()); + permission_addresses.append(ACCOUNT3.try_into().unwrap()); + permission_addresses.append(ACCOUNT4.try_into().unwrap()); + + let mut permissions = ArrayTrait::new(); + permissions.append(true); + permissions.append(true); + permissions.append(true); + + start_cheat_caller_address(contract_address, owner); + + let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; + permissionable_dispatcher.set_permission(permission_addresses, permissions); + + // check events are emitted + spy + .assert_emitted( + @array![ + ( + contract_address, + PermissionableComponent::Event::PermissionUpdated( + PermissionableComponent::PermissionUpdated { + owner: owner, + permissioned_address: ACCOUNT4.try_into().unwrap(), + has_permission: true + } + ) + ) + ] + ); +} + From 07fdf5cea4fb36e0b211fbe4b4f8df0c319b0709 Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Fri, 30 Aug 2024 15:22:17 +0100 Subject: [PATCH 06/11] fix changes requested on the PR --- .../permissionable/permissionable.cairo | 3 --- tests/test_permissionable_component.cairo | 19 +++++++++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index 30cd5ad..4b44751 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -50,10 +50,7 @@ pub mod PermissionableComponent { // ERRORS // ************************************************************************* pub mod Errors { - pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; - pub const NOT_OWNER: felt252 = 'Account: not account owner'; pub const INVALID_LENGTH: felt252 = 'Account: invalid length'; - pub const NOT_PERMITTED: felt252 = 'Account: not permitted'; } diff --git a/tests/test_permissionable_component.cairo b/tests/test_permissionable_component.cairo index 8ad0db7..9d0baf2 100644 --- a/tests/test_permissionable_component.cairo +++ b/tests/test_permissionable_component.cairo @@ -127,7 +127,7 @@ fn test_permissionable() { } #[test] -fn test_not_permitted() { +fn test_has_permissions() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; @@ -148,16 +148,27 @@ fn test_not_permitted() { let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; permissionable_dispatcher.set_permission(permission_addresses, permissions); - let has_permission = permissionable_dispatcher + let has_permission2 = permissionable_dispatcher + .has_permission(owner, ACCOUNT2.try_into().unwrap()); + + assert(has_permission2 == true, 'Account: permitted'); + + let has_permission3 = permissionable_dispatcher + .has_permission(owner, ACCOUNT3.try_into().unwrap()); + + assert(has_permission3 == true, 'Account: permitted'); + + let has_permission4 = permissionable_dispatcher .has_permission(owner, ACCOUNT4.try_into().unwrap()); - assert(has_permission == false, 'Account: permitted'); + assert(has_permission4 == false, 'Account: permitted'); + stop_cheat_caller_address(contract_address); } #[test] -fn test_permissionable_emit_event() { +fn test_set_permission_emits_event() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; From e22110a661377f0dea2f1c9f29b3ee443e694e0c Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Sun, 1 Sep 2024 07:18:26 +0100 Subject: [PATCH 07/11] check for caller is the account owner --- src/components/permissionable/permissionable.cairo | 4 ++++ src/components/presets/account_preset.cairo | 4 ---- tests/test_permissionable_component.cairo | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index 4b44751..1f373e2 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -51,6 +51,7 @@ pub mod PermissionableComponent { // ************************************************************************* pub mod Errors { pub const INVALID_LENGTH: felt252 = 'Account: invalid length'; + pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; } @@ -76,6 +77,9 @@ pub mod PermissionableComponent { let account_comp = get_dep_component!(@self, Account); let owner = account_comp.owner(); + // call is account owner + assert(owner == get_caller_address(), Errors::UNAUTHORIZED); + let length = permissioned_addresses.len(); let mut index: u32 = 0; while index < length { diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index f31f428..5fa7848 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -155,10 +155,6 @@ pub mod AccountPreset { permissioned_addresses: Array, permissions: Array ) { - // validate signer - let caller = get_caller_address(); - assert(self.is_valid_signer(caller), 'Account: unauthorized'); - // set permissions self.permissionable.set_permission(permissioned_addresses, permissions) } diff --git a/tests/test_permissionable_component.cairo b/tests/test_permissionable_component.cairo index 9d0baf2..3a1956d 100644 --- a/tests/test_permissionable_component.cairo +++ b/tests/test_permissionable_component.cairo @@ -1,5 +1,5 @@ // ************************************************************************* -// COMPONENT COMPONENT TEST +// PERMISSIONABLE COMPONENT TEST // ************************************************************************* use starknet::{ContractAddress, account::Call, get_block_timestamp}; use snforge_std::{ @@ -75,7 +75,7 @@ fn __setup__() -> (ContractAddress, ContractAddress) { #[test] #[should_panic(expected: ('Account: invalid length',))] -fn test_when_permissioned_addresses_and_permissions_not_equal() { +fn test_should_fail_if_unequal_permissioned_addresses_and_permissions() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; @@ -98,7 +98,7 @@ fn test_when_permissioned_addresses_and_permissions_not_equal() { #[test] -fn test_permissionable() { +fn test_set_permission() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; From 445f57c059174d2d7078290789a4ac4c222cc2bf Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Sun, 1 Sep 2024 13:59:02 +0100 Subject: [PATCH 08/11] unit test for signatory component --- .../permissionable/permissionable.cairo | 4 - src/components/presets/account_preset.cairo | 4 + tests/test_permissionable_component.cairo | 6 +- tests/test_signatory_component.cairo | 131 ++++++++++++++++++ 4 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 tests/test_signatory_component.cairo diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index 1f373e2..4b44751 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -51,7 +51,6 @@ pub mod PermissionableComponent { // ************************************************************************* pub mod Errors { pub const INVALID_LENGTH: felt252 = 'Account: invalid length'; - pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; } @@ -77,9 +76,6 @@ pub mod PermissionableComponent { let account_comp = get_dep_component!(@self, Account); let owner = account_comp.owner(); - // call is account owner - assert(owner == get_caller_address(), Errors::UNAUTHORIZED); - let length = permissioned_addresses.len(); let mut index: u32 = 0; while index < length { diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index 5fa7848..f31f428 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -155,6 +155,10 @@ pub mod AccountPreset { permissioned_addresses: Array, permissions: Array ) { + // validate signer + let caller = get_caller_address(); + assert(self.is_valid_signer(caller), 'Account: unauthorized'); + // set permissions self.permissionable.set_permission(permissioned_addresses, permissions) } diff --git a/tests/test_permissionable_component.cairo b/tests/test_permissionable_component.cairo index 3a1956d..9d0baf2 100644 --- a/tests/test_permissionable_component.cairo +++ b/tests/test_permissionable_component.cairo @@ -1,5 +1,5 @@ // ************************************************************************* -// PERMISSIONABLE COMPONENT TEST +// COMPONENT COMPONENT TEST // ************************************************************************* use starknet::{ContractAddress, account::Call, get_block_timestamp}; use snforge_std::{ @@ -75,7 +75,7 @@ fn __setup__() -> (ContractAddress, ContractAddress) { #[test] #[should_panic(expected: ('Account: invalid length',))] -fn test_should_fail_if_unequal_permissioned_addresses_and_permissions() { +fn test_when_permissioned_addresses_and_permissions_not_equal() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; @@ -98,7 +98,7 @@ fn test_should_fail_if_unequal_permissioned_addresses_and_permissions() { #[test] -fn test_set_permission() { +fn test_permissionable() { let (contract_address, _) = __setup__(); let acct_dispatcher = IAccountDispatcher { contract_address: contract_address }; diff --git a/tests/test_signatory_component.cairo b/tests/test_signatory_component.cairo new file mode 100644 index 0000000..a3b1147 --- /dev/null +++ b/tests/test_signatory_component.cairo @@ -0,0 +1,131 @@ +// ************************************************************************* +// COMPONENT COMPONENT TEST +// ************************************************************************* +use starknet::{ContractAddress, account::Call, get_block_timestamp}; +use snforge_std::{ + declare, start_cheat_caller_address, stop_cheat_caller_address, start_cheat_transaction_hash, + start_cheat_nonce, spy_events, EventSpyAssertionsTrait, ContractClassTrait, ContractClass, + start_cheat_block_timestamp, stop_cheat_block_timestamp +}; +use core::hash::HashStateTrait; +use core::pedersen::PedersenTrait; + +use token_bound_accounts::interfaces::IAccount::{ + IAccountDispatcher, IAccountDispatcherTrait, IAccountSafeDispatcher, IAccountSafeDispatcherTrait +}; + +use token_bound_accounts::interfaces::IPermissionable::{ + IPermissionableDispatcher, IPermissionableDispatcherTrait +}; + +use token_bound_accounts::interfaces::ISignatory::{ISignatoryDispatcher, ISignatoryDispatcherTrait}; + +use token_bound_accounts::interfaces::IExecutable::{ + IExecutableDispatcher, IExecutableDispatcherTrait +}; +use token_bound_accounts::interfaces::IUpgradeable::{ + IUpgradeableDispatcher, IUpgradeableDispatcherTrait +}; +use token_bound_accounts::components::presets::account_preset::AccountPreset; +use token_bound_accounts::components::account::account::AccountComponent; + + +use token_bound_accounts::components::signatory::signatory::SignatoryComponent; + +use token_bound_accounts::test_helper::{ + hello_starknet::{IHelloStarknetDispatcher, IHelloStarknetDispatcherTrait, HelloStarknet}, + erc721_helper::{IERC721Dispatcher, IERC721DispatcherTrait, ERC721}, + simple_account::{ISimpleAccountDispatcher, ISimpleAccountDispatcherTrait, SimpleAccount} +}; + +const ACCOUNT1: felt252 = 5729; +const ACCOUNT2: felt252 = 1234; +const ACCOUNT3: felt252 = 6908; +const ACCOUNT4: felt252 = 4697; + +#[derive(Drop)] +struct SignedTransactionData { + private_key: felt252, + public_key: felt252, + transaction_hash: felt252, + r: felt252, + s: felt252 +} + +fn SIGNED_TX_DATA() -> SignedTransactionData { + SignedTransactionData { + private_key: 1234, + public_key: 883045738439352841478194533192765345509759306772397516907181243450667673002, + transaction_hash: 2717105892474786771566982177444710571376803476229898722748888396642649184538, + r: 3068558690657879390136740086327753007413919701043650133111397282816679110801, + s: 3355728545224320878895493649495491771252432631648740019139167265522817576501 + } +} + + +// ************************************************************************* +// SETUP +// ************************************************************************* +fn __setup__() -> (ContractAddress, ContractAddress) { + // deploy erc721 helper contract + let erc721_contract = declare("ERC721").unwrap(); + let mut erc721_constructor_calldata = array!['tokenbound', 'TBA']; + let (erc721_contract_address, _) = erc721_contract + .deploy(@erc721_constructor_calldata) + .unwrap(); + + // deploy recipient contract + let account_contract = declare("SimpleAccount").unwrap(); + let (recipient, _) = account_contract + .deploy( + @array![883045738439352841478194533192765345509759306772397516907181243450667673002] + ) + .unwrap(); + + // mint a new token + let dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + dispatcher.mint(recipient, 1.try_into().unwrap()); + + // deploy account contract + let account_contract = declare("AccountPreset").unwrap(); + let mut acct_constructor_calldata = array![erc721_contract_address.try_into().unwrap(), 1, 0]; + let (account_contract_address, _) = account_contract + .deploy(@acct_constructor_calldata) + .unwrap(); + + (account_contract_address, erc721_contract_address) +} + +#[test] +fn test_is_valid_signer() { + let (contract_address, erc721_contract_address) = __setup__(); + + let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; + start_cheat_caller_address(contract_address, token_owner); + let is_valid_signer = signatory_dispatcher.is_valid_signer(token_owner); + + assert(is_valid_signer == true, 'should accept valid signature'); + stop_cheat_caller_address(contract_address); +} + + +#[test] +fn test_is_valid_signature() { + let (contract_address, erc721_contract_address) = __setup__(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + + let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; + + start_cheat_caller_address(contract_address, token_owner); + let mut good_signature = array![data.r, data.s]; + let is_valid = signatory_dispatcher.is_valid_signature(hash, good_signature.span()); + assert(is_valid == 'VALID', 'should accept valid signature'); + stop_cheat_caller_address(contract_address); +} + From 5ab435a652768a31a1543cb90122a151ee9b6969 Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Mon, 2 Sep 2024 01:41:20 +0100 Subject: [PATCH 09/11] add check for caller address --- src/components/permissionable/permissionable.cairo | 3 +++ src/components/presets/account_preset.cairo | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/permissionable/permissionable.cairo b/src/components/permissionable/permissionable.cairo index 4b44751..c7ae7fc 100644 --- a/src/components/permissionable/permissionable.cairo +++ b/src/components/permissionable/permissionable.cairo @@ -51,6 +51,7 @@ pub mod PermissionableComponent { // ************************************************************************* pub mod Errors { pub const INVALID_LENGTH: felt252 = 'Account: invalid length'; + pub const UNAUTHORIZED: felt252 = 'Account: unauthorized'; } @@ -76,6 +77,8 @@ pub mod PermissionableComponent { let account_comp = get_dep_component!(@self, Account); let owner = account_comp.owner(); + assert(owner == get_caller_address(), Errors::UNAUTHORIZED); + let length = permissioned_addresses.len(); let mut index: u32 = 0; while index < length { diff --git a/src/components/presets/account_preset.cairo b/src/components/presets/account_preset.cairo index f31f428..5fa7848 100644 --- a/src/components/presets/account_preset.cairo +++ b/src/components/presets/account_preset.cairo @@ -155,10 +155,6 @@ pub mod AccountPreset { permissioned_addresses: Array, permissions: Array ) { - // validate signer - let caller = get_caller_address(); - assert(self.is_valid_signer(caller), 'Account: unauthorized'); - // set permissions self.permissionable.set_permission(permissioned_addresses, permissions) } From 87735f5741ec2e30fd960ee60c18f0c5ea3517ef Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Tue, 3 Sep 2024 06:51:40 +0100 Subject: [PATCH 10/11] root owner and permissioned address signature test --- tests/test_signatory_component.cairo | 64 ++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_signatory_component.cairo b/tests/test_signatory_component.cairo index a3b1147..f13f8cd 100644 --- a/tests/test_signatory_component.cairo +++ b/tests/test_signatory_component.cairo @@ -31,6 +31,7 @@ use token_bound_accounts::components::account::account::AccountComponent; use token_bound_accounts::components::signatory::signatory::SignatoryComponent; +use token_bound_accounts::components::permissionable::permissionable::PermissionableComponent; use token_bound_accounts::test_helper::{ hello_starknet::{IHelloStarknetDispatcher, IHelloStarknetDispatcherTrait, HelloStarknet}, @@ -85,6 +86,7 @@ fn __setup__() -> (ContractAddress, ContractAddress) { // mint a new token let dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; dispatcher.mint(recipient, 1.try_into().unwrap()); + dispatcher.mint(recipient, 2.try_into().unwrap()); // deploy account contract let account_contract = declare("AccountPreset").unwrap(); @@ -110,6 +112,49 @@ fn test_is_valid_signer() { stop_cheat_caller_address(contract_address); } +#[test] +fn test_is_valid_signer_permissioned_address() { + let (contract_address, erc721_contract_address) = __setup__(); + + let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + let permissionable_dispatcher = IPermissionableDispatcher { contract_address }; + let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; + + let mut permission_addresses = ArrayTrait::new(); + permission_addresses.append(ACCOUNT2.try_into().unwrap()); + permission_addresses.append(ACCOUNT3.try_into().unwrap()); + + let mut permissions = ArrayTrait::new(); + permissions.append(true); + permissions.append(true); + + start_cheat_caller_address(contract_address, token_owner); + permissionable_dispatcher.set_permission(permission_addresses, permissions); + let is_valid_signer = signatory_dispatcher.is_valid_signer(ACCOUNT2.try_into().unwrap()); + + assert(is_valid_signer == true, 'should accept valid signer'); + stop_cheat_caller_address(contract_address); +} + +#[test] +fn test_is_valid_signer_root_owner() { + let (contract_address, erc721_contract_address) = __setup__(); + + let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + let token_owner1 = token_dispatcher.ownerOf(1.try_into().unwrap()); + token_dispatcher.ownerOf(2.try_into().unwrap()); + + let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; + + start_cheat_caller_address(contract_address, token_owner1); + + let is_valid_signer = signatory_dispatcher.is_valid_signer(token_owner1); + + assert(is_valid_signer == true, 'should accept valid signer'); + stop_cheat_caller_address(contract_address); +} + #[test] fn test_is_valid_signature() { @@ -129,3 +174,22 @@ fn test_is_valid_signature() { stop_cheat_caller_address(contract_address); } +#[test] +fn test_is_valid_signature_root_owner() { + let (contract_address, erc721_contract_address) = __setup__(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + + let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; + let token_owner1 = token_dispatcher.ownerOf(1.try_into().unwrap()); + token_dispatcher.ownerOf(2.try_into().unwrap()); + let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; + + start_cheat_caller_address(contract_address, token_owner1); + let mut good_signature = array![data.r, data.s]; + let is_valid = signatory_dispatcher.is_valid_signature(hash, good_signature.span()); + assert(is_valid == 'VALID', 'should accept valid signature'); + stop_cheat_caller_address(contract_address); +} + From a7f2d898dc8ace81595870cd8da1c1be0098cfb6 Mon Sep 17 00:00:00 2001 From: mubarak23 Date: Tue, 3 Sep 2024 13:54:21 +0100 Subject: [PATCH 11/11] fix test functions base on review requested --- src/components/account/account.cairo | 10 +++++++++- src/interfaces/IAccount.cairo | 3 +++ tests/test_signatory_component.cairo | 27 +++++++++++++++------------ 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/components/account/account.cairo b/src/components/account/account.cairo index a5120c1..2dbb1ef 100644 --- a/src/components/account/account.cairo +++ b/src/components/account/account.cairo @@ -111,6 +111,12 @@ pub mod AccountComponent { return false; } } + + fn get_root_owner( + self: @ComponentState, token_contract: ContractAddress, token_id: u256 + ) -> ContractAddress { + self._get_root_owner(token_contract, token_id) + } } // ************************************************************************* @@ -210,9 +216,11 @@ pub mod AccountComponent { self: @ComponentState, token_contract: ContractAddress, token_id: u256 ) -> ContractAddress { // TODO: implement logic to get root owner - 123.try_into().unwrap() + + 1.try_into().unwrap() } + /// @notice internal transaction for returning the contract address and token ID of the NFT fn _get_token(self: @ComponentState) -> (ContractAddress, u256, felt252) { let contract = self.account_token_contract.read(); diff --git a/src/interfaces/IAccount.cairo b/src/interfaces/IAccount.cairo index 8f5e26d..50fbc6d 100644 --- a/src/interfaces/IAccount.cairo +++ b/src/interfaces/IAccount.cairo @@ -15,4 +15,7 @@ pub trait IAccount { fn owner(self: @TContractState) -> ContractAddress; fn state(self: @TContractState) -> u256; fn supports_interface(self: @TContractState, interface_id: felt252) -> bool; + fn get_root_owner( + self: @TContractState, token_contract: ContractAddress, token_id: u256 + ) -> ContractAddress; } diff --git a/tests/test_signatory_component.cairo b/tests/test_signatory_component.cairo index f13f8cd..e2f52b5 100644 --- a/tests/test_signatory_component.cairo +++ b/tests/test_signatory_component.cairo @@ -142,14 +142,15 @@ fn test_is_valid_signer_root_owner() { let (contract_address, erc721_contract_address) = __setup__(); let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; - let token_owner1 = token_dispatcher.ownerOf(1.try_into().unwrap()); - token_dispatcher.ownerOf(2.try_into().unwrap()); + let account_dispatcher = IAccountDispatcher { contract_address }; + let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + let root_owner = account_dispatcher.get_root_owner(token_owner, 1.try_into().unwrap()); let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; - start_cheat_caller_address(contract_address, token_owner1); + start_cheat_caller_address(contract_address, root_owner); - let is_valid_signer = signatory_dispatcher.is_valid_signer(token_owner1); + let is_valid_signer = signatory_dispatcher.is_valid_signer(root_owner); assert(is_valid_signer == true, 'should accept valid signer'); stop_cheat_caller_address(contract_address); @@ -175,21 +176,23 @@ fn test_is_valid_signature() { } #[test] -fn test_is_valid_signature_root_owner() { +fn test_is_valid_signature1_root_owner() { let (contract_address, erc721_contract_address) = __setup__(); - let data = SIGNED_TX_DATA(); let hash = data.transaction_hash; - let token_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address }; - let token_owner1 = token_dispatcher.ownerOf(1.try_into().unwrap()); - token_dispatcher.ownerOf(2.try_into().unwrap()); + let account_dispatcher = IAccountDispatcher { contract_address }; + let token_owner = token_dispatcher.ownerOf(1.try_into().unwrap()); + + let root_owner = account_dispatcher.get_root_owner(token_owner, 1.try_into().unwrap()); let signatory_dispatcher = ISignatoryDispatcher { contract_address: contract_address }; - start_cheat_caller_address(contract_address, token_owner1); + start_cheat_caller_address(contract_address, root_owner); + let mut good_signature = array![data.r, data.s]; - let is_valid = signatory_dispatcher.is_valid_signature(hash, good_signature.span()); - assert(is_valid == 'VALID', 'should accept valid signature'); + let is_valid_signer = signatory_dispatcher.is_valid_signature(hash, good_signature.span()); + assert(is_valid_signer == 'VALID', 'should accept valid signature'); + stop_cheat_caller_address(contract_address); }