improve packing of the delegated snapshot accumulator, fix vote count…

…ing (#14)

* improve packing of the delegated snapshot accumulator
fixes #4

* use a constant

* fixes #12

* improve tests with assertions on voting power

src/governance_token.cairo

@@ -50,15 +50,35 @@ mod GovernanceToken {
     use super::{IERC20, IGovernanceToken, ContractAddress};
     use traits::{Into, TryInto};
     use option::{OptionTrait};
-    use starknet::{get_caller_address, get_block_timestamp};
+    use starknet::{get_caller_address, get_block_timestamp, StorePacking};
     use zeroable::{Zeroable};
+    use integer::{u256_safe_divmod, u256_as_non_zero};
 
-    #[derive(Copy, Drop, starknet::Store)]
+    #[derive(Copy, Drop, PartialEq)]
     struct DelegatedSnapshot {
         timestamp: u64,
         delegated_cumulative: u256,
     }
 
+    const TWO_POW_64: u128 = 0x10000000000000000_u128;
+
+    impl DelegatedSnapshotStorePacking of StorePacking<DelegatedSnapshot, felt252> {
+        fn pack(value: DelegatedSnapshot) -> felt252 {
+            (value.delegated_cumulative + u256 {
+                high: value.timestamp.into() * TWO_POW_64, low: 0
+            })
+                .try_into()
+                .unwrap()
+        }
+        fn unpack(value: felt252) -> DelegatedSnapshot {
+            let (timestamp, delegated_cumulative, _) = u256_safe_divmod(
+                value.into(), u256_as_non_zero(u256 { low: 0, high: TWO_POW_64 })
+            );
+            DelegatedSnapshot { timestamp: timestamp.low.try_into().unwrap(), delegated_cumulative }
+        }
+    }
+
+
     #[storage]
     struct Storage {
         name: felt252,

src/governor.cairo

@@ -37,9 +37,9 @@ struct ProposalInfo {
     // the relevant timestamps
     timestamps: ProposalTimestamps,
     // how many yes votes have been collected
-    yes: u128,
+    yea: u128,
     // how many no votes have been collected
-    no: u128,
+    nay: u128,
 }
 
 #[derive(Copy, Drop, Serde, starknet::Store)]
@@ -64,7 +64,7 @@ trait IGovernor<TStorage> {
     fn propose(ref self: TStorage, call: Call) -> felt252;
 
     // Vote on the given proposal.
-    fn vote(ref self: TStorage, id: felt252, vote: bool);
+    fn vote(ref self: TStorage, id: felt252, yea: bool);
 
     // Cancel the given proposal. Cancellation can happen by any address if the average voting weight is below the proposal_creation_threshold.
     fn cancel(ref self: TStorage, id: felt252);
@@ -132,14 +132,14 @@ mod Governor {
                     ProposalInfo {
                         proposer, timestamps: ProposalTimestamps {
                             creation: timestamp_current, executed: 0
-                        }, yes: 0, no: 0
+                        }, yea: 0, nay: 0
                     }
                 );
 
             id
         }
 
-        fn vote(ref self: ContractState, id: felt252, vote: bool) {
+        fn vote(ref self: ContractState, id: felt252, yea: bool) {
             let config = self.config.read();
             let mut proposal = self.proposals.read(id);
 
@@ -155,14 +155,16 @@ mod Governor {
 
             let weight = config
                 .voting_token
-                .get_average_delegated_over_last(
-                    delegate: voter, period: config.voting_weight_smoothing_duration
+                .get_average_delegated(
+                    delegate: voter,
+                    start: voting_start_time - config.voting_weight_smoothing_duration,
+                    end: voting_start_time,
                 );
 
-            if vote {
-                proposal.yes = proposal.yes + weight;
+            if yea {
+                proposal.yea = proposal.yea + weight;
             } else {
-                proposal.no = proposal.no + weight;
+                proposal.nay = proposal.nay + weight;
             }
             self.proposals.write(id, proposal);
             self.voted.write((voter, id), true);
@@ -201,7 +203,7 @@ mod Governor {
             proposal = ProposalInfo {
                 proposer: contract_address_const::<0>(), timestamps: ProposalTimestamps {
                     creation: 0, executed: 0
-                }, yes: 0, no: 0
+                }, yea: 0, nay: 0
             };
 
             self.proposals.write(id, proposal);
@@ -228,8 +230,8 @@ mod Governor {
                 'VOTING_NOT_ENDED'
             );
 
-            assert((proposal.yes + proposal.no) >= config.quorum, 'QUORUM_NOT_MET');
-            assert(proposal.yes >= proposal.no, 'NO_MAJORITY');
+            assert((proposal.yea + proposal.nay) >= config.quorum, 'QUORUM_NOT_MET');
+            assert(proposal.yea >= proposal.nay, 'NO_MAJORITY');
 
             proposal.timestamps = ProposalTimestamps {
                 creation: proposal.timestamps.creation, executed: timestamp_current

src/tests/governance_token_test.cairo

@@ -2,7 +2,8 @@ use array::{ArrayTrait};
 use debug::PrintTrait;
 use governance::governance_token::{
     IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait, IERC20Dispatcher,
-    IERC20DispatcherTrait, GovernanceToken
+    IERC20DispatcherTrait, GovernanceToken,
+    GovernanceToken::{DelegatedSnapshotStorePacking, DelegatedSnapshot},
 };
 use starknet::{
     get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress,
@@ -35,6 +36,86 @@ fn deploy(
     );
 }
 
+#[test]
+fn test_governance_token_delegated_snapshot_store_pack() {
+    assert(
+        DelegatedSnapshotStorePacking::pack(
+            DelegatedSnapshot { timestamp: 0, delegated_cumulative: 0 }
+        ) == 0,
+        'zero'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::pack(
+            DelegatedSnapshot { timestamp: 0, delegated_cumulative: 1 }
+        ) == 1,
+        'one cumulative'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::pack(
+            DelegatedSnapshot { timestamp: 1, delegated_cumulative: 0 }
+        ) == 0x1000000000000000000000000000000000000000000000000,
+        'one timestamp'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::pack(
+            DelegatedSnapshot { timestamp: 1, delegated_cumulative: 1 }
+        ) == 0x1000000000000000000000000000000000000000000000001,
+        'one both'
+    );
+
+    assert(
+        DelegatedSnapshotStorePacking::pack(
+            DelegatedSnapshot {
+                timestamp: 576460752303423488, // this timestamp equal to 2**59 is so large it's invalid
+                delegated_cumulative: 6277101735386680763835789423207666416102355444464034512895 // max u192
+            }
+        ) == 3618502788666131113263695016908177884250476444008934042335404944711319814143,
+        'very large values'
+    );
+}
+
+#[test]
+fn test_governance_token_delegated_snapshot_store_unpack() {
+    assert(
+        DelegatedSnapshotStorePacking::unpack(0) == DelegatedSnapshot {
+            timestamp: 0, delegated_cumulative: 0
+        },
+        'zero'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::unpack(1) == DelegatedSnapshot {
+            timestamp: 0, delegated_cumulative: 1
+        },
+        'one cumulative'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::unpack(
+            0x1000000000000000000000000000000000000000000000000
+        ) == DelegatedSnapshot {
+            timestamp: 1, delegated_cumulative: 0
+        },
+        'one timestamp'
+    );
+    assert(
+        DelegatedSnapshotStorePacking::unpack(
+            0x1000000000000000000000000000000000000000000000001
+        ) == DelegatedSnapshot {
+            timestamp: 1, delegated_cumulative: 1
+        },
+        'one both'
+    );
+
+    assert(
+        DelegatedSnapshotStorePacking::unpack(
+            3618502788666131113263695016908177884250476444008934042335404944711319814143
+        ) == DelegatedSnapshot {
+            timestamp: 576460752303423488,
+            delegated_cumulative: 6277101735386680763835789423207666416102355444464034512895
+        },
+        'max both'
+    );
+}
+
 #[test]
 #[available_gas(3000000)]
 fn test_deploy_constructor() {