Adds storage packing for timestamp/executed, stores the timestamp of … (

#13)

* Adds storage packing for timestamp/executed, stores the timestamp of execution, refactors the erc20 interface to include camel case (fixes #10)

* address comments

README.md

@@ -23,11 +23,11 @@ The structure is as follows:
     - Timelock ownership may be transferred to a new governance contract in future, e.g. to migrate to a volition-based voting contract
     - None of the proposal metadata is stored in governor, simply the number of votes
     - Proposals can be canceled at any time if the voting weight of the proposer falls below the threshold
-- `Token` is an ERC20 token meant for voting in contracts like `Governor`
+- `GovernanceToken` is an ERC20 token meant for voting in contracts like `Governor`
     - Users must delegate their tokens to vote, and may delegate to themselves
     - Allows other contracts to get the average voting weight for *any* historical period
     - Average votes are used to compute voting weight in the `Governor`, over a configurable period of time
-- `Airdrop` can be used to distribute Token
+- `Airdrop` can be used to distribute GovernanceToken
     - Compute a merkle root by computing a list of amounts and recipients, hashing them, and arranging them into a merkle binary tree
     - Deploy the airdrop with the root and the token address
     - Transfer the total amount of tokens to the `Airdrop` contract

src/airdrop.cairo

@@ -7,8 +7,9 @@ struct Claim {
     amount: u128,
 }
 
+// The only method required by Airdrop is transfer, so we use a simplified interface
 #[starknet::interface]
-trait IERC20<TStorage> {
+trait ITransferrableERC20<TStorage> {
     fn transfer(ref self: TStorage, recipient: ContractAddress, amount: u256);
 }
 
@@ -19,7 +20,10 @@ trait IAirdrop<TStorage> {
 
 #[starknet::contract]
 mod Airdrop {
-    use super::{IAirdrop, ContractAddress, Claim, IERC20Dispatcher, IERC20DispatcherTrait};
+    use super::{
+        IAirdrop, ContractAddress, Claim, ITransferrableERC20Dispatcher,
+        ITransferrableERC20DispatcherTrait
+    };
     use array::{ArrayTrait, SpanTrait};
     use hash::{pedersen};
     use traits::{Into, TryInto};
@@ -61,7 +65,7 @@ mod Airdrop {
     #[storage]
     struct Storage {
         root: felt252,
-        token: IERC20Dispatcher,
+        token: ITransferrableERC20Dispatcher,
         claimed: LegacyMap<felt252, bool>,
     }
 
@@ -77,9 +81,9 @@ mod Airdrop {
     }
 
     #[constructor]
-    fn constructor(ref self: ContractState, token: IERC20Dispatcher, root: felt252) {
+    fn constructor(ref self: ContractState, token: ContractAddress, root: felt252) {
         self.root.write(root);
-        self.token.write(token);
+        self.token.write(ITransferrableERC20Dispatcher { contract_address: token });
     }
 
     #[external(v0)]

src/token.cairo → src/governance_token.cairo

@@ -2,24 +2,27 @@ use core::traits::TryInto;
 use starknet::ContractAddress;
 
 #[starknet::interface]
-trait IToken<TStorage> {
-    // ERC20 methods
+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 approve(ref self: TStorage, spender: ContractAddress, amount: u256) -> bool;
-    fn increase_allowance(ref self: TStorage, spender: ContractAddress, added_value: u256) -> bool;
-    fn decrease_allowance(
-        ref self: TStorage, spender: ContractAddress, subtracted_value: u256
+    fn transferFrom(
+        ref self: TStorage, sender: ContractAddress, recipient: ContractAddress, amount: u256
     ) -> bool;
+    fn approve(ref self: TStorage, spender: ContractAddress, amount: u256) -> bool;
+}
 
+#[starknet::interface]
+trait IGovernanceToken<TStorage> {
     // Delegate tokens from the caller to the given delegate address
     fn delegate(ref self: TStorage, to: ContractAddress);
 
@@ -43,8 +46,8 @@ trait IToken<TStorage> {
 }
 
 #[starknet::contract]
-mod Token {
-    use super::{IToken, ContractAddress};
+mod GovernanceToken {
+    use super::{IERC20, IGovernanceToken, ContractAddress};
     use traits::{Into, TryInto};
     use option::{OptionTrait};
     use starknet::{get_caller_address, get_block_timestamp};
@@ -211,7 +214,7 @@ mod Token {
     }
 
     #[external(v0)]
-    impl TokenImpl of IToken<ContractState> {
+    impl ERC20Impl of IERC20<ContractState> {
         fn name(self: @ContractState) -> felt252 {
             self.name.read()
         }
@@ -224,9 +227,15 @@ mod Token {
         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 {
@@ -269,6 +278,14 @@ mod Token {
             self.emit(Transfer { from: sender, to: recipient, value: amount });
             true
         }
+        fn transferFrom(
+            ref self: ContractState,
+            sender: ContractAddress,
+            recipient: ContractAddress,
+            amount: u256
+        ) -> bool {
+            self.transfer_from(sender, recipient, amount)
+        }
         fn approve(ref self: ContractState, spender: ContractAddress, amount: u256) -> bool {
             let owner = get_caller_address();
             self
@@ -277,18 +294,10 @@ mod Token {
             self.emit(Approval { owner, spender, value: amount });
             true
         }
+    }
 
-        fn increase_allowance(
-            ref self: ContractState, spender: ContractAddress, added_value: u256
-        ) -> bool {
-            self.approve(spender, self.allowance(get_caller_address(), spender) + added_value)
-        }
-        fn decrease_allowance(
-            ref self: ContractState, spender: ContractAddress, subtracted_value: u256
-        ) -> bool {
-            self.approve(spender, self.allowance(get_caller_address(), spender) - subtracted_value)
-        }
-
+    #[external(v0)]
+    impl TokenImpl of IGovernanceToken<ContractState> {
         fn delegate(ref self: ContractState, to: ContractAddress) {
             let caller = get_caller_address();
             let old = self.delegates.read(caller);

src/governor.cairo

@@ -1,26 +1,51 @@
-use governance::token::ITokenDispatcherTrait;
-use starknet::{ContractAddress};
+use governance::governance_token::IGovernanceTokenDispatcherTrait;
+use starknet::{ContractAddress, StorePacking};
 use array::{Array};
-use governance::token::{ITokenDispatcher};
+use governance::governance_token::{IGovernanceTokenDispatcher};
 use starknet::account::{Call};
+use option::{Option, OptionTrait};
+use integer::{u128_safe_divmod, u128_as_non_zero};
+use traits::{Into, TryInto};
+
+#[derive(Copy, Drop, Serde, PartialEq)]
+struct ProposalTimestamps {
+    // the timestamp when the proposal was created
+    creation: u64,
+    // the timestamp when the proposal was executed
+    executed: u64,
+}
+
+const TWO_POW_64: u128 = 0x10000000000000000_u128;
+
+impl ProposalTimestampsStorePacking of StorePacking<ProposalTimestamps, u128> {
+    fn pack(value: ProposalTimestamps) -> u128 {
+        value.creation.into() + (value.executed.into() * TWO_POW_64)
+    }
+
+    fn unpack(value: u128) -> ProposalTimestamps {
+        let (executed, creation) = u128_safe_divmod(value, u128_as_non_zero(TWO_POW_64));
+        ProposalTimestamps {
+            creation: creation.try_into().unwrap(), executed: executed.try_into().unwrap()
+        }
+    }
+}
 
 #[derive(Copy, Drop, Serde, starknet::Store, PartialEq)]
 struct ProposalInfo {
+    // the address of the proposer
     proposer: ContractAddress,
-    // when the proposal was created
-    creation_timestamp: u64,
-    // how many yes votes has been collected
+    // the relevant timestamps
+    timestamps: ProposalTimestamps,
+    // how many yes votes have been collected
     yes: u128,
-    // how many no votes has been collected
+    // how many no votes have been collected
     no: u128,
-    // whether the proposal has been executed
-    executed: bool
 }
 
 #[derive(Copy, Drop, Serde, starknet::Store)]
 struct Config {
     // the token used for voting
-    voting_token: ITokenDispatcher,
+    voting_token: IGovernanceTokenDispatcher,
     // how long after a proposal is created does voting start
     voting_start_delay: u64,
     // the period during which votes are collected
@@ -56,10 +81,13 @@ trait IGovernor<TStorage> {
 
 #[starknet::contract]
 mod Governor {
-    use super::{ContractAddress, Array, IGovernor, ITokenDispatcher, Config, ProposalInfo, Call};
+    use super::{
+        ContractAddress, Array, IGovernor, IGovernanceTokenDispatcher, Config, ProposalInfo, Call,
+        ProposalTimestamps
+    };
     use starknet::{get_block_timestamp, get_caller_address, contract_address_const};
     use governance::call_trait::{CallTrait};
-    use governance::token::{ITokenDispatcherTrait};
+    use governance::governance_token::{IGovernanceTokenDispatcherTrait};
     use zeroable::{Zeroable};
 
     #[storage]
@@ -102,11 +130,9 @@ mod Governor {
                 .write(
                     id,
                     ProposalInfo {
-                        proposer,
-                        creation_timestamp: timestamp_current,
-                        yes: 0,
-                        no: 0,
-                        executed: false
+                        proposer, timestamps: ProposalTimestamps {
+                            creation: timestamp_current, executed: 0
+                        }, yes: 0, no: 0
                     }
                 );
 
@@ -119,7 +145,7 @@ mod Governor {
 
             assert(proposal.proposer.is_non_zero(), 'DOES_NOT_EXIST');
             let timestamp_current = get_block_timestamp();
-            let voting_start_time = (proposal.creation_timestamp + config.voting_start_delay);
+            let voting_start_time = (proposal.timestamps.creation + config.voting_start_delay);
             let voter = get_caller_address();
             let voted = self.voted.read((voter, id));
 
@@ -166,18 +192,16 @@ mod Governor {
             }
 
             assert(
-                timestamp_current < (proposal.creation_timestamp
+                timestamp_current < (proposal.timestamps.creation
                     + config.voting_start_delay
                     + config.voting_period),
                 'VOTING_ENDED'
             );
 
             proposal = ProposalInfo {
-                proposer: contract_address_const::<0>(),
-                creation_timestamp: 0,
-                yes: 0,
-                no: 0,
-                executed: false
+                proposer: contract_address_const::<0>(), timestamps: ProposalTimestamps {
+                    creation: 0, executed: 0
+                }, yes: 0, no: 0
             };
 
             self.proposals.write(id, proposal);
@@ -190,12 +214,15 @@ mod Governor {
             let mut proposal = self.proposals.read(id);
 
             assert(proposal.proposer.is_non_zero(), 'DOES_NOT_EXIST');
-            assert(!proposal.executed, 'ALREADY_EXECUTED');
+            assert(proposal.timestamps.executed.is_zero(), 'ALREADY_EXECUTED');
 
             let timestamp_current = get_block_timestamp();
+            // we cannot tell if a proposal is executed if it is executed at timestamp 0
+            // this can only happen in testing, but it makes this method locally correct
+            assert(timestamp_current.is_non_zero(), 'TIMESTAMP_ZERO');
 
             assert(
-                timestamp_current >= (proposal.creation_timestamp
+                timestamp_current >= (proposal.timestamps.creation
                     + config.voting_start_delay
                     + config.voting_period),
                 'VOTING_NOT_ENDED'
@@ -204,7 +231,9 @@ mod Governor {
             assert((proposal.yes + proposal.no) >= config.quorum, 'QUORUM_NOT_MET');
             assert(proposal.yes >= proposal.no, 'NO_MAJORITY');
 
-            proposal.executed = true;
+            proposal.timestamps = ProposalTimestamps {
+                creation: proposal.timestamps.creation, executed: timestamp_current
+            };
 
             self.proposals.write(id, proposal);
 

src/lib.cairo

@@ -1,6 +1,6 @@
 mod airdrop;
 mod governor;
-mod token;
+mod governance_token;
 mod timelock;
 mod call_trait;
 

src/tests.cairo

@@ -3,7 +3,7 @@ mod airdrop_test;
 #[cfg(test)]
 mod governor_test;
 #[cfg(test)]
-mod token_test;
+mod governance_token_test;
 #[cfg(test)]
 mod timelock_test;
 #[cfg(test)]