update scarb version; use openzeppelin's components; fix failing even…

…ts in tests
snapshot-labs · Jun 13, 2024 · 7b65149 · 7b65149
1 parent df4b6fd
commit 7b65149
Show file tree

Hide file tree

Showing 51 changed files with 421 additions and 621 deletions.
diff --git a/starknet/Scarb.lock b/starknet/Scarb.lock
@@ -0,0 +1,14 @@
+# Code generated by scarb DO NOT EDIT.
+version = 1
+
+[[package]]
+name = "openzeppelin"
+version = "0.13.0"
+source = "git+https://github.com/OpenZeppelin/cairo-contracts.git#592095f1cc344b50c269252a09626c678342301f"
+
+[[package]]
+name = "sx"
+version = "0.1.0"
+dependencies = [
+ "openzeppelin",
+]
diff --git a/starknet/Scarb.toml b/starknet/Scarb.toml
@@ -12,7 +12,7 @@ casm-add-pythonic-hints = true
 build-external-contracts = ["openzeppelin::account::account::Account", "openzeppelin::account::interface::AccountABI"]
 
 [dependencies]
-openzeppelin = { git = "https://github.com/snapshot-labs/openzeppelin-cairo-contracts.git", branch = "feat/erc20votes-#631-frozen" }
+openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", version = "0.13.0" }
 starknet = ">=2.2.0"
 
 [scripts]

diff --git a/starknet/src/authenticators/eth_sig.cairo b/starknet/src/authenticators/eth_sig.cairo
@@ -89,14 +89,14 @@ mod EthSigAuthenticator {
     use starknet::{ContractAddress, EthAddress};
     use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait};
     use sx::types::{Strategy, IndexedStrategy, Choice, UserAddress};
