From f50679ca0fec25e1bee6f0ff8197cc3a4ee754fa Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Fri, 4 Oct 2024 13:57:54 +0200 Subject: [PATCH 01/14] remove state from eip712 mod --- starknet/src/authenticators/eth_sig.cairo | 24 +- starknet/src/lib.cairo | 1 - starknet/src/utils/eip712.cairo | 366 ++++++++++------------ 3 files changed, 174 insertions(+), 217 deletions(-) diff --git a/starknet/src/authenticators/eth_sig.cairo b/starknet/src/authenticators/eth_sig.cairo index f62effc7..1e32633b 100644 --- a/starknet/src/authenticators/eth_sig.cairo +++ b/starknet/src/authenticators/eth_sig.cairo @@ -89,7 +89,7 @@ mod EthSigAuthenticator { use starknet::{ContractAddress, EthAddress}; use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait}; use sx::types::{Strategy, IndexedStrategy, Choice, UserAddress}; - use sx::utils::{EIP712, LegacyHashEthAddress, ByteReverse}; + use sx::utils::{eip712, LegacyHashEthAddress, ByteReverse}; #[storage] struct Storage { @@ -112,9 +112,7 @@ mod EthSigAuthenticator { ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); - let state = EIP712::unsafe_new_contract_state(); - EIP712::InternalImpl::verify_propose_sig( - @state, + eip712::verify_propose_sig( r, s, v, @@ -149,9 +147,7 @@ mod EthSigAuthenticator { ) { // No need to check salts here, as double voting is prevented by the space itself. - let state = EIP712::unsafe_new_contract_state(); - EIP712::InternalImpl::verify_vote_sig( - @state, + eip712::verify_vote_sig( r, s, v, @@ -187,18 +183,8 @@ mod EthSigAuthenticator { ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); - let state = EIP712::unsafe_new_contract_state(); - EIP712::InternalImpl::verify_update_proposal_sig( - @state, - r, - s, - v, - space, - author, - proposal_id, - @execution_strategy, - metadata_uri.span(), - salt + eip712::verify_update_proposal_sig( + r, s, v, space, author, proposal_id, @execution_strategy, metadata_uri.span(), salt ); self._used_salts.write((author, salt), true); ISpaceDispatcher { contract_address: space } diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index daeb152f..5e863b41 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -118,7 +118,6 @@ mod utils { use default::ContractAddressDefault; mod eip712; - use eip712::EIP712; mod endian; use endian::ByteReverse; diff --git a/starknet/src/utils/eip712.cairo b/starknet/src/utils/eip712.cairo index 4adeb7e4..44a13f83 100644 --- a/starknet/src/utils/eip712.cairo +++ b/starknet/src/utils/eip712.cairo @@ -1,207 +1,179 @@ -#[starknet::contract] -mod EIP712 { - use starknet::{EthAddress, ContractAddress, secp256_trait}; - use starknet::eth_signature::verify_eth_signature; - use starknet::secp256k1::Secp256k1Point; - use sx::types::{Strategy, IndexedStrategy, Choice}; - use sx::utils::{endian, ByteReverse, KeccakStructHash, TIntoU256}; - use sx::utils::constants::{ - DOMAIN_HASH_LOW, DOMAIN_HASH_HIGH, ETHEREUM_PREFIX, PROPOSE_TYPEHASH_LOW, - PROPOSE_TYPEHASH_HIGH, VOTE_TYPEHASH_LOW, VOTE_TYPEHASH_HIGH, UPDATE_PROPOSAL_TYPEHASH_LOW, - UPDATE_PROPOSAL_TYPEHASH_HIGH, INDEXED_STRATEGY_TYPEHASH_LOW, - INDEXED_STRATEGY_TYPEHASH_HIGH, - }; - - #[storage] - struct Storage {} - - #[generate_trait] - impl InternalImpl of InternalTrait { - /// Verifies the signature of the propose calldata. - fn verify_propose_sig( - self: @ContractState, - r: u256, - s: u256, - v: u32, - space: ContractAddress, - author: EthAddress, - metadata_uri: Span, - execution_strategy: @Strategy, - user_proposal_validation_params: Span, - salt: u256, - ) { - let digest: u256 = self - .get_propose_digest( - space, - author, - metadata_uri, - execution_strategy, - user_proposal_validation_params, - salt - ); - verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), author); - } +use starknet::{EthAddress, ContractAddress, secp256_trait}; +use starknet::eth_signature::verify_eth_signature; +use starknet::secp256k1::Secp256k1Point; +use sx::types::{Strategy, IndexedStrategy, Choice}; +use sx::utils::{endian, ByteReverse, KeccakStructHash, TIntoU256}; +use sx::utils::constants::{ + DOMAIN_HASH_LOW, DOMAIN_HASH_HIGH, ETHEREUM_PREFIX, PROPOSE_TYPEHASH_LOW, PROPOSE_TYPEHASH_HIGH, + VOTE_TYPEHASH_LOW, VOTE_TYPEHASH_HIGH, UPDATE_PROPOSAL_TYPEHASH_LOW, + UPDATE_PROPOSAL_TYPEHASH_HIGH, INDEXED_STRATEGY_TYPEHASH_LOW, INDEXED_STRATEGY_TYPEHASH_HIGH, +}; - /// Verifies the signature of the vote calldata. - fn verify_vote_sig( - self: @ContractState, - r: u256, - s: u256, - v: u32, - space: ContractAddress, - voter: EthAddress, - proposal_id: u256, - choice: Choice, - user_voting_strategies: Span, - metadata_uri: Span, - ) { - let digest: u256 = self - .get_vote_digest( - space, voter, proposal_id, choice, user_voting_strategies, metadata_uri - ); - verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), voter); - } +/// Verifies the signature of the propose calldata. +fn verify_propose_sig( + r: u256, + s: u256, + v: u32, + space: ContractAddress, + author: EthAddress, + metadata_uri: Span, + execution_strategy: @Strategy, + user_proposal_validation_params: Span, + salt: u256, +) { + let digest: u256 = get_propose_digest( + space, author, metadata_uri, execution_strategy, user_proposal_validation_params, salt + ); + verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), author); +} - /// Verifies the signature of the update proposal calldata. - fn verify_update_proposal_sig( - self: @ContractState, - r: u256, - s: u256, - v: u32, - space: ContractAddress, - author: EthAddress, - proposal_id: u256, - execution_strategy: @Strategy, - metadata_uri: Span, - salt: u256 - ) { - let digest: u256 = self - .get_update_proposal_digest( - space, author, proposal_id, execution_strategy, metadata_uri, salt - ); - verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), author); - } +/// Verifies the signature of the vote calldata. +fn verify_vote_sig( + r: u256, + s: u256, + v: u32, + space: ContractAddress, + voter: EthAddress, + proposal_id: u256, + choice: Choice, + user_voting_strategies: Span, + metadata_uri: Span, +) { + let digest: u256 = get_vote_digest( + space, voter, proposal_id, choice, user_voting_strategies, metadata_uri + ); + verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), voter); +} - /// Returns the digest of the propose calldata. - fn get_propose_digest( - self: @ContractState, - space: ContractAddress, - author: EthAddress, - metadata_uri: Span, - execution_strategy: @Strategy, - user_proposal_validation_params: Span, - salt: u256 - ) -> u256 { - let encoded_data = array![ - u256 { low: PROPOSE_TYPEHASH_LOW, high: PROPOSE_TYPEHASH_HIGH }, - Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), - starknet::get_contract_address().into(), - space.into(), - author.into(), - metadata_uri.keccak_struct_hash(), - execution_strategy.keccak_struct_hash(), - user_proposal_validation_params.keccak_struct_hash(), - salt - ]; - let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); - self.hash_typed_data(message_hash) - } +/// Verifies the signature of the update proposal calldata. +fn verify_update_proposal_sig( + r: u256, + s: u256, + v: u32, + space: ContractAddress, + author: EthAddress, + proposal_id: u256, + execution_strategy: @Strategy, + metadata_uri: Span, + salt: u256 +) { + let digest: u256 = get_update_proposal_digest( + space, author, proposal_id, execution_strategy, metadata_uri, salt + ); + verify_eth_signature(digest, secp256_trait::signature_from_vrs(v, r, s), author); +} - /// Returns the digest of the vote calldata. - fn get_vote_digest( - self: @ContractState, - space: ContractAddress, - voter: EthAddress, - proposal_id: u256, - choice: Choice, - user_voting_strategies: Span, - metadata_uri: Span, - ) -> u256 { - let encoded_data = array![ - u256 { low: VOTE_TYPEHASH_LOW, high: VOTE_TYPEHASH_HIGH }, - Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), - starknet::get_contract_address().into(), - space.into(), - voter.into(), - proposal_id, - choice.into(), - user_voting_strategies.keccak_struct_hash(), - metadata_uri.keccak_struct_hash() - ]; - let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); - self.hash_typed_data(message_hash) - } +/// Returns the digest of the propose calldata. +fn get_propose_digest( + space: ContractAddress, + author: EthAddress, + metadata_uri: Span, + execution_strategy: @Strategy, + user_proposal_validation_params: Span, + salt: u256 +) -> u256 { + let encoded_data = array![ + u256 { low: PROPOSE_TYPEHASH_LOW, high: PROPOSE_TYPEHASH_HIGH }, + Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), + starknet::get_contract_address().into(), + space.into(), + author.into(), + metadata_uri.keccak_struct_hash(), + execution_strategy.keccak_struct_hash(), + user_proposal_validation_params.keccak_struct_hash(), + salt + ]; + let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); + hash_typed_data(message_hash) +} - /// Returns the digest of the update proposal calldata. - fn get_update_proposal_digest( - self: @ContractState, - space: ContractAddress, - author: EthAddress, - proposal_id: u256, - execution_strategy: @Strategy, - metadata_uri: Span, - salt: u256 - ) -> u256 { - let encoded_data = array![ - u256 { low: UPDATE_PROPOSAL_TYPEHASH_LOW, high: UPDATE_PROPOSAL_TYPEHASH_HIGH }, - Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), - starknet::get_contract_address().into(), - space.into(), - author.into(), - proposal_id, - execution_strategy.keccak_struct_hash(), - metadata_uri.keccak_struct_hash(), - salt - ]; - let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); - self.hash_typed_data(message_hash) - } +/// Returns the digest of the vote calldata. +fn get_vote_digest( + space: ContractAddress, + voter: EthAddress, + proposal_id: u256, + choice: Choice, + user_voting_strategies: Span, + metadata_uri: Span, +) -> u256 { + let encoded_data = array![ + u256 { low: VOTE_TYPEHASH_LOW, high: VOTE_TYPEHASH_HIGH }, + Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), + starknet::get_contract_address().into(), + space.into(), + voter.into(), + proposal_id, + choice.into(), + user_voting_strategies.keccak_struct_hash(), + metadata_uri.keccak_struct_hash() + ]; + let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); + hash_typed_data(message_hash) +} - /// Hashes typed data according to the EIP-712 specification. - fn hash_typed_data(self: @ContractState, message_hash: u256) -> u256 { - let encoded_data = InternalImpl::add_prefix_array( - array![u256 { low: DOMAIN_HASH_LOW, high: DOMAIN_HASH_HIGH }, message_hash], - ETHEREUM_PREFIX - ); - let (mut u64_arr, overflow) = endian::into_le_u64_array(encoded_data); - keccak::cairo_keccak(ref u64_arr, overflow, 2).byte_reverse() - } +/// Returns the digest of the update proposal calldata. +fn get_update_proposal_digest( + space: ContractAddress, + author: EthAddress, + proposal_id: u256, + execution_strategy: @Strategy, + metadata_uri: Span, + salt: u256 +) -> u256 { + let encoded_data = array![ + u256 { low: UPDATE_PROPOSAL_TYPEHASH_LOW, high: UPDATE_PROPOSAL_TYPEHASH_HIGH }, + Felt252IntoU256::into(starknet::get_tx_info().unbox().chain_id), + starknet::get_contract_address().into(), + space.into(), + author.into(), + proposal_id, + execution_strategy.keccak_struct_hash(), + metadata_uri.keccak_struct_hash(), + salt + ]; + let message_hash = keccak::keccak_u256s_be_inputs(encoded_data.span()).byte_reverse(); + hash_typed_data(message_hash) +} - /// Prefixes a 16 bit prefix to an array of 256 bit values. - fn add_prefix_array(input: Array, mut prefix: u128) -> Array { - let mut out = ArrayTrait::::new(); - let mut input = input; - loop { - match input.pop_front() { - Option::Some(num) => { - let (w1, high_carry) = InternalImpl::add_prefix_u128(num.high, prefix); - let (w0, low_carry) = InternalImpl::add_prefix_u128(num.low, high_carry); - out.append(u256 { low: w0, high: w1 }); - prefix = low_carry; - }, - Option::None(_) => { - // left shift so that the prefix is in the high bits - out - .append( - u256 { - high: prefix * 0x10000000000000000000000000000_u128, low: 0_u128 - } - ); - break (); - } - }; - }; - out - } +/// Hashes typed data according to the EIP-712 specification. +fn hash_typed_data(message_hash: u256) -> u256 { + let encoded_data = add_prefix_array( + array![u256 { low: DOMAIN_HASH_LOW, high: DOMAIN_HASH_HIGH }, message_hash], ETHEREUM_PREFIX + ); + let (mut u64_arr, overflow) = endian::into_le_u64_array(encoded_data); + keccak::cairo_keccak(ref u64_arr, overflow, 2).byte_reverse() +} +/// Prefixes a 16 bit prefix to an array of 256 bit values. +fn add_prefix_array(input: Array, mut prefix: u128) -> Array { + let mut out = ArrayTrait::::new(); + let mut input = input; + loop { + match input.pop_front() { + Option::Some(num) => { + let (w1, high_carry) = add_prefix_u128(num.high, prefix); + let (w0, low_carry) = add_prefix_u128(num.low, high_carry); + out.append(u256 { low: w0, high: w1 }); + prefix = low_carry; + }, + Option::None(_) => { + // left shift so that the prefix is in the high bits + out + .append( + u256 { high: prefix * 0x10000000000000000000000000000_u128, low: 0_u128 } + ); + break (); + } + }; + }; + out +} - /// Adds a 16 bit prefix to a 128 bit input, returning the result and a carry. - fn add_prefix_u128(input: u128, prefix: u128) -> (u128, u128) { - let with_prefix = u256 { low: input, high: prefix }; - let carry = with_prefix & 0xffff; - // Removing the carry and shifting back. - let out = (with_prefix - carry) / 0x10000; - (out.low, carry.low) - } - } +/// Adds a 16 bit prefix to a 128 bit input, returning the result and a carry. +fn add_prefix_u128(input: u128, prefix: u128) -> (u128, u128) { + let with_prefix = u256 { low: input, high: prefix }; + let carry = with_prefix & 0xffff; + // Removing the carry and shifting back. + let out = (with_prefix - carry) / 0x10000; + (out.low, carry.low) } From 636ca523e10303d1ae02f674a663f7d0f5c46bbd Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:40:52 +0200 Subject: [PATCH 02/14] move SNIP12 to components --- starknet/src/authenticators/stark_sig.cairo | 85 ++++++++++++--------- starknet/src/lib.cairo | 1 - starknet/src/utils/snip12.cairo | 80 +++++++++---------- 3 files changed, 88 insertions(+), 78 deletions(-) diff --git a/starknet/src/authenticators/stark_sig.cairo b/starknet/src/authenticators/stark_sig.cairo index 8bf28e09..1a23b570 100644 --- a/starknet/src/authenticators/stark_sig.cairo +++ b/starknet/src/authenticators/stark_sig.cairo @@ -81,11 +81,23 @@ mod StarkSigAuthenticator { use starknet::{ContractAddress, info}; use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait}; use sx::types::{Strategy, IndexedStrategy, UserAddress, Choice}; - use sx::utils::SNIP12; + use sx::utils::snip12::SNIP12Component; + + component!(path: SNIP12Component, storage: snip12, event: SNIP12Event); + + impl SNIP12InternalImpl = SNIP12Component::InternalImpl; #[storage] struct Storage { - _used_salts: LegacyMap::<(ContractAddress, felt252), bool> + _used_salts: LegacyMap::<(ContractAddress, felt252), bool>, + #[substorage(v0)] + snip12: SNIP12Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + SNIP12Event: SNIP12Component::Event } #[abi(embed_v0)] @@ -102,17 +114,17 @@ mod StarkSigAuthenticator { ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); - let state = SNIP12::unsafe_new_contract_state(); - SNIP12::InternalImpl::verify_propose_sig( - @state, - signature, - space, - author, - metadata_uri.span(), - @execution_strategy, - user_proposal_validation_params.span(), - salt - ); + self + .snip12 + .verify_propose_sig( + signature, + space, + author, + metadata_uri.span(), + @execution_strategy, + user_proposal_validation_params.span(), + salt + ); self._used_salts.write((author, salt), true); ISpaceDispatcher { contract_address: space } @@ -136,17 +148,17 @@ mod StarkSigAuthenticator { ) { // No need to check salts here, as double voting is prevented by the space itself. - let state = SNIP12::unsafe_new_contract_state(); - SNIP12::InternalImpl::verify_vote_sig( - @state, - signature, - space, - voter, - proposal_id, - choice, - user_voting_strategies.span(), - metadata_uri.span() - ); + self + .snip12 + .verify_vote_sig( + signature, + space, + voter, + proposal_id, + choice, + user_voting_strategies.span(), + metadata_uri.span() + ); ISpaceDispatcher { contract_address: space } .vote( @@ -170,17 +182,17 @@ mod StarkSigAuthenticator { ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); - let state = SNIP12::unsafe_new_contract_state(); - SNIP12::InternalImpl::verify_update_proposal_sig( - @state, - signature, - space, - author, - proposal_id, - @execution_strategy, - metadata_uri.span(), - salt - ); + self + .snip12 + .verify_update_proposal_sig( + signature, + space, + author, + proposal_id, + @execution_strategy, + metadata_uri.span(), + salt + ); self._used_salts.write((author, salt), true); ISpaceDispatcher { contract_address: space } @@ -191,7 +203,6 @@ mod StarkSigAuthenticator { } #[constructor] fn constructor(ref self: ContractState, name: felt252, version: felt252) { - let mut state = SNIP12::unsafe_new_contract_state(); - SNIP12::InternalImpl::initializer(ref state, name, version); + self.snip12.initializer(name, version); } } diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 5e863b41..b77b4c88 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -153,7 +153,6 @@ mod utils { use single_slot_proof::SingleSlotProof; mod snip12; - use snip12::SNIP12; mod struct_hash; use struct_hash::StructHash; diff --git a/starknet/src/utils/snip12.cairo b/starknet/src/utils/snip12.cairo index 65dc5d1e..5a4f51f0 100644 --- a/starknet/src/utils/snip12.cairo +++ b/starknet/src/utils/snip12.cairo @@ -1,7 +1,7 @@ /// SNIP12 style typed data signing implementation. /// See here for more info: https://community.starknet.io/t/snip-off-chain-signatures-a-la-eip712/98029 -#[starknet::contract] -mod SNIP12 { +#[starknet::component] +mod SNIP12Component { use starknet::ContractAddress; use openzeppelin::account::interface::{AccountABIDispatcher, AccountABIDispatcherTrait}; use sx::types::{Strategy, IndexedStrategy, Choice}; @@ -17,14 +17,16 @@ mod SNIP12 { } #[generate_trait] - impl InternalImpl of InternalTrait { - fn initializer(ref self: ContractState, name: felt252, version: felt252) { - self._domain_hash.write(InternalImpl::get_domain_hash(name, version)); + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { + fn initializer(ref self: ComponentState, name: felt252, version: felt252) { + self._domain_hash.write(get_domain_hash(name, version)); } /// Verifies the signature of the propose calldata. fn verify_propose_sig( - self: @ContractState, + self: @ComponentState, signature: Array, space: ContractAddress, author: ContractAddress, @@ -43,12 +45,12 @@ mod SNIP12 { salt ); - InternalImpl::verify_signature(digest, signature, author); + verify_signature(digest, signature, author); } /// Verifies the signature of the vote calldata. fn verify_vote_sig( - self: @ContractState, + self: @ComponentState, signature: Array, space: ContractAddress, voter: ContractAddress, @@ -61,12 +63,12 @@ mod SNIP12 { .get_vote_digest( space, voter, proposal_id, choice, user_voting_strategies, metadata_uri ); - InternalImpl::verify_signature(digest, signature, voter); + verify_signature(digest, signature, voter); } /// Verifies the signature of the update proposal calldata. fn verify_update_proposal_sig( - self: @ContractState, + self: @ComponentState, signature: Array, space: ContractAddress, author: ContractAddress, @@ -79,12 +81,12 @@ mod SNIP12 { .get_update_proposal_digest( space, author, proposal_id, execution_strategy, metadata_uri, salt ); - InternalImpl::verify_signature(digest, signature, author); + verify_signature(digest, signature, author); } /// Returns the digest of the propose calldata. fn get_propose_digest( - self: @ContractState, + self: @ComponentState, space: ContractAddress, author: ContractAddress, metadata_uri: Span, @@ -105,7 +107,7 @@ mod SNIP12 { /// Returns the digest of the vote calldata. fn get_vote_digest( - self: @ContractState, + self: @ComponentState, space: ContractAddress, voter: ContractAddress, proposal_id: u256, @@ -127,7 +129,7 @@ mod SNIP12 { /// Returns the digest of the update proposal calldata. fn get_update_proposal_digest( - self: @ContractState, + self: @ComponentState, space: ContractAddress, author: ContractAddress, proposal_id: u256, @@ -146,20 +148,9 @@ mod SNIP12 { self.hash_typed_data(encoded_data.span().struct_hash(), author) } - - /// Returns the domain hash of the contract. - fn get_domain_hash(name: felt252, version: felt252) -> felt252 { - let mut encoded_data = array![]; - DOMAIN_TYPEHASH.serialize(ref encoded_data); - name.serialize(ref encoded_data); - version.serialize(ref encoded_data); - starknet::get_tx_info().unbox().chain_id.serialize(ref encoded_data); - encoded_data.span().struct_hash() - } - /// Hashes typed data according to the starknet equiavalent to the EIP-712 specification. fn hash_typed_data( - self: @ContractState, message_hash: felt252, signer: ContractAddress + self: @ComponentState, message_hash: felt252, signer: ContractAddress ) -> felt252 { let mut encoded_data = array![]; STARKNET_MESSAGE.serialize(ref encoded_data); @@ -168,21 +159,30 @@ mod SNIP12 { message_hash.serialize(ref encoded_data); encoded_data.span().struct_hash() } + } - /// Verifies the signature of a message by calling the account contract. - fn verify_signature(digest: felt252, signature: Array, account: ContractAddress) { - // Only SNIP-6 compliant accounts are supported. - assert( - AccountABIDispatcher { contract_address: account } - .supports_interface(ERC165_ACCOUNT_INTERFACE_ID) == true, - 'Invalid Account' - ); + /// Returns the domain hash of the contract. + fn get_domain_hash(name: felt252, version: felt252) -> felt252 { + let mut encoded_data = array![]; + DOMAIN_TYPEHASH.serialize(ref encoded_data); + name.serialize(ref encoded_data); + version.serialize(ref encoded_data); + starknet::get_tx_info().unbox().chain_id.serialize(ref encoded_data); + encoded_data.span().struct_hash() + } + /// Verifies the signature of a message by calling the account contract. + fn verify_signature(digest: felt252, signature: Array, account: ContractAddress) { + // Only SNIP-6 compliant accounts are supported. + assert( + AccountABIDispatcher { contract_address: account } + .supports_interface(ERC165_ACCOUNT_INTERFACE_ID) == true, + 'Invalid Account' + ); - assert( - AccountABIDispatcher { contract_address: account } - .is_valid_signature(digest, signature) == 'VALID', - 'Invalid Signature' - ); - } + assert( + AccountABIDispatcher { contract_address: account } + .is_valid_signature(digest, signature) == 'VALID', + 'Invalid Signature' + ); } } From 16989be324406b7aa98b42d1156ab39cca167bbe Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:26:47 +0200 Subject: [PATCH 03/14] add simple quorum component --- .../src/execution_strategies/timelock.cairo | 33 ++-- starknet/src/lib.cairo | 1 - .../mocks/vanilla_execution_strategy.cairo | 35 ++-- starknet/src/utils/simple_quorum.cairo | 170 ++++++++++-------- 4 files changed, 129 insertions(+), 110 deletions(-) diff --git a/starknet/src/execution_strategies/timelock.cairo b/starknet/src/execution_strategies/timelock.cairo index dff1304f..c5c87740 100644 --- a/starknet/src/execution_strategies/timelock.cairo +++ b/starknet/src/execution_strategies/timelock.cairo @@ -37,20 +37,28 @@ mod TimelockExecutionStrategy { use sx::interfaces::IExecutionStrategy; use super::ITimelockExecutionStrategy; use sx::types::{Proposal, ProposalStatus}; - use sx::utils::{SimpleQuorum, SpaceManager}; + use sx::utils::{simple_quorum::SimpleQuorumComponent, SpaceManager}; component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); impl OwnableInternalImpl = OwnableComponent::InternalImpl; impl OwnableImpl = OwnableComponent::OwnableImpl; + component!(path: SimpleQuorumComponent, storage: simple_quorum, event: SimpleQuorumEvent); + + #[abi(embed_v0)] + impl SimpleQuorumImpl = SimpleQuorumComponent::SimpleQuorumImpl; + impl SimpleQuorumInternalImpl = SimpleQuorumComponent::InternalImpl; + #[storage] struct Storage { _timelock_delay: u32, _veto_guardian: ContractAddress, _proposal_execution_time: LegacyMap::, #[substorage(v0)] - ownable: OwnableComponent::Storage + ownable: OwnableComponent::Storage, + #[substorage(v0)] + simple_quorum: SimpleQuorumComponent::Storage, } #[event] @@ -65,7 +73,9 @@ mod TimelockExecutionStrategy { ProposalExecuted: ProposalExecuted, ProposalVetoed: ProposalVetoed, #[flat] - OwnableEvent: OwnableComponent::Event + OwnableEvent: OwnableComponent::Event, + #[flat] + SimpleQuorumEvent: SimpleQuorumComponent::Event, } #[derive(Drop, PartialEq, starknet::Event)] @@ -132,8 +142,7 @@ mod TimelockExecutionStrategy { ) { self.ownable.initializer(owner); - let mut state = SimpleQuorum::unsafe_new_contract_state(); - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + self.simple_quorum.initializer(quorum); let mut state = SpaceManager::unsafe_new_contract_state(); SpaceManager::InternalImpl::initializer(ref state, spaces); @@ -157,10 +166,9 @@ mod TimelockExecutionStrategy { // it is actually safe. let state = SpaceManager::unsafe_new_contract_state(); SpaceManager::InternalImpl::assert_only_spaces(@state); - let state = SimpleQuorum::unsafe_new_contract_state(); - let proposal_status = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let proposal_status = self + .simple_quorum + .get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert( proposal_status == ProposalStatus::Accepted(()) || proposal_status == ProposalStatus::VotingPeriodAccepted(()), @@ -201,10 +209,9 @@ mod TimelockExecutionStrategy { votes_against: u256, votes_abstain: u256, ) -> ProposalStatus { - let state = SimpleQuorum::unsafe_new_contract_state(); - SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ) + self + .simple_quorum + .get_proposal_status(@proposal, votes_for, votes_against, votes_abstain) } fn get_strategy_type(self: @ContractState) -> felt252 { diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index b77b4c88..997ca6d5 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -147,7 +147,6 @@ mod utils { mod simple_majority; mod simple_quorum; - use simple_quorum::SimpleQuorum; mod single_slot_proof; use single_slot_proof::SingleSlotProof; diff --git a/starknet/src/tests/mocks/vanilla_execution_strategy.cairo b/starknet/src/tests/mocks/vanilla_execution_strategy.cairo index 4e481436..f4909584 100644 --- a/starknet/src/tests/mocks/vanilla_execution_strategy.cairo +++ b/starknet/src/tests/mocks/vanilla_execution_strategy.cairo @@ -2,20 +2,26 @@ mod VanillaExecutionStrategy { use sx::interfaces::{IExecutionStrategy, IQuorum}; use sx::types::{Proposal, ProposalStatus}; - use sx::utils::SimpleQuorum; + use sx::utils::simple_quorum::SimpleQuorumComponent; + component!(path: SimpleQuorumComponent, storage: simple_quorum, event: SimpleQuorumEvent); + + #[abi(embed_v0)] + impl SimpleQuorumImpl = SimpleQuorumComponent::SimpleQuorumImpl; + impl SimpleQuorumInternalImpl = SimpleQuorumComponent::InternalImpl; #[storage] struct Storage { - _num_executed: felt252 + _num_executed: felt252, + #[substorage(v0)] + simple_quorum: SimpleQuorumComponent::Storage, } - #[abi(embed_v0)] - impl QuorumImpl of IQuorum { - fn quorum(self: @ContractState) -> u256 { - let mut state: SimpleQuorum::ContractState = SimpleQuorum::unsafe_new_contract_state(); - SimpleQuorum::InternalImpl::quorum(@state) - } + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + SimpleQuorumEvent: SimpleQuorumComponent::Event, } /// The vanilla execution strategy is a dummy execution strategy that simply increments a `_num_executed` variable for every @@ -48,11 +54,9 @@ mod VanillaExecutionStrategy { votes_against: u256, votes_abstain: u256, ) -> ProposalStatus { - let mut state: SimpleQuorum::ContractState = SimpleQuorum::unsafe_new_contract_state(); - - SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain, - ) + self + .simple_quorum + .get_proposal_status(@proposal, votes_for, votes_against, votes_abstain,) } fn get_strategy_type(self: @ContractState) -> felt252 { @@ -62,10 +66,7 @@ mod VanillaExecutionStrategy { #[constructor] fn constructor(ref self: ContractState, quorum: u256) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state = SimpleQuorum::unsafe_new_contract_state(); - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + self.simple_quorum.initializer(quorum); } #[generate_trait] diff --git a/starknet/src/utils/simple_quorum.cairo b/starknet/src/utils/simple_quorum.cairo index 0fe79eb6..4e2712f5 100644 --- a/starknet/src/utils/simple_quorum.cairo +++ b/starknet/src/utils/simple_quorum.cairo @@ -1,5 +1,5 @@ -#[starknet::contract] -mod SimpleQuorum { +#[starknet::component] +mod SimpleQuorumComponent { use starknet::{ContractAddress, info}; use sx::interfaces::IQuorum; use sx::types::{Proposal, FinalizationStatus, ProposalStatus}; @@ -10,15 +10,13 @@ mod SimpleQuorum { } #[generate_trait] - impl InternalImpl of InternalTrait { - fn initializer(ref self: ContractState, quorum: u256) { + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { + fn initializer(ref self: ComponentState, quorum: u256) { self._quorum.write(quorum); } - fn quorum(self: @ContractState) -> u256 { - self._quorum.read() - } - /// Returns the status of a proposal. /// To be accepted, a proposal must be supported by a majority of votes, have reached the quorum, /// and have the current timestamp be greater or equal to `max_end_timestamp`. @@ -28,7 +26,7 @@ mod SimpleQuorum { /// Spaces that don't want to deal with early accepting proposals should set `min_voting_duration` and `max_voting_duration` /// to the same value. fn get_proposal_status( - self: @ContractState, + self: @ComponentState, proposal: @Proposal, votes_for: u256, votes_against: u256, @@ -61,255 +59,269 @@ mod SimpleQuorum { } } - #[abi(embed_v0)] - impl Quorum of IQuorum { - fn quorum(self: @ContractState) -> u256 { + #[embeddable_as(SimpleQuorumImpl)] + impl SimpleQuorum< + TContractState, +HasComponent + > of IQuorum> { + fn quorum(self: @ComponentState) -> u256 { self._quorum.read() } } } + #[cfg(test)] mod tests { - use super::SimpleQuorum; + use super::SimpleQuorumComponent; + use super::SimpleQuorumComponent::InternalTrait; use sx::types::{Proposal, proposal::ProposalDefault, FinalizationStatus, ProposalStatus}; + // You need an actual contract to test a component. + #[starknet::contract] + mod MockContract { + use super::SimpleQuorumComponent; + use sx::types::{Proposal, ProposalStatus}; + + component!(path: SimpleQuorumComponent, storage: simple_quorum, event: SimpleQuorumEvent); + + #[abi(embed_v0)] + impl SimpleQuorumImpl = + SimpleQuorumComponent::SimpleQuorumImpl; + impl SimpleQuorumInternalImpl = SimpleQuorumComponent::InternalImpl; + + #[storage] + struct Storage { + #[substorage(v0)] + simple_quorum: SimpleQuorumComponent::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + pub enum Event { + #[flat] + SimpleQuorumEvent: SimpleQuorumComponent::Event, + } + + #[constructor] + fn constructor(ref self: ContractState, quorum: u256) { + self.simple_quorum.initializer(quorum); + } + } + + type ComponentState = SimpleQuorumComponent::ComponentState; + + fn COMPONENT_STATE() -> ComponentState { + SimpleQuorumComponent::component_state_for_testing() + } + #[test] #[available_gas(10000000)] fn cancelled() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let mut proposal: Proposal = Default::default(); proposal.finalization_status = FinalizationStatus::Cancelled(()); let votes_for = 0; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Cancelled(()), 'failed cancelled'); } #[test] #[available_gas(10000000)] fn executed() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let mut proposal: Proposal = Default::default(); proposal.finalization_status = FinalizationStatus::Executed(()); let votes_for = 0; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Executed(()), 'failed executed'); } #[test] #[available_gas(10000000)] fn voting_delay() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let mut proposal: Proposal = Default::default(); proposal.start_timestamp = 42424242; let votes_for = 0; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingDelay(()), 'failed voting_delay'); } #[test] #[available_gas(10000000)] fn voting_period() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let mut proposal: Proposal = Default::default(); proposal.min_end_timestamp = 42424242; let votes_for = 0; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingPeriod(()), 'failed min_end_timestamp'); } #[test] #[available_gas(10000000)] fn shortcut_accepted() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); proposal.max_end_timestamp = 10; let votes_for = quorum; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingPeriodAccepted(()), 'failed shortcut_accepted'); } #[test] #[available_gas(10000000)] fn shortcut_only_abstains() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); proposal.max_end_timestamp = 10; let votes_for = 0; let votes_against = 0; let votes_abstain = quorum; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_only_abstains'); } #[test] #[available_gas(10000000)] fn shortcut_only_againsts() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); proposal.max_end_timestamp = 10; let votes_for = 0; let votes_against = quorum; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_only_againsts'); } #[test] #[available_gas(10000000)] fn shortcut_balanced() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); proposal.max_end_timestamp = 10; let votes_for = quorum; let votes_against = quorum; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_balanced'); } #[test] #[available_gas(10000000)] fn balanced() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); let votes_for = quorum; let votes_against = quorum; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Rejected(()), 'failed balanced'); } #[test] #[available_gas(10000000)] fn accepted() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 2; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); let votes_for = quorum; let votes_against = quorum - 1; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Accepted(()), 'failed accepted'); } #[test] #[available_gas(10000000)] fn accepted_with_abstains() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 5; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); let votes_for = 2; let votes_against = 1; let votes_abstain = 10; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Accepted(()), 'failed accepted abstains'); } #[test] #[available_gas(10000000)] fn rejected_only_againsts() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 0; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); let votes_for = 0; let votes_against = 1; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Rejected(()), 'failed rejected'); } #[test] #[available_gas(10000000)] fn quorum_not_reached() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 3; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); let votes_for = 2; let votes_against = 0; let votes_abstain = 0; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Rejected(()), 'failed quorum_not_reached'); } #[test] #[available_gas(10000000)] fn quorum_not_reached_with_against_votes() { - let mut state = SimpleQuorum::unsafe_new_contract_state(); + let mut state = COMPONENT_STATE(); let quorum = 6; - SimpleQuorum::InternalImpl::initializer(ref state, quorum); + state.initializer(quorum); let mut proposal: Proposal = Default::default(); // quorum only takes into account for and abstain votes let votes_for = 3; let votes_against = 2; let votes_abstain = 1; - let result = SimpleQuorum::InternalImpl::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let result = state.get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); assert(result == ProposalStatus::Rejected(()), 'failed quorum_not_reached'); } } From fff5cde3ea4fae74035c7073ec13835760b6b718 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:28:38 +0200 Subject: [PATCH 04/14] annotate ownableImpl with embed_v0 --- .../src/execution_strategies/timelock.cairo | 23 +------- starknet/src/interfaces/i_space.cairo | 7 --- starknet/src/space/space.cairo | 15 +---- .../tests/execution_strategies/timelock.cairo | 58 ++++++++++++------- starknet/src/tests/space/space.cairo | 4 +- starknet/src/tests/space/upgrade.cairo | 4 +- 6 files changed, 48 insertions(+), 63 deletions(-) diff --git a/starknet/src/execution_strategies/timelock.cairo b/starknet/src/execution_strategies/timelock.cairo index c5c87740..7cefe614 100644 --- a/starknet/src/execution_strategies/timelock.cairo +++ b/starknet/src/execution_strategies/timelock.cairo @@ -20,13 +20,6 @@ trait ITimelockExecutionStrategy { fn disable_space(ref self: TContractState, space: ContractAddress); fn is_space_enabled(self: @TContractState, space: ContractAddress) -> bool; - - /// Transfers the ownership to `new_owner`. - fn transfer_ownership(ref self: TContractState, new_owner: ContractAddress); - /// Renounces the ownership of the contract. CAUTION: This is a one-way operation. - fn renounce_ownership(ref self: TContractState); - - fn owner(self: @TContractState) -> ContractAddress; } #[starknet::contract] @@ -41,8 +34,9 @@ mod TimelockExecutionStrategy { component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); - impl OwnableInternalImpl = OwnableComponent::InternalImpl; + #[abi(embed_v0)] impl OwnableImpl = OwnableComponent::OwnableImpl; + impl OwnableInternalImpl = OwnableComponent::InternalImpl; component!(path: SimpleQuorumComponent, storage: simple_quorum, event: SimpleQuorumEvent); @@ -318,19 +312,6 @@ mod TimelockExecutionStrategy { let state = SpaceManager::unsafe_new_contract_state(); SpaceManager::InternalImpl::is_space_enabled(@state, space) } - - // TODO: use Ownable impl as abi_embed(v0)? - fn owner(self: @ContractState) -> ContractAddress { - self.ownable.owner() - } - - fn transfer_ownership(ref self: ContractState, new_owner: ContractAddress) { - self.ownable.transfer_ownership(new_owner); - } - - fn renounce_ownership(ref self: ContractState) { - self.ownable.renounce_ownership(); - } } #[generate_trait] diff --git a/starknet/src/interfaces/i_space.cairo b/starknet/src/interfaces/i_space.cairo index bb54d2de..f82de2d2 100644 --- a/starknet/src/interfaces/i_space.cairo +++ b/starknet/src/interfaces/i_space.cairo @@ -7,9 +7,6 @@ use sx::types::{ trait ISpace { // -- View functions -- - /// Returns the owner of the contract. - fn owner(self: @TContractState) -> ContractAddress; - /// The voting delay in seconds between when a proposal is created and the start of the voting period. fn voting_delay(self: @TContractState) -> u32; @@ -60,10 +57,6 @@ trait ISpace { /// struct is passed to be able to update different settings in a single call. Settings that should not /// be updated should have the `no_update` value (see `UpdateSettingsCalldata`). fn update_settings(ref self: TContractState, input: UpdateSettingsCalldata); - /// Transfers the ownership to `new_owner`. - fn transfer_ownership(ref self: TContractState, new_owner: ContractAddress); - /// Renounces the ownership of the contract. CAUTION: This is a one-way operation. - fn renounce_ownership(ref self: TContractState); // -- Actions -- /// Initializes the contract. Can only be called once. diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index bd8565e5..4d940c4a 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -22,8 +22,9 @@ mod Space { component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); - impl OwnableInternalImpl = OwnableComponent::InternalImpl; + #[abi(embed_v0)] impl OwnableImpl = OwnableComponent::OwnableImpl; + impl OwnableInternalImpl = OwnableComponent::InternalImpl; #[storage] struct Storage { @@ -486,10 +487,6 @@ mod Space { // We leave the implementation empty. } - fn owner(self: @ContractState) -> ContractAddress { - self.ownable.owner() - } - fn max_voting_duration(self: @ContractState) -> u32 { self._max_voting_duration.read() } @@ -710,14 +707,6 @@ mod Space { fn vote_power(self: @ContractState, proposal_id: u256, choice: Choice) -> u256 { self._vote_power.read((proposal_id, choice)) } - - fn transfer_ownership(ref self: ContractState, new_owner: ContractAddress) { - self.ownable.transfer_ownership(new_owner); - } - - fn renounce_ownership(ref self: ContractState) { - self.ownable.renounce_ownership(); - } } #[generate_trait] diff --git a/starknet/src/tests/execution_strategies/timelock.cairo b/starknet/src/tests/execution_strategies/timelock.cairo index a2ef2b0e..73fe1e52 100644 --- a/starknet/src/tests/execution_strategies/timelock.cairo +++ b/starknet/src/tests/execution_strategies/timelock.cairo @@ -5,6 +5,7 @@ mod tests { use sx::interfaces::{ IQuorum, IQuorumDispatcher, IQuorumDispatcherTrait, ISpaceDispatcher, ISpaceDispatcherTrait }; + use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; use sx::execution_strategies::{ TimelockExecutionStrategy, TimelockExecutionStrategy::CallWithSalt, timelock::{ITimelockExecutionStrategyDispatcher, ITimelockExecutionStrategyDispatcherTrait} @@ -27,6 +28,7 @@ mod tests { let mut config = setup::setup(); config.min_voting_duration = 0; let (_, space) = setup::deploy(@config); + let ownable_space = IOwnableDispatcher { contract_address: space.contract_address }; let spaces = array![space.contract_address]; let timelock_delay = 100; @@ -55,7 +57,7 @@ mod tests { // Set timelock as space controller testing::set_caller_address(config.owner); testing::set_contract_address(config.owner); - space.transfer_ownership(timelock.contract_address); + ownable_space.transfer_ownership(timelock.contract_address); (config, space, timelock) } @@ -129,7 +131,9 @@ mod tests { ); // Execute proposal on timelock - should renounce ownership of space timelock.execute_queued_proposal(payload.span()); - assert(space.owner().is_zero(), 'renounce ownership failed'); + + let ownable_space = IOwnableDispatcher { contract_address: space.contract_address }; + assert(ownable_space.owner().is_zero(), 'renounce ownership failed'); } #[test] @@ -150,8 +154,10 @@ mod tests { } ] ); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); + + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); timelock.disable_space(space.contract_address); assert(timelock.is_space_enabled(space.contract_address) == false, 'disable space failed'); // Try to execute proposal on space, should fail because the space is disabled @@ -424,7 +430,9 @@ mod tests { // Execute proposal on timelock - should renounce ownership of space and update settings timelock.execute_queued_proposal(payload.span()); assert(space.max_voting_duration() == 1000, 'update settings failed'); - assert(space.owner().is_zero(), 'renounce ownership failed'); + + let ownable_space = IOwnableDispatcher { contract_address: space.contract_address }; + assert(ownable_space.owner().is_zero(), 'renounce ownership failed'); } #[test] @@ -515,9 +523,12 @@ mod tests { ] ); space.execute(proposal_id, payload.clone()); + + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); + // Veto proposal from unauthorized account - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); timelock.veto(poseidon::poseidon_hash_span(payload.span())); } @@ -526,8 +537,9 @@ mod tests { fn set_timelock_delay() { let (_, _, timelock) = setup_test(); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); timelock.set_timelock_delay(1000); assert(timelock.timelock_delay() == 1000, 'timelock delay not set'); } @@ -549,8 +561,9 @@ mod tests { fn set_veto_guardian() { let (_, _, timelock) = setup_test(); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); timelock.set_veto_guardian(timelock.veto_guardian()); assert(timelock.veto_guardian() == timelock.veto_guardian(), 'veto guardian not set'); } @@ -571,10 +584,11 @@ mod tests { fn set_owner() { let (_, _, timelock) = setup_test(); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); - timelock.transfer_ownership(timelock.owner()); - assert(timelock.owner() == timelock.owner(), 'owner not set'); + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); + ownable_timelock.transfer_ownership(timelock.veto_guardian()); + assert(ownable_timelock.owner() == timelock.veto_guardian(), 'owner not set'); } #[test] @@ -585,7 +599,9 @@ mod tests { // Only the owner can transfer ownership testing::set_caller_address(timelock.veto_guardian()); testing::set_contract_address(timelock.veto_guardian()); - timelock.transfer_ownership(starknet::contract_address_const::<0x9999>()); + + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + ownable_timelock.transfer_ownership(starknet::contract_address_const::<0x9999>()); } #[test] @@ -593,8 +609,9 @@ mod tests { fn enable_space() { let (_, _, timelock) = setup_test(); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); timelock.enable_space(starknet::contract_address_const::<0x123456789>()); assert( timelock.is_space_enabled(starknet::contract_address_const::<0x123456789>()), @@ -618,8 +635,9 @@ mod tests { fn disable_space() { let (_, _, timelock) = setup_test(); - testing::set_caller_address(timelock.owner()); - testing::set_contract_address(timelock.owner()); + let ownable_timelock = IOwnableDispatcher { contract_address: timelock.contract_address }; + testing::set_caller_address(ownable_timelock.owner()); + testing::set_contract_address(ownable_timelock.owner()); timelock.enable_space(starknet::contract_address_const::<0x123456789>()); timelock.disable_space(starknet::contract_address_const::<0x123456789>()); assert( diff --git a/starknet/src/tests/space/space.cairo b/starknet/src/tests/space/space.cairo index 58259d7a..1668b512 100644 --- a/starknet/src/tests/space/space.cairo +++ b/starknet/src/tests/space/space.cairo @@ -2,6 +2,7 @@ mod tests { use starknet::{ContractAddress, syscalls, testing, info}; use openzeppelin::tests::utils; + use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait}; use sx::space::space::{Space, Space::{ProposalCreated, VoteCast, ProposalUpdated},}; use sx::tests::mocks::vanilla_authenticator::{ @@ -111,6 +112,7 @@ mod tests { .unwrap(); let space = ISpaceDispatcher { contract_address: space_address }; + let ownable_space = IOwnableDispatcher { contract_address: space_address }; space .initialize( @@ -127,7 +129,7 @@ mod tests { array![] ); - assert(space.owner() == owner, 'owner incorrect'); + assert(ownable_space.owner() == owner, 'owner incorrect'); assert(space.min_voting_duration() == min_voting_duration, 'min incorrect'); assert(space.max_voting_duration() == max_voting_duration, 'max incorrect'); assert(space.voting_delay() == voting_delay, 'voting delay incorrect'); diff --git a/starknet/src/tests/space/upgrade.cairo b/starknet/src/tests/space/upgrade.cairo index 39a6d197..f791097c 100644 --- a/starknet/src/tests/space/upgrade.cairo +++ b/starknet/src/tests/space/upgrade.cairo @@ -6,6 +6,7 @@ mod tests { use sx::tests::mocks::vanilla_authenticator::{ VanillaAuthenticator, IVanillaAuthenticatorDispatcher, IVanillaAuthenticatorDispatcherTrait }; + use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; use sx::tests::mocks::vanilla_execution_strategy::VanillaExecutionStrategy; use sx::tests::mocks::vanilla_voting_strategy::VanillaVotingStrategy; use sx::tests::mocks::vanilla_proposal_validation::VanillaProposalValidationStrategy; @@ -61,7 +62,8 @@ mod tests { // Set the owner to be the execution strategy testing::set_contract_address(config.owner); - space.transfer_ownership(execution_contract_address); + let ownable_space = IOwnableDispatcher { contract_address: space.contract_address }; + ownable_space.transfer_ownership(execution_contract_address); let selector = sx::utils::constants::UPGRADE_SELECTOR; let mut tx_calldata = array![]; From f617ebb0250c616b95eb8afc0493881230fb6e7c Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 9 Oct 2024 12:51:57 +0200 Subject: [PATCH 05/14] add space manager component --- .../src/execution_strategies/timelock.cairo | 28 ++-- starknet/src/lib.cairo | 1 - starknet/src/utils/space_manager.cairo | 138 +++++++++--------- 3 files changed, 86 insertions(+), 81 deletions(-) diff --git a/starknet/src/execution_strategies/timelock.cairo b/starknet/src/execution_strategies/timelock.cairo index 7cefe614..0870c366 100644 --- a/starknet/src/execution_strategies/timelock.cairo +++ b/starknet/src/execution_strategies/timelock.cairo @@ -30,7 +30,7 @@ mod TimelockExecutionStrategy { use sx::interfaces::IExecutionStrategy; use super::ITimelockExecutionStrategy; use sx::types::{Proposal, ProposalStatus}; - use sx::utils::{simple_quorum::SimpleQuorumComponent, SpaceManager}; + use sx::utils::{simple_quorum::SimpleQuorumComponent, space_manager::SpaceManagerComponent}; component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); @@ -44,6 +44,9 @@ mod TimelockExecutionStrategy { impl SimpleQuorumImpl = SimpleQuorumComponent::SimpleQuorumImpl; impl SimpleQuorumInternalImpl = SimpleQuorumComponent::InternalImpl; + component!(path: SpaceManagerComponent, storage: space_manager, event: SpaceManagerEvent); + impl SpaceManagerInternalImpl = SpaceManagerComponent::InternalImpl; + #[storage] struct Storage { _timelock_delay: u32, @@ -53,6 +56,8 @@ mod TimelockExecutionStrategy { ownable: OwnableComponent::Storage, #[substorage(v0)] simple_quorum: SimpleQuorumComponent::Storage, + #[substorage(v0)] + space_manager: SpaceManagerComponent::Storage, } #[event] @@ -70,6 +75,8 @@ mod TimelockExecutionStrategy { OwnableEvent: OwnableComponent::Event, #[flat] SimpleQuorumEvent: SimpleQuorumComponent::Event, + #[flat] + SpaceManagerEvent: SpaceManagerComponent::Event, } #[derive(Drop, PartialEq, starknet::Event)] @@ -138,8 +145,7 @@ mod TimelockExecutionStrategy { self.simple_quorum.initializer(quorum); - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer(ref state, spaces); + self.space_manager.initializer(spaces); self._timelock_delay.write(timelock_delay); self._veto_guardian.write(veto_guardian); @@ -156,10 +162,7 @@ mod TimelockExecutionStrategy { votes_abstain: u256, payload: Array ) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::assert_only_spaces(@state); + self.space_manager.assert_only_spaces(); let proposal_status = self .simple_quorum .get_proposal_status(@proposal, votes_for, votes_against, votes_abstain); @@ -296,21 +299,16 @@ mod TimelockExecutionStrategy { fn enable_space(ref self: ContractState, space: ContractAddress) { self.ownable.assert_only_owner(); - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::enable_space(ref state, space); + self.space_manager.enable_space(space); } fn disable_space(ref self: ContractState, space: ContractAddress) { self.ownable.assert_only_owner(); - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::disable_space(ref state, space); + self.space_manager.disable_space(space); } fn is_space_enabled(self: @ContractState, space: ContractAddress) -> bool { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::is_space_enabled(@state, space) + self.space_manager.is_space_enabled(space) } } diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 997ca6d5..543dd388 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -157,7 +157,6 @@ mod utils { use struct_hash::StructHash; mod space_manager; - use space_manager::SpaceManager; } mod external { diff --git a/starknet/src/utils/space_manager.cairo b/starknet/src/utils/space_manager.cairo index 198799c6..ece0d0a4 100644 --- a/starknet/src/utils/space_manager.cairo +++ b/starknet/src/utils/space_manager.cairo @@ -1,5 +1,5 @@ -#[starknet::contract] -mod SpaceManager { +#[starknet::component] +mod SpaceManagerComponent { use starknet::{ContractAddress, info}; #[storage] @@ -25,8 +25,12 @@ mod SpaceManager { } #[generate_trait] - impl InternalImpl of InternalTrait { - fn initializer(ref self: ContractState, mut spaces: Span) { + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { + fn initializer( + ref self: ComponentState, mut spaces: Span + ) { loop { match spaces.pop_front() { Option::Some(space) => { @@ -40,23 +44,23 @@ mod SpaceManager { } } - fn enable_space(ref self: ContractState, space: ContractAddress) { + fn enable_space(ref self: ComponentState, space: ContractAddress) { assert(space.is_non_zero() && !self._spaces.read(space), 'Invalid Space'); self._spaces.write(space, true); self.emit(Event::SpaceEnabled(SpaceEnabled { space: space })); } - fn disable_space(ref self: ContractState, space: ContractAddress) { + fn disable_space(ref self: ComponentState, space: ContractAddress) { assert(self._spaces.read(space), 'Invalid Space'); self._spaces.write(space, false); self.emit(Event::SpaceDisabled(SpaceDisabled { space: space })); } - fn is_space_enabled(self: @ContractState, space: ContractAddress) -> bool { + fn is_space_enabled(self: @ComponentState, space: ContractAddress) -> bool { return self._spaces.read(space); } - fn assert_only_spaces(self: @ContractState) { + fn assert_only_spaces(self: @ComponentState) { assert(self._spaces.read(info::get_caller_address()), 'Unauthorized Space'); } } @@ -65,19 +69,43 @@ mod SpaceManager { #[cfg(test)] mod tests { use starknet::{ContractAddress}; - use super::SpaceManager; + use super::SpaceManagerComponent; + use super::SpaceManagerComponent::InternalTrait; + + #[starknet::contract] + mod MockContract { + use super::SpaceManagerComponent; + + component!(path: SpaceManagerComponent, storage: space_manager, event: SpaceManagerEvent); + + #[storage] + struct Storage { + #[substorage(v0)] + space_manager: SpaceManagerComponent::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + SpaceManagerEvent: SpaceManagerComponent::Event + } + + impl SpaceManagerInternalImpl = SpaceManagerComponent::InternalImpl; + } + + type ComponentState = SpaceManagerComponent::ComponentState; + + fn COMPONENT_STATE() -> ComponentState { + SpaceManagerComponent::component_state_for_testing() + } #[test] #[available_gas(10000000)] fn initializer() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer( - ref state, array![starknet::contract_address_const::<0x123456789>()].span() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![starknet::contract_address_const::<0x123456789>()].span()); assert( - SpaceManager::InternalImpl::is_space_enabled( - @state, starknet::contract_address_const::<0x123456789>() - ), + state.is_space_enabled(starknet::contract_address_const::<0x123456789>()), 'initializer failed' ); } @@ -86,39 +114,33 @@ mod tests { #[available_gas(10000000)] #[should_panic(expected: ('Invalid Space',))] fn initializer_duplicate_address() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer( - ref state, - array![ - starknet::contract_address_const::<0x123456789>(), - starknet::contract_address_const::<0x123456789>() - ] - .span() - ); + let mut state = COMPONENT_STATE(); + state + .initializer( + array![ + starknet::contract_address_const::<0x123456789>(), + starknet::contract_address_const::<0x123456789>() + ] + .span() + ); } #[test] #[available_gas(10000000)] #[should_panic(expected: ('Invalid Space',))] fn initializer_zero_address() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer( - ref state, array![starknet::contract_address_const::<0x0>()].span() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![starknet::contract_address_const::<0x0>()].span()); } #[test] #[available_gas(10000000)] fn enable_space() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer(ref state, array![].span()); - SpaceManager::InternalImpl::enable_space( - ref state, starknet::contract_address_const::<0x123456789>() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![].span()); + state.enable_space(starknet::contract_address_const::<0x123456789>()); assert( - SpaceManager::InternalImpl::is_space_enabled( - @state, starknet::contract_address_const::<0x123456789>() - ), + state.is_space_enabled(starknet::contract_address_const::<0x123456789>()), 'enable_space failed' ); } @@ -127,41 +149,29 @@ mod tests { #[available_gas(10000000)] #[should_panic(expected: ('Invalid Space',))] fn enable_space_duplicate_address() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer(ref state, array![].span()); - SpaceManager::InternalImpl::enable_space( - ref state, starknet::contract_address_const::<0x123456789>() - ); - SpaceManager::InternalImpl::enable_space( - ref state, starknet::contract_address_const::<0x123456789>() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![].span()); + state.enable_space(starknet::contract_address_const::<0x123456789>()); + state.enable_space(starknet::contract_address_const::<0x123456789>()); } #[test] #[available_gas(10000000)] #[should_panic(expected: ('Invalid Space',))] fn enable_space_zero_address() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer(ref state, array![].span()); - SpaceManager::InternalImpl::enable_space( - ref state, starknet::contract_address_const::<0x0>() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![].span()); + state.enable_space(starknet::contract_address_const::<0x0>()); } #[test] #[available_gas(10000000)] fn disable_space() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer( - ref state, array![starknet::contract_address_const::<0x123456789>()].span() - ); - SpaceManager::InternalImpl::disable_space( - ref state, starknet::contract_address_const::<0x123456789>() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![starknet::contract_address_const::<0x123456789>()].span()); + state.disable_space(starknet::contract_address_const::<0x123456789>()); assert( - !SpaceManager::InternalImpl::is_space_enabled( - @state, starknet::contract_address_const::<0x123456789>() - ), + !state.is_space_enabled(starknet::contract_address_const::<0x123456789>()), 'disable_space failed' ); } @@ -170,10 +180,8 @@ mod tests { #[available_gas(10000000)] #[should_panic(expected: ('Invalid Space',))] fn disable_space_not_enabled() { - let mut state = SpaceManager::unsafe_new_contract_state(); - SpaceManager::InternalImpl::initializer(ref state, array![].span()); - SpaceManager::InternalImpl::disable_space( - ref state, starknet::contract_address_const::<0x123456789>() - ); + let mut state = COMPONENT_STATE(); + state.initializer(array![].span()); + state.disable_space(starknet::contract_address_const::<0x123456789>()); } } From 726aded05fdd63980f09950f9c5dfde1df11dc14 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:19:31 +0200 Subject: [PATCH 06/14] add reinitializable component --- starknet/src/lib.cairo | 1 - starknet/src/space/space.cairo | 29 ++++++++++++++---------- starknet/src/tests/mocks/space_v2.cairo | 24 +++++++++++++++----- starknet/src/utils/reinitializable.cairo | 21 +++++++++++------ 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 543dd388..00fe7298 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -142,7 +142,6 @@ mod utils { mod proposition_power; mod reinitializable; - use reinitializable::Reinitializable; mod simple_majority; diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index 4d940c4a..07bb6c0c 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -16,9 +16,10 @@ mod Space { proposal::ProposalDefault }; use sx::utils::{ - Reinitializable, BitSetter, LegacyHashChoice, LegacyHashUserAddress, LegacyHashVotePower, + BitSetter, LegacyHashChoice, LegacyHashUserAddress, LegacyHashVotePower, LegacyHashVoteRegistry, constants::{INITIALIZE_SELECTOR, POST_UPGRADE_INITIALIZER_SELECTOR} }; + use sx::utils::reinitializable::ReinitializableComponent; component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); @@ -26,6 +27,12 @@ mod Space { impl OwnableImpl = OwnableComponent::OwnableImpl; impl OwnableInternalImpl = OwnableComponent::InternalImpl; + component!( + path: ReinitializableComponent, storage: reinitializable, event: ReinitializableEvent + ); + + impl ReinitializableInternalImpl = ReinitializableComponent::InternalImpl; + #[storage] struct Storage { _min_voting_duration: u32, @@ -43,6 +50,8 @@ mod Space { _vote_registry: LegacyMap::<(u256, UserAddress), bool>, #[substorage(v0)] ownable: OwnableComponent::Storage, + #[substorage(v0)] + reinitializable: ReinitializableComponent::Storage } #[event] @@ -66,8 +75,11 @@ mod Space { VotingDelayUpdated: VotingDelayUpdated, Upgraded: Upgraded, #[flat] - OwnableEvent: OwnableComponent::Event + OwnableEvent: OwnableComponent::Event, + #[flat] + ReinitializableEvent: ReinitializableComponent::Event } + #[derive(Drop, PartialEq, starknet::Event)] struct SpaceCreated { @@ -216,11 +228,7 @@ mod Space { ); // Checking that the contract is not already initialized - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state: Reinitializable::ContractState = - Reinitializable::unsafe_new_contract_state(); - Reinitializable::InternalImpl::initialize(ref state); + self.reinitializable.initialize(); assert(voting_strategies.len() != 0, 'empty voting strategies'); assert(authenticators.len() != 0, 'empty authenticators'); @@ -456,9 +464,7 @@ mod Space { starknet::replace_class_syscall(class_hash)?; // Allowing initializer to be called again. - let mut state: Reinitializable::ContractState = - Reinitializable::unsafe_new_contract_state(); - Reinitializable::InternalImpl::reset(ref state); + self.reinitializable.reset(); // Call `post_upgrade_initializer` on the new version. syscalls::call_contract_syscall( @@ -481,8 +487,7 @@ mod Space { fn post_upgrade_initializer(ref self: ContractState, initialize_calldata: Array,) { // This code is left here to indicate to future developers that this // function should be called only once! - let mut state = Reinitializable::unsafe_new_contract_state(); - Reinitializable::InternalImpl::initialize(ref state); + self.reinitializable.initialize(); // This contract being the first version, we don't expect anyone to upgrade to it. // We leave the implementation empty. } diff --git a/starknet/src/tests/mocks/space_v2.cairo b/starknet/src/tests/mocks/space_v2.cairo index cd2e0dc4..92d7fee3 100644 --- a/starknet/src/tests/mocks/space_v2.cairo +++ b/starknet/src/tests/mocks/space_v2.cairo @@ -7,20 +7,32 @@ trait ISpaceV2 { #[starknet::contract] mod SpaceV2 { use super::ISpaceV2; - use sx::utils::reinitializable::Reinitializable; + use sx::utils::reinitializable::ReinitializableComponent; + + component!( + path: ReinitializableComponent, storage: reinitializable, event: ReinitializableEvent + ); + + impl ReinitializableInternalImpl = ReinitializableComponent::InternalImpl; #[storage] struct Storage { - _var: felt252 + _var: felt252, + #[substorage(v0)] + reinitializable: ReinitializableComponent::Storage, + } + + #[event] + #[derive(Drop, PartialEq, starknet::Event)] + enum Event { + #[flat] + ReinitializableEvent: ReinitializableComponent::Event } #[abi(embed_v0)] impl SpaceV2 of ISpaceV2 { fn post_upgrade_initializer(ref self: ContractState, var: felt252) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state = Reinitializable::unsafe_new_contract_state(); - Reinitializable::InternalImpl::initialize(ref state); + self.reinitializable.initialize(); self._var.write(var); } diff --git a/starknet/src/utils/reinitializable.cairo b/starknet/src/utils/reinitializable.cairo index 951b4700..d160cc6f 100644 --- a/starknet/src/utils/reinitializable.cairo +++ b/starknet/src/utils/reinitializable.cairo @@ -1,6 +1,6 @@ /// A helper module for initializing / re-initializing. -#[starknet::contract] -mod Reinitializable { +#[starknet::component] +mod ReinitializableComponent { use starknet::ContractAddress; #[storage] @@ -8,26 +8,33 @@ mod Reinitializable { _initialized: bool } + // Event is needed to derive PartialEq on it so we can test it in other modules. + #[event] + #[derive(Drop, PartialEq, starknet::Event)] + enum Event {} + #[generate_trait] - impl InternalImpl of InternalTrait { + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { /// Initialize the contract. Must not have been initialized before. - fn initialize(ref self: ContractState) { + fn initialize(ref self: ComponentState) { self.not_initialized(); self._initialized.write(true); } /// Reset the initialization state of the contract, allowing it to be initialized again in the future. - fn reset(ref self: ContractState) { + fn reset(ref self: ComponentState) { self._initialized.write(false); } /// Asserts that the contract has been initialized. - fn initialized(self: @ContractState) { + fn initialized(self: @ComponentState) { assert(self._initialized.read() == true, 'Not Initialized'); } /// Asserts that the contract has not been initialized. - fn not_initialized(self: @ContractState) { + fn not_initialized(self: @ComponentState) { assert(self._initialized.read() == false, 'Already Initialized'); } } From 26db88b4d70c6ad5f3c38ef549bfc99b69f84619 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:23:24 +0200 Subject: [PATCH 07/14] scarb: fmt --- starknet/src/space/space.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index 07bb6c0c..b651e478 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -79,7 +79,7 @@ mod Space { #[flat] ReinitializableEvent: ReinitializableComponent::Event } - + #[derive(Drop, PartialEq, starknet::Event)] struct SpaceCreated { From f2eee0df68a71cdd9c12a11413d490b472142e1b Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:41:31 +0200 Subject: [PATCH 08/14] add Single Slot Proof Component --- starknet/src/lib.cairo | 1 - starknet/src/utils/single_slot_proof.cairo | 18 ++++--- .../voting_strategies/evm_slot_value.cairo | 39 ++++++++------- .../oz_votes_storage_proof.cairo | 47 +++++++++++------- .../oz_votes_trace_208_storage_proof.cairo | 48 +++++++++++-------- 5 files changed, 93 insertions(+), 60 deletions(-) diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 00fe7298..5c766fc9 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -148,7 +148,6 @@ mod utils { mod simple_quorum; mod single_slot_proof; - use single_slot_proof::SingleSlotProof; mod snip12; diff --git a/starknet/src/utils/single_slot_proof.cairo b/starknet/src/utils/single_slot_proof.cairo index 3a68345c..77f45bf1 100644 --- a/starknet/src/utils/single_slot_proof.cairo +++ b/starknet/src/utils/single_slot_proof.cairo @@ -1,5 +1,5 @@ -#[starknet::contract] -mod SingleSlotProof { +#[starknet::component] +mod SingleSlotProofComponent { use starknet::{ContractAddress, EthAddress}; use sx::external::herodotus::{ Words64, BinarySearchTree, ITimestampRemappersDispatcher, @@ -15,9 +15,11 @@ mod SingleSlotProof { } #[generate_trait] - impl InternalImpl of InternalTrait { + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { fn initializer( - ref self: ContractState, + ref self: ComponentState, timestamp_remappers: ContractAddress, facts_registry: ContractAddress ) { @@ -26,7 +28,7 @@ mod SingleSlotProof { } fn get_storage_slot( - self: @ContractState, + self: @ComponentState, timestamp: u32, l1_contract_address: EthAddress, slot_key: u256, @@ -45,7 +47,9 @@ mod SingleSlotProof { slot_value } - fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { + fn cache_timestamp( + ref self: ComponentState, timestamp: u32, tree: BinarySearchTree + ) { // Maps timestamp to closest L1 block number that occurred before the timestamp. If the queried // timestamp is less than the earliest timestamp or larger than the latest timestamp in the mapper // then the call will return Option::None and the transaction will revert. @@ -59,7 +63,7 @@ mod SingleSlotProof { self._cached_remapped_timestamps.write(timestamp, l1_block_number); } - fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { + fn cached_timestamps(self: @ComponentState, timestamp: u32) -> u256 { let l1_block_number = self._cached_remapped_timestamps.read(timestamp); assert(l1_block_number.is_non_zero(), 'Timestamp not cached'); l1_block_number diff --git a/starknet/src/voting_strategies/evm_slot_value.cairo b/starknet/src/voting_strategies/evm_slot_value.cairo index 199969df..b80c5806 100644 --- a/starknet/src/voting_strategies/evm_slot_value.cairo +++ b/starknet/src/voting_strategies/evm_slot_value.cairo @@ -4,11 +4,27 @@ mod EvmSlotValueVotingStrategy { use sx::external::herodotus::BinarySearchTree; use sx::types::{UserAddress, UserAddressTrait}; use sx::interfaces::IVotingStrategy; - use sx::utils::{SingleSlotProof, TIntoU256}; + use sx::utils::{single_slot_proof::SingleSlotProofComponent, TIntoU256}; use sx::utils::endian::ByteReverse; + component!( + path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent + ); + + impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; + #[storage] - struct Storage {} + struct Storage { + #[substorage(v0)] + single_slot_proof: SingleSlotProofComponent::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + SingleSlotProofEvent: SingleSlotProofComponent::Event + } #[abi(embed_v0)] impl EvmSlotValueVotingStrategy of IVotingStrategy { @@ -50,12 +66,8 @@ mod EvmSlotValueVotingStrategy { let slot_key = InternalImpl::get_mapping_slot_key(voter.into(), slot_index); // Returns the value of the storage slot at the block number corresponding to the given timestamp. - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let state = SingleSlotProof::unsafe_new_contract_state(); - let slot_value = SingleSlotProof::InternalImpl::get_storage_slot( - @state, timestamp, evm_contract_address, slot_key, mpt_proof - ); + let slot_value = self.single_slot_proof + .get_storage_slot(timestamp, evm_contract_address, slot_key, mpt_proof); slot_value } @@ -74,8 +86,7 @@ mod EvmSlotValueVotingStrategy { /// * `timestamp` - The timestamp at which to query. /// * `tree` - The tree proof required to query the remapper. fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cache_timestamp(ref state, timestamp, tree); + self.single_slot_proof.cache_timestamp(timestamp, tree); } /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. @@ -88,8 +99,7 @@ mod EvmSlotValueVotingStrategy { /// /// * `u256` - The cached L1 block number corresponding to the timestamp. fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - let state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cached_timestamps(@state, timestamp) + self.single_slot_proof.cached_timestamps(timestamp) } } @@ -106,10 +116,7 @@ mod EvmSlotValueVotingStrategy { timestamp_remappers: ContractAddress, facts_registry: ContractAddress ) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::initializer(ref state, timestamp_remappers, facts_registry); + self.single_slot_proof.initializer(timestamp_remappers, facts_registry); } } diff --git a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo index 7f805e8a..08d78a9e 100644 --- a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo @@ -4,11 +4,28 @@ mod OZVotesStorageProofVotingStrategy { use sx::external::herodotus::BinarySearchTree; use sx::types::{UserAddress, UserAddressTrait}; use sx::interfaces::IVotingStrategy; - use sx::utils::{SingleSlotProof, TIntoU256}; + use sx::utils::{single_slot_proof::SingleSlotProofComponent, TIntoU256}; use sx::utils::endian::ByteReverse; + component!( + path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent + ); + + impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; + + #[storage] - struct Storage {} + struct Storage { + #[substorage(v0)] + single_slot_proof: SingleSlotProofComponent::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + SingleSlotProofEvent: SingleSlotProofComponent::Event + } #[abi(embed_v0)] impl OZVotesStorageProofVotingStrategy of IVotingStrategy { @@ -57,16 +74,17 @@ mod OZVotesStorageProofVotingStrategy { // Get the slot containing the final checkpoint // Migration to components planned ; disregard the `unsafe` keyword, // it is actually safe. - let state = SingleSlotProof::unsafe_new_contract_state(); - let checkpoint = SingleSlotProof::InternalImpl::get_storage_slot( - @state, timestamp, evm_contract_address, slot_key, checkpoint_mpt_proof - ); + let checkpoint = self + .single_slot_proof + .get_storage_slot(timestamp, evm_contract_address, slot_key, checkpoint_mpt_proof); // Verify the checkpoint is indeed the final checkpoint by checking the next slot is zero. assert( - SingleSlotProof::InternalImpl::get_storage_slot( - @state, timestamp, evm_contract_address, slot_key + 1, exclusion_mpt_proof - ) + self + .single_slot_proof + .get_storage_slot( + timestamp, evm_contract_address, slot_key + 1, exclusion_mpt_proof + ) .is_zero(), 'Invalid Checkpoint' ); @@ -91,8 +109,7 @@ mod OZVotesStorageProofVotingStrategy { /// * `timestamp` - The timestamp at which to query. /// * `tree` - The tree proof required to query the remapper. fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cache_timestamp(ref state, timestamp, tree); + self.single_slot_proof.cache_timestamp(timestamp, tree); } /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. @@ -105,8 +122,7 @@ mod OZVotesStorageProofVotingStrategy { /// /// * `u256` - The cached L1 block number corresponding to the timestamp. fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - let state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cached_timestamps(@state, timestamp) + self.single_slot_proof.cached_timestamps(timestamp) } } @@ -146,10 +162,7 @@ mod OZVotesStorageProofVotingStrategy { timestamp_remappers: ContractAddress, facts_registry: ContractAddress ) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::initializer(ref state, timestamp_remappers, facts_registry); + self.single_slot_proof.initializer(timestamp_remappers, facts_registry); } } diff --git a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo index 15e3c81f..89a3d8dd 100644 --- a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo @@ -4,11 +4,27 @@ mod OZVotesTrace208StorageProofVotingStrategy { use sx::external::herodotus::BinarySearchTree; use sx::types::{UserAddress, UserAddressTrait}; use sx::interfaces::IVotingStrategy; - use sx::utils::{SingleSlotProof, TIntoU256}; + use sx::utils::{single_slot_proof::SingleSlotProofComponent, TIntoU256}; use sx::utils::endian::ByteReverse; + component!( + path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent + ); + + impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; + #[storage] - struct Storage {} + struct Storage { + #[substorage(v0)] + single_slot_proof: SingleSlotProofComponent::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + SingleSlotProofEvent: SingleSlotProofComponent::Event + } #[abi(embed_v0)] impl OZVotesTrace208StorageProofVotingStrategy of IVotingStrategy { @@ -55,18 +71,17 @@ mod OZVotesTrace208StorageProofVotingStrategy { ); // Get the slot containing the final checkpoint - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let state = SingleSlotProof::unsafe_new_contract_state(); - let checkpoint = SingleSlotProof::InternalImpl::get_storage_slot( - @state, timestamp, evm_contract_address, slot_key, checkpoint_mpt_proof - ); + let checkpoint = self + .single_slot_proof + .get_storage_slot(timestamp, evm_contract_address, slot_key, checkpoint_mpt_proof); // Verify the checkpoint is indeed the final checkpoint by checking the next slot is zero. assert( - SingleSlotProof::InternalImpl::get_storage_slot( - @state, timestamp, evm_contract_address, slot_key + 1, exclusion_mpt_proof - ) + self + .single_slot_proof + .get_storage_slot( + timestamp, evm_contract_address, slot_key + 1, exclusion_mpt_proof + ) .is_zero(), 'Invalid Checkpoint' ); @@ -91,8 +106,7 @@ mod OZVotesTrace208StorageProofVotingStrategy { /// * `timestamp` - The timestamp at which to query. /// * `tree` - The tree proof required to query the remapper. fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cache_timestamp(ref state, timestamp, tree); + self.single_slot_proof.cache_timestamp(timestamp, tree); } /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. @@ -105,8 +119,7 @@ mod OZVotesTrace208StorageProofVotingStrategy { /// /// * `u256` - The cached L1 block number corresponding to the timestamp. fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - let state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::cached_timestamps(@state, timestamp) + self.single_slot_proof.cached_timestamps(timestamp) } } @@ -146,10 +159,7 @@ mod OZVotesTrace208StorageProofVotingStrategy { timestamp_remappers: ContractAddress, facts_registry: ContractAddress ) { - // Migration to components planned ; disregard the `unsafe` keyword, - // it is actually safe. - let mut state = SingleSlotProof::unsafe_new_contract_state(); - SingleSlotProof::InternalImpl::initializer(ref state, timestamp_remappers, facts_registry); + self.single_slot_proof.initializer(timestamp_remappers, facts_registry); } } From f2ad4a51d967d1a2386d06f20e638fbbc3cbbe32 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:43:02 +0200 Subject: [PATCH 09/14] scarb fmt --- starknet/src/voting_strategies/evm_slot_value.cairo | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/starknet/src/voting_strategies/evm_slot_value.cairo b/starknet/src/voting_strategies/evm_slot_value.cairo index b80c5806..080eff6e 100644 --- a/starknet/src/voting_strategies/evm_slot_value.cairo +++ b/starknet/src/voting_strategies/evm_slot_value.cairo @@ -66,7 +66,8 @@ mod EvmSlotValueVotingStrategy { let slot_key = InternalImpl::get_mapping_slot_key(voter.into(), slot_index); // Returns the value of the storage slot at the block number corresponding to the given timestamp. - let slot_value = self.single_slot_proof + let slot_value = self + .single_slot_proof .get_storage_slot(timestamp, evm_contract_address, slot_key, mpt_proof); slot_value From 7b21f43c01b0cdc865db39bb929ecfa6b35622cd Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:59:08 +0200 Subject: [PATCH 10/14] use storage prefix to avoid clashes --- starknet/src/utils/reinitializable.cairo | 10 +++++----- starknet/src/utils/simple_quorum.cairo | 8 ++++---- starknet/src/utils/single_slot_proof.cairo | 20 ++++++++++---------- starknet/src/utils/snip12.cairo | 6 +++--- starknet/src/utils/space_manager.cairo | 19 ++++++++++--------- 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/starknet/src/utils/reinitializable.cairo b/starknet/src/utils/reinitializable.cairo index d160cc6f..a536364e 100644 --- a/starknet/src/utils/reinitializable.cairo +++ b/starknet/src/utils/reinitializable.cairo @@ -5,7 +5,7 @@ mod ReinitializableComponent { #[storage] struct Storage { - _initialized: bool + Reinitializable_initialized: bool } // Event is needed to derive PartialEq on it so we can test it in other modules. @@ -20,22 +20,22 @@ mod ReinitializableComponent { /// Initialize the contract. Must not have been initialized before. fn initialize(ref self: ComponentState) { self.not_initialized(); - self._initialized.write(true); + self.Reinitializable_initialized.write(true); } /// Reset the initialization state of the contract, allowing it to be initialized again in the future. fn reset(ref self: ComponentState) { - self._initialized.write(false); + self.Reinitializable_initialized.write(false); } /// Asserts that the contract has been initialized. fn initialized(self: @ComponentState) { - assert(self._initialized.read() == true, 'Not Initialized'); + assert(self.Reinitializable_initialized.read() == true, 'Not Initialized'); } /// Asserts that the contract has not been initialized. fn not_initialized(self: @ComponentState) { - assert(self._initialized.read() == false, 'Already Initialized'); + assert(self.Reinitializable_initialized.read() == false, 'Already Initialized'); } } } diff --git a/starknet/src/utils/simple_quorum.cairo b/starknet/src/utils/simple_quorum.cairo index 4e2712f5..ad2e6554 100644 --- a/starknet/src/utils/simple_quorum.cairo +++ b/starknet/src/utils/simple_quorum.cairo @@ -6,7 +6,7 @@ mod SimpleQuorumComponent { #[storage] struct Storage { - _quorum: u256 + Simplequorum_quorum: u256 } #[generate_trait] @@ -14,7 +14,7 @@ mod SimpleQuorumComponent { TContractState, +HasComponent > of InternalTrait { fn initializer(ref self: ComponentState, quorum: u256) { - self._quorum.write(quorum); + self.Simplequorum_quorum.write(quorum); } /// Returns the status of a proposal. @@ -32,7 +32,7 @@ mod SimpleQuorumComponent { votes_against: u256, votes_abstain: u256, ) -> ProposalStatus { - let quorum_reached = votes_for + votes_abstain >= self._quorum.read(); + let quorum_reached = votes_for + votes_abstain >= self.Simplequorum_quorum.read(); let supported = votes_for > votes_against; let accepted = quorum_reached && supported; @@ -64,7 +64,7 @@ mod SimpleQuorumComponent { TContractState, +HasComponent > of IQuorum> { fn quorum(self: @ComponentState) -> u256 { - self._quorum.read() + self.Simplequorum_quorum.read() } } } diff --git a/starknet/src/utils/single_slot_proof.cairo b/starknet/src/utils/single_slot_proof.cairo index 77f45bf1..50b21169 100644 --- a/starknet/src/utils/single_slot_proof.cairo +++ b/starknet/src/utils/single_slot_proof.cairo @@ -9,9 +9,9 @@ mod SingleSlotProofComponent { #[storage] struct Storage { - _timestamp_remappers: ContractAddress, - _facts_registry: ContractAddress, - _cached_remapped_timestamps: LegacyMap:: + Singleslotproof_timestamp_remappers: ContractAddress, + Singleslotproof_facts_registry: ContractAddress, + Singleslotproof_cached_remapped_timestamps: LegacyMap:: } #[generate_trait] @@ -23,8 +23,8 @@ mod SingleSlotProofComponent { timestamp_remappers: ContractAddress, facts_registry: ContractAddress ) { - self._timestamp_remappers.write(timestamp_remappers); - self._facts_registry.write(facts_registry); + self.Singleslotproof_timestamp_remappers.write(timestamp_remappers); + self.Singleslotproof_facts_registry.write(facts_registry); } fn get_storage_slot( @@ -35,12 +35,12 @@ mod SingleSlotProofComponent { mpt_proof: Span ) -> u256 { // Checks if the timestamp is already cached. - let l1_block_number = self._cached_remapped_timestamps.read(timestamp); + let l1_block_number = self.Singleslotproof_cached_remapped_timestamps.read(timestamp); assert(l1_block_number.is_non_zero(), 'Timestamp not cached'); // Returns the value of the storage slot of account: `l1_contract_address` at key: `slot_key` and block number: `l1_block_number`. let slot_value = IEVMFactsRegistryDispatcher { - contract_address: self._facts_registry.read() + contract_address: self.Singleslotproof_facts_registry.read() } .get_storage(l1_block_number, l1_contract_address.into(), slot_key, mpt_proof); @@ -54,17 +54,17 @@ mod SingleSlotProofComponent { // timestamp is less than the earliest timestamp or larger than the latest timestamp in the mapper // then the call will return Option::None and the transaction will revert. let l1_block_number = ITimestampRemappersDispatcher { - contract_address: self._timestamp_remappers.read() + contract_address: self.Singleslotproof_timestamp_remappers.read() } .get_closest_l1_block_number(tree, timestamp.into()) .expect('TimestampRemappers call failed') .expect('Timestamp out of range'); - self._cached_remapped_timestamps.write(timestamp, l1_block_number); + self.Singleslotproof_cached_remapped_timestamps.write(timestamp, l1_block_number); } fn cached_timestamps(self: @ComponentState, timestamp: u32) -> u256 { - let l1_block_number = self._cached_remapped_timestamps.read(timestamp); + let l1_block_number = self.Singleslotproof_cached_remapped_timestamps.read(timestamp); assert(l1_block_number.is_non_zero(), 'Timestamp not cached'); l1_block_number } diff --git a/starknet/src/utils/snip12.cairo b/starknet/src/utils/snip12.cairo index 5a4f51f0..11edd26b 100644 --- a/starknet/src/utils/snip12.cairo +++ b/starknet/src/utils/snip12.cairo @@ -13,7 +13,7 @@ mod SNIP12Component { #[storage] struct Storage { - _domain_hash: felt252 + Snip12_domain_hash: felt252 } #[generate_trait] @@ -21,7 +21,7 @@ mod SNIP12Component { TContractState, +HasComponent > of InternalTrait { fn initializer(ref self: ComponentState, name: felt252, version: felt252) { - self._domain_hash.write(get_domain_hash(name, version)); + self.Snip12_domain_hash.write(get_domain_hash(name, version)); } /// Verifies the signature of the propose calldata. @@ -154,7 +154,7 @@ mod SNIP12Component { ) -> felt252 { let mut encoded_data = array![]; STARKNET_MESSAGE.serialize(ref encoded_data); - self._domain_hash.read().serialize(ref encoded_data); + self.Snip12_domain_hash.read().serialize(ref encoded_data); signer.serialize(ref encoded_data); message_hash.serialize(ref encoded_data); encoded_data.span().struct_hash() diff --git a/starknet/src/utils/space_manager.cairo b/starknet/src/utils/space_manager.cairo index ece0d0a4..b2cd395f 100644 --- a/starknet/src/utils/space_manager.cairo +++ b/starknet/src/utils/space_manager.cairo @@ -4,7 +4,7 @@ mod SpaceManagerComponent { #[storage] struct Storage { - _spaces: LegacyMap:: + Spacemanager_spaces: LegacyMap:: } #[event] @@ -35,9 +35,10 @@ mod SpaceManagerComponent { match spaces.pop_front() { Option::Some(space) => { assert( - (*space).is_non_zero() && !self._spaces.read(*space), 'Invalid Space' + (*space).is_non_zero() && !self.Spacemanager_spaces.read(*space), + 'Invalid Space' ); - self._spaces.write(*space, true); + self.Spacemanager_spaces.write(*space, true); }, Option::None(()) => { break; } }; @@ -45,23 +46,23 @@ mod SpaceManagerComponent { } fn enable_space(ref self: ComponentState, space: ContractAddress) { - assert(space.is_non_zero() && !self._spaces.read(space), 'Invalid Space'); - self._spaces.write(space, true); + assert(space.is_non_zero() && !self.Spacemanager_spaces.read(space), 'Invalid Space'); + self.Spacemanager_spaces.write(space, true); self.emit(Event::SpaceEnabled(SpaceEnabled { space: space })); } fn disable_space(ref self: ComponentState, space: ContractAddress) { - assert(self._spaces.read(space), 'Invalid Space'); - self._spaces.write(space, false); + assert(self.Spacemanager_spaces.read(space), 'Invalid Space'); + self.Spacemanager_spaces.write(space, false); self.emit(Event::SpaceDisabled(SpaceDisabled { space: space })); } fn is_space_enabled(self: @ComponentState, space: ContractAddress) -> bool { - return self._spaces.read(space); + return self.Spacemanager_spaces.read(space); } fn assert_only_spaces(self: @ComponentState) { - assert(self._spaces.read(info::get_caller_address()), 'Unauthorized Space'); + assert(self.Spacemanager_spaces.read(info::get_caller_address()), 'Unauthorized Space'); } } } From 250b40c2535e6e928b30c749662d54836b017447 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:03:28 +0200 Subject: [PATCH 11/14] add SingleSlotProofImpl as embeddable --- .../src/interfaces/i_single_slot_proof.cairo | 28 ++++++++++++++++ starknet/src/lib.cairo | 5 +++ starknet/src/utils/single_slot_proof.cairo | 6 ++++ .../voting_strategies/evm_slot_value.cairo | 32 ++---------------- .../oz_votes_storage_proof.cairo | 33 ++----------------- .../oz_votes_trace_208_storage_proof.cairo | 33 ++----------------- 6 files changed, 47 insertions(+), 90 deletions(-) create mode 100644 starknet/src/interfaces/i_single_slot_proof.cairo diff --git a/starknet/src/interfaces/i_single_slot_proof.cairo b/starknet/src/interfaces/i_single_slot_proof.cairo new file mode 100644 index 00000000..19c8992d --- /dev/null +++ b/starknet/src/interfaces/i_single_slot_proof.cairo @@ -0,0 +1,28 @@ +use sx::external::herodotus::BinarySearchTree; + +/// Optional trait that execution strategies can decide to implement. +#[starknet::interface] +trait ISingleSlotProof { + /// Queries the Timestamp Remapper contract for the closest L1 block number that occurred before + /// the given timestamp and then caches the result. If the queried timestamp is less than the earliest + /// timestamp or larger than the latest timestamp in the mapper then the transaction will revert. + /// This function should be used to cache a remapped timestamp before it's used when calling the + /// `get_storage_slot` function with the same timestamp. + /// + /// # Arguments + /// + /// * `timestamp` - The timestamp at which to query. + /// * `tree` - The tree proof required to query the remapper. + fn cache_timestamp(ref self: TContractState, timestamp: u32, tree: BinarySearchTree); + + /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. + /// + /// # Arguments + /// + /// * `timestamp` - The timestamp to query. + /// + /// # Returns + /// + /// * `u256` - The cached L1 block number corresponding to the timestamp. + fn cached_timestamps(self: @TContractState, timestamp: u32) -> u256; +} diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 5c766fc9..a014c763 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -72,6 +72,11 @@ mod interfaces { IVotingStrategy, IVotingStrategyDispatcher, IVotingStrategyDispatcherTrait }; + mod i_single_slot_proof; + use i_single_slot_proof::{ + ISingleSlotProof, ISingleSlotProofDispatcher, ISingleSlotProofDispatcherTrait + }; + mod i_space; use i_space::{ISpace, ISpaceDispatcher, ISpaceDispatcherTrait}; diff --git a/starknet/src/utils/single_slot_proof.cairo b/starknet/src/utils/single_slot_proof.cairo index 50b21169..9b77fa54 100644 --- a/starknet/src/utils/single_slot_proof.cairo +++ b/starknet/src/utils/single_slot_proof.cairo @@ -6,6 +6,7 @@ mod SingleSlotProofComponent { ITimestampRemappersDispatcherTrait, IEVMFactsRegistryDispatcher, IEVMFactsRegistryDispatcherTrait }; + use sx::interfaces::i_single_slot_proof::ISingleSlotProof; #[storage] struct Storage { @@ -46,7 +47,12 @@ mod SingleSlotProofComponent { slot_value } + } + #[embeddable_as(SingleSlotProofImpl)] + impl SimpleQuorum< + TContractState, +HasComponent + > of ISingleSlotProof> { fn cache_timestamp( ref self: ComponentState, timestamp: u32, tree: BinarySearchTree ) { diff --git a/starknet/src/voting_strategies/evm_slot_value.cairo b/starknet/src/voting_strategies/evm_slot_value.cairo index 080eff6e..25de36f0 100644 --- a/starknet/src/voting_strategies/evm_slot_value.cairo +++ b/starknet/src/voting_strategies/evm_slot_value.cairo @@ -11,6 +11,8 @@ mod EvmSlotValueVotingStrategy { path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent ); + #[abi(embed_v0)] + impl SingleSlotProofImpl = SingleSlotProofComponent::SingleSlotProofImpl; impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; #[storage] @@ -74,36 +76,6 @@ mod EvmSlotValueVotingStrategy { } } - #[generate_trait] - impl SingleSlotProofImpl of SingleSlotProofTrait { - /// Queries the Timestamp Remapper contract for the closest L1 block number that occurred before - /// the given timestamp and then caches the result. If the queried timestamp is less than the earliest - /// timestamp or larger than the latest timestamp in the mapper then the transaction will revert. - /// This function should be used to cache a remapped timestamp before it's used when calling the - /// `get_storage_slot` function with the same timestamp. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp at which to query. - /// * `tree` - The tree proof required to query the remapper. - fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - self.single_slot_proof.cache_timestamp(timestamp, tree); - } - - /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp to query. - /// - /// # Returns - /// - /// * `u256` - The cached L1 block number corresponding to the timestamp. - fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - self.single_slot_proof.cached_timestamps(timestamp) - } - } - #[generate_trait] impl InternalImpl of InternalTrait { fn get_mapping_slot_key(mapping_key: u256, slot_index: u256) -> u256 { diff --git a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo index 08d78a9e..8a9a1736 100644 --- a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo @@ -11,6 +11,9 @@ mod OZVotesStorageProofVotingStrategy { path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent ); + #[abi(embed_v0)] + impl SingleSlotProofImpl = + SingleSlotProofComponent::SingleSlotProofImpl; impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; @@ -96,36 +99,6 @@ mod OZVotesStorageProofVotingStrategy { } } - #[generate_trait] - impl SingleSlotProofImpl of SingleSlotProofTrait { - /// Queries the Timestamp Remapper contract for the closest L1 block number that occurred before - /// the given timestamp and then caches the result. If the queried timestamp is less than the earliest - /// timestamp or larger than the latest timestamp in the mapper then the transaction will revert. - /// This function should be used to cache a remapped timestamp before it's used when calling the - /// `get_storage_slot` function with the same timestamp. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp at which to query. - /// * `tree` - The tree proof required to query the remapper. - fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - self.single_slot_proof.cache_timestamp(timestamp, tree); - } - - /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp to query. - /// - /// # Returns - /// - /// * `u256` - The cached L1 block number corresponding to the timestamp. - fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - self.single_slot_proof.cached_timestamps(timestamp) - } - } - #[generate_trait] impl InternalImpl of InternalTrait { fn final_checkpoint_slot_key(mapping_key: u256, slot_index: u256, offset: u32) -> u256 { diff --git a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo index 89a3d8dd..822987c8 100644 --- a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo @@ -11,6 +11,9 @@ mod OZVotesTrace208StorageProofVotingStrategy { path: SingleSlotProofComponent, storage: single_slot_proof, event: SingleSlotProofEvent ); + #[abi(embed_v0)] + impl SingleSlotProofImpl = + SingleSlotProofComponent::SingleSlotProofImpl; impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; #[storage] @@ -93,36 +96,6 @@ mod OZVotesTrace208StorageProofVotingStrategy { } } - #[generate_trait] - impl SingleSlotProofImpl of SingleSlotProofTrait { - /// Queries the Timestamp Remapper contract for the closest L1 block number that occurred before - /// the given timestamp and then caches the result. If the queried timestamp is less than the earliest - /// timestamp or larger than the latest timestamp in the mapper then the transaction will revert. - /// This function should be used to cache a remapped timestamp before it's used when calling the - /// `get_storage_slot` function with the same timestamp. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp at which to query. - /// * `tree` - The tree proof required to query the remapper. - fn cache_timestamp(ref self: ContractState, timestamp: u32, tree: BinarySearchTree) { - self.single_slot_proof.cache_timestamp(timestamp, tree); - } - - /// View function exposing the cached remapped timestamps. Reverts if the timestamp is not cached. - /// - /// # Arguments - /// - /// * `timestamp` - The timestamp to query. - /// - /// # Returns - /// - /// * `u256` - The cached L1 block number corresponding to the timestamp. - fn cached_timestamps(self: @ContractState, timestamp: u32) -> u256 { - self.single_slot_proof.cached_timestamps(timestamp) - } - } - #[generate_trait] impl InternalImpl of InternalTrait { fn final_checkpoint_slot_key(mapping_key: u256, slot_index: u256, offset: u32) -> u256 { From 86e78d326036c79969071d8498f4e6344ef02395 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:04:53 +0200 Subject: [PATCH 12/14] chore: fmt --- starknet/src/voting_strategies/evm_slot_value.cairo | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/starknet/src/voting_strategies/evm_slot_value.cairo b/starknet/src/voting_strategies/evm_slot_value.cairo index 25de36f0..5f12361b 100644 --- a/starknet/src/voting_strategies/evm_slot_value.cairo +++ b/starknet/src/voting_strategies/evm_slot_value.cairo @@ -12,7 +12,8 @@ mod EvmSlotValueVotingStrategy { ); #[abi(embed_v0)] - impl SingleSlotProofImpl = SingleSlotProofComponent::SingleSlotProofImpl; + impl SingleSlotProofImpl = + SingleSlotProofComponent::SingleSlotProofImpl; impl SingleSlotProofInternalImpl = SingleSlotProofComponent::InternalImpl; #[storage] From 46b80c5e7fa405f7e86be026dda56e5a8d135d11 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:43:51 +0200 Subject: [PATCH 13/14] ensure ozvotes / evm_slot_value strategies expose the SingleSlotProof external functions --- starknet/src/lib.cairo | 1 + starknet/src/tests.cairo | 3 ++ starknet/src/tests/mocks/facts_registry.cairo | 21 ++++++++++++ .../src/tests/mocks/timestamp_remappers.cairo | 22 +++++++++++++ .../src/tests/utils/single_slot_proof.cairo | 33 +++++++++++++++++++ .../voting_strategies/evm_slot_value.cairo | 30 +++++++++++++++++ .../oz_votes_storage_proof.cairo | 30 +++++++++++++++++ .../oz_votes_trace_208_storage_proof.cairo | 30 +++++++++++++++++ 8 files changed, 170 insertions(+) create mode 100644 starknet/src/tests/mocks/facts_registry.cairo create mode 100644 starknet/src/tests/mocks/timestamp_remappers.cairo create mode 100644 starknet/src/tests/utils/single_slot_proof.cairo diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index a014c763..81483908 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -166,5 +166,6 @@ mod external { mod herodotus; } +#[cfg(test)] mod tests; diff --git a/starknet/src/tests.cairo b/starknet/src/tests.cairo index 4edaa337..438e0f25 100644 --- a/starknet/src/tests.cairo +++ b/starknet/src/tests.cairo @@ -36,9 +36,11 @@ mod space { mod mocks { mod erc20_votes_preset; mod executor; + mod facts_registry; mod no_voting_power; mod proposal_validation_always_fail; mod space_v2; + mod timestamp_remappers; mod vanilla_authenticator; mod vanilla_execution_strategy; mod vanilla_proposal_validation; @@ -51,4 +53,5 @@ mod setup { mod utils { mod strategy_trait; + mod single_slot_proof; } diff --git a/starknet/src/tests/mocks/facts_registry.cairo b/starknet/src/tests/mocks/facts_registry.cairo new file mode 100644 index 00000000..f8e2e99d --- /dev/null +++ b/starknet/src/tests/mocks/facts_registry.cairo @@ -0,0 +1,21 @@ +#[starknet::contract] +mod MockFactsRegistry { + use sx::external::herodotus::IEVMFactsRegistry; + use sx::external::herodotus::{BinarySearchTree, MapperId, Words64}; + + #[storage] + struct Storage {} + + #[abi(embed_v0)] + impl FactsRegistry of IEVMFactsRegistry { + fn get_storage( + self: @ContractState, + block: u256, + account: felt252, + slot: u256, + mpt_proof: Span + ) -> u256 { + return 1; + } + } +} diff --git a/starknet/src/tests/mocks/timestamp_remappers.cairo b/starknet/src/tests/mocks/timestamp_remappers.cairo new file mode 100644 index 00000000..f0d058a9 --- /dev/null +++ b/starknet/src/tests/mocks/timestamp_remappers.cairo @@ -0,0 +1,22 @@ +#[starknet::contract] +mod MockTimestampRemappers { + use sx::external::herodotus::ITimestampRemappers; + use sx::external::herodotus::{BinarySearchTree, MapperId}; + + #[storage] + struct Storage {} + + #[abi(embed_v0)] + impl TimestampRemappers of ITimestampRemappers { + fn get_closest_l1_block_number( + self: @ContractState, tree: BinarySearchTree, timestamp: u256 + ) -> Result, felt252> { + return Result::Ok(Option::Some(1)); + } + + // Getter for the last timestamp of a given mapper. + fn get_last_mapper_timestamp(self: @ContractState, mapper_id: MapperId) -> u256 { + return 1; + } + } +} diff --git a/starknet/src/tests/utils/single_slot_proof.cairo b/starknet/src/tests/utils/single_slot_proof.cairo new file mode 100644 index 00000000..edf2fb71 --- /dev/null +++ b/starknet/src/tests/utils/single_slot_proof.cairo @@ -0,0 +1,33 @@ +use sx::tests::mocks::{ + timestamp_remappers::MockTimestampRemappers, facts_registry::MockFactsRegistry +}; +use starknet::ContractAddress; +use sx::external::herodotus::BinarySearchTree; + +fn deploy_timestamp_remappers() -> ContractAddress { + let (contract_address, _) = starknet::syscalls::deploy_syscall( + MockTimestampRemappers::TEST_CLASS_HASH.try_into().unwrap(), 0, array![].span(), false, + ) + .unwrap(); + contract_address +} + +fn deploy_facts_registry() -> ContractAddress { + let (contract_address, _) = starknet::syscalls::deploy_syscall( + MockFactsRegistry::TEST_CLASS_HASH.try_into().unwrap(), 0, array![].span(), false, + ) + .unwrap(); + contract_address +} + +impl DefaultBinarySearchTree of Default { + fn default() -> BinarySearchTree { + BinarySearchTree { + mapper_id: 1, + last_pos: 1, + peaks: array![].span(), + proofs: array![].span(), + left_neighbor: Option::None, + } + } +} diff --git a/starknet/src/voting_strategies/evm_slot_value.cairo b/starknet/src/voting_strategies/evm_slot_value.cairo index 5f12361b..4781298b 100644 --- a/starknet/src/voting_strategies/evm_slot_value.cairo +++ b/starknet/src/voting_strategies/evm_slot_value.cairo @@ -97,6 +97,36 @@ mod EvmSlotValueVotingStrategy { #[cfg(test)] mod tests { use super::EvmSlotValueVotingStrategy; + use sx::interfaces::{ + ISingleSlotProof, ISingleSlotProofDispatcher, ISingleSlotProofDispatcherTrait + }; + use sx::tests::mocks::timestamp_remappers::MockTimestampRemappers; + use sx::tests::mocks::facts_registry::MockFactsRegistry; + use sx::external::herodotus::BinarySearchTree; + use sx::tests::utils::single_slot_proof::{ + deploy_timestamp_remappers, deploy_facts_registry, DefaultBinarySearchTree + }; + + #[test] + #[available_gas(10000000)] + fn ensure_ssp_is_exposed() { + let constructor_calldata = array![ + deploy_timestamp_remappers().into(), deploy_facts_registry().into() + ]; + let (contract_address, _) = starknet::syscalls::deploy_syscall( + EvmSlotValueVotingStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false, + ) + .unwrap(); + + let ssp = ISingleSlotProofDispatcher { contract_address }; + let tt = 1337; + ssp.cache_timestamp(tt, DefaultBinarySearchTree::default()); + + assert(ssp.cached_timestamps(tt) == 1, 'Timestamp not cached'); + } #[test] #[available_gas(10000000)] diff --git a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo index 8a9a1736..d87678ec 100644 --- a/starknet/src/voting_strategies/oz_votes_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_storage_proof.cairo @@ -142,6 +142,36 @@ mod OZVotesStorageProofVotingStrategy { #[cfg(test)] mod tests { use super::OZVotesStorageProofVotingStrategy; + use sx::interfaces::{ + ISingleSlotProof, ISingleSlotProofDispatcher, ISingleSlotProofDispatcherTrait + }; + use sx::tests::mocks::timestamp_remappers::MockTimestampRemappers; + use sx::tests::mocks::facts_registry::MockFactsRegistry; + use sx::external::herodotus::BinarySearchTree; + use sx::tests::utils::single_slot_proof::{ + deploy_timestamp_remappers, deploy_facts_registry, DefaultBinarySearchTree + }; + + #[test] + #[available_gas(10000000)] + fn ensure_ssp_is_exposed() { + let constructor_calldata = array![ + deploy_timestamp_remappers().into(), deploy_facts_registry().into() + ]; + let (contract_address, _) = starknet::syscalls::deploy_syscall( + OZVotesStorageProofVotingStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false, + ) + .unwrap(); + + let ssp = ISingleSlotProofDispatcher { contract_address }; + let tt = 1337; + ssp.cache_timestamp(tt, DefaultBinarySearchTree::default()); + + assert(ssp.cached_timestamps(tt) == 1, 'Timestamp not cached'); + } #[test] #[available_gas(10000000)] diff --git a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo index 822987c8..d60cc911 100644 --- a/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo +++ b/starknet/src/voting_strategies/oz_votes_trace_208_storage_proof.cairo @@ -139,6 +139,36 @@ mod OZVotesTrace208StorageProofVotingStrategy { #[cfg(test)] mod tests { use super::OZVotesTrace208StorageProofVotingStrategy; + use sx::interfaces::{ + ISingleSlotProof, ISingleSlotProofDispatcher, ISingleSlotProofDispatcherTrait + }; + use sx::tests::mocks::timestamp_remappers::MockTimestampRemappers; + use sx::tests::mocks::facts_registry::MockFactsRegistry; + use sx::external::herodotus::BinarySearchTree; + use sx::tests::utils::single_slot_proof::{ + deploy_timestamp_remappers, deploy_facts_registry, DefaultBinarySearchTree + }; + + #[test] + #[available_gas(10000000)] + fn ensure_ssp_is_exposed() { + let constructor_calldata = array![ + deploy_timestamp_remappers().into(), deploy_facts_registry().into() + ]; + let (contract_address, _) = starknet::syscalls::deploy_syscall( + OZVotesTrace208StorageProofVotingStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false, + ) + .unwrap(); + + let ssp = ISingleSlotProofDispatcher { contract_address }; + let tt = 1337; + ssp.cache_timestamp(tt, DefaultBinarySearchTree::default()); + + assert(ssp.cached_timestamps(tt) == 1, 'Timestamp not cached'); + } #[test] #[available_gas(10000000)] From 8eaa9a0144f660f60b2a6781d51764b4c3c503d9 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Mon, 21 Oct 2024 13:11:58 +0200 Subject: [PATCH 14/14] remove cfg test --- starknet/src/lib.cairo | 1 - starknet/src/tests.cairo | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 81483908..a014c763 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -166,6 +166,5 @@ mod external { mod herodotus; } -#[cfg(test)] mod tests; diff --git a/starknet/src/tests.cairo b/starknet/src/tests.cairo index 438e0f25..b0167966 100644 --- a/starknet/src/tests.cairo +++ b/starknet/src/tests.cairo @@ -53,5 +53,6 @@ mod setup { mod utils { mod strategy_trait; + #[cfg(test)] mod single_slot_proof; }