chore: Improve Governancecodebase (#52)

* improve codebase formatting

* use array macro for instantiating arrays everywhere

* leave only comments and typos

* consistency for 'delegate'

* fix

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:
 

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<TContractState> {
-    // 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<felt252>) -> 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<Claim>, remaining_proof: Span<felt252>
     ) -> 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<TContractState> {
 
 #[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>) -> felt252 {
         match proof.pop_front() {
             Option::Some(proof_element) => {
@@ -100,14 +97,14 @@ pub mod Airdrop {
         Claimed: Claimed,
     }
 
+    const BITMAP_SIZE: NonZero<u64> = 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<u64> = 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<Claim>) -> felt252 {
         assert(!claims.is_empty(), 'NO_CLAIMS');
 
-        let mut claim_hashes: Array<felt252> = ArrayTrait::new();
+        let mut claim_hashes: Array<felt252> = array![];
 
         let mut last_claim_id: Option<u64> = Option::None;
 
@@ -140,7 +137,7 @@ pub mod Airdrop {
         while current_layer
             .len()
             .is_non_one() {
-                let mut next_layer: Array<felt252> = ArrayTrait::new();
+                let mut next_layer: Array<felt252> = 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<Claim> = ArrayTrait::new();
+            let mut unclaimed: Array<Claim> = 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()
         }
 

src/airdrop_claim_check.cairo

@@ -21,8 +21,6 @@ trait IAirdropClaimCheck<TContractState> {
 
 #[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};

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<felt252> = ArrayTrait::new();
+    let mut constructor_args: Array<felt252> = 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<Claim> = 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::Claimed>(airdrop.contract_address).is_none(), true);
 }
 
-
 mod refundable {
     use super::{
         deploy_token, deploy_with_refundee, get_contract_address, contract_address_const,

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<T, S, +Serde<T>, +HashStateTrait<S>, +Drop<S>>
         while let Option::Some(word) = arr.pop_front() {
             state = state.update(word)
         };
+
         state
     }
 }

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() {

src/execution_state.cairo

@@ -22,7 +22,7 @@ pub struct ExecutionState {
     pub canceled: u64
 }
 
-pub(crate) impl ExecutionStateStorePacking of StorePacking<ExecutionState, felt252> {
+impl ExecutionStateStorePacking of StorePacking<ExecutionState, felt252> {
     fn pack(value: ExecutionState) -> felt252 {
         u256 {
             low: TwoU64TupleStorePacking::pack((value.created, value.executed)),