From b997970bc395cde591668572ada434c410be76f5 Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Tue, 26 Nov 2024 01:43:50 +0530 Subject: [PATCH] fix: added limit as a settable variable and removed unused imports --- src/interface.cairo | 3 ++ src/lib.cairo | 27 ++++++++++---- tests/common.cairo | 2 +- tests/test_channel_alias.cairo | 6 +-- tests/test_channel_delegate.cairo | 8 ++-- tests/test_channel_settings.cairo | 10 ++--- tests/test_push_admin.cairo | 59 +++++++++++++++++++++++++++++- tests/test_send_notification.cairo | 8 ++-- tests/test_user_operations.cairo | 8 ++-- 9 files changed, 100 insertions(+), 31 deletions(-) diff --git a/src/interface.cairo b/src/interface.cairo index 62907c5..c3b625c 100644 --- a/src/interface.cairo +++ b/src/interface.cairo @@ -5,6 +5,8 @@ pub trait IPushComm { // Push Admin fn set_push_governance_address(ref self: TContractState, governance_address: ContractAddress); fn set_push_token_address(ref self: TContractState, push_token_address: ContractAddress); + fn set_chain_name(ref self: TContractState, chain_name: felt252); + fn set_identity_bytes_limit(ref self: TContractState, limit: usize); // Channel fn verify_channel_alias(ref self: TContractState, channel_address: EthAddress); @@ -35,6 +37,7 @@ pub trait IPushComm { ) -> bool; fn users_count(self: @TContractState) -> u256; fn chain_name(self: @TContractState) -> felt252; + fn identity_bytes_limit(self: @TContractState) -> usize; fn push_token_address(self: @TContractState) -> ContractAddress; fn push_governance_address(self: @TContractState) -> ContractAddress; fn user_to_channel_notifs(self: @TContractState, user: ContractAddress, channel: ContractAddress) -> ByteArray; diff --git a/src/lib.cairo b/src/lib.cairo index b1e9793..3021301 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -50,6 +50,8 @@ pub mod PushComm { push_token_address: ContractAddress, // Chain Info chain_name: felt252, + // identity bytes limit + MAX_IDENTITY_BYTES_LIMIT: usize } #[starknet::storage_node] @@ -149,6 +151,7 @@ pub mod PushComm { self.ownable.initializer(owner); self.chain_name.write(chain_name); self.governance.write(push_governance); + self.MAX_IDENTITY_BYTES_LIMIT.write(9145); } #[abi(embed_v0)] @@ -261,12 +264,9 @@ pub mod PushComm { recipient: ContractAddress, identity: ByteArray ) -> bool { - // Define the maximum allowed bytes based on felts limit - let MAX_IDENTITY_BYTES_LIMIT: usize = 9145; // equivalent to 295 felts - // Check that the identity length is within the limit let identity_length = identity.len(); - if identity_length > MAX_IDENTITY_BYTES_LIMIT { + if identity_length > self.MAX_IDENTITY_BYTES_LIMIT.read() { return false; } @@ -321,13 +321,10 @@ pub mod PushComm { let modified_notif_settings = format!("@{}+@{}", notif_id, notif_settings); - // Define the maximum allowed bytes based on felts limit - let MAX_IDENTITY_BYTES_LIMIT: usize = 9145; // equivalent to 295 felts - // Check that the notif_settings length is within the limit let modified_notif_settings_length = modified_notif_settings.len(); assert!( - modified_notif_settings_length <= MAX_IDENTITY_BYTES_LIMIT, "notif_settings exceeds limit" + modified_notif_settings_length <= self.MAX_IDENTITY_BYTES_LIMIT.read(), "notif_settings exceeds limit" ); self @@ -400,6 +397,16 @@ pub mod PushComm { self.push_token_address.write(push_token_address); } + fn set_chain_name(ref self: ContractState, chain_name: felt252) { + self.ownable.assert_only_owner(); + self.chain_name.write(chain_name); + } + + fn set_identity_bytes_limit(ref self: ContractState, limit: usize) { + self.ownable.assert_only_owner(); + self.MAX_IDENTITY_BYTES_LIMIT.write(limit); + } + // Getters Functions fn push_token_address(self: @ContractState) -> ContractAddress { self.push_token_address.read() @@ -423,6 +430,10 @@ pub mod PushComm { self.chain_name.read() } + fn identity_bytes_limit(self: @ContractState) -> usize { + self.MAX_IDENTITY_BYTES_LIMIT.read() + } + fn user_to_channel_notifs(self: @ContractState, user: ContractAddress, channel: ContractAddress) -> ByteArray { self.user_to_channel_notifs.entry(user).entry(channel).read() } diff --git a/tests/common.cairo b/tests/common.cairo index 9c4e61e..97c56b3 100644 --- a/tests/common.cairo +++ b/tests/common.cairo @@ -1,5 +1,5 @@ use starknet::ContractAddress; -use snforge_std::{declare, ContractClassTrait, cheat_caller_address, CheatSpan}; +use snforge_std::{declare, ContractClassTrait}; // Constants pub fn PUSH_ADMIN() -> ContractAddress { diff --git a/tests/test_channel_alias.cairo b/tests/test_channel_alias.cairo index 06d1873..1dd7b25 100644 --- a/tests/test_channel_alias.cairo +++ b/tests/test_channel_alias.cairo @@ -1,8 +1,8 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{EthAddress}; use snforge_std::{ - declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events, - EventSpyAssertionsTrait, Event, EventSpyTrait + cheat_caller_address, CheatSpan, spy_events, + EventSpyAssertionsTrait }; use push_comm::{PushComm, interface::IPushCommDispatcher, interface::IPushCommDispatcherTrait}; use super::common::{USER_1, deploy_contract, CHAIN_NAME}; diff --git a/tests/test_channel_delegate.cairo b/tests/test_channel_delegate.cairo index de60bbc..db24917 100644 --- a/tests/test_channel_delegate.cairo +++ b/tests/test_channel_delegate.cairo @@ -1,11 +1,11 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress}; use snforge_std::{ - declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events, - EventSpyAssertionsTrait, Event, EventSpyTrait + cheat_caller_address, CheatSpan, spy_events, + EventSpyAssertionsTrait }; use push_comm::{PushComm, interface::IPushCommDispatcher, interface::IPushCommDispatcherTrait}; -use super::common::{USER_1, deploy_contract, CHAIN_NAME}; +use super::common::{USER_1, deploy_contract}; #[test] diff --git a/tests/test_channel_settings.cairo b/tests/test_channel_settings.cairo index 9dda4d6..a1559cf 100644 --- a/tests/test_channel_settings.cairo +++ b/tests/test_channel_settings.cairo @@ -1,11 +1,11 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress}; use snforge_std::{ - declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events, - EventSpyAssertionsTrait, Event, EventSpyTrait + cheat_caller_address, CheatSpan, spy_events, + EventSpyAssertionsTrait }; use push_comm::{PushComm, interface::IPushCommDispatcher, interface::IPushCommDispatcherTrait}; -use super::common::{USER_1, deploy_contract, CHAIN_NAME}; +use super::common::{USER_1, deploy_contract}; #[test] fn test_channel_channel_user_settings() { @@ -25,7 +25,7 @@ fn test_channel_channel_user_settings() { push_comm.change_user_channel_settings(CHANNEL_ADDRESS, notif_id, notif_settings.clone()); let modified_notif_settings = format!("@{}+@{}", notif_id, notif_settings); - let modified_notif_settings_clone = modified_notif_settings.clone(); + // let modified_notif_settings_clone = modified_notif_settings.clone(); let setting_saved = push_comm.user_to_channel_notifs(USER_1(), CHANNEL_ADDRESS); assert(setting_saved == modified_notif_settings, 'Settings saved'); diff --git a/tests/test_push_admin.cairo b/tests/test_push_admin.cairo index 71aced8..446d3a2 100644 --- a/tests/test_push_admin.cairo +++ b/tests/test_push_admin.cairo @@ -1,6 +1,6 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress}; -use snforge_std::{declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events}; +use snforge_std::{cheat_caller_address, CheatSpan}; use push_comm::interface::{IPushCommDispatcher, IPushCommDispatcherTrait}; use super::common::{USER_1, PUSH_ADMIN, deploy_contract}; @@ -62,3 +62,58 @@ fn test_non_admin_set_push_token_address() { push_comm.set_push_governance_address(TOKEN_ADDRESS); } +#[test] +fn test_admin_set_chain_name() { + let contract_address = deploy_contract(); + let push_comm = IPushCommDispatcher { contract_address }; + + let chain_name: felt252 = 'starknet2'.try_into().unwrap(); + + // admin sets the migration status + cheat_caller_address(contract_address, PUSH_ADMIN(), CheatSpan::TargetCalls(1)); + push_comm.set_chain_name(chain_name); + + let updated_chain_name = push_comm.chain_name(); + assert(chain_name == updated_chain_name, 'Chain Name Update Failed'); +} + +#[test] +#[should_panic(expected: 'Caller is not the owner')] +fn test_non_admin_set_chain_name() { + let contract_address = deploy_contract(); + let push_comm = IPushCommDispatcher { contract_address }; + + let chain_name: felt252 = 'starknet2'.try_into().unwrap(); + + // admin sets the migration status + cheat_caller_address(contract_address, USER_1(), CheatSpan::TargetCalls(1)); + push_comm.set_chain_name(chain_name); +} + +#[test] +fn test_admin_set_identity_bytes_limit() { + let contract_address = deploy_contract(); + let push_comm = IPushCommDispatcher { contract_address }; + + let identity_bytes_limit: usize = 9200_usize; + + // admin sets the migration status + cheat_caller_address(contract_address, PUSH_ADMIN(), CheatSpan::TargetCalls(1)); + push_comm.set_identity_bytes_limit(identity_bytes_limit); + + let updated_identity_bytes_limit = push_comm.identity_bytes_limit(); + assert(identity_bytes_limit == updated_identity_bytes_limit, 'Identity bytes Update Failed'); +} + +#[test] +#[should_panic(expected: 'Caller is not the owner')] +fn test_non_admin_set_identity_bytes_limit() { + let contract_address = deploy_contract(); + let push_comm = IPushCommDispatcher { contract_address }; + + let identity_bytes_limit: usize = 9200_usize; + + // admin sets the migration status + cheat_caller_address(contract_address, USER_1(), CheatSpan::TargetCalls(1)); + push_comm.set_identity_bytes_limit(identity_bytes_limit); +} \ No newline at end of file diff --git a/tests/test_send_notification.cairo b/tests/test_send_notification.cairo index c846e5b..b5f9a2e 100644 --- a/tests/test_send_notification.cairo +++ b/tests/test_send_notification.cairo @@ -1,11 +1,11 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress}; use snforge_std::{ - declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events, - EventSpyAssertionsTrait, Event, EventSpyTrait + cheat_caller_address, CheatSpan, spy_events, + EventSpyAssertionsTrait }; use push_comm::{PushComm, interface::IPushCommDispatcher, interface::IPushCommDispatcherTrait}; -use super::common::{USER_1, deploy_contract, CHAIN_NAME}; +use super::common::{USER_1, deploy_contract}; #[test] diff --git a/tests/test_user_operations.cairo b/tests/test_user_operations.cairo index 29d2f91..4979bc5 100644 --- a/tests/test_user_operations.cairo +++ b/tests/test_user_operations.cairo @@ -1,11 +1,11 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress}; use snforge_std::{ - declare, ContractClassTrait, cheat_caller_address, CheatSpan, spy_events, - EventSpyAssertionsTrait, Event, EventSpyTrait + cheat_caller_address, CheatSpan, spy_events, + EventSpyAssertionsTrait }; use push_comm::{PushComm, interface::IPushCommDispatcher, interface::IPushCommDispatcherTrait}; -use super::common::{USER_1, deploy_contract, CHAIN_NAME}; +use super::common::{USER_1, deploy_contract}; #[test]