diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a2b69666..ca69722d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,7 +54,7 @@ jobs: - name: Step 2 - Install Scarb uses: software-mansion/setup-scarb@v1 with: - scarb-version: 0.6.0-alpha.1 + scarb-version: 0.6.2 - name: Step 3 - Check formatting working-directory: ./starknet diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index 0e9ee514..3e8732e4 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -257,6 +257,7 @@ mod Space { metadata_URI: Array, ) { assert_only_authenticator(@self); + assert(author.is_non_zero(), 'Zero Address'); let proposal_id = self._next_proposal_id.read(); // Proposal Validation @@ -312,6 +313,7 @@ mod Space { metadata_URI: Array ) { assert_only_authenticator(@self); + assert(voter.is_non_zero(), 'Zero Address'); let proposal = self._proposals.read(proposal_id); assert_proposal_exists(@proposal); @@ -377,6 +379,7 @@ mod Space { metadata_URI: Array, ) { assert_only_authenticator(@self); + assert(author.is_non_zero(), 'Zero Address'); let mut proposal = self._proposals.read(proposal_id); assert_proposal_exists(@proposal); assert(proposal.author == author, 'Only Author'); diff --git a/starknet/src/tests/test_space.cairo b/starknet/src/tests/test_space.cairo index 1c36f42a..7e66afc0 100644 --- a/starknet/src/tests/test_space.cairo +++ b/starknet/src/tests/test_space.cairo @@ -405,4 +405,168 @@ mod tests { ArrayTrait::::new().serialize(ref vote_calldata); authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED', 'ENTRYPOINT_FAILED'))] + fn test_propose_zero_address() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let quorum = u256_from_felt252(1); + let mut constructor_calldata = ArrayTrait::::new(); + quorum.serialize(ref constructor_calldata); + + let (vanilla_execution_strategy_address, _) = deploy_syscall( + VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false + ) + .unwrap(); + let vanilla_execution_strategy = StrategyImpl::from_address( + vanilla_execution_strategy_address + ); + // author is the zero address + let author = UserAddress::Starknet(contract_address_const::<0x0>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED', 'ENTRYPOINT_FAILED'))] + fn test_update_zero_address() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let quorum = u256_from_felt252(1); + let mut constructor_calldata = ArrayTrait::::new(); + quorum.serialize(ref constructor_calldata); + + let (vanilla_execution_strategy_address, _) = deploy_syscall( + VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false + ) + .unwrap(); + let vanilla_execution_strategy = StrategyImpl::from_address( + vanilla_execution_strategy_address + ); + // author is the zero address + let author = UserAddress::Starknet(contract_address_const::<0x0>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + // Update Proposal + let mut update_calldata = array::ArrayTrait::::new(); + author.serialize(ref update_calldata); + let proposal_id = u256_from_felt252(1); + proposal_id.serialize(ref update_calldata); + // Keeping the same execution strategy contract but changing the payload + let mut new_payload = ArrayTrait::::new(); + new_payload.append(1); + let execution_strategy = Strategy { + address: vanilla_execution_strategy.address, params: new_payload + }; + execution_strategy.serialize(ref update_calldata); + ArrayTrait::::new().serialize(ref update_calldata); + + authenticator + .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED', 'ENTRYPOINT_FAILED'))] + fn test_vote_zero_address() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let quorum = u256_from_felt252(1); + let mut constructor_calldata = ArrayTrait::::new(); + quorum.serialize(ref constructor_calldata); + + let (vanilla_execution_strategy_address, _) = deploy_syscall( + VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + constructor_calldata.span(), + false + ) + .unwrap(); + let vanilla_execution_strategy = StrategyImpl::from_address( + vanilla_execution_strategy_address + ); + let author = UserAddress::Starknet(contract_address_const::<0x5678>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + // Update Proposal + let mut update_calldata = array::ArrayTrait::::new(); + author.serialize(ref update_calldata); + let proposal_id = u256_from_felt252(1); + proposal_id.serialize(ref update_calldata); + // Keeping the same execution strategy contract but changing the payload + let mut new_payload = ArrayTrait::::new(); + new_payload.append(1); + let execution_strategy = Strategy { + address: vanilla_execution_strategy.address, params: new_payload + }; + execution_strategy.serialize(ref update_calldata); + ArrayTrait::::new().serialize(ref update_calldata); + + authenticator + .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + + // Increasing block block_number by 1 to pass voting delay + testing::set_block_number(1_u64); + + let mut vote_calldata = array::ArrayTrait::::new(); + // Voter is the zero address + let voter = UserAddress::Starknet(contract_address_const::<0x0>()); + voter.serialize(ref vote_calldata); + let proposal_id = u256_from_felt252(1); + proposal_id.serialize(ref vote_calldata); + let choice = Choice::For(()); + choice.serialize(ref vote_calldata); + let mut user_voting_strategies = ArrayTrait::::new(); + user_voting_strategies + .append(IndexedStrategy { index: 0_u8, params: ArrayTrait::::new() }); + user_voting_strategies.serialize(ref vote_calldata); + ArrayTrait::::new().serialize(ref vote_calldata); + + // Vote on Proposal + authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); + } } diff --git a/starknet/src/types/user_address.cairo b/starknet/src/types/user_address.cairo index 1066eb96..69bfb177 100644 --- a/starknet/src/types/user_address.cairo +++ b/starknet/src/types/user_address.cairo @@ -1,5 +1,6 @@ use starknet::{ContractAddress, EthAddress}; use traits::{PartialEq, TryInto, Into}; +use zeroable::Zeroable; use serde::Serde; use array::ArrayTrait; use sx::utils::legacy_hash::LegacyHashUserAddress; @@ -57,3 +58,66 @@ impl UserAddressImpl of UserAddressTrait { } } } + +impl UserAddressZeroable of Zeroable { + fn zero() -> UserAddress { + panic_with_felt252('Undefined') + } + fn is_zero(self: UserAddress) -> bool { + match self { + UserAddress::Starknet(address) => address.is_zero(), + UserAddress::Ethereum(address) => address.is_zero(), + UserAddress::Custom(address) => address.is_zero(), + } + } + fn is_non_zero(self: UserAddress) -> bool { + !self.is_zero() + } +} + +#[cfg(test)] +mod tests { + use zeroable::Zeroable; + use super::{UserAddress, UserAddressZeroable}; + use starknet::{EthAddress, contract_address_const}; + + #[test] + fn test_is_zero() { + assert(UserAddress::Starknet(contract_address_const::<0>()).is_zero(), 'is not zero'); + assert(UserAddress::Ethereum(EthAddress { address: 0 }).is_zero(), 'is not zero'); + assert(UserAddress::Custom(0_u256).is_zero(), 'is not zero'); + } + + #[test] + fn test_is_zero_false_positive() { + assert( + UserAddress::Starknet(contract_address_const::<1>()).is_zero() == false, + 'false positive not zero' + ); + assert( + UserAddress::Ethereum(EthAddress { address: 1 }).is_zero() == false, + 'false positive not zero' + ); + assert(UserAddress::Custom(1_u256).is_zero() == false, 'false positive not zero'); + } + + #[test] + fn test_is_non_zero() { + assert(UserAddress::Starknet(contract_address_const::<1>()).is_non_zero(), 'is zero'); + assert(UserAddress::Ethereum(EthAddress { address: 1 }).is_non_zero(), 'is zero'); + assert(UserAddress::Custom(1_u256).is_non_zero(), 'is zero'); + } + + #[test] + fn test_is_non_zero_false_positive() { + assert( + UserAddress::Starknet(contract_address_const::<0>()).is_non_zero() == false, + 'false positive is zero' + ); + assert( + UserAddress::Ethereum(EthAddress { address: 0 }).is_non_zero() == false, + 'false positive is zero' + ); + assert(UserAddress::Custom(0_u256).is_non_zero() == false, 'false positve not zero'); + } +}