timelock optimization

src/factory.cairo

@@ -3,20 +3,14 @@ use governance::governor::{Config as GovernorConfig};
 use governance::governor::{IGovernorDispatcher};
 use governance::governance_token::{IGovernanceTokenDispatcher};
 use governance::airdrop::{IAirdropDispatcher};
-use governance::timelock::{ITimelockDispatcher};
+use governance::timelock::{ITimelockDispatcher, TimelockConfig};
 
 #[derive(Copy, Drop, Serde)]
 struct AirdropConfig {
     root: felt252,
     total: u128,
 }
 
-#[derive(Copy, Drop, Serde)]
-struct TimelockConfig {
-    delay: u64,
-    window: u64,
-}
-
 #[derive(Copy, Drop, Serde)]
 struct DeploymentParameters {
     name: felt252,

src/factory_test.cairo

@@ -4,11 +4,10 @@ use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
 use governance::governor::{Config as GovernorConfig};
 use governance::factory::{
     IFactoryDispatcher, IFactoryDispatcherTrait, Factory, DeploymentParameters, AirdropConfig,
-    TimelockConfig,
 };
 use governance::governance_token::{GovernanceToken};
 use governance::governor::{Governor};
-use governance::timelock::{Timelock};
+use governance::timelock::{Timelock, TimelockConfig};
 use governance::airdrop::{Airdrop};
 use starknet::{
     get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress,
@@ -100,5 +99,6 @@ fn test_deploy() {
             },
         'governor.config'
     );
-    assert(result.timelock.get_configuration() == (320, 60), 'timelock config');
+    assert(result.timelock.get_configuration().delay == 320, 'timelock config (delay)');
+    assert(result.timelock.get_configuration().window == 60, 'timelock config (window)');
 }

src/governor.cairo

@@ -295,7 +295,7 @@ mod Governor {
             let data = call.execute();
 
             self.emit(Executed { id, });
-            
+
             data
         }
 

src/timelock.cairo

@@ -1,6 +1,10 @@
+use core::option::OptionTrait;
+use core::traits::TryInto;
 use core::result::ResultTrait;
 use starknet::{ContractAddress};
 use starknet::account::{Call};
