From 46a327319f8baf29ba63ddf7c74c1bf297c9dc49 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Mon, 1 Jan 2024 22:05:20 +0100 Subject: [PATCH 01/10] timelock optimization --- src/timelock.cairo | 123 ++++++++++++++++++++++++++++++---------- src/timelock_test.cairo | 10 ++-- 2 files changed, 99 insertions(+), 34 deletions(-) diff --git a/src/timelock.cairo b/src/timelock.cairo index 0cbd3a0..d932539 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -1,6 +1,8 @@ +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}; #[starknet::interface] trait ITimelock { @@ -14,23 +16,84 @@ trait ITimelock { fn execute(ref self: TStorage, calls: Span) -> Array>; // 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) -> TwoTimestamps; // 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) -> TwoTimestamps; // 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, delay_and_window: TwoTimestamps); +} + +impl U128IntoU64 of Into { + fn into(self: u128) -> u64 { + u64_try_from_felt252(u128_to_felt252(self)).unwrap() + } +} + +const TIMESTAMP_C_FILTER: u128 = 0xFFFFFFFFF000000000000000000; +const TIMESTAMP_B_FILTER: u128 = 0xFFFFFFFFF000000000; +const TIMESTAMP_A_FILTER: u128 = 0xFFFFFFFFF; +const B_SPACE: u128 = 0x1000000000; +const C_SPACE: u128 = 0x1000000000000000000; + +#[derive(Copy, Drop, Serde)] +struct ThreeTimestamps { + timestamps: u128 +} + +#[generate_trait] +impl ThreeTimestampsImpl of ThreeTimestampsTrait { + fn a(self: @ThreeTimestamps) -> u64 { + (*self.timestamps & TIMESTAMP_A_FILTER).into() + } + fn b(self: @ThreeTimestamps) -> u64 { + (*self.timestamps & TIMESTAMP_B_FILTER).into() + } + fn c(self: @ThreeTimestamps) -> u64 { + (*self.timestamps & TIMESTAMP_C_FILTER).into() + } + fn all(self: @ThreeTimestamps) -> (u64, u64, u64) { + ((*self.timestamps & TIMESTAMP_A_FILTER).into(), (*self.timestamps & TIMESTAMP_B_FILTER).into(), (*self.timestamps & TIMESTAMP_C_FILTER).into()) + } + fn new(a: u64, b: u64, c: u64) -> ThreeTimestamps { + ThreeTimestamps { + timestamps: a.into()+b.into()*B_SPACE+c.into()*C_SPACE + } + } +} + +#[derive(Copy, Drop, Serde)] +struct TwoTimestamps { + timestamps: u128 +} + +#[generate_trait] +impl TwoTimestampsImpl of TwoTimestampsTrait { + fn a(self: @TwoTimestamps) -> u64 { + (*self.timestamps & TIMESTAMP_A_FILTER).into() + } + fn b(self: @TwoTimestamps) -> u64 { + (*self.timestamps & TIMESTAMP_B_FILTER).into() + } + fn all(self: @TwoTimestamps) -> (u64, u64) { + ((*self.timestamps & TIMESTAMP_A_FILTER).into(), (*self.timestamps & TIMESTAMP_B_FILTER).into()) + } + fn new(a: u64, b: u64) -> TwoTimestamps { + TwoTimestamps { + timestamps: a.into()+b.into()*B_SPACE + } + } } #[starknet::contract] mod Timelock { - use super::{ITimelock, ContractAddress, Call}; + use super::{ITimelock, ContractAddress, Call, TwoTimestamps, ThreeTimestamps, TwoTimestampsImpl, ThreeTimestampsImpl}; use governance::call_trait::{CallTrait, HashCall}; use hash::{LegacyHash}; use array::{ArrayTrait, SpanTrait}; @@ -69,18 +132,15 @@ mod Timelock { #[storage] struct Storage { owner: ContractAddress, - delay: u64, - window: u64, - execution_started: LegacyMap, - executed: LegacyMap, - canceled: LegacyMap, + delay_and_window: TwoTimestamps, + // started_executed_canceled + execution_state: LegacyMap, } #[constructor] - fn constructor(ref self: ContractState, owner: ContractAddress, delay: u64, window: u64) { + fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: TwoTimestamps) { self.owner.write(owner); - self.delay.write(delay); - self.window.write(window); + self.delay_and_window.write(delay_and_window); } // Take a list of calls and convert it to a unique identifier for the execution @@ -113,10 +173,12 @@ mod Timelock { self.check_owner(); let id = to_id(calls); + let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); + - assert(self.execution_started.read(id).is_zero(), 'ALREADY_QUEUED'); + assert(execution_started.is_zero(), 'ALREADY_QUEUED'); - self.execution_started.write(id, get_block_timestamp()); + self.execution_state.write(id, ThreeTimestampsImpl::new(get_block_timestamp(), execution_executed, execution_canceled)); self.emit(Queued { id, calls, }); @@ -125,10 +187,11 @@ 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'); + let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); + assert(execution_started.is_non_zero(), 'DOES_NOT_EXIST'); + assert(execution_executed.is_zero(), 'ALREADY_EXECUTED'); - self.execution_started.write(id, 0); + self.execution_state.write(id, ThreeTimestampsImpl::new(0, execution_executed, execution_canceled)); self.emit(Canceled { id, }); } @@ -136,15 +199,17 @@ mod Timelock { fn execute(ref self: ContractState, mut calls: Span) -> Array> { let id = to_id(calls); - assert(self.executed.read(id).is_zero(), 'ALREADY_EXECUTED'); + let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); + + assert(execution_executed.is_zero(), 'ALREADY_EXECUTED'); - let (earliest, latest) = self.get_execution_window(id); + let (earliest, latest) = self.get_execution_window(id).all(); let time_current = get_block_timestamp(); assert(time_current >= earliest, 'TOO_EARLY'); assert(time_current < latest, 'TOO_LATE'); - self.executed.write(id, time_current); + self.execution_state.write(id, ThreeTimestampsImpl::new(execution_started, time_current, execution_canceled)); let mut results: Array> = ArrayTrait::new(); @@ -160,26 +225,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) -> TwoTimestamps { + let start_time = self.execution_state.read(id).a(); // 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 (delay, window) = (self.get_configuration()).all(); let earliest = start_time + delay; + let latest = earliest + window; - (earliest, latest) + TwoTimestampsImpl::new(earliest, 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) -> TwoTimestamps { + self.delay_and_window.read() } fn transfer(ref self: ContractState, to: ContractAddress) { @@ -188,11 +254,10 @@ mod Timelock { self.owner.write(to); } - fn configure(ref self: ContractState, delay: u64, window: u64) { + fn configure(ref self: ContractState, delay_and_window: TwoTimestamps) { self.check_self_call(); - self.delay.write(delay); - self.window.write(window); + self.delay_and_window.write(delay_and_window); } } } diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index 8ae1a29..91a6413 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -1,6 +1,6 @@ use array::{Array, ArrayTrait, SpanTrait}; use debug::PrintTrait; -use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock}; +use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, TwoTimestamps, TwoTimestampsImpl, ThreeTimestamps, ThreeTimestampsImpl}; use governance::governance_token_test::{deploy as deploy_token}; use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use governance::governance_token::{IGovernanceTokenDispatcher, IGovernanceTokenDispatcherTrait}; @@ -30,7 +30,7 @@ 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(); + let (window, delay) = timelock.get_configuration().all(); assert(window == 10239, 'window'); assert(delay == 3600, 'delay'); let owner = timelock.get_owner(); @@ -70,7 +70,7 @@ fn test_queue_execute() { let id = timelock.queue(single_call(transfer_call(token, recipient, 500_u256))); - let (earliest, latest) = timelock.get_execution_window(id); + let (earliest, latest) = timelock.get_execution_window(id).all(); assert(earliest == 86401, 'earliest'); assert(latest == 90001, 'latest'); @@ -134,7 +134,7 @@ 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); + let (earliest, latest) = timelock.get_execution_window(id).all(); set_block_timestamp(earliest - 1); timelock.execute(single_call(transfer_call(token, recipient, 500_u256))); } @@ -153,7 +153,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); + let (earliest, latest) = timelock.get_execution_window(id).all(); set_block_timestamp(latest); timelock.execute(single_call(transfer_call(token, recipient, 500_u256))); } From f71e04aa2dcc5032c0a9389e30f70fd3a5ed47a7 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Sat, 6 Jan 2024 21:48:21 +0100 Subject: [PATCH 02/10] storepacking trait --- src/timelock.cairo | 125 +++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 68 deletions(-) diff --git a/src/timelock.cairo b/src/timelock.cairo index d932539..7a95324 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -3,6 +3,7 @@ use core::result::ResultTrait; use starknet::{ContractAddress}; use starknet::account::{Call}; use core::integer::{u128_to_felt252, u64_try_from_felt252}; +use starknet::storage_access::{StorePacking}; #[starknet::interface] trait ITimelock { @@ -16,84 +17,73 @@ trait ITimelock { fn execute(ref self: TStorage, calls: Span) -> Array>; // 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) -> TwoTimestamps; + fn get_execution_window(self: @TStorage, id: felt252) -> DelayAndWindow; // Get the current owner fn get_owner(self: @TStorage) -> ContractAddress; // Returns the delay and the window for call execution - fn get_configuration(self: @TStorage) -> TwoTimestamps; + fn get_configuration(self: @TStorage) -> DelayAndWindow; // 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_and_window: TwoTimestamps); + fn configure(ref self: TStorage, delay_and_window: DelayAndWindow); } impl U128IntoU64 of Into { fn into(self: u128) -> u64 { u64_try_from_felt252(u128_to_felt252(self)).unwrap() } -} +} -const TIMESTAMP_C_FILTER: u128 = 0xFFFFFFFFF000000000000000000; -const TIMESTAMP_B_FILTER: u128 = 0xFFFFFFFFF000000000; -const TIMESTAMP_A_FILTER: u128 = 0xFFFFFFFFF; -const B_SPACE: u128 = 0x1000000000; -const C_SPACE: u128 = 0x1000000000000000000; +const TIMESTAMP_C_MASK: u128 = 0xFFFFFFFFF000000000000000000; +const TIMESTAMP_B_MASK: u128 = 0xFFFFFFFFF000000000; +const TIMESTAMP_A_MASK: u128 = 0xFFFFFFFFF; +const TWO_POW_36: u128 = 0x1000000000; +const TWO_POW_72: u128 = 0x1000000000000000000; #[derive(Copy, Drop, Serde)] -struct ThreeTimestamps { - timestamps: u128 +struct ExecutionState { + started: u64, + executed: u64, + canceled: u64 } -#[generate_trait] -impl ThreeTimestampsImpl of ThreeTimestampsTrait { - fn a(self: @ThreeTimestamps) -> u64 { - (*self.timestamps & TIMESTAMP_A_FILTER).into() - } - fn b(self: @ThreeTimestamps) -> u64 { - (*self.timestamps & TIMESTAMP_B_FILTER).into() - } - fn c(self: @ThreeTimestamps) -> u64 { - (*self.timestamps & TIMESTAMP_C_FILTER).into() - } - fn all(self: @ThreeTimestamps) -> (u64, u64, u64) { - ((*self.timestamps & TIMESTAMP_A_FILTER).into(), (*self.timestamps & TIMESTAMP_B_FILTER).into(), (*self.timestamps & TIMESTAMP_C_FILTER).into()) +impl ExecutionStateStorePacking of StorePacking { + fn pack(value: ExecutionState) -> u128 { + value.started.into()+value.executed.into()*TWO_POW_36+value.canceled.into()*TWO_POW_72 } - fn new(a: u64, b: u64, c: u64) -> ThreeTimestamps { - ThreeTimestamps { - timestamps: a.into()+b.into()*B_SPACE+c.into()*C_SPACE + fn unpack(value: u128) -> ExecutionState { + ExecutionState { + started: (value & TIMESTAMP_A_MASK).into(), + executed: (value & TIMESTAMP_B_MASK).into(), + canceled: (value & TIMESTAMP_C_MASK).into() } } } #[derive(Copy, Drop, Serde)] -struct TwoTimestamps { - timestamps: u128 +struct DelayAndWindow { + delay: u64, + window: u64 } -#[generate_trait] -impl TwoTimestampsImpl of TwoTimestampsTrait { - fn a(self: @TwoTimestamps) -> u64 { - (*self.timestamps & TIMESTAMP_A_FILTER).into() +impl DelayAndWindowStorePacking of StorePacking { + fn pack(value: DelayAndWindow) -> u128 { + value.delay.into()+value.window.into()*TWO_POW_36 } - fn b(self: @TwoTimestamps) -> u64 { - (*self.timestamps & TIMESTAMP_B_FILTER).into() - } - fn all(self: @TwoTimestamps) -> (u64, u64) { - ((*self.timestamps & TIMESTAMP_A_FILTER).into(), (*self.timestamps & TIMESTAMP_B_FILTER).into()) - } - fn new(a: u64, b: u64) -> TwoTimestamps { - TwoTimestamps { - timestamps: a.into()+b.into()*B_SPACE + fn unpack(value: u128) -> DelayAndWindow { + DelayAndWindow { + delay: (value & TIMESTAMP_A_MASK).into(), + window: (value & TIMESTAMP_B_MASK).into(), } } } #[starknet::contract] mod Timelock { - use super::{ITimelock, ContractAddress, Call, TwoTimestamps, ThreeTimestamps, TwoTimestampsImpl, ThreeTimestampsImpl}; + use super::{ITimelock, ContractAddress, Call, DelayAndWindow, ExecutionState, DelayAndWindowStorePacking, ExecutionStateStorePacking}; use governance::call_trait::{CallTrait, HashCall}; use hash::{LegacyHash}; use array::{ArrayTrait, SpanTrait}; @@ -132,13 +122,13 @@ mod Timelock { #[storage] struct Storage { owner: ContractAddress, - delay_and_window: TwoTimestamps, + delay_and_window: DelayAndWindow, // started_executed_canceled - execution_state: LegacyMap, + execution_state: LegacyMap, } #[constructor] - fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: TwoTimestamps) { + fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: DelayAndWindow) { self.owner.write(owner); self.delay_and_window.write(delay_and_window); } @@ -173,12 +163,11 @@ mod Timelock { self.check_owner(); let id = to_id(calls); - let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); - + let execution_state = self.execution_state.read(id); - assert(execution_started.is_zero(), 'ALREADY_QUEUED'); + assert(execution_state.started.is_zero(), 'ALREADY_QUEUED'); - self.execution_state.write(id, ThreeTimestampsImpl::new(get_block_timestamp(), execution_executed, execution_canceled)); + self.execution_state.write(id, ExecutionState{started: get_block_timestamp(), executed: execution_state.executed, canceled: execution_state.canceled}); self.emit(Queued { id, calls, }); @@ -187,11 +176,11 @@ mod Timelock { fn cancel(ref self: ContractState, id: felt252) { self.check_owner(); - let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); - assert(execution_started.is_non_zero(), 'DOES_NOT_EXIST'); - assert(execution_executed.is_zero(), 'ALREADY_EXECUTED'); + 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, ThreeTimestampsImpl::new(0, execution_executed, execution_canceled)); + self.execution_state.write(id, ExecutionState {started: 0, executed: execution_state.executed, canceled: execution_state.canceled}); self.emit(Canceled { id, }); } @@ -199,17 +188,17 @@ mod Timelock { fn execute(ref self: ContractState, mut calls: Span) -> Array> { let id = to_id(calls); - let (execution_started, execution_executed, execution_canceled) = self.execution_state.read(id).all(); + let execution_state = self.execution_state.read(id); - assert(execution_executed.is_zero(), 'ALREADY_EXECUTED'); + assert(execution_state.executed.is_zero(), 'ALREADY_EXECUTED'); - let (earliest, latest) = self.get_execution_window(id).all(); + 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.delay, 'TOO_EARLY'); + assert(time_current < execution_window.window, 'TOO_LATE'); - self.execution_state.write(id, ThreeTimestampsImpl::new(execution_started, time_current, execution_canceled)); + self.execution_state.write(id, ExecutionState {started: execution_state.started, executed: time_current, canceled: execution_state.canceled}); let mut results: Array> = ArrayTrait::new(); @@ -225,26 +214,26 @@ mod Timelock { results } - fn get_execution_window(self: @ContractState, id: felt252) -> TwoTimestamps { - let start_time = self.execution_state.read(id).a(); + fn get_execution_window(self: @ContractState, id: felt252) -> DelayAndWindow { + 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()).all(); + let configuration = (self.get_configuration()); - let earliest = start_time + delay; + let earliest = start_time + configuration.delay; - let latest = earliest + window; + let latest = earliest + configuration.window; - TwoTimestampsImpl::new(earliest, latest) + DelayAndWindow {delay: earliest, window: latest} } fn get_owner(self: @ContractState) -> ContractAddress { self.owner.read() } - fn get_configuration(self: @ContractState) -> TwoTimestamps { + fn get_configuration(self: @ContractState) -> DelayAndWindow { self.delay_and_window.read() } @@ -254,7 +243,7 @@ mod Timelock { self.owner.write(to); } - fn configure(ref self: ContractState, delay_and_window: TwoTimestamps) { + fn configure(ref self: ContractState, delay_and_window: DelayAndWindow) { self.check_self_call(); self.delay_and_window.write(delay_and_window); From 9d0212d1b72c904c65a14ac36547d55b198970d9 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Sat, 6 Jan 2024 21:56:53 +0100 Subject: [PATCH 03/10] execution_window --- src/timelock.cairo | 31 +++++++++++++++++++++++++------ src/timelock_test.cairo | 22 +++++++++++----------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/timelock.cairo b/src/timelock.cairo index 7a95324..6aae25d 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -17,7 +17,7 @@ trait ITimelock { fn execute(ref self: TStorage, calls: Span) -> Array>; // 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) -> DelayAndWindow; + fn get_execution_window(self: @TStorage, id: felt252) -> ExecutionWindow; // Get the current owner fn get_owner(self: @TStorage) -> ContractAddress; @@ -81,9 +81,28 @@ impl DelayAndWindowStorePacking of StorePacking { } } +#[derive(Copy, Drop, Serde)] +struct ExecutionWindow { + earliest: u64, + latest: u64 +} + +impl ExecutionWindowStorePacking of StorePacking { + fn pack(value: ExecutionWindow) -> u128 { + value.earliest.into()+value.latest.into()*TWO_POW_36 + } + fn unpack(value: u128) -> ExecutionWindow { + ExecutionWindow { + earliest: (value & TIMESTAMP_A_MASK).into(), + latest: (value & TIMESTAMP_B_MASK).into(), + } + } +} + + #[starknet::contract] mod Timelock { - use super::{ITimelock, ContractAddress, Call, DelayAndWindow, ExecutionState, DelayAndWindowStorePacking, ExecutionStateStorePacking}; + use super::{ITimelock, ContractAddress, Call, DelayAndWindow, ExecutionState, DelayAndWindowStorePacking, ExecutionStateStorePacking, ExecutionWindow, ExecutionWindowStorePacking}; use governance::call_trait::{CallTrait, HashCall}; use hash::{LegacyHash}; use array::{ArrayTrait, SpanTrait}; @@ -195,8 +214,8 @@ mod Timelock { let execution_window = self.get_execution_window(id); let time_current = get_block_timestamp(); - assert(time_current >= execution_window.delay, 'TOO_EARLY'); - assert(time_current < execution_window.window, 'TOO_LATE'); + assert(time_current >= execution_window.earliest, 'TOO_EARLY'); + assert(time_current < execution_window.latest, 'TOO_LATE'); self.execution_state.write(id, ExecutionState {started: execution_state.started, executed: time_current, canceled: execution_state.canceled}); @@ -214,7 +233,7 @@ mod Timelock { results } - fn get_execution_window(self: @ContractState, id: felt252) -> DelayAndWindow { + 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 @@ -226,7 +245,7 @@ mod Timelock { let latest = earliest + configuration.window; - DelayAndWindow {delay: earliest, window: latest} + ExecutionWindow {earliest: earliest, latest: latest} } fn get_owner(self: @ContractState) -> ContractAddress { diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index 91a6413..fa38bd1 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -1,6 +1,6 @@ use array::{Array, ArrayTrait, SpanTrait}; use debug::PrintTrait; -use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, TwoTimestamps, TwoTimestampsImpl, ThreeTimestamps, ThreeTimestampsImpl}; +use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, DelayAndWindow, DelayAndWindowStorePacking, 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 +30,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().all(); - assert(window == 10239, 'window'); - assert(delay == 3600, 'delay'); + let configuration = timelock.get_configuration(); + assert(configuration.window == 10239, 'window'); + assert(configuration.delay == 3600, 'delay'); let owner = timelock.get_owner(); assert(owner == contract_address_const::<2300>(), 'owner'); } @@ -70,9 +70,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).all(); - 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 +134,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).all(); - 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 +153,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).all(); - 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))); } From 485949c3599750f7263982497c725f8c8ddd7e6f Mon Sep 17 00:00:00 2001 From: Flydexo Date: Sat, 6 Jan 2024 22:13:50 +0100 Subject: [PATCH 04/10] 64 bits timestamps --- src/timelock.cairo | 57 +++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/timelock.cairo b/src/timelock.cairo index 6aae25d..6048246 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -1,3 +1,4 @@ +use core::option::OptionTrait; use core::traits::TryInto; use core::result::ResultTrait; use starknet::{ContractAddress}; @@ -31,17 +32,9 @@ trait ITimelock { fn configure(ref self: TStorage, delay_and_window: DelayAndWindow); } -impl U128IntoU64 of Into { - fn into(self: u128) -> u64 { - u64_try_from_felt252(u128_to_felt252(self)).unwrap() - } -} -const TIMESTAMP_C_MASK: u128 = 0xFFFFFFFFF000000000000000000; -const TIMESTAMP_B_MASK: u128 = 0xFFFFFFFFF000000000; -const TIMESTAMP_A_MASK: u128 = 0xFFFFFFFFF; -const TWO_POW_36: u128 = 0x1000000000; -const TWO_POW_72: u128 = 0x1000000000000000000; +const U64_MASK: u128 = 0xFFFFFFFFFFFFFFFF; +const TWO_POW_64: u128 = 0x10000000000000000; #[derive(Copy, Drop, Serde)] struct ExecutionState { @@ -50,15 +43,19 @@ struct ExecutionState { canceled: u64 } -impl ExecutionStateStorePacking of StorePacking { - fn pack(value: ExecutionState) -> u128 { - value.started.into()+value.executed.into()*TWO_POW_36+value.canceled.into()*TWO_POW_72 +impl ExecutionStateStorePacking of StorePacking { + fn pack(value: ExecutionState) -> felt252 { + u256 { + low: value.started.into()+value.executed.into()*TWO_POW_64, + high: value.canceled.into() + }.try_into().unwrap() } - fn unpack(value: u128) -> ExecutionState { + fn unpack(value: felt252) -> ExecutionState { + let u256_value: u256 = value.into(); ExecutionState { - started: (value & TIMESTAMP_A_MASK).into(), - executed: (value & TIMESTAMP_B_MASK).into(), - canceled: (value & TIMESTAMP_C_MASK).into() + started: (u256_value.low & U64_MASK).try_into().unwrap(), + executed: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), + canceled: (u256_value.high).try_into().unwrap() } } } @@ -69,14 +66,15 @@ struct DelayAndWindow { window: u64 } -impl DelayAndWindowStorePacking of StorePacking { - fn pack(value: DelayAndWindow) -> u128 { - value.delay.into()+value.window.into()*TWO_POW_36 +impl DelayAndWindowStorePacking of StorePacking { + fn pack(value: DelayAndWindow) -> felt252 { + (value.delay.into()+value.window.into()*TWO_POW_64).into() } - fn unpack(value: u128) -> DelayAndWindow { + fn unpack(value: felt252) -> DelayAndWindow { + let u256_value: u256 = value.into(); DelayAndWindow { - delay: (value & TIMESTAMP_A_MASK).into(), - window: (value & TIMESTAMP_B_MASK).into(), + delay: (u256_value.low & U64_MASK).try_into().unwrap(), + window: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), } } } @@ -87,14 +85,15 @@ struct ExecutionWindow { latest: u64 } -impl ExecutionWindowStorePacking of StorePacking { - fn pack(value: ExecutionWindow) -> u128 { - value.earliest.into()+value.latest.into()*TWO_POW_36 +impl ExecutionWindowStorePacking of StorePacking { + fn pack(value: ExecutionWindow) -> felt252 { + (value.earliest.into()+value.latest.into()*TWO_POW_64).into() } - fn unpack(value: u128) -> ExecutionWindow { + fn unpack(value: felt252) -> ExecutionWindow { + let u256_value: u256 = value.into(); ExecutionWindow { - earliest: (value & TIMESTAMP_A_MASK).into(), - latest: (value & TIMESTAMP_B_MASK).into(), + earliest: (u256_value.low & U64_MASK).try_into().unwrap(), + latest: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), } } } From a0f454272516a68484c7b8c4c622864ad03956f2 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Sat, 6 Jan 2024 22:35:59 +0100 Subject: [PATCH 05/10] optimized & fixed tests --- src/factory_test.cairo | 5 +++-- src/timelock.cairo | 6 +++--- src/timelock_test.cairo | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/factory_test.cairo b/src/factory_test.cairo index 7380e23..ad70c0d 100644 --- a/src/factory_test.cairo +++ b/src/factory_test.cairo @@ -8,7 +8,7 @@ use governance::factory::{ }; use governance::governance_token::{GovernanceToken}; use governance::governor::{Governor}; -use governance::timelock::{Timelock}; +use governance::timelock::{Timelock, DelayAndWindow}; use governance::airdrop::{Airdrop}; use starknet::{ get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress, @@ -100,5 +100,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)'); } diff --git a/src/timelock.cairo b/src/timelock.cairo index 6048246..8d87528 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -54,7 +54,7 @@ impl ExecutionStateStorePacking of StorePacking { let u256_value: u256 = value.into(); ExecutionState { started: (u256_value.low & U64_MASK).try_into().unwrap(), - executed: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), + executed: (u256_value.low/TWO_POW_64).try_into().unwrap(), canceled: (u256_value.high).try_into().unwrap() } } @@ -74,7 +74,7 @@ impl DelayAndWindowStorePacking of StorePacking { let u256_value: u256 = value.into(); DelayAndWindow { delay: (u256_value.low & U64_MASK).try_into().unwrap(), - window: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), + window: (u256_value.low/TWO_POW_64).try_into().unwrap(), } } } @@ -93,7 +93,7 @@ impl ExecutionWindowStorePacking of StorePacking { let u256_value: u256 = value.into(); ExecutionWindow { earliest: (u256_value.low & U64_MASK).try_into().unwrap(), - latest: ((u256_value.low/TWO_POW_64) & U64_MASK).try_into().unwrap(), + latest: (u256_value.low/TWO_POW_64).try_into().unwrap(), } } } diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index fa38bd1..6406277 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -31,8 +31,8 @@ fn test_deploy() { let timelock = deploy(contract_address_const::<2300>(), 10239, 3600); let configuration = timelock.get_configuration(); - assert(configuration.window == 10239, 'window'); - assert(configuration.delay == 3600, 'delay'); + assert(configuration.delay == 10239, 'delay'); + assert(configuration.window == 3600, 'window'); let owner = timelock.get_owner(); assert(owner == contract_address_const::<2300>(), 'owner'); } From 75c69b64b6523d7bf68bb5c1debf6345c0ea310d Mon Sep 17 00:00:00 2001 From: Flydexo Date: Mon, 8 Jan 2024 22:59:24 +0100 Subject: [PATCH 06/10] tuple store & factory --- src/factory.cairo | 8 +--- src/factory_test.cairo | 3 +- src/timelock.cairo | 85 +++++++++++++++++++++++++++-------------- src/timelock_test.cairo | 2 +- 4 files changed, 60 insertions(+), 38 deletions(-) diff --git a/src/factory.cairo b/src/factory.cairo index e90a44c..04239bc 100644 --- a/src/factory.cairo +++ b/src/factory.cairo @@ -3,7 +3,7 @@ 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 { @@ -11,12 +11,6 @@ struct AirdropConfig { total: u128, } -#[derive(Copy, Drop, Serde)] -struct TimelockConfig { - delay: u64, - window: u64, -} - #[derive(Copy, Drop, Serde)] struct DeploymentParameters { name: felt252, diff --git a/src/factory_test.cairo b/src/factory_test.cairo index ad70c0d..73b77f7 100644 --- a/src/factory_test.cairo +++ b/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, DelayAndWindow}; +use governance::timelock::{Timelock, TimelockConfig}; use governance::airdrop::{Airdrop}; use starknet::{ get_contract_address, deploy_syscall, ClassHash, contract_address_const, ContractAddress, diff --git a/src/timelock.cairo b/src/timelock.cairo index 8d87528..b5aab81 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -24,18 +24,49 @@ trait ITimelock { fn get_owner(self: @TStorage) -> ContractAddress; // Returns the delay and the window for call execution - fn get_configuration(self: @TStorage) -> DelayAndWindow; + 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_and_window: DelayAndWindow); + fn configure(ref self: TStorage, delay_and_window: TimelockConfig); } const U64_MASK: u128 = 0xFFFFFFFFFFFFFFFF; const TWO_POW_64: u128 = 0x10000000000000000; +impl ThreeU64TupleStorePacking of StorePacking<(u64, u64, u64), felt252> { + fn pack(value: (u64, u64, u64)) -> felt252 { + let (a, b, c) = value; + u256 { + low: a.into()+b.into()*TWO_POW_64, + high: c.into() + }.try_into().unwrap() + } + fn unpack(value: felt252) -> (u64, u64, u64) { + let u256_value: u256 = value.into(); + ( + (u256_value.low & U64_MASK).try_into().unwrap(), + (u256_value.low/TWO_POW_64).try_into().unwrap(), + (u256_value.high).try_into().unwrap() + ) + } +} + +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) { + ( + (value & U64_MASK).try_into().unwrap(), + (value/TWO_POW_64).try_into().unwrap() + ) + } +} + #[derive(Copy, Drop, Serde)] struct ExecutionState { started: u64, @@ -43,38 +74,36 @@ struct ExecutionState { canceled: u64 } -impl ExecutionStateStorePacking of StorePacking { - fn pack(value: ExecutionState) -> felt252 { - u256 { - low: value.started.into()+value.executed.into()*TWO_POW_64, - high: value.canceled.into() - }.try_into().unwrap() +#[inline(always)] +impl ExecutionStateStorePacking of StorePacking { + fn pack(value: ExecutionState) -> (u64, u64, u64) { + (value.started, value.executed, value.canceled) } - fn unpack(value: felt252) -> ExecutionState { - let u256_value: u256 = value.into(); + fn unpack(value: (u64, u64, u64)) -> ExecutionState { + let (started, executed, canceled) = value; ExecutionState { - started: (u256_value.low & U64_MASK).try_into().unwrap(), - executed: (u256_value.low/TWO_POW_64).try_into().unwrap(), - canceled: (u256_value.high).try_into().unwrap() + started, + executed, + canceled } } } #[derive(Copy, Drop, Serde)] -struct DelayAndWindow { +struct TimelockConfig { delay: u64, - window: u64 + window: u64, } -impl DelayAndWindowStorePacking of StorePacking { - fn pack(value: DelayAndWindow) -> felt252 { - (value.delay.into()+value.window.into()*TWO_POW_64).into() +impl TimelockConfigStorePacking of StorePacking { + fn pack(value: TimelockConfig) -> (u64, u64) { + (value.delay, value.window) } - fn unpack(value: felt252) -> DelayAndWindow { - let u256_value: u256 = value.into(); - DelayAndWindow { - delay: (u256_value.low & U64_MASK).try_into().unwrap(), - window: (u256_value.low/TWO_POW_64).try_into().unwrap(), + fn unpack(value: (u64, u64)) -> TimelockConfig { + let (delay, window) = value; + TimelockConfig { + delay, + window } } } @@ -101,7 +130,7 @@ impl ExecutionWindowStorePacking of StorePacking { #[starknet::contract] mod Timelock { - use super::{ITimelock, ContractAddress, Call, DelayAndWindow, ExecutionState, DelayAndWindowStorePacking, ExecutionStateStorePacking, ExecutionWindow, ExecutionWindowStorePacking}; + 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}; @@ -140,13 +169,13 @@ mod Timelock { #[storage] struct Storage { owner: ContractAddress, - delay_and_window: DelayAndWindow, + delay_and_window: TimelockConfig, // started_executed_canceled execution_state: LegacyMap, } #[constructor] - fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: DelayAndWindow) { + fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: TimelockConfig) { self.owner.write(owner); self.delay_and_window.write(delay_and_window); } @@ -251,7 +280,7 @@ mod Timelock { self.owner.read() } - fn get_configuration(self: @ContractState) -> DelayAndWindow { + fn get_configuration(self: @ContractState) -> TimelockConfig { self.delay_and_window.read() } @@ -261,7 +290,7 @@ mod Timelock { self.owner.write(to); } - fn configure(ref self: ContractState, delay_and_window: DelayAndWindow) { + fn configure(ref self: ContractState, delay_and_window: TimelockConfig) { self.check_self_call(); self.delay_and_window.write(delay_and_window); diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index 6406277..9e686af 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -1,6 +1,6 @@ use array::{Array, ArrayTrait, SpanTrait}; use debug::PrintTrait; -use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, DelayAndWindow, DelayAndWindowStorePacking, ExecutionState, ExecutionStateStorePacking}; +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}; From b22e08154130a9521c73dda11ec5e4a3c57c19dc Mon Sep 17 00:00:00 2001 From: Flydexo Date: Wed, 10 Jan 2024 22:26:19 +0100 Subject: [PATCH 07/10] resolves comments --- src/governor.cairo | 2 +- src/timelock.cairo | 126 ++++++++++++++++++++++------------------ src/timelock_test.cairo | 5 +- 3 files changed, 74 insertions(+), 59 deletions(-) diff --git a/src/governor.cairo b/src/governor.cairo index de10e0c..8099b61 100644 --- a/src/governor.cairo +++ b/src/governor.cairo @@ -295,7 +295,7 @@ mod Governor { let data = call.execute(); self.emit(Executed { id, }); - + data } diff --git a/src/timelock.cairo b/src/timelock.cairo index b5aab81..ca6eacb 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -3,7 +3,7 @@ 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}; +use core::integer::{u128_to_felt252, u64_try_from_felt252, u128_safe_divmod}; use starknet::storage_access::{StorePacking}; #[starknet::interface] @@ -29,41 +29,34 @@ trait ITimelock { // 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_and_window: TimelockConfig); + fn configure(ref self: TStorage, config: TimelockConfig); } -const U64_MASK: u128 = 0xFFFFFFFFFFFFFFFF; 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: a.into()+b.into()*TWO_POW_64, - high: c.into() - }.try_into().unwrap() + 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(); - ( - (u256_value.low & U64_MASK).try_into().unwrap(), - (u256_value.low/TWO_POW_64).try_into().unwrap(), - (u256_value.high).try_into().unwrap() - ) + 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 + a.into() + b.into() * TWO_POW_64 } fn unpack(value: u128) -> (u64, u64) { - ( - (value & U64_MASK).try_into().unwrap(), - (value/TWO_POW_64).try_into().unwrap() - ) + let (q, r) = u128_safe_divmod(value, TWO_POW_64.try_into().unwrap()); + (r.try_into().unwrap(), q.try_into().unwrap()) } } @@ -75,17 +68,13 @@ struct ExecutionState { } #[inline(always)] -impl ExecutionStateStorePacking of StorePacking { - fn pack(value: ExecutionState) -> (u64, u64, u64) { - (value.started, value.executed, value.canceled) +impl ExecutionStateStorePacking of StorePacking { + fn pack(value: ExecutionState) -> felt252 { + ThreeU64TupleStorePacking::pack((value.started, value.executed, value.canceled)) } - fn unpack(value: (u64, u64, u64)) -> ExecutionState { - let (started, executed, canceled) = value; - ExecutionState { - started, - executed, - canceled - } + fn unpack(value: felt252) -> ExecutionState { + let (started, executed, canceled) = ThreeU64TupleStorePacking::unpack(value); + ExecutionState { started, executed, canceled } } } @@ -95,16 +84,14 @@ struct TimelockConfig { window: u64, } -impl TimelockConfigStorePacking of StorePacking { - fn pack(value: TimelockConfig) -> (u64, u64) { - (value.delay, value.window) +#[inline(always)] +impl TimelockConfigStorePacking of StorePacking { + fn pack(value: TimelockConfig) -> u128 { + TwoU64TupleStorePacking::pack((value.delay, value.window)) } - fn unpack(value: (u64, u64)) -> TimelockConfig { - let (delay, window) = value; - TimelockConfig { - delay, - window - } + fn unpack(value: u128) -> TimelockConfig { + let (delay, window) = TwoU64TupleStorePacking::unpack(value); + TimelockConfig { delay, window } } } @@ -114,23 +101,25 @@ struct ExecutionWindow { latest: u64 } -impl ExecutionWindowStorePacking of StorePacking { - fn pack(value: ExecutionWindow) -> felt252 { - (value.earliest.into()+value.latest.into()*TWO_POW_64).into() +#[inline(always)] +impl ExecutionWindowStorePacking of StorePacking { + fn pack(value: ExecutionWindow) -> u128 { + TwoU64TupleStorePacking::pack((value.earliest, value.latest)) } - fn unpack(value: felt252) -> ExecutionWindow { - let u256_value: u256 = value.into(); - ExecutionWindow { - earliest: (u256_value.low & U64_MASK).try_into().unwrap(), - latest: (u256_value.low/TWO_POW_64).try_into().unwrap(), - } + fn unpack(value: u128) -> ExecutionWindow { + let (earliest, latest) = TwoU64TupleStorePacking::unpack(value); + ExecutionWindow { earliest, latest, } } } #[starknet::contract] mod Timelock { - use super::{ITimelock, ContractAddress, Call, TimelockConfig, ExecutionState, TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow, ExecutionWindowStorePacking, TwoU64TupleStorePacking, ThreeU64TupleStorePacking}; + 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}; @@ -169,15 +158,15 @@ mod Timelock { #[storage] struct Storage { owner: ContractAddress, - delay_and_window: TimelockConfig, + config: TimelockConfig, // started_executed_canceled execution_state: LegacyMap, } #[constructor] - fn constructor(ref self: ContractState, owner: ContractAddress, delay_and_window: TimelockConfig) { + fn constructor(ref self: ContractState, owner: ContractAddress, config: TimelockConfig) { self.owner.write(owner); - self.delay_and_window.write(delay_and_window); + self.config.write(config); } // Take a list of calls and convert it to a unique identifier for the execution @@ -213,8 +202,13 @@ mod Timelock { let execution_state = self.execution_state.read(id); assert(execution_state.started.is_zero(), 'ALREADY_QUEUED'); + assert(execution_state.canceled.is_non_zero(), 'PREVIOUSLY_CANCELED'); - self.execution_state.write(id, ExecutionState{started: get_block_timestamp(), executed: execution_state.executed, canceled: execution_state.canceled}); + self + .execution_state + .write( + id, ExecutionState { started: get_block_timestamp(), executed: 0, canceled: 0 } + ); self.emit(Queued { id, calls, }); @@ -227,7 +221,16 @@ mod Timelock { 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 + .execution_state + .write( + id, + ExecutionState { + started: 0, + executed: execution_state.executed, + canceled: execution_state.canceled + } + ); self.emit(Canceled { id, }); } @@ -245,7 +248,16 @@ mod Timelock { assert(time_current >= execution_window.earliest, 'TOO_EARLY'); assert(time_current < execution_window.latest, 'TOO_LATE'); - self.execution_state.write(id, ExecutionState {started: execution_state.started, executed: time_current, canceled: execution_state.canceled}); + self + .execution_state + .write( + id, + ExecutionState { + started: execution_state.started, + executed: time_current, + canceled: execution_state.canceled + } + ); let mut results: Array> = ArrayTrait::new(); @@ -270,10 +282,10 @@ mod Timelock { let configuration = (self.get_configuration()); let earliest = start_time + configuration.delay; - + let latest = earliest + configuration.window; - ExecutionWindow {earliest: earliest, latest: latest} + ExecutionWindow { earliest: earliest, latest: latest } } fn get_owner(self: @ContractState) -> ContractAddress { @@ -281,7 +293,7 @@ mod Timelock { } fn get_configuration(self: @ContractState) -> TimelockConfig { - self.delay_and_window.read() + self.config.read() } fn transfer(ref self: ContractState, to: ContractAddress) { @@ -290,10 +302,10 @@ mod Timelock { self.owner.write(to); } - fn configure(ref self: ContractState, delay_and_window: TimelockConfig) { + fn configure(ref self: ContractState, config: TimelockConfig) { self.check_self_call(); - self.delay_and_window.write(delay_and_window); + self.config.write(config); } } } diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index 9e686af..d043c85 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -1,6 +1,9 @@ use array::{Array, ArrayTrait, SpanTrait}; use debug::PrintTrait; -use governance::timelock::{ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, TimelockConfig, TimelockConfigStorePacking, ExecutionState, ExecutionStateStorePacking}; +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}; From 13c9caba9ae9033b96beaa96ab9a2dab093cf08e Mon Sep 17 00:00:00 2001 From: Flydexo Date: Thu, 11 Jan 2024 21:26:46 +0100 Subject: [PATCH 08/10] separated file, removed CANCELED, inline --- src/lib.cairo | 1 + src/timelock.cairo | 105 ++++++++++++------------------------- src/utils.cairo | 1 + src/utils/timestamps.cairo | 31 +++++++++++ 4 files changed, 66 insertions(+), 72 deletions(-) create mode 100644 src/utils.cairo create mode 100644 src/utils/timestamps.cairo diff --git a/src/lib.cairo b/src/lib.cairo index 21e58e6..9d658dd 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -5,6 +5,7 @@ mod governance_token; mod governor; mod interfaces; mod timelock; +mod utils; #[cfg(test)] mod airdrop_test; diff --git a/src/timelock.cairo b/src/timelock.cairo index ca6eacb..c120ccb 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -3,62 +3,8 @@ 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 { - // Queue a list of calls to be executed after the delay. Only the owner may call this. - fn queue(ref self: TStorage, calls: Span) -> felt252; - - // Cancel a queued proposal before it is executed. Only the owner may call this. - fn cancel(ref self: TStorage, id: felt252); - - // Execute a list of calls that have previously been queued. Anyone may call this. - fn execute(ref self: TStorage, calls: Span) -> Array>; - - // 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) -> 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) -> 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, 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()) - } -} +use governance::utils::timestamps::{ThreeU64TupleStorePacking, TwoU64TupleStorePacking}; #[derive(Copy, Drop, Serde)] struct ExecutionState { @@ -67,11 +13,12 @@ struct ExecutionState { canceled: u64 } -#[inline(always)] impl ExecutionStateStorePacking of StorePacking { + #[inline(always)] fn pack(value: ExecutionState) -> felt252 { ThreeU64TupleStorePacking::pack((value.started, value.executed, value.canceled)) } + #[inline(always)] fn unpack(value: felt252) -> ExecutionState { let (started, executed, canceled) = ThreeU64TupleStorePacking::unpack(value); ExecutionState { started, executed, canceled } @@ -84,41 +31,55 @@ struct TimelockConfig { window: u64, } -#[inline(always)] impl TimelockConfigStorePacking of StorePacking { + #[inline(always)] fn pack(value: TimelockConfig) -> u128 { TwoU64TupleStorePacking::pack((value.delay, value.window)) } + #[inline(always)] fn unpack(value: u128) -> TimelockConfig { let (delay, window) = TwoU64TupleStorePacking::unpack(value); TimelockConfig { delay, window } } } +#[starknet::interface] +trait ITimelock { + // Queue a list of calls to be executed after the delay. Only the owner may call this. + fn queue(ref self: TStorage, calls: Span) -> felt252; + + // Cancel a queued proposal before it is executed. Only the owner may call this. + fn cancel(ref self: TStorage, id: felt252); + + // Execute a list of calls that have previously been queued. Anyone may call this. + fn execute(ref self: TStorage, calls: Span) -> Array>; + + // 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) -> 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) -> 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, config: TimelockConfig); +} + #[derive(Copy, Drop, Serde)] struct ExecutionWindow { earliest: u64, latest: u64 } -#[inline(always)] -impl ExecutionWindowStorePacking of StorePacking { - 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, TimelockConfig, ExecutionState, - TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow, - ExecutionWindowStorePacking, TwoU64TupleStorePacking, ThreeU64TupleStorePacking + TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow }; use governance::call_trait::{CallTrait, HashCall}; use hash::{LegacyHash}; @@ -285,7 +246,7 @@ mod Timelock { let latest = earliest + configuration.window; - ExecutionWindow { earliest: earliest, latest: latest } + ExecutionWindow { earliest, latest } } fn get_owner(self: @ContractState) -> ContractAddress { diff --git a/src/utils.cairo b/src/utils.cairo new file mode 100644 index 0000000..ea30424 --- /dev/null +++ b/src/utils.cairo @@ -0,0 +1 @@ +mod timestamps; diff --git a/src/utils/timestamps.cairo b/src/utils/timestamps.cairo new file mode 100644 index 0000000..7c275f0 --- /dev/null +++ b/src/utils/timestamps.cairo @@ -0,0 +1,31 @@ +use core::integer::{u128_to_felt252, u64_try_from_felt252, u128_safe_divmod}; +use starknet::storage_access::{StorePacking}; + +const TWO_POW_64: u128 = 0x10000000000000000; + +impl ThreeU64TupleStorePacking of StorePacking<(u64, u64, u64), felt252> { + #[inline(always)] + fn pack(value: (u64, u64, u64)) -> felt252 { + let (a, b, c) = value; + u256 { low: TwoU64TupleStorePacking::pack((a, b)), high: c.into() }.try_into().unwrap() + } + #[inline(always)] + 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()) + } +} + +impl TwoU64TupleStorePacking of StorePacking<(u64, u64), u128> { + #[inline(always)] + fn pack(value: (u64, u64)) -> u128 { + let (a, b) = value; + a.into() + b.into() * TWO_POW_64 + } + #[inline(always)] + 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()) + } +} From 694ea0436348cc34a330fe6f952f995aee03ac26 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Thu, 11 Jan 2024 21:27:03 +0100 Subject: [PATCH 09/10] rm canceled checl --- src/timelock.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/timelock.cairo b/src/timelock.cairo index c120ccb..a4d3dab 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -163,7 +163,6 @@ mod Timelock { let execution_state = self.execution_state.read(id); assert(execution_state.started.is_zero(), 'ALREADY_QUEUED'); - assert(execution_state.canceled.is_non_zero(), 'PREVIOUSLY_CANCELED'); self .execution_state From c570a51de2e9ab466e80cc723419da3d1d8548b1 Mon Sep 17 00:00:00 2001 From: Flydexo Date: Fri, 12 Jan 2024 18:05:55 +0100 Subject: [PATCH 10/10] fix conflicts --- src/factory.cairo | 2 +- src/lib.cairo | 14 +++++++------- src/timelock.cairo | 14 +++++++------- src/timelock_test.cairo | 4 ++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/factory.cairo b/src/factory.cairo index 46d76e7..7ab3d54 100644 --- a/src/factory.cairo +++ b/src/factory.cairo @@ -2,8 +2,8 @@ use governance::airdrop::{IAirdropDispatcher}; use governance::governance_token::{IGovernanceTokenDispatcher}; use governance::governor::{Config as GovernorConfig}; use governance::governor::{IGovernorDispatcher}; -use starknet::{ContractAddress}; use governance::timelock::{ITimelockDispatcher, TimelockConfig}; +use starknet::{ContractAddress}; #[derive(Copy, Drop, Serde)] struct AirdropConfig { diff --git a/src/lib.cairo b/src/lib.cairo index 9d658dd..90dd1f1 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -1,23 +1,23 @@ mod airdrop; -mod call_trait; -mod factory; -mod governance_token; -mod governor; -mod interfaces; -mod timelock; -mod utils; #[cfg(test)] mod airdrop_test; +mod call_trait; #[cfg(test)] mod call_trait_test; +mod factory; #[cfg(test)] mod factory_test; +mod governance_token; #[cfg(test)] mod governance_token_test; +mod governor; #[cfg(test)] mod governor_test; +mod interfaces; #[cfg(test)] mod test_utils; +mod timelock; #[cfg(test)] mod timelock_test; +mod utils; diff --git a/src/timelock.cairo b/src/timelock.cairo index 7bfa41e..1931c5a 100644 --- a/src/timelock.cairo +++ b/src/timelock.cairo @@ -1,10 +1,10 @@ use core::option::OptionTrait; -use core::traits::TryInto; use core::result::ResultTrait; +use core::traits::TryInto; +use governance::utils::timestamps::{ThreeU64TupleStorePacking, TwoU64TupleStorePacking}; use starknet::account::{Call}; use starknet::contract_address::{ContractAddress}; use starknet::storage_access::{StorePacking}; -use governance::utils::timestamps::{ThreeU64TupleStorePacking, TwoU64TupleStorePacking}; #[derive(Copy, Drop, Serde)] struct ExecutionState { @@ -77,16 +77,16 @@ struct ExecutionWindow { #[starknet::contract] mod Timelock { - use super::{ - ITimelock, ContractAddress, Call, TimelockConfig, ExecutionState, - TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow - }; + use core::hash::LegacyHash; use governance::call_trait::{CallTrait, HashCall}; use starknet::{ get_caller_address, get_contract_address, SyscallResult, syscalls::call_contract_syscall, ContractAddressIntoFelt252, get_block_timestamp }; - use core::hash::LegacyHash; + use super::{ + ITimelock, ContractAddress, Call, TimelockConfig, ExecutionState, + TimelockConfigStorePacking, ExecutionStateStorePacking, ExecutionWindow + }; #[derive(starknet::Event, Drop)] struct Queued { diff --git a/src/timelock_test.cairo b/src/timelock_test.cairo index cc2ae47..b36050f 100644 --- a/src/timelock_test.cairo +++ b/src/timelock_test.cairo @@ -1,11 +1,11 @@ use array::{Array, ArrayTrait, SpanTrait}; use debug::PrintTrait; +use governance::governance_token_test::{deploy as deploy_token, IGovernanceTokenDispatcher}; +use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use governance::timelock::{ ITimelockDispatcher, ITimelockDispatcherTrait, Timelock, TimelockConfig, TimelockConfigStorePacking, ExecutionState, ExecutionStateStorePacking }; -use governance::governance_token_test::{deploy as deploy_token, IGovernanceTokenDispatcher}; -use governance::interfaces::erc20::{IERC20Dispatcher, IERC20DispatcherTrait}; use starknet::account::{Call}; use starknet::class_hash::Felt252TryIntoClassHash; use starknet::{