From 732741d5da86858469c94a97952e528e7a20b598 Mon Sep 17 00:00:00 2001 From: sprtd Date: Thu, 24 Oct 2024 20:36:49 +0100 Subject: [PATCH] refac: replace ERC20 LegacyMap with Map --- crates/cheatnet/tests/cheatcodes/store.rs | 2 +- .../map_simple_value_simple_key.cairo | 8 ++-- crates/forge/tests/data/contracts/erc20.cairo | 24 +++++----- .../tests/data/erc20_package/src/erc20.cairo | 16 +++---- crates/forge/tests/integration/test_state.rs | 6 ++- crates/sncast/README.md | 25 ++++------ .../tests/data/contracts/map/src/lib.cairo | 14 ++++-- .../map_script/contracts/src/lib.cairo | 10 +++- .../state_script/contracts/src/lib.cairo | 9 ++-- design_documents/stark_curve_signatures.md | 8 ++-- design_documents/store_load_cheatcodes.md | 46 ++++++++++--------- 11 files changed, 92 insertions(+), 76 deletions(-) diff --git a/crates/cheatnet/tests/cheatcodes/store.rs b/crates/cheatnet/tests/cheatcodes/store.rs index 64d9f6b3b9..cdcea8fa33 100644 --- a/crates/cheatnet/tests/cheatcodes/store.rs +++ b/crates/cheatnet/tests/cheatcodes/store.rs @@ -60,4 +60,4 @@ fn store_state_map_simple_value() { test_env.call_contract(&contract_address, "read", &[map_key]), &[inserted_value.into()], ); -} +} \ No newline at end of file diff --git a/crates/cheatnet/tests/contracts/src/store_load/map_simple_value_simple_key.cairo b/crates/cheatnet/tests/contracts/src/store_load/map_simple_value_simple_key.cairo index 3e125a0ad4..35b57b246c 100644 --- a/crates/cheatnet/tests/contracts/src/store_load/map_simple_value_simple_key.cairo +++ b/crates/cheatnet/tests/contracts/src/store_load/map_simple_value_simple_key.cairo @@ -1,17 +1,19 @@ #[starknet::contract] mod MapSimpleValueSimpleKey { + use starknet::{ + storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map}}; #[storage] struct Storage { - values: LegacyMap, + values: Map, } #[external(v0)] fn insert(ref self: ContractState, key: felt252, value: felt252) { - self.values.write(key, value); + self.values.entry(key).write(value); } #[external(v0)] fn read(self: @ContractState, key: felt252) -> felt252 { - self.values.read(key) + self.values.entry(key).read() } } diff --git a/crates/forge/tests/data/contracts/erc20.cairo b/crates/forge/tests/data/contracts/erc20.cairo index 45dc482068..2a8691d2b0 100644 --- a/crates/forge/tests/data/contracts/erc20.cairo +++ b/crates/forge/tests/data/contracts/erc20.cairo @@ -21,10 +21,10 @@ trait IERC20 { #[starknet::contract] mod ERC20 { + use starknet::{get_caller_address, contract_address_const, ContractAddress, + storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map}, + }; use zeroable::Zeroable; - use starknet::get_caller_address; - use starknet::contract_address_const; - use starknet::ContractAddress; #[storage] struct Storage { @@ -32,8 +32,8 @@ mod ERC20 { symbol: felt252, decimals: u8, total_supply: u256, - balances: LegacyMap::, - allowances: LegacyMap::<(ContractAddress, ContractAddress), u256>, + balances: Map, + allowances: Map<(ContractAddress, ContractAddress), u256>, } #[event] @@ -69,7 +69,7 @@ mod ERC20 { self.decimals.write(decimals_); assert(!recipient.is_zero(), 'ERC20: mint to the 0 address'); self.total_supply.write(initial_supply); - self.balances.write(recipient, initial_supply); + self.balances.entry(recipient).write(initial_supply); self .emit( Event::Transfer( @@ -99,13 +99,13 @@ mod ERC20 { } fn balance_of(self: @ContractState, account: ContractAddress) -> u256 { - self.balances.read(account) + self.balances.entry(account).read() } fn allowance( self: @ContractState, owner: ContractAddress, spender: ContractAddress ) -> u256 { - self.allowances.read((owner, spender)) + self.allowances.entry((owner, spender)).read() } fn transfer(ref self: ContractState, recipient: ContractAddress, amount: u256) { @@ -160,8 +160,8 @@ mod ERC20 { ) { assert(!sender.is_zero(), 'ERC20: transfer from 0'); assert(!recipient.is_zero(), 'ERC20: transfer to 0'); - self.balances.write(sender, self.balances.read(sender) - amount); - self.balances.write(recipient, self.balances.read(recipient) + amount); + self.balances.entry(sender).write(self.balances.entry(sender).read() - amount); + self.balances.entry(recipient).write(self.balances.entry(recipient).read() + amount); self.emit(Event::Transfer(Transfer { from: sender, to: recipient, value: amount })); } @@ -181,8 +181,8 @@ mod ERC20 { ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256 ) { assert(!spender.is_zero(), 'ERC20: approve from 0'); - self.allowances.write((owner, spender), amount); + self.allowances.entry((owner, spender)).write(amount); self.emit(Event::Approval(Approval { owner, spender, value: amount })); } } -} \ No newline at end of file +} diff --git a/crates/forge/tests/data/erc20_package/src/erc20.cairo b/crates/forge/tests/data/erc20_package/src/erc20.cairo index caf99b269b..4b219662c3 100644 --- a/crates/forge/tests/data/erc20_package/src/erc20.cairo +++ b/crates/forge/tests/data/erc20_package/src/erc20.cairo @@ -21,19 +21,17 @@ trait IERC20 { #[starknet::contract] mod ERC20 { + use starknet::{ contract_address_const, get_caller_address, ContractAddress, + storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map}, }; use zeroable::Zeroable; - use starknet::get_caller_address; - use starknet::contract_address_const; - use starknet::ContractAddress; - #[storage] struct Storage { name: felt252, symbol: felt252, decimals: u8, total_supply: u256, - balances: LegacyMap::, - allowances: LegacyMap::<(ContractAddress, ContractAddress), u256>, + balances: Map, + allowances: Map<(ContractAddress, ContractAddress), u256>, } #[event] @@ -69,7 +67,7 @@ mod ERC20 { self.decimals.write(decimals_); assert(!recipient.is_zero(), 'ERC20: mint to the 0 address'); self.total_supply.write(initial_supply); - self.balances.write(recipient, initial_supply); + self.balances.entry(recipient).write(initial_supply); self .emit( Event::Transfer( @@ -160,8 +158,8 @@ mod ERC20 { ) { assert(!sender.is_zero(), 'ERC20: transfer from 0'); assert(!recipient.is_zero(), 'ERC20: transfer to 0'); - self.balances.write(sender, self.balances.read(sender) - amount); - self.balances.write(recipient, self.balances.read(recipient) + amount); + self.balances.entry(sender).write(self.balances.read(sender) - amount); + self.balances.entry(recipient).write(self.balances.read(recipient) + amount); self.emit(Event::Transfer(Transfer { from: sender, to: recipient, value: amount })); } diff --git a/crates/forge/tests/integration/test_state.rs b/crates/forge/tests/integration/test_state.rs index 64f9aa45a5..1cfc50da22 100644 --- a/crates/forge/tests/integration/test_state.rs +++ b/crates/forge/tests/integration/test_state.rs @@ -415,7 +415,7 @@ fn storage_access_default_values() { #[storage] struct Storage { balance: felt252, - legacy_map: LegacyMap, + legacy_map: Map, custom_struct: CustomStruct, } } @@ -427,7 +427,7 @@ fn storage_access_default_values() { let default_felt252 = state.balance.read(); assert(default_felt252 == 0, 'Incorrect storage value'); - let default_map_value = state.legacy_map.read(22); + let default_map_value = state.legacy_map.entry(22).read(); assert(default_map_value == 0, 'Incorrect map value'); let default_custom_struct = state.custom_struct.read(); @@ -859,3 +859,5 @@ fn felt252_dict_usage() { assert_passed(&result); } + + diff --git a/crates/sncast/README.md b/crates/sncast/README.md index 0b92258188..857f62bb34 100644 --- a/crates/sncast/README.md +++ b/crates/sncast/README.md @@ -7,14 +7,15 @@ Note, that `sncast` only officially supports contracts written in Cairo 2. ## Table of contents - * [Installation](#installation) - * [Documentation](#documentation) - * [Example usages](#example-usages) - * [Declaring contracts](#declare-a-contract) - * [Deploying contracts](#deploy-a-contract) - * [Invoking contracts](#invoke-a-contract) - * [Calling contracts](#call-a-contract) - * [Development](#development) + +- [Installation](#installation) +- [Documentation](#documentation) +- [Example usages](#example-usages) + - [Declaring contracts](#declare-a-contract) + - [Deploying contracts](#deploy-a-contract) + - [Invoking contracts](#invoke-a-contract) + - [Calling contracts](#call-a-contract) +- [Development](#development) ## Installation @@ -23,7 +24,7 @@ You can download latest version of `sncast` [here](https://github.com/foundry-rs ## Documentation -For more details on Starknet Foundry `sncast`, please visit [our docs](https://foundry-rs.github.io/starknet-foundry/starknet/index.html) +For more details on Starknet Foundry `sncast`, please visit [our docs](https://foundry-rs.github.io/starknet-foundry/starknet/index.html) ## Example usages @@ -42,7 +43,6 @@ class_hash: 0x8448a68b5ea1affc45e3fd4b8b480ea36a51dc34e337a16d2567d32d0c6f8a transaction_hash: 0x7ad0d6e449e33b6581a4bb8df866c0fce3919a5ee05a30840ba521dafee217f ``` - With arguments taken from `snfoundry.toml` file (default profile name): ```shell @@ -66,7 +66,6 @@ contract_address: 0x301316d47a81b39c5e27cca4a7b8ca4773edbf1103218588d6da4d3ed530 transaction_hash: 0x64a62a000240e034d1862c2bbfa154aac6a8195b4b2e570f38bf4fd47a5ab1e ``` - With arguments taken from `snfoundry.toml` file (default profile name): ```shell @@ -77,7 +76,6 @@ contract_address: 0x301316d47a81b39c5e27cca4a7b8ca4773edbf1103218588d6da4d3ed530 transaction_hash: 0x64a62a000240e034d1862c2bbfa154aac6a8195b4b2e570f38bf4fd47a5ab1e ``` - ### Invoke a contract ```shell @@ -92,7 +90,6 @@ command: Invoke transaction_hash: 0x7ad0d6e449e33b6581a4bb8df866c0fce3919a5ee05a30840ba521dafee217f ``` - With arguments taken from `snfoundry.toml` file (default profile name): ```shell @@ -118,7 +115,6 @@ command: call response: [0x0] ``` - With arguments taken from `snfoundry.toml` file (default profile name): ```shell @@ -131,7 +127,6 @@ command: call response: [0x0] ``` - ## Development Refer to [documentation](https://foundry-rs.github.io/starknet-foundry/development/environment-setup.html) to make sure you have all the pre-requisites, and to obtain an information on how to help to develop `sncast`. diff --git a/crates/sncast/tests/data/contracts/map/src/lib.cairo b/crates/sncast/tests/data/contracts/map/src/lib.cairo index aa849d5799..f82b5776f9 100644 --- a/crates/sncast/tests/data/contracts/map/src/lib.cairo +++ b/crates/sncast/tests/data/contracts/map/src/lib.cairo @@ -7,19 +7,25 @@ trait IMap { #[starknet::contract] mod Map { + use starknet::{ + storage::{ + StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, + StoragePathEntry, Map + } + }; #[storage] struct Storage { - storage: LegacyMap::, + storage: Map, } #[abi(embed_v0)] - impl Map of super::IMap { + impl MapImpl of super::IMap { fn put(ref self: ContractState, key: felt252, value: felt252) { - self.storage.write(key, value); + self.storage.entry(key).write(value); } fn get(self: @ContractState, key: felt252) -> felt252 { - self.storage.read(key) + self.storage.entry(key).read() } } } diff --git a/crates/sncast/tests/data/scripts/map_script/contracts/src/lib.cairo b/crates/sncast/tests/data/scripts/map_script/contracts/src/lib.cairo index 62ac3789d5..a77c4af280 100644 --- a/crates/sncast/tests/data/scripts/map_script/contracts/src/lib.cairo +++ b/crates/sncast/tests/data/scripts/map_script/contracts/src/lib.cairo @@ -9,7 +9,10 @@ trait IMap { #[starknet::contract] mod Mapa { use starknet::{ - storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map} + storage::{ + StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, + StoragePathEntry, Map + } }; #[storage] @@ -36,7 +39,10 @@ mod Mapa { #[starknet::contract] mod Mapa2 { use starknet::{ - storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map} + storage::{ + StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, + StoragePathEntry, Map + } }; #[storage] diff --git a/crates/sncast/tests/data/scripts/state_script/contracts/src/lib.cairo b/crates/sncast/tests/data/scripts/state_script/contracts/src/lib.cairo index 5621017c4f..d26ee8888f 100644 --- a/crates/sncast/tests/data/scripts/state_script/contracts/src/lib.cairo +++ b/crates/sncast/tests/data/scripts/state_script/contracts/src/lib.cairo @@ -8,19 +8,22 @@ trait IState { #[starknet::contract] mod State { + use starknet::{ + storage::{StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, StoragePathEntry, Map}}; + #[storage] struct Storage { - storage: LegacyMap::, + storage: Map, } #[abi(embed_v0)] impl State of super::IState { fn put(ref self: ContractState, key: felt252, value: felt252) { - self.storage.write(key, value); + self.storage.entry(key).write(value); } fn get(self: @ContractState, key: felt252) -> felt252 { - self.storage.read(key) + self.storage.entry(key).read() } fn dummy(self: @ContractState) -> felt252 { diff --git a/design_documents/stark_curve_signatures.md b/design_documents/stark_curve_signatures.md index 60d63129ee..e783d78c24 100644 --- a/design_documents/stark_curve_signatures.md +++ b/design_documents/stark_curve_signatures.md @@ -3,7 +3,7 @@ ## Context Some users would like to have a way of generating and signing with the `stark curve`. It would be useful for testing -custom account implementations. +custom account implementations. ## Existing solutions @@ -17,7 +17,7 @@ My proposal would be to introduce a `StarkCurveKeyPair` struct which would imple ```cairo struct StarkCurveKeyPair { private_key: felt252, - public_key: felt252 + public_key: felt252 } trait StarkCurveKeyPairTrait { @@ -36,9 +36,9 @@ use snforge_std::{StarkCurveKeyPair, StarkCurveKeyPairTrait}; fn test_stark_curve() { let mut key_pair = StarkCurveKeyPairTrait::generate(); let message_hash = 12345; - + let signature = key_pair.sign(message_hash); - + assert(key_pair.verify(message_hash, signature), 'Signature is incorrect'); } ``` diff --git a/design_documents/store_load_cheatcodes.md b/design_documents/store_load_cheatcodes.md index 58d8956b19..de27634bda 100644 --- a/design_documents/store_load_cheatcodes.md +++ b/design_documents/store_load_cheatcodes.md @@ -1,26 +1,27 @@ # `store/load` Cheatcodes ## Context + People might want to manipulate the storage of contracts from the test context. -For example: we might want to modify the `authority` of the contract, without actually having an exposed endpoint in the contract, +For example: we might want to modify the `authority` of the contract, without actually having an exposed endpoint in the contract, since it might impose some security issues. Having the function, to modify the stored `authority` will allow to change it, without re-deploying the contract. -Also - some specific storage variables may not be exposed directly to the user via the contracts' interface, since it +Also - some specific storage variables may not be exposed directly to the user via the contracts' interface, since it would bloat the interface and would not be needed for anything but tests (which is generally an antipattern in programming). ## Existing solutions The [store](https://book.getfoundry.sh/cheatcodes/store) and [load](https://book.getfoundry.sh/cheatcodes/load) cheatcodes known from foundry, -provide the functionality to store and load memory in the VM for the given contract. +provide the functionality to store and load memory in the VM for the given contract. Having the correct format of data (conversion), is up to the user, since the interface accepts bytes, and returns bytes as well. - ## Proposed solution -My proposal would be to use the generated `contract_state_for_testing` and leverage it's typed structure, to -enable the users to get the address of the variable, via the `.address()` function, and -implement a `store/load` functions, which use the: +My proposal would be to use the generated `contract_state_for_testing` and leverage it's typed structure, to +enable the users to get the address of the variable, via the `.address()` function, and +implement a `store/load` functions, which use the: + - Contract address - Calculated variable address - Value (in case of `store`) @@ -31,7 +32,9 @@ implement a `store/load` functions, which use the: 2. Implementing & importing `storage_access` for the objects we want to write (needs to be implemented anyway, just a bit inconvenient) ## Example usage + ### Contract + ```cairo #[starknet::contract] mod HelloStarknet { @@ -43,12 +46,13 @@ mod HelloStarknet { #[storage] struct Storage { - balance: felt252, + balance: felt252, map: LegacyMap, custom_struct: CustomStruct, } } ``` + ### Test ```cairo @@ -74,45 +78,45 @@ fn test_felt252() { let contract_address = deploy_hello_contract(); let state = HelloStarknet::contract_state_for_testing(); let variable_address = state.balance.address(); - + let new_balance = 420; store::(contract_address, variable_address, new_balance); let stored_value = load::(contract_address, variable_address); - - assert(stored_value == 420, 'Wrong balance stored'); + + assert(stored_value == 420, 'Wrong balance stored'); } #[test] fn test_legacy_map() { let contract_address = deploy_hello_contract(); let state = HelloStarknet::contract_state_for_testing(); - - + + let key = 420; let value = 69; let variable_address = state.map.address(key); - + store::(contract_address, variable_address, value); let stored_value = load::(contract_address, variable_address); - - assert(stored_value == 69, 'Wrong k:v stored'); + + assert(stored_value == 69, 'Wrong k:v stored'); } #[test] fn test_custom_struct() { let contract_address = deploy_hello_contract(); let state = HelloStarknet::contract_state_for_testing(); - - + + let a = 420; let b = 69; let value = CustomStruct {a, b}; let variable_address = state.custom_struct.address(); - + store::(contract_address, variable_address, value); let stored_value = load::(contract_address, variable_address); - + assert(stored_value.a == 420, 'Wrong custom_struct.a stored'); - assert(stored_value.b == 69, 'Wrong custom_struct.a stored'); + assert(stored_value.b == 69, 'Wrong custom_struct.a stored'); } ```