From 16f9c87be97e0856038a30fe18191291419569a1 Mon Sep 17 00:00:00 2001 From: Tristan <122918260+TAdev0@users.noreply.github.com> Date: Fri, 14 Jun 2024 03:34:56 +0200 Subject: [PATCH] chore: Improve `Governance`codebase (#52) * improve codebase formatting * use array macro for instantiating arrays everywhere * leave only comments and typos * consistency for 'delegate' * fix --- README.md | 4 +-- src/airdrop.cairo | 27 ++++++++--------- src/airdrop_claim_check.cairo | 2 -- src/airdrop_test.cairo | 16 ++-------- src/call_trait.cairo | 8 ++--- src/call_trait_test.cairo | 4 --- src/execution_state.cairo | 2 +- src/governor.cairo | 47 ++++++++++++++---------------- src/governor_test.cairo | 55 +++++++++++++---------------------- src/lib.cairo | 23 +++++++++------ src/staker.cairo | 24 +++++++-------- src/staker_test.cairo | 14 +-------- src/test/test_token.cairo | 6 +++- 13 files changed, 94 insertions(+), 138 deletions(-) diff --git a/README.md b/README.md index 899bf84..95da23f 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Contracts in this repository are designed so that they may be used together _or_ - A delegate may create a proposal to make a batch of `calls` if their voting weight exceeds the proposal creation threshold - After the `voting_start_delay`, users may choose to vote `yea` or `nay` on the created proposal for the duration of the `voting_period` - A voter's voting weight is computed based on their average delegation over the period `voting_weight_smoothing_duration` from before the start time of the proposal -- If a proposal receives at least `quorum` in voting weight, and the simple majority of total votes is yea, and the voting period is over, the proposal may be executed exactly once +- If a proposal receives at least `quorum` in voting weight, and the simple majority of total votes is `yea`, and the voting period is over, the proposal may be executed exactly once - Must happen after the voting has ended and the execution delay has passed - If any of the calls fails, the transaction will revert, and anyone may attempt to execute the proposal again, up until the end of the execution window - Proposals can be canceled at any time by _anyone_ iff the voting weight of the proposer falls below the proposal creation threshold, up until it is executed @@ -49,7 +49,7 @@ Contracts in this repository are designed so that they may be used together _or_ ## Testing -Make sure you have [Scarb with asdf](https://docs.swmansion.com/scarb/download#install-via-asdf) installed. +Make sure you have [Scarb with asdf](https://docs.swmansion.com/scarb/download#install-via-asdf) installed. You can look at the _.tool-versions_ file to know which version of Scarb is currently used. To run unit tests: diff --git a/src/airdrop.cairo b/src/airdrop.cairo index a3ef7c1..50c8984 100644 --- a/src/airdrop.cairo +++ b/src/airdrop.cairo @@ -1,4 +1,3 @@ -use core::array::{Array, Span}; use governance::interfaces::erc20::{IERC20Dispatcher}; use starknet::{ContractAddress}; @@ -24,27 +23,27 @@ pub struct Config { #[starknet::interface] pub trait IAirdrop { - // Returns the address of the token distributed by this airdrop + // Returns the address of the token distributed by this airdrop. fn get_token(self: @TContractState) -> ContractAddress; - // Returns the config of this deployed airdrop + // Returns the config of this deployed airdrop. fn get_config(self: @TContractState) -> Config; // Claims the given allotment of tokens. // Because this method is idempotent, it does not revert in case of a second submission of the same claim. // This makes it simpler to batch many claims together in a single transaction. - // Returns true iff the claim was processed. Returns false if the claim was already claimed. + // Returns true if the claim was processed. Returns false if the claim was already claimed. // Panics if the proof is invalid. fn claim(ref self: TContractState, claim: Claim, proof: Span) -> bool; // Claims the batch of up to 128 claims that must be aligned with a single bitmap, i.e. the id of the first must be a multiple of 128 // and the claims should be sequentially in order. The proof verification is optimized in this method. - // Returns the number of claims that were executed + // Returns the number of claims that were executed. fn claim_128( ref self: TContractState, claims: Span, remaining_proof: Span ) -> u8; - // Return whether the claim with the given ID has been claimed + // Returns whether the claim with the given ID has been claimed. fn is_claimed(self: @TContractState, claim_id: u64) -> bool; // Refunds the current token balance. Can be called by anyone after the refund timestamp. @@ -53,7 +52,6 @@ pub trait IAirdrop { #[starknet::contract] pub mod Airdrop { - use core::array::{ArrayTrait, SpanTrait}; use core::hash::{LegacyHash}; use core::num::traits::one::{One}; use core::num::traits::zero::{Zero}; @@ -62,7 +60,6 @@ pub mod Airdrop { use starknet::{get_block_timestamp, get_contract_address}; use super::{Config, IAirdrop, ContractAddress, Claim, IERC20Dispatcher}; - pub fn hash_function(a: felt252, b: felt252) -> felt252 { let a_u256: u256 = a.into(); if a_u256 < b.into() { @@ -72,7 +69,7 @@ pub mod Airdrop { } } - // Compute the pedersen root of a merkle tree by combining the current node with each sibling up the tree + // computes the pedersen root of a merkle tree by combining the current node with each sibling up the tree pub fn compute_pedersen_root(current: felt252, mut proof: Span) -> felt252 { match proof.pop_front() { Option::Some(proof_element) => { @@ -100,14 +97,14 @@ pub mod Airdrop { Claimed: Claimed, } + const BITMAP_SIZE: NonZero = 128; + #[constructor] fn constructor(ref self: ContractState, token: ContractAddress, config: Config) { self.token.write(IERC20Dispatcher { contract_address: token }); self.config.write(config); } - const BITMAP_SIZE: NonZero = 128; - fn claim_id_to_bitmap_index(claim_id: u64) -> (u64, u8) { let (word, index) = DivRem::div_rem(claim_id, BITMAP_SIZE); (word, index.try_into().unwrap()) @@ -120,7 +117,7 @@ pub mod Airdrop { pub fn compute_root_of_group(mut claims: Span) -> felt252 { assert(!claims.is_empty(), 'NO_CLAIMS'); - let mut claim_hashes: Array = ArrayTrait::new(); + let mut claim_hashes: Array = array![]; let mut last_claim_id: Option = Option::None; @@ -140,7 +137,7 @@ pub mod Airdrop { while current_layer .len() .is_non_one() { - let mut next_layer: Array = ArrayTrait::new(); + let mut next_layer: Array = array![]; while let Option::Some(hash) = current_layer .pop_front() { @@ -180,7 +177,6 @@ pub mod Airdrop { false } else { self.claimed_bitmap.write(word, bitmap | exp2(index.try_into().unwrap())); - self.token.read().transfer(claim.claimee, claim.amount.into()); self.emit(Claimed { claim }); @@ -212,7 +208,7 @@ pub mod Airdrop { let mut bitmap = self.claimed_bitmap.read(word); let mut index: u8 = 0; - let mut unclaimed: Array = ArrayTrait::new(); + let mut unclaimed: Array = array![]; while let Option::Some(claim) = claims .pop_front() { @@ -246,6 +242,7 @@ pub mod Airdrop { fn is_claimed(self: @ContractState, claim_id: u64) -> bool { let (word, index) = claim_id_to_bitmap_index(claim_id); let bitmap = self.claimed_bitmap.read(word); + (bitmap & exp2(index)).is_non_zero() } diff --git a/src/airdrop_claim_check.cairo b/src/airdrop_claim_check.cairo index 29ee8a5..474d97e 100644 --- a/src/airdrop_claim_check.cairo +++ b/src/airdrop_claim_check.cairo @@ -21,8 +21,6 @@ trait IAirdropClaimCheck { #[starknet::contract] mod AirdropClaimCheck { - use core::array::{SpanTrait}; - use core::option::{OptionTrait}; use governance::airdrop::{IAirdropDispatcherTrait}; use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use super::{IAirdropClaimCheck, IAirdropDispatcher, CheckParams, CheckResult}; diff --git a/src/airdrop_test.cairo b/src/airdrop_test.cairo index 8d63ac1..bea4105 100644 --- a/src/airdrop_test.cairo +++ b/src/airdrop_test.cairo @@ -1,10 +1,5 @@ -use core::array::{ArrayTrait, SpanTrait}; use core::hash::{LegacyHash}; use core::num::traits::zero::{Zero}; -use core::option::{OptionTrait}; - -use core::result::{Result, ResultTrait}; -use core::traits::{TryInto, Into}; use governance::airdrop::{ IAirdropDispatcher, IAirdropDispatcherTrait, Airdrop, Config, Airdrop::{compute_pedersen_root, hash_function, hash_claim, compute_root_of_group}, Claim @@ -13,14 +8,14 @@ use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use governance::test::test_token::{TestToken, deploy as deploy_token}; use starknet::testing::{pop_log, set_block_timestamp}; use starknet::{ - get_contract_address, syscalls::{deploy_syscall}, ClassHash, contract_address_const, - ContractAddress + ClassHash, ContractAddress, contract_address_const, get_contract_address, + syscalls::{deploy_syscall} }; fn deploy_with_refundee( token: ContractAddress, root: felt252, refundable_timestamp: u64, refund_to: ContractAddress ) -> IAirdropDispatcher { - let mut constructor_args: Array = ArrayTrait::new(); + let mut constructor_args: Array = array![]; Serde::serialize( @(token, Config { root, refundable_timestamp, refund_to }), ref constructor_args ); @@ -241,7 +236,6 @@ fn test_compute_root_of_group() { ); } - #[test] fn test_compute_root_of_group_large() { let mut arr: Array = array![]; @@ -280,7 +274,6 @@ fn test_compute_root_of_group_large_odd() { ); } - #[test] fn test_claim_two_claims() { let token = deploy_token(get_contract_address(), 1234567); @@ -711,7 +704,6 @@ fn test_multiple_claims_from_generated_tree() { assert_eq!(log.claim, claim_0); } - #[test] #[should_panic(expected: ('FIRST_CLAIM_MUST_BE_MULT_128', 'ENTRYPOINT_FAILED'))] fn test_claim_128_fails_if_not_id_aligned() { @@ -730,7 +722,6 @@ fn test_claim_128_fails_if_not_id_aligned() { airdrop.claim_128(array![claim_b, claim_a].span(), array![].span()); } - #[test] #[should_panic(expected: ('CLAIMS_EMPTY', 'ENTRYPOINT_FAILED'))] fn test_claim_128_empty() { @@ -826,7 +817,6 @@ fn test_claim_128_double_claim() { assert_eq!(pop_log::(airdrop.contract_address).is_none(), true); } - mod refundable { use super::{ deploy_token, deploy_with_refundee, get_contract_address, contract_address_const, diff --git a/src/call_trait.cairo b/src/call_trait.cairo index 802a86a..67f816b 100644 --- a/src/call_trait.cairo +++ b/src/call_trait.cairo @@ -1,11 +1,6 @@ -use core::array::{ArrayTrait, SpanTrait}; use core::hash::{HashStateTrait, HashStateExTrait, Hash}; -use core::result::{ResultTrait}; -use core::serde::{Serde}; -use core::traits::{Into}; use starknet::account::{Call}; -use starknet::{ContractAddress}; -use starknet::{SyscallResult, syscalls::call_contract_syscall}; +use starknet::syscalls::{call_contract_syscall}; // Care must be taken when using this implementation: Serde of the type T must be safe for hashing. // This means that no two values of type T have the same serialization. @@ -17,6 +12,7 @@ pub(crate) impl HashSerializable, +HashStateTrait, +Drop> while let Option::Some(word) = arr.pop_front() { state = state.update(word) }; + state } } diff --git a/src/call_trait_test.cairo b/src/call_trait_test.cairo index a6f5af4..3b5bf04 100644 --- a/src/call_trait_test.cairo +++ b/src/call_trait_test.cairo @@ -1,7 +1,5 @@ -use core::array::{Array, ArrayTrait}; use core::hash::{LegacyHash, HashStateExTrait, HashStateTrait}; use core::poseidon::{PoseidonTrait}; -use core::serde::{Serde}; use governance::call_trait::{CallTrait, HashSerializable}; use governance::test::test_token::{deploy as deploy_token}; use starknet::{contract_address_const, get_contract_address, account::{Call}}; @@ -71,7 +69,6 @@ fn test_execute_contract_not_deployed() { call.execute(); } - #[test] #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_execute_invalid_entry_point() { @@ -82,7 +79,6 @@ fn test_execute_invalid_entry_point() { call.execute(); } - #[test] #[should_panic(expected: ('Failed to deserialize param #1', 'ENTRYPOINT_FAILED'))] fn test_execute_invalid_call_data_too_short() { diff --git a/src/execution_state.cairo b/src/execution_state.cairo index 946ddbc..f73b04a 100644 --- a/src/execution_state.cairo +++ b/src/execution_state.cairo @@ -22,7 +22,7 @@ pub struct ExecutionState { pub canceled: u64 } -pub(crate) impl ExecutionStateStorePacking of StorePacking { +impl ExecutionStateStorePacking of StorePacking { fn pack(value: ExecutionState) -> felt252 { u256 { low: TwoU64TupleStorePacking::pack((value.created, value.executed)), diff --git a/src/governor.cairo b/src/governor.cairo index f71b5c1..e20d7ba 100644 --- a/src/governor.cairo +++ b/src/governor.cairo @@ -1,7 +1,3 @@ -use core::array::{Array}; -use core::byte_array::{ByteArray}; -use core::option::{Option, OptionTrait}; -use core::traits::{Into, TryInto}; use governance::execution_state::{ExecutionState}; use governance::staker::{IStakerDispatcher}; use starknet::account::{Call}; @@ -43,13 +39,13 @@ pub struct Config { #[starknet::interface] pub trait IGovernor { - // Propose executing the given call from this contract. + // Proposes executing the given call from this contract. fn propose(ref self: TContractState, calls: Span) -> felt252; - // Vote on the given proposal. + // Votes on the given proposal. fn vote(ref self: TContractState, id: felt252, yea: bool); - // Cancel the proposal with the given ID. Only callable by the proposer. + // Cancels the proposal with the given ID. Only callable by the proposer. fn cancel(ref self: TContractState, id: felt252); // Execute the given proposal. @@ -58,27 +54,27 @@ pub trait IGovernor { // Attaches the given text to the proposal. Simply emits an event containing the proposal description. fn describe(ref self: TContractState, id: felt252, description: ByteArray); - // Combined propose and describe methods + // Combines propose and describe methods. fn propose_and_describe( ref self: TContractState, calls: Span, description: ByteArray ) -> felt252; - // Get the staker that is used by this governor contract. + // Gets the staker that is used by this governor contract. fn get_staker(self: @TContractState) -> IStakerDispatcher; - // Get the latest configuration for this governor contract. + // Gets the latest configuration for this governor contract. fn get_config(self: @TContractState) -> Config; - // Get the latest configuration for this governor contract and its config version ID + // Gets the latest configuration for this governor contract and its config version ID. fn get_config_with_version(self: @TContractState) -> (Config, u64); - // Get the configuration with the given version ID. + // Gets the configuration with the given version ID. fn get_config_version(self: @TContractState, version: u64) -> Config; - // Get the proposal info for the given proposal id. + // Gets the proposal info for the given proposal id. fn get_proposal(self: @TContractState, id: felt252) -> ProposalInfo; - // Gets the proposal and the config version with which it was created + // Gets the proposal and the config version with which it was created. fn get_proposal_with_config(self: @TContractState, id: felt252) -> (ProposalInfo, Config); // Returns the vote cast by the given voter on the given proposal ID. @@ -87,10 +83,10 @@ pub trait IGovernor { // - 1 means voted against fn get_vote(self: @TContractState, id: felt252, voter: ContractAddress) -> u8; - // Change the configuration of the governor. Only affects proposals created after the configuration change. Must be called by self, e.g. via a proposal. + // Changes the configuration of the governor. Only affects proposals created after the configuration change. Must be called by self, e.g. via a proposal. fn reconfigure(ref self: TContractState, config: Config) -> u64; - // Replace the code at this address. This must be self-called via a proposal. + // Replaces the code at this address. This must be self-called via a proposal. fn upgrade(ref self: TContractState, class_hash: ClassHash); } @@ -102,15 +98,14 @@ pub mod Governor { use governance::call_trait::{HashSerializable, CallTrait}; use governance::staker::{IStakerDispatcherTrait}; use starknet::{ - get_block_timestamp, get_caller_address, contract_address_const, get_contract_address, + get_block_timestamp, get_caller_address, get_contract_address, syscalls::{replace_class_syscall} }; use super::{ - IStakerDispatcher, ContractAddress, Array, IGovernor, Config, ProposalInfo, Call, - ExecutionState, ByteArray, ClassHash + IStakerDispatcher, ContractAddress, IGovernor, Config, ProposalInfo, Call, ExecutionState, + ClassHash }; - #[derive(starknet::Event, Drop)] pub struct Proposed { pub id: felt252, @@ -165,12 +160,12 @@ pub mod Governor { struct Storage { staker: IStakerDispatcher, config: Config, + config_versions: LegacyMap, + latest_config_version: u64, nonce: u64, proposals: LegacyMap, - vote: LegacyMap<(felt252, ContractAddress), u8>, latest_proposal_by_proposer: LegacyMap, - latest_config_version: u64, - config_versions: LegacyMap, + vote: LegacyMap<(felt252, ContractAddress), u8>, } #[constructor] @@ -194,7 +189,6 @@ pub mod Governor { .finalize() } - #[generate_trait] impl GovernorInternal of GovernorInternalTrait { fn check_self_call(self: @ContractState) { @@ -277,6 +271,7 @@ pub mod Governor { ) -> felt252 { let id = self.propose(calls); self.describe(id, description); + id } @@ -325,7 +320,7 @@ pub mod Governor { assert(proposal.proposer == get_caller_address(), 'PROPOSER_ONLY'); assert(proposal.execution_state.canceled.is_zero(), 'ALREADY_CANCELED'); - // This is prevented so that proposers cannot grief voters by creating proposals that they plan to cancel after the result is known + // this is prevented so that proposers cannot grief voters by creating proposals that they plan to cancel after the result is known assert( get_block_timestamp() < (proposal.execution_state.created + config.voting_start_delay), @@ -428,6 +423,7 @@ pub mod Governor { fn get_proposal_with_config(self: @ContractState, id: felt252) -> (ProposalInfo, Config) { let proposal = self.get_proposal(id); let config = self.get_config_version(proposal.config_version); + (proposal, config) } @@ -442,6 +438,7 @@ pub mod Governor { self.config_versions.write(version, config); self.latest_config_version.write(version); self.emit(Reconfigured { new_config: config, version, }); + version } diff --git a/src/governor_test.cairo b/src/governor_test.cairo index 3a53cad..6063e81 100644 --- a/src/governor_test.cairo +++ b/src/governor_test.cairo @@ -1,12 +1,4 @@ -use core::array::SpanTrait; -use core::array::{ArrayTrait}; use core::num::traits::zero::{Zero}; -use core::option::{OptionTrait}; - -use core::result::{Result, ResultTrait}; -use core::serde::Serde; -use core::traits::{TryInto}; - use governance::execution_state::{ExecutionState}; use governance::governor::{ IGovernorDispatcher, IGovernorDispatcherTrait, Governor, Config, ProposalInfo, @@ -22,39 +14,39 @@ use starknet::{ testing::{set_block_timestamp, set_contract_address, pop_log} }; - -pub(crate) fn recipient() -> ContractAddress { +fn recipient() -> ContractAddress { 'recipient'.try_into().unwrap() } -pub(crate) fn proposer() -> ContractAddress { +fn proposer() -> ContractAddress { 'proposer'.try_into().unwrap() } -pub(crate) fn delegate() -> ContractAddress { +fn delegate() -> ContractAddress { 'delegate'.try_into().unwrap() } -pub(crate) fn voter1() -> ContractAddress { +fn voter1() -> ContractAddress { 'voter1'.try_into().unwrap() } -pub(crate) fn voter2() -> ContractAddress { +fn voter2() -> ContractAddress { 'voter2'.try_into().unwrap() } -pub(crate) fn anyone() -> ContractAddress { +fn anyone() -> ContractAddress { 'anyone'.try_into().unwrap() } fn advance_time(by: u64) -> u64 { let next = get_block_timestamp() + by; set_block_timestamp(next); + next } fn transfer_call(token: IERC20Dispatcher, recipient: ContractAddress, amount: u256) -> Call { - let mut calldata: Array = ArrayTrait::new(); + let mut calldata: Array = array![]; Serde::serialize(@(recipient, amount), ref calldata); Call { @@ -66,7 +58,7 @@ fn transfer_call(token: IERC20Dispatcher, recipient: ContractAddress, amount: u2 } fn deploy(staker: IStakerDispatcher, config: Config) -> IGovernorDispatcher { - let mut constructor_args: Array = ArrayTrait::new(); + let mut constructor_args: Array = array![]; Serde::serialize(@(staker, config), ref constructor_args); let (address, _) = deploy_syscall( @@ -104,7 +96,7 @@ fn create_proposal( fn create_proposal_with_call( governor: IGovernorDispatcher, token: IERC20Dispatcher, staker: IStakerDispatcher, call: Call ) -> felt252 { - // Delegate token to the proposer so that he reaches threshold. + // delegate token to the proposer so that he reaches threshold token .approve(staker.contract_address, governor.get_config().proposal_creation_threshold.into()); staker.stake(proposer()); @@ -115,6 +107,7 @@ fn create_proposal_with_call( set_contract_address(proposer()); let id = governor.propose(array![call].span()); set_contract_address(address_before); + id } @@ -144,7 +137,6 @@ fn test_setup() { assert_eq!(governor.get_config(), config); } - #[test] fn test_propose() { let (staker, token, governor, config) = setup(); @@ -310,7 +302,6 @@ fn test_describe_proposal_successful() { ); } - #[test] fn test_propose_and_describe_successful() { let (staker, token, governor, config) = setup(); @@ -394,7 +385,7 @@ fn test_vote_no_staking_after_period_starts() { advance_time(config.voting_start_delay); - // Delegate token to the voter to give him voting power. + // delegate token to the voter to give him voting power token.approve(staker.contract_address, 900); staker.stake(voter1()); @@ -406,7 +397,6 @@ fn test_vote_no_staking_after_period_starts() { assert_eq!(proposal.yea, 0); } - #[test] #[should_panic(expected: ('DOES_NOT_EXIST', 'ENTRYPOINT_FAILED'))] fn test_vote_invalid_proposal() { @@ -452,7 +442,7 @@ fn test_vote_already_voted_should_fail() { governor.vote(id, true); assert_eq!(governor.get_vote(id, voter1()), 3); - // Trying to vote twice on the same proposal should fail + // trying to vote twice on the same proposal should fail governor.vote(id, true); } @@ -462,13 +452,13 @@ fn test_vote_already_voted_different_vote_should_fail() { let (staker, token, governor, config) = setup(); let id = create_proposal(governor, token, staker); - // Fast forward to voting period + // fast forward to voting period advance_time(config.voting_start_delay); set_contract_address(voter1()); governor.vote(id, true); - // Different vote should still fail + // different vote should still fail governor.vote(id, false); } @@ -581,7 +571,7 @@ fn test_cancel_after_voting_end_should_fail() { advance_time(config.voting_start_delay); set_contract_address(proposer); - governor.cancel(id); // Try to cancel the proposal after voting completed + governor.cancel(id); // try to cancel the proposal after voting completed } #[test] @@ -639,14 +629,13 @@ fn test_execute_before_voting_ends_should_fail() { advance_time(config.voting_start_delay); - // Execute the proposal. The vote is still active, this should fail. + // Execute the proposal. If the vote is still active, this should fail. governor .execute( id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() ); } - #[test] #[should_panic(expected: ('QUORUM_NOT_MET', 'ENTRYPOINT_FAILED'))] fn test_execute_quorum_not_met_should_fail() { @@ -656,7 +645,7 @@ fn test_execute_quorum_not_met_should_fail() { advance_time(config.voting_start_delay + config.voting_period + config.execution_delay); set_contract_address(proposer()); - // Execute the proposal. The quorum was not met, this should fail. + // Execute the proposal. If the quorum was not met, this should fail. governor .execute( id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() @@ -698,7 +687,7 @@ fn test_execute_no_majority_should_fail() { advance_time(config.voting_period + config.execution_delay); - // Execute the proposal. The majority of votes are no, this should fail. + // Execute the proposal. If the majority of votes are no, this should fail. governor .execute( id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() @@ -729,7 +718,6 @@ fn test_execute_before_execution_window_begins() { ); } - #[test] #[should_panic(expected: ('EXECUTION_WINDOW_OVER', 'ENTRYPOINT_FAILED'))] fn test_execute_after_execution_window_ends() { @@ -800,7 +788,7 @@ fn test_verify_votes_are_counted_over_voting_weight_smoothing_duration_from_star advance_time(config.voting_period + config.execution_delay); - // Execute the proposal. The quorum was not met, this should fail. + // Execute the proposal. If the quorum was not met, this should fail. governor .execute( id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() @@ -935,7 +923,7 @@ fn test_execute_already_executed_should_fail() { governor .execute( id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); // Try to execute again + ); // try to execute again } #[test] @@ -1026,7 +1014,6 @@ fn test_reconfigure_fails_if_not_self_call() { ); } - #[test] fn test_reconfigure_succeeds_self_call() { let (staker, token, governor, config) = setup(); diff --git a/src/lib.cairo b/src/lib.cairo index 286e6ad..5c79614 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -1,28 +1,33 @@ pub mod airdrop; mod airdrop_claim_check; - #[cfg(test)] -pub(crate) mod airdrop_test; +mod airdrop_test; + pub mod call_trait; #[cfg(test)] -pub(crate) mod call_trait_test; +mod call_trait_test; + pub mod execution_state; #[cfg(test)] -pub(crate) mod execution_state_test; +mod execution_state_test; + pub mod governor; #[cfg(test)] -pub(crate) mod governor_test; +mod governor_test; + pub mod staker; #[cfg(test)] -pub(crate) mod staker_test; -pub(crate) mod interfaces { +mod staker_test; + +mod interfaces { pub(crate) mod erc20; } -pub(crate) mod utils { +mod utils { pub(crate) mod exp2; } #[cfg(test)] -pub(crate) mod test { +mod test { pub(crate) mod test_token; } + diff --git a/src/staker.cairo b/src/staker.cairo index 1c7ac56..2fa2dc1 100644 --- a/src/staker.cairo +++ b/src/staker.cairo @@ -2,24 +2,24 @@ use starknet::{ContractAddress}; #[starknet::interface] pub trait IStaker { - // Returns the token this staker references + // Returns the token this staker references. fn get_token(self: @TContractState) -> ContractAddress; - // Returns the amount staked from the staker to the delegate + // Returns the amount staked from the staker to the delegate. fn get_staked( self: @TContractState, staker: ContractAddress, delegate: ContractAddress ) -> u128; - // Transfer the approved amount of token from the caller into this contract and delegates it to the given address + // Transfers the approved amount of token from the caller into this contract and delegates it to the given address. fn stake(ref self: TContractState, delegate: ContractAddress); - // Transfer the specified amount of token from the caller into this contract and delegates the voting weight to the specified delegate + // Transfers the specified amount of token from the caller into this contract and delegates the voting weight to the specified delegate. fn stake_amount(ref self: TContractState, delegate: ContractAddress, amount: u128); - // Unstakes and withdraws all of the tokens delegated by the sender to the delegate from the contract to the given recipient address + // Unstakes and withdraws all of the tokens delegated by the sender to the delegate from the contract to the given recipient address. fn withdraw(ref self: TContractState, delegate: ContractAddress, recipient: ContractAddress); - // Unstakes and withdraws the specified amount of tokens delegated by the sender to the delegate from the contract to the given recipient address + // Unstakes and withdraws the specified amount of tokens delegated by the sender to the delegate from the contract to the given recipient address. fn withdraw_amount( ref self: TContractState, delegate: ContractAddress, @@ -27,23 +27,23 @@ pub trait IStaker { amount: u128 ); - // Get the currently delegated amount of token. Note this is not flash-loan resistant. + // Gets the currently delegated amount of token. Note this is not flash-loan resistant. fn get_delegated(self: @TContractState, delegate: ContractAddress) -> u128; - // Get how much delegated tokens an address has at a certain timestamp. + // Gets how much delegated tokens an address has at a certain timestamp. fn get_delegated_at(self: @TContractState, delegate: ContractAddress, timestamp: u64) -> u128; - // Get the cumulative delegated amount * seconds for an address at a certain timestamp. + // Gets the cumulative delegated amount * seconds for an address at a certain timestamp. fn get_delegated_cumulative( self: @TContractState, delegate: ContractAddress, timestamp: u64 ) -> u256; - // Get the average amount delegated over the given period, where end > start and end <= current time + // Gets the average amount delegated over the given period, where end > start and end <= current time. fn get_average_delegated( self: @TContractState, delegate: ContractAddress, start: u64, end: u64 ) -> u128; - // Get the average amount delegated over the last period seconds + // Gets the average amount delegated over the last period seconds. fn get_average_delegated_over_last( self: @TContractState, delegate: ContractAddress, period: u64 ) -> u128; @@ -52,8 +52,6 @@ pub trait IStaker { #[starknet::contract] pub mod Staker { use core::num::traits::zero::{Zero}; - use core::option::{OptionTrait}; - use core::traits::{Into, TryInto}; use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use starknet::{ get_caller_address, get_contract_address, get_block_timestamp, diff --git a/src/staker_test.cairo b/src/staker_test.cairo index f0059d6..6f06ddb 100644 --- a/src/staker_test.cairo +++ b/src/staker_test.cairo @@ -1,17 +1,12 @@ -use core::array::{ArrayTrait}; use core::num::traits::zero::{Zero}; -use core::option::{OptionTrait}; -use core::result::{Result, ResultTrait}; -use core::traits::{TryInto}; use governance::execution_state_test::{assert_pack_unpack}; use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; - use governance::staker::{ IStakerDispatcher, IStakerDispatcherTrait, Staker, Staker::{DelegatedSnapshotStorePacking, DelegatedSnapshot}, }; use governance::test::test_token::{TestToken, deploy as deploy_token}; -use starknet::testing::{set_contract_address, set_block_timestamp, pop_log}; +use starknet::testing::{set_block_timestamp, pop_log}; use starknet::{ get_contract_address, syscalls::deploy_syscall, ClassHash, contract_address_const, ContractAddress, @@ -116,7 +111,6 @@ fn test_staker_delegated_snapshot_store_pack() { ), 0x1000000000000000000000000000000000000000000000001 ); - assert_eq!( DelegatedSnapshotStorePacking::pack( DelegatedSnapshot { @@ -146,7 +140,6 @@ fn test_staker_delegated_snapshot_store_unpack() { DelegatedSnapshotStorePacking::unpack(0x1000000000000000000000000000000000000000000000001), DelegatedSnapshot { timestamp: 1, delegated_cumulative: 1 } ); - assert_eq!( DelegatedSnapshotStorePacking::unpack( 3618502788666131113263695016908177884250476444008934042335404944711319814143 @@ -156,7 +149,6 @@ fn test_staker_delegated_snapshot_store_unpack() { delegated_cumulative: 6277101735386680763835789423207666416102355444464034512895 // 2**192 - 1 } ); - assert_eq!( DelegatedSnapshotStorePacking::unpack( // max felt252 @@ -258,7 +250,6 @@ fn test_approve_sets_allowance() { assert(erc20.allowance(get_contract_address(), spender) == 5151, 'allowance'); } - #[test] fn test_delegate_count_lags() { let (staker, token) = setup(12345); @@ -280,7 +271,6 @@ fn test_delegate_count_lags() { assert(staker.get_delegated_at(delegatee, 4) == 12345, 'a 2 seconds after'); } - #[test] fn test_get_delegated_cumulative() { let (staker, token) = setup(12345); @@ -369,7 +359,6 @@ fn test_get_average_delegated() { assert(staker.get_average_delegated(delegatee, 4, 10) == 8230, 'average (4 sec * 12345)/6'); } - #[test] fn test_transfer_delegates_moved() { let (staker, token) = setup(12345); @@ -385,7 +374,6 @@ fn test_transfer_delegates_moved() { assert_eq!(staker.get_average_delegated(delegatee, 0, 5), ((3 * (12345 - 500)) / 5)); } - #[test] fn test_delegate_undelegate() { let (staker, token) = setup(12345); diff --git a/src/test/test_token.cairo b/src/test/test_token.cairo index 888d82a..7df9d4d 100644 --- a/src/test/test_token.cairo +++ b/src/test/test_token.cairo @@ -1,6 +1,6 @@ use governance::interfaces::erc20::{IERC20Dispatcher}; - use starknet::{ContractAddress, syscalls::{deploy_syscall}}; + #[starknet::contract] pub(crate) mod TestToken { use core::num::traits::zero::{Zero}; @@ -37,11 +37,13 @@ pub(crate) mod TestToken { fn balanceOf(self: @ContractState, account: ContractAddress) -> u256 { self.balances.read(account).into() } + fn allowance( self: @ContractState, owner: ContractAddress, spender: ContractAddress ) -> u256 { self.allowances.read((owner, spender)).into() } + fn transfer(ref self: ContractState, recipient: ContractAddress, amount: u256) -> bool { let balance = self.balances.read(get_caller_address()); assert(balance >= amount, 'INSUFFICIENT_TRANSFER_BALANCE'); @@ -50,6 +52,7 @@ pub(crate) mod TestToken { self.emit(Transfer { from: get_caller_address(), to: recipient, value: amount }); true } + fn transferFrom( ref self: ContractState, sender: ContractAddress, @@ -66,6 +69,7 @@ pub(crate) mod TestToken { self.emit(Transfer { from: sender, to: recipient, value: amount }); true } + fn approve(ref self: ContractState, spender: ContractAddress, amount: u256) -> bool { self.allowances.write((get_caller_address(), spender), amount.try_into().unwrap()); true