+use core::integer::{u128_to_felt252, u64_try_from_felt252, u128_safe_divmod};
+use starknet::storage_access::{StorePacking};
 
 #[starknet::interface]
 trait ITimelock<TStorage> {
@@ -14,23 +18,108 @@ trait ITimelock<TStorage> {
     fn execute(ref self: TStorage, calls: Span<Call>) -> Array<Span<felt252>>;
 
     // Return the execution window, i.e. the start and end timestamp in which the call can be executed
-    fn get_execution_window(self: @TStorage, id: felt252) -> (u64, u64);
+    fn get_execution_window(self: @TStorage, id: felt252) -> ExecutionWindow;
 
     // Get the current owner
     fn get_owner(self: @TStorage) -> ContractAddress;
 
     // Returns the delay and the window for call execution
-    fn get_configuration(self: @TStorage) -> (u64, u64);
+    fn get_configuration(self: @TStorage) -> TimelockConfig;
 
     // Transfer ownership, i.e. the address that can queue and cancel calls. This must be self-called via #queue.
     fn transfer(ref self: TStorage, to: ContractAddress);
     // Configure the delay and the window for call execution. This must be self-called via #queue.
-    fn configure(ref self: TStorage, delay: u64, window: u64);
+    fn configure(ref self: TStorage, config: TimelockConfig);
 }
 
+
+const TWO_POW_64: u128 = 0x10000000000000000;
+
+#[inline(always)]
+impl ThreeU64TupleStorePacking of StorePacking<(u64, u64, u64), felt252> {
+    fn pack(value: (u64, u64, u64)) -> felt252 {
+        let (a, b, c) = value;
+        u256 { low: TwoU64TupleStorePacking::pack((a, b)), high: c.into() }.try_into().unwrap()
+    }
+    fn unpack(value: felt252) -> (u64, u64, u64) {
+        let u256_value: u256 = value.into();
+        let (a, b) = TwoU64TupleStorePacking::unpack(u256_value.low);
+        (a, b, (u256_value.high).try_into().unwrap())
+    }
+}
+
+#[inline(always)]
+impl TwoU64TupleStorePacking of StorePacking<(u64, u64), u128> {
+    fn pack(value: (u64, u64)) -> u128 {
+        let (a, b) = value;
+        a.into() + b.into() * TWO_POW_64
+    }
+    fn unpack(value: u128) -> (u64, u64) {
+        let (q, r) = u128_safe_divmod(value, TWO_POW_64.try_into().unwrap());
+        (r.try_into().unwrap(), q.try_into().unwrap())
+    }
+}
+
+#[derive(Copy, Drop, Serde)]
+struct ExecutionState {
+    started: u64,
+    executed: u64,
+    canceled: u64
+}
+
+#[inline(always)]
+impl ExecutionStateStorePacking of StorePacking<ExecutionState, felt252> {
+    fn pack(value: ExecutionState) -> felt252 {
+        ThreeU64TupleStorePacking::pack((value.started, value.executed, value.canceled))
+    }
+    fn unpack(value: felt252) -> ExecutionState {
+        let (started, executed, canceled) = ThreeU64TupleStorePacking::unpack(value);
+        ExecutionState { started, executed, canceled }
+    }
+}
+
+#[derive(Copy, Drop, Serde)]
+struct TimelockConfig {
+    delay: u64,
+    window: u64,
+}
+
+#[inline(always)]
+impl TimelockConfigStorePacking of StorePacking<TimelockConfig, u128> {
+    fn pack(value: TimelockConfig) -> u128 {
+        TwoU64TupleStorePacking::pack((value.delay, value.window))
+    }
+    fn unpack(value: u128) -> TimelockConfig {
+        let (delay, window) = TwoU64TupleStorePacking::unpack(value);
+        TimelockConfig { delay, window }
+    }
+}
+
+#[derive(Copy, Drop, Serde)]
+struct ExecutionWindow {
+    earliest: u64,
+    latest: u64
+}
+
+#[inline(always)]
+impl ExecutionWindowStorePacking of StorePacking<ExecutionWindow, u128> {
+    fn pack(value: ExecutionWindow) -> u128 {
+        TwoU64TupleStorePacking::pack((value.earliest, value.latest))
+    }
+    fn unpack(value: u128) -> ExecutionWindow {
+        let (earliest, latest) = TwoU64TupleStorePacking::unpack(value);
+        ExecutionWindow { earliest, latest, }
+    }
+}
+
+
 #[starknet::contract]
 mod Timelock {
-    use super::{ITimelock, ContractAddress, Call};
+    use super::{
+        ITimelock, ContractAddress, Call, TimelockConfig, ExecutionState,
+        TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow,
+        ExecutionWindowStorePacking, TwoU64TupleStorePacking, ThreeU64TupleStorePacking
+    };
     use governance::call_trait::{CallTrait, HashCall};
     use hash::{LegacyHash};
     use array::{ArrayTrait, SpanTrait};
@@ -69,18 +158,15 @@ mod Timelock {
     #[storage]
     struct Storage {
         owner: ContractAddress,
-        delay: u64,
-        window: u64,
-        execution_started: LegacyMap<felt252, u64>,
-        executed: LegacyMap<felt252, u64>,
-        canceled: LegacyMap<felt252, u64>,
+        config: TimelockConfig,
+        // started_executed_canceled
+        execution_state: LegacyMap<felt252, ExecutionState>,
     }
 
     #[constructor]
-    fn constructor(ref self: ContractState, owner: ContractAddress, delay: u64, window: u64) {
+    fn constructor(ref self: ContractState, owner: ContractAddress, config: TimelockConfig) {
         self.owner.write(owner);
-        self.delay.write(delay);
-        self.window.write(window);
+        self.config.write(config);
     }
 
     // Take a list of calls and convert it to a unique identifier for the execution
@@ -113,10 +199,16 @@ mod Timelock {
             self.check_owner();
 
             let id = to_id(calls);
+            let execution_state = self.execution_state.read(id);
 
-            assert(self.execution_started.read(id).is_zero(), 'ALREADY_QUEUED');
+            assert(execution_state.started.is_zero(), 'ALREADY_QUEUED');
+            assert(execution_state.canceled.is_non_zero(), 'PREVIOUSLY_CANCELED');
 
-            self.execution_started.write(id, get_block_timestamp());
+            self
+                .execution_state
+                .write(
+                    id, ExecutionState { started: get_block_timestamp(), executed: 0, canceled: 0 }
+                );
 
             self.emit(Queued { id, calls, });
 
@@ -125,26 +217,47 @@ mod Timelock {
 
         fn cancel(ref self: ContractState, id: felt252) {
             self.check_owner();
-            assert(self.execution_started.read(id).is_non_zero(), 'DOES_NOT_EXIST');
-            assert(self.executed.read(id).is_zero(), 'ALREADY_EXECUTED');
-
-            self.execution_started.write(id, 0);
+            let execution_state = self.execution_state.read(id);
+            assert(execution_state.started.is_non_zero(), 'DOES_NOT_EXIST');
+            assert(execution_state.executed.is_zero(), 'ALREADY_EXECUTED');
+
+            self
+                .execution_state
+                .write(
+                    id,
+                    ExecutionState {
+                        started: 0,
+                        executed: execution_state.executed,
+                        canceled: execution_state.canceled
+                    }
+                );
 
             self.emit(Canceled { id, });
         }
 
         fn execute(ref self: ContractState, mut calls: Span<Call>) -> Array<Span<felt252>> {
             let id = to_id(calls);
 
-            assert(self.executed.read(id).is_zero(), 'ALREADY_EXECUTED');
+            let execution_state = self.execution_state.read(id);
 
-            let (earliest, latest) = self.get_execution_window(id);
+            assert(execution_state.executed.is_zero(), 'ALREADY_EXECUTED');
+
+            let execution_window = self.get_execution_window(id);
             let time_current = get_block_timestamp();
 
-            assert(time_current >= earliest, 'TOO_EARLY');
-            assert(time_current < latest, 'TOO_LATE');
+            assert(time_current >= execution_window.earliest, 'TOO_EARLY');
+            assert(time_current < execution_window.latest, 'TOO_LATE');
 
-            self.executed.write(id, time_current);
+            self
+                .execution_state
+                .write(
+                    id,
+                    ExecutionState {
+                        started: execution_state.started,
+                        executed: time_current,
+                        canceled: execution_state.canceled
+                    }
+                );
 
             let mut results: Array<Span<felt252>> = ArrayTrait::new();
 
@@ -160,26 +273,27 @@ mod Timelock {
             results
         }
 
-        fn get_execution_window(self: @ContractState, id: felt252) -> (u64, u64) {
-            let start_time = self.execution_started.read(id);
+        fn get_execution_window(self: @ContractState, id: felt252) -> ExecutionWindow {
+            let start_time = self.execution_state.read(id).started;
 
             // this is how we prevent the 0 timestamp from being considered valid
             assert(start_time != 0, 'DOES_NOT_EXIST');
 
-            let (delay, window) = (self.get_configuration());
+            let configuration = (self.get_configuration());
+
+            let earliest = start_time + configuration.delay;
 
-            let earliest = start_time + delay;
-            let latest = earliest + window;
+            let latest = earliest + configuration.window;
 
-            (earliest, latest)
+            ExecutionWindow { earliest: earliest, latest: latest }
         }
 
         fn get_owner(self: @ContractState) -> ContractAddress {
             self.owner.read()
         }
 
-        fn get_configuration(self: @ContractState) -> (u64, u64) {
-            (self.delay.read(), self.window.read())
+        fn get_configuration(self: @ContractState) -> TimelockConfig {
+            self.config.read()
         }
 
         fn transfer(ref self: ContractState, to: ContractAddress) {
@@ -188,11 +302,10 @@ mod Timelock {
             self.owner.write(to);
         }
 
-        fn configure(ref self: ContractState, delay: u64, window: u64) {
+        fn configure(ref self: ContractState, config: TimelockConfig) {
             self.check_self_call();
 
-            self.delay.write(delay);
-            self.window.write(window);
+            self.config.write(config);
         }
     }
 }

src/timelock_test.cairo

@@ -1,6 +1,9 @@
 use array::{Array, ArrayTrait, SpanTrait};
 use debug::PrintTrait;
-use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock};
+use governance::timelock::{
+    ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, TimelockConfig,
+    TimelockConfigStorePacking, ExecutionState, ExecutionStateStorePacking
+};
 use governance::governance_token_test::{deploy as deploy_token};
 use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait};
 use governance::governance_token::{IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait};
@@ -30,9 +33,9 @@ fn deploy(owner: ContractAddress, delay: u64, window: u64) -> ITimelockDispatche
 fn test_deploy() {
     let timelock = deploy(contract_address_const::<2300>(), 10239, 3600);
 
-    let (window, delay) = timelock.get_configuration();
-    assert(window == 10239, 'window');
-    assert(delay == 3600, 'delay');
+    let configuration = timelock.get_configuration();
+    assert(configuration.delay == 10239, 'delay');
+    assert(configuration.window == 3600, 'window');
     let owner = timelock.get_owner();
     assert(owner == contract_address_const::<2300>(), 'owner');
 }
@@ -70,9 +73,9 @@ fn test_queue_execute() {
 
     let id = timelock.queue(single_call(transfer_call(token, recipient, 500_u256)));
 
-    let (earliest, latest) = timelock.get_execution_window(id);
-    assert(earliest == 86401, 'earliest');
-    assert(latest == 90001, 'latest');
+    let execution_window = timelock.get_execution_window(id);
+    assert(execution_window.earliest == 86401, 'earliest');
+    assert(execution_window.latest == 90001, 'latest');
 
     set_block_timestamp(86401);
 
@@ -134,8 +137,8 @@ fn test_queue_executed_too_early() {
 
     let id = timelock.queue(single_call(transfer_call(token, recipient, 500_u256)));
 
-    let (earliest, latest) = timelock.get_execution_window(id);
-    set_block_timestamp(earliest - 1);
+    let execution_window = timelock.get_execution_window(id);
+    set_block_timestamp(execution_window.earliest - 1);
     timelock.execute(single_call(transfer_call(token, recipient, 500_u256)));
 }
 
@@ -153,7 +156,7 @@ fn test_queue_executed_too_late() {
 
     let id = timelock.queue(single_call(transfer_call(token, recipient, 500_u256)));
 
-    let (earliest, latest) = timelock.get_execution_window(id);
-    set_block_timestamp(latest);
+    let execution_window = timelock.get_execution_window(id);
+    set_block_timestamp(execution_window.latest);
     timelock.execute(single_call(transfer_call(token, recipient, 500_u256)));
 }