-    use sx::utils::{EIP712, LegacyHashEthAddress, LegacyHashUsedSalts, ByteReverse};
+    use sx::utils::{EIP712, LegacyHashEthAddress, ByteReverse};
 
     #[storage]
     struct Storage {
         _used_salts: LegacyMap::<(EthAddress, u256), bool>
     }
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl EthSigAuthenticator of IEthSigAuthenticator<ContractState> {
         fn authenticate_propose(
             ref self: ContractState,

diff --git a/starknet/src/authenticators/eth_tx.cairo b/starknet/src/authenticators/eth_tx.cairo
@@ -69,7 +69,6 @@ mod EthTxAuthenticator {
     use starknet::{ContractAddress, EthAddress, Felt252TryIntoEthAddress, EthAddressIntoFelt252,};
     use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait};
     use sx::types::{UserAddress, Strategy, IndexedStrategy, Choice};
-    use sx::utils::LegacyHashFelt252EthAddress;
     use sx::utils::constants::{PROPOSE_SELECTOR, VOTE_SELECTOR, UPDATE_PROPOSAL_SELECTOR};
 
     #[storage]
@@ -78,7 +77,7 @@ mod EthTxAuthenticator {
         _commits: LegacyMap::<(felt252, EthAddress), bool>
     }
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl EthTxAuthenticator of IEthTxAuthenticator<ContractState> {
         fn authenticate_propose(
             ref self: ContractState,

diff --git a/starknet/src/authenticators/stark_sig.cairo b/starknet/src/authenticators/stark_sig.cairo
@@ -88,7 +88,7 @@ mod StarkSigAuthenticator {
         _used_salts: LegacyMap::<(ContractAddress, felt252), bool>
     }
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl StarkSigAuthenticator of IStarkSigAuthenticator<ContractState> {
         fn authenticate_propose(
             ref self: ContractState,

diff --git a/starknet/src/authenticators/stark_tx.cairo b/starknet/src/authenticators/stark_tx.cairo
@@ -70,7 +70,7 @@ mod StarkTxAuthenticator {
     #[storage]
     struct Storage {}
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl StarkTxAuthenticator of IStarkTxAuthenticator<ContractState> {
         fn authenticate_propose(
             ref self: ContractState,

diff --git a/starknet/src/execution_strategies/eth_relayer.cairo b/starknet/src/execution_strategies/eth_relayer.cairo
@@ -3,6 +3,7 @@ mod EthRelayerExecutionStrategy {
     use starknet::{info, syscalls, EthAddress};
     use sx::interfaces::IExecutionStrategy;
     use sx::types::{Proposal, ProposalStatus};
+    use starknet::SyscallResultTrait;
 
     #[storage]
     struct Storage {}
@@ -20,7 +21,7 @@ mod EthRelayerExecutionStrategy {
     /// * votes_against - The number of votes against the proposal.
     /// * votes_abstain - The number of votes abstaining from the proposal.
     /// * payload - An array containing the serialized L1 execution strategy address and the L1 execution hash.
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl EthRelayerExecutionStrategy of IExecutionStrategy<ContractState> {
         fn execute(
             ref self: ContractState,
@@ -43,9 +44,9 @@ mod EthRelayerExecutionStrategy {
 
             // Decode payload into L1 execution strategy and L1 (keccak) execution hash
             let mut payload = payload.span();
-            let (l1_execution_strategy, l1_execution_hash) = Serde::<(
-                EthAddress, u256
-            )>::deserialize(ref payload)
+            let (l1_execution_strategy, l1_execution_hash) = Serde::<
+                (EthAddress, u256)
+            >::deserialize(ref payload)
                 .unwrap();
 
             // Serialize the payload to be sent to the L1 execution strategy
@@ -58,7 +59,8 @@ mod EthRelayerExecutionStrategy {
             votes_abstain.serialize(ref l1_payload);
             l1_execution_hash.serialize(ref l1_payload);
 
-            syscalls::send_message_to_l1_syscall(l1_execution_strategy.into(), l1_payload.span());
+            syscalls::send_message_to_l1_syscall(l1_execution_strategy.into(), l1_payload.span())
+                .unwrap_syscall();
         }
 
         fn get_strategy_type(self: @ContractState) -> felt252 {

diff --git a/starknet/src/execution_strategies/no_execution_simple_majority.cairo b/starknet/src/execution_strategies/no_execution_simple_majority.cairo
@@ -10,7 +10,7 @@ mod NoExecutionSimpleMajorityExecutionStrategy {
     #[storage]
     struct Storage {}
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl NoExecutionSimpleMajorityExecutionStrategy of IExecutionStrategy<ContractState> {
         fn execute(
             ref self: ContractState,

diff --git a/starknet/src/execution_strategies/timelock.cairo b/starknet/src/execution_strategies/timelock.cairo
@@ -33,17 +33,24 @@ trait ITimelockExecutionStrategy<TContractState> {
 mod TimelockExecutionStrategy {
     use core::zeroable::Zeroable;
     use starknet::{ContractAddress, info, syscalls};
-    use openzeppelin::access::ownable::Ownable;
+    use openzeppelin::access::ownable::OwnableComponent;
     use sx::interfaces::IExecutionStrategy;
     use super::ITimelockExecutionStrategy;
     use sx::types::{Proposal, ProposalStatus};
     use sx::utils::{SimpleQuorum, SpaceManager};
 
+    component!(path: OwnableComponent, storage: ownable, event: OwnableEvent);
+
+    impl OwnableInternalImpl = OwnableComponent::InternalImpl<ContractState>;
+    impl OwnableImpl = OwnableComponent::OwnableImpl<ContractState>;
+
     #[storage]
     struct Storage {
         _timelock_delay: u32,
         _veto_guardian: ContractAddress,
-        _proposal_execution_time: LegacyMap::<felt252, u32>
+        _proposal_execution_time: LegacyMap::<felt252, u32>,
+        #[substorage(v0)]
+        ownable: OwnableComponent::Storage
     }
 
     #[event]
@@ -56,7 +63,9 @@ mod TimelockExecutionStrategy {
         ProposalQueued: ProposalQueued,
         CallExecuted: CallExecuted,
         ProposalExecuted: ProposalExecuted,
-        ProposalVetoed: ProposalVetoed
+        ProposalVetoed: ProposalVetoed,
+        #[flat]
+        OwnableEvent: OwnableComponent::Event
     }
 
     #[derive(Drop, PartialEq, starknet::Event)]
@@ -121,10 +130,7 @@ mod TimelockExecutionStrategy {
         timelock_delay: u32,
         quorum: u256
     ) {
-        // Migration to components planned ; disregard the `unsafe` keyword,
-        // it is actually safe.
-        let mut state = Ownable::unsafe_new_contract_state();
-        Ownable::InternalImpl::initializer(ref state, owner);
+        self.ownable.initializer(owner);
 
         let mut state = SimpleQuorum::unsafe_new_contract_state();
         SimpleQuorum::InternalImpl::initializer(ref state, quorum);
@@ -136,7 +142,7 @@ mod TimelockExecutionStrategy {
         self._veto_guardian.write(veto_guardian);
     }
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl ExecutionStrategy of IExecutionStrategy<ContractState> {
         fn execute(
             ref self: ContractState,
@@ -183,9 +189,7 @@ mod TimelockExecutionStrategy {
                                 )
                             );
                     },
-                    Option::None(()) => {
-                        break;
-                    }
+                    Option::None(()) => { break; }
                 };
             }
         }
@@ -208,7 +212,7 @@ mod TimelockExecutionStrategy {
         }
     }
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl TimelockExecutionStrategy of ITimelockExecutionStrategy<ContractState> {
         fn execute_queued_proposal(ref self: ContractState, mut payload: Span<felt252>) {
             let execution_payload_hash = poseidon::poseidon_hash_span(payload);
@@ -232,9 +236,7 @@ mod TimelockExecutionStrategy {
 
                         self.emit(Event::CallExecuted(CallExecuted { call: call }));
                     },
-                    Option::None(()) => {
-                        break;
-                    }
+                    Option::None(()) => { break; }
                 };
             };
 
@@ -262,10 +264,7 @@ mod TimelockExecutionStrategy {
         }
 
         fn set_veto_guardian(ref self: ContractState, new_veto_guardian: ContractAddress) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let state = Ownable::unsafe_new_contract_state();
-            Ownable::InternalImpl::assert_only_owner(@state);
+            self.ownable.assert_only_owner();
             self._veto_guardian.write(new_veto_guardian);
 
             self
@@ -279,10 +278,7 @@ mod TimelockExecutionStrategy {
         }
 
         fn set_timelock_delay(ref self: ContractState, new_timelock_delay: u32) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let state = Ownable::unsafe_new_contract_state();
-            Ownable::InternalImpl::assert_only_owner(@state);
+            self.ownable.assert_only_owner();
             self._timelock_delay.write(new_timelock_delay);
 
             self
@@ -298,19 +294,13 @@ mod TimelockExecutionStrategy {
         }
 
         fn enable_space(ref self: ContractState, space: ContractAddress) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let state = Ownable::unsafe_new_contract_state();
-            Ownable::InternalImpl::assert_only_owner(@state);
+            self.ownable.assert_only_owner();
             let mut state = SpaceManager::unsafe_new_contract_state();
             SpaceManager::InternalImpl::enable_space(ref state, space);
         }
 
         fn disable_space(ref self: ContractState, space: ContractAddress) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let state = Ownable::unsafe_new_contract_state();
-            Ownable::InternalImpl::assert_only_owner(@state);
+            self.ownable.assert_only_owner();
             let mut state = SpaceManager::unsafe_new_contract_state();
             SpaceManager::InternalImpl::disable_space(ref state, space);
         }
@@ -322,25 +312,17 @@ mod TimelockExecutionStrategy {
             SpaceManager::InternalImpl::is_space_enabled(@state, space)
         }
 
+        // TODO: use Ownable impl as abi_embed(v0)?
         fn owner(self: @ContractState) -> ContractAddress {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let state = Ownable::unsafe_new_contract_state();
-            Ownable::OwnableImpl::owner(@state)
+            self.ownable.owner()
         }
 
         fn transfer_ownership(ref self: ContractState, new_owner: ContractAddress) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let mut state = Ownable::unsafe_new_contract_state();
-            Ownable::OwnableImpl::transfer_ownership(ref state, new_owner);
+            self.ownable.transfer_ownership(new_owner);
         }
 
         fn renounce_ownership(ref self: ContractState) {
-            // Migration to components planned ; disregard the `unsafe` keyword,
-            // it is actually safe.
-            let mut state = Ownable::unsafe_new_contract_state();
-            Ownable::OwnableImpl::renounce_ownership(ref state);
+            self.ownable.renounce_ownership();
         }
     }
 

diff --git a/starknet/src/factory/factory.cairo b/starknet/src/factory/factory.cairo
@@ -18,12 +18,12 @@ mod Factory {
     use sx::utils::constants::INITIALIZE_SELECTOR;
 
     #[event]
-    #[derive(Drop, starknet::Event)]
+    #[derive(Drop, PartialEq, starknet::Event)]
     enum Event {
         NewContractDeployed: NewContractDeployed
     }
 
-    #[derive(Drop, starknet::Event)]
+    #[derive(Drop, PartialEq, starknet::Event)]
     struct NewContractDeployed {
         class_hash: ClassHash,
         contract_address: ContractAddress
@@ -32,7 +32,7 @@ mod Factory {
     #[storage]
     struct Storage {}
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl Factory of IFactory<ContractState> {
         fn deploy(
             ref self: ContractState,

diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo
@@ -130,8 +130,8 @@ mod utils {
 
     mod legacy_hash;
     use legacy_hash::{
-        LegacyHashEthAddress, LegacyHashFelt252EthAddress, LegacyHashUsedSalts, LegacyHashChoice,
-        LegacyHashUserAddress, LegacyHashVotePower, LegacyHashVoteRegistry, LegacyHashSpanFelt252
+        LegacyHashEthAddress, LegacyHashChoice, LegacyHashUserAddress, LegacyHashVotePower,
+        LegacyHashVoteRegistry, LegacyHashSpanFelt252
     };
 
     mod math;

diff --git a/starknet/src/proposal_validation_strategies/proposition_power.cairo b/starknet/src/proposal_validation_strategies/proposition_power.cairo
@@ -10,7 +10,7 @@ mod PropositionPowerProposalValidationStrategy {
     #[storage]
     struct Storage {}
 
-    #[external(v0)]
+    #[abi(embed_v0)]
     impl PropositionPowerProposalValidationStrategy of IProposalValidationStrategy<ContractState> {
         /// Designed to be used by spaces that which to use voting strategies to define who can create proposals.
         /// The allowed strategies simply need to be supplied in the `params` field.
@@ -29,7 +29,9 @@ mod PropositionPowerProposalValidationStrategy {
         fn validate(
             self: @ContractState,
             author: UserAddress,
-            params: Span<felt252>, // [proposal_threshold: u256, allowed_strategies: Array<Strategy>]
+            params: Span<
+                felt252
+            >, // [proposal_threshold: u256, allowed_strategies: Array<Strategy>]
             user_params: Span<felt252> // [user_strategies: Array<IndexedStrategy>]
         ) -> bool {
             proposition_power::validate(author, params, user_params)