refactoring and cleanup, adding some more testing around logs

README.md

@@ -2,16 +2,17 @@
 
 [![Tests](https://github.com/EkuboProtocol/governance/actions/workflows/test.yaml/badge.svg)](https://github.com/EkuboProtocol/governance/actions/workflows/test.yaml)
 
-Simple-as contracts for token governance on Starknet.
+Simple contracts for token governance on Starknet.
 
 ## Principles
 
 These contracts follow the Compound governance architecture.
-Contracts are focused on immutability, so it's broken up into a set of very modular components.
+The contracts are not upgradeable, so the project is broken up into modular and replaceable components.
+All contracts are intended to be upgraded by simply migrating to new ones. 
 
-All contracts are intended to be upgraded by simply migrating to new ones. Even the token contract can be migrated, if necessary, by deploying a new contract that allows burning the old token to mint the new one. It's likely volition will make voting use cases significantly cheaper, given the amount of indexed data required by the token contract, so upgrades of this sort are to be expected.
+Even the token contract can be migrated, if necessary, by deploying a new contract that allows burning the old token to mint the new one.
 
-The structure is as follows:
+## Components
 
 - `Timelock` is an owned contract that allows a list of calls to be queued by an owner
     - Anyone can execute the calls after a period of time, once queued by the owner

src/airdrop.cairo

@@ -1,32 +1,24 @@
-use starknet::ContractAddress;
+use starknet::{ContractAddress};
 use array::{Array};
 
-#[derive(Copy, Drop, Serde, Hash)]
+#[derive(Copy, Drop, Serde, Hash, PartialEq)]
 struct Claim {
     claimee: ContractAddress,
     amount: u128,
 }
 
-// The only method required by Airdrop is transfer, so we use a simplified interface
-#[starknet::interface]
-trait ITransferrableERC20<TStorage> {
-    fn transfer(ref self: TStorage, recipient: ContractAddress, amount: u256);
-}
-
 #[starknet::interface]
 trait IAirdrop<TStorage> {
+    // Claims the given allotment of tokens
     fn claim(ref self: TStorage, claim: Claim, proof: Array::<felt252>);
 }
 
 #[starknet::contract]
 mod Airdrop {
-    use super::{
-        IAirdrop, ContractAddress, Claim, ITransferrableERC20Dispatcher,
-        ITransferrableERC20DispatcherTrait
-    };
+    use super::{IAirdrop, ContractAddress, Claim};
+    use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
     use hash::{LegacyHash};
     use array::{ArrayTrait, SpanTrait};
-    use traits::{Into, TryInto};
     use starknet::{ContractAddressIntoFelt252};
 
     fn lt<X, +Copy<X>, +Into<X, u256>>(lhs: @X, rhs: @X) -> bool {
@@ -55,7 +47,7 @@ mod Airdrop {
     #[storage]
     struct Storage {
         root: felt252,
-        token: ITransferrableERC20Dispatcher,
+        token: IERC20Dispatcher,
         claimed: LegacyMap<felt252, bool>,
     }
 
@@ -71,9 +63,9 @@ mod Airdrop {
     }
 
     #[constructor]
-    fn constructor(ref self: ContractState, token: ContractAddress, root: felt252) {
+    fn constructor(ref self: ContractState, token: IERC20Dispatcher, root: felt252) {
         self.root.write(root);
-        self.token.write(ITransferrableERC20Dispatcher { contract_address: token });
+        self.token.write(token);
     }
 
     #[external(v0)]

src/tests/airdrop_test.cairo → src/airdrop_test.cairo

@@ -1,17 +1,19 @@
-use governance::governance_token::{IGovernanceTokenDispatcherTrait};
+use governance::governance_token::{
+    IGovernanceTokenDispatcherTrait, GovernanceToken, IGovernanceTokenDispatcher
+};
 use array::{ArrayTrait};
 use debug::PrintTrait;
 use governance::airdrop::{
     IAirdropDispatcher, IAirdropDispatcherTrait, Airdrop, Airdrop::compute_pedersen_root, Claim,
     Airdrop::lt
 };
 use hash::{LegacyHash};
-use governance::governance_token::{IERC20Dispatcher, IERC20DispatcherTrait};
+use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
 use starknet::{
     get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress
 };
-use governance::governance_token::{GovernanceToken, IGovernanceTokenDispatcher};
-use governance::tests::governance_token_test::{deploy as deploy_token};
+use starknet::testing::{pop_log};
+use governance::governance_token_test::{deploy as deploy_token};
 use starknet::class_hash::Felt252TryIntoClassHash;
 use traits::{TryInto, Into};
 
@@ -105,6 +107,16 @@ fn test_claim_single_recipient() {
     let proof = ArrayTrait::new();
 
     airdrop.claim(claim, proof);
+
+    let log = pop_log::<Airdrop::Claimed>(airdrop.contract_address).unwrap();
+    assert(log.claim == claim, 'claim');
+
+    pop_log::<GovernanceToken::Transfer>(token.contract_address);
+    pop_log::<GovernanceToken::Transfer>(token.contract_address);
+    let log = pop_log::<GovernanceToken::Transfer>(token.contract_address).unwrap();
+    assert(log.from == airdrop.contract_address, 'from');
+    assert(log.to == claim.claimee, 'to');
+    assert(log.value == claim.amount.into(), 'amount');
 }
 
 

src/tests/call_trait_test.cairo → src/call_trait_test.cairo

@@ -2,7 +2,7 @@ use debug::PrintTrait;
 use governance::call_trait::{CallTrait, HashCall};
 use starknet::{contract_address_const, account::{Call}};
 use array::{Array, ArrayTrait};
-use governance::tests::governance_token_test::{deploy as deploy_token};
+use governance::governance_token_test::{deploy as deploy_token};
 use serde::{Serde};
 use hash::{LegacyHash};
 

src/governance_token.cairo

@@ -1,25 +1,4 @@
-use core::traits::TryInto;
-use starknet::ContractAddress;
-
-#[starknet::interface]
-trait IERC20<TStorage> {
-    fn name(self: @TStorage) -> felt252;
-    fn symbol(self: @TStorage) -> felt252;
-    fn decimals(self: @TStorage) -> u8;
-    fn total_supply(self: @TStorage) -> u256;
-    fn totalSupply(self: @TStorage) -> u256;
-    fn balance_of(self: @TStorage, account: ContractAddress) -> u256;
-    fn balanceOf(self: @TStorage, account: ContractAddress) -> u256;
-    fn allowance(self: @TStorage, owner: ContractAddress, spender: ContractAddress) -> u256;
-    fn transfer(ref self: TStorage, recipient: ContractAddress, amount: u256) -> bool;
-    fn transfer_from(
-        ref self: TStorage, sender: ContractAddress, recipient: ContractAddress, amount: u256
-    ) -> bool;
-    fn transferFrom(
-        ref self: TStorage, sender: ContractAddress, recipient: ContractAddress, amount: u256
-    ) -> bool;
-    fn approve(ref self: TStorage, spender: ContractAddress, amount: u256) -> bool;
-}
+use starknet::{ContractAddress};
 
 #[starknet::interface]
 trait IGovernanceToken<TStorage> {
@@ -31,6 +10,7 @@ trait IGovernanceToken<TStorage> {
 
     // Get how much delegated tokens an address has at a certain timestamp.
     fn get_delegated_at(self: @TStorage, delegate: ContractAddress, timestamp: u64) -> u128;
+
     // Get the cumulative delegated amount * seconds for an address at a certain timestamp.
     fn get_delegated_cumulative(self: @TStorage, delegate: ContractAddress, timestamp: u64) -> u256;
 
@@ -47,7 +27,8 @@ trait IGovernanceToken<TStorage> {
 
 #[starknet::contract]
 mod GovernanceToken {
-    use super::{IERC20, IGovernanceToken, ContractAddress};
+    use governance::interfaces::erc20::{IERC20};
+    use super::{IGovernanceToken, ContractAddress};
     use traits::{Into, TryInto};
     use option::{OptionTrait};
     use starknet::{get_caller_address, get_block_timestamp, StorePacking};
@@ -97,6 +78,12 @@ mod GovernanceToken {
         self.symbol.write(symbol);
         self.total_supply.write(total_supply);
         self.balances.write(get_caller_address(), total_supply);
+        self
+            .emit(
+                Transfer {
+                    from: Zeroable::zero(), to: get_caller_address(), value: total_supply.into()
+                }
+            );
     }
 
     #[derive(starknet::Event, Drop)]
@@ -237,32 +224,41 @@ mod GovernanceToken {
         fn name(self: @ContractState) -> felt252 {
             self.name.read()
         }
+
         fn symbol(self: @ContractState) -> felt252 {
             self.symbol.read()
         }
+
         fn decimals(self: @ContractState) -> u8 {
             18_u8
         }
+
         fn total_supply(self: @ContractState) -> u256 {
             self.total_supply.read().into()
         }
+
         fn totalSupply(self: @ContractState) -> u256 {
             self.total_supply()
         }
+
         fn balance_of(self: @ContractState, account: ContractAddress) -> u256 {
             self.balances.read(account).into()
         }
+
         fn balanceOf(self: @ContractState, account: ContractAddress) -> u256 {
             self.balance_of(account)
         }
+
         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 {
             self.transfer_from(get_caller_address(), recipient, amount)
         }
+
         fn transfer_from(
             ref self: ContractState,
             sender: ContractAddress,
@@ -297,6 +293,7 @@ mod GovernanceToken {
             self.emit(Transfer { from: sender, to: recipient, value: amount });
             true
         }
+
         fn transferFrom(
             ref self: ContractState,
             sender: ContractAddress,
@@ -305,6 +302,7 @@ mod GovernanceToken {
         ) -> bool {
             self.transfer_from(sender, recipient, amount)
         }
+
         fn approve(ref self: ContractState, spender: ContractAddress, amount: u256) -> bool {
             let owner = get_caller_address();
             self
@@ -348,7 +346,13 @@ mod GovernanceToken {
             return if (num_snapshots.is_zero()) {
                 0
             } else {
-                self.find_delegated_cumulative(delegate, 0, num_snapshots, timestamp)
+                self
+                    .find_delegated_cumulative(
+                        delegate: delegate,
+                        min_index: 0,
+                        max_index_exclusive: num_snapshots,
+                        timestamp: timestamp
+                    )
             };
         }
 

src/tests/governance_token_test.cairo → src/governance_token_test.cairo

@@ -1,15 +1,15 @@
 use array::{ArrayTrait};
 use debug::PrintTrait;
+use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
 use governance::governance_token::{
-    IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait, IERC20Dispatcher,
-    IERC20DispatcherTrait, GovernanceToken,
+    IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait, GovernanceToken,
     GovernanceToken::{DelegatedSnapshotStorePacking, DelegatedSnapshot},
 };
 use starknet::{
     get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress,
 };
 use starknet::class_hash::Felt252TryIntoClassHash;
-use starknet::testing::{set_contract_address, set_block_timestamp};
+use starknet::testing::{set_contract_address, set_block_timestamp, pop_log};
 use traits::{TryInto};
 
 use result::{Result, ResultTrait};
@@ -121,6 +121,11 @@ fn test_deploy_constructor() {
     assert(erc20.balanceOf(contract_address_const::<1234512345>()) == 0, 'random balance');
     assert(erc20.total_supply() == 12345, 'total supply');
     assert(erc20.totalSupply() == 12345, 'total supply');
+
+    let log = pop_log::<GovernanceToken::Transfer>(erc20.contract_address).unwrap();
+    assert(log.from.is_zero(), 'from zero');
+    assert(log.to == get_contract_address(), 'to deployer');
+    assert(log.value == 12345, 'value');
 }
 
 #[test]
@@ -132,6 +137,12 @@ fn test_transfer_entire_balance() {
     erc20.transfer(recipient, 12345);
     assert(erc20.balance_of(get_contract_address()) == 0, 'zero after');
     assert(erc20.balance_of(recipient) == 12345, '12345 after');
+
+    pop_log::<GovernanceToken::Transfer>(erc20.contract_address);
+    let log = pop_log::<GovernanceToken::Transfer>(erc20.contract_address).unwrap();
+    assert(log.from == get_contract_address(), 'from');
+    assert(log.to == recipient, 'to');
+    assert(log.value == 12345, 'value');
 }
 
 #[test]
@@ -145,6 +156,12 @@ fn test_transfer_lt_total_balance() {
     assert(erc20.balanceOf(get_contract_address()) == 12300, 'remaining');
     assert(erc20.balance_of(recipient) == 45, '45 transferred');
     assert(erc20.balanceOf(recipient) == 45, '45 transferred');
+
+    pop_log::<GovernanceToken::Transfer>(erc20.contract_address);
+    let log = pop_log::<GovernanceToken::Transfer>(erc20.contract_address).unwrap();
+    assert(log.from == get_contract_address(), 'from');
+    assert(log.to == recipient, 'to');
+    assert(log.value == 45, 'value');
 }
 
 #[test]
@@ -404,4 +421,11 @@ fn test_delegate_undelegate() {
 
     assert(token.get_delegated(delegatee) == 0, 'delegated');
     assert(token.get_average_delegated(delegatee, 0, 8) == ((3 * 12345) / 8), 'average');
+
+    assert(token.get_delegated_at(delegatee, timestamp: 1) == 0, 'at 1');
+    assert(token.get_delegated_at(delegatee, timestamp: 2) == 0, 'at 2');
+    assert(token.get_delegated_at(delegatee, timestamp: 3) == 12345, 'at 3');
+    assert(token.get_delegated_at(delegatee, timestamp: 4) == 12345, 'at 4');
+    assert(token.get_delegated_at(delegatee, timestamp: 5) == 12345, 'at 5');
+    assert(token.get_delegated_at(delegatee, timestamp: 6) == 0, 'at 6');
 }

src/governor.cairo

@@ -1,7 +1,6 @@
-use governance::governance_token::IGovernanceTokenDispatcherTrait;
+use governance::governance_token::{IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait};
 use starknet::{ContractAddress, StorePacking};
 use array::{Array};
-use governance::governance_token::{IGovernanceTokenDispatcher};
 use starknet::account::{Call};
 use option::{Option, OptionTrait};
 use integer::{u128_safe_divmod, u128_as_non_zero};

src/tests/governor_test.cairo → src/governor_test.cairo

@@ -5,14 +5,12 @@ use governance::governor::{
     IGovernorDispatcher, IGovernorDispatcherTrait, Governor, Config, ProposalInfo,
     ProposalTimestamps
 };
-use governance::governance_token::{
-    IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait, IERC20Dispatcher,
-    IERC20DispatcherTrait
-};
+use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
+use governance::governance_token::{IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait};
 use governance::call_trait::{CallTrait};
 use starknet::account::{Call};
-use governance::tests::timelock_test::{single_call, transfer_call, deploy as deploy_timelock};
-use governance::tests::utils;
+use governance::timelock_test::{single_call, transfer_call, deploy as deploy_timelock};
+use governance::{test_utils as utils};
 use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait};
 use starknet::{
     get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress,
@@ -23,7 +21,7 @@ use traits::{TryInto};
 
 use result::{Result, ResultTrait};
 use option::{OptionTrait};
-use governance::tests::governance_token_test::{deploy as deploy_token};
+use governance::governance_token_test::{deploy as deploy_token};
 use serde::Serde;
 use zeroable::{Zeroable};
 

src/interfaces.cairo

@@ -0,0 +1 @@
+mod erc20;

src/interfaces/erc20.cairo

@@ -0,0 +1,21 @@
+use starknet::{ContractAddress};
+
+#[starknet::interface]
+trait IERC20<TStorage> {
+    fn name(self: @TStorage) -> felt252;
+    fn symbol(self: @TStorage) -> felt252;
+    fn decimals(self: @TStorage) -> u8;
+    fn total_supply(self: @TStorage) -> u256;
+    fn totalSupply(self: @TStorage) -> u256;
+    fn balance_of(self: @TStorage, account: ContractAddress) -> u256;
+    fn balanceOf(self: @TStorage, account: ContractAddress) -> u256;
+    fn allowance(self: @TStorage, owner: ContractAddress, spender: ContractAddress) -> u256;
+    fn transfer(ref self: TStorage, recipient: ContractAddress, amount: u256) -> bool;
+    fn transfer_from(
+        ref self: TStorage, sender: ContractAddress, recipient: ContractAddress, amount: u256
+    ) -> bool;
+    fn transferFrom(
+        ref self: TStorage, sender: ContractAddress, recipient: ContractAddress, amount: u256
+    ) -> bool;
+    fn approve(ref self: TStorage, spender: ContractAddress, amount: u256) -> bool;
+}