Removes the backwards compatibility of storage for governor config (#60)

* Removes the backwards compatibility of storage for governor config * improve the comment
EkuboProtocol · Sep 4, 2024 · 51a5123 · 51a5123
1 parent d5a5dd5
commit 51a5123
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 7 deletions.
diff --git a/src/governor.cairo b/src/governor.cairo
@@ -88,6 +88,9 @@ pub trait IGovernor<TContractState> {
 
     // Replaces the code at this address. This must be self-called via a proposal.
     fn upgrade(ref self: TContractState, class_hash: ClassHash);
+
+    // Migrates to the latest version of storage layout, from the version of storage before v2.1.0
+    fn _migrate_old_config_storage(ref self: TContractState);
 }
 
 #[starknet::contract(account)]
@@ -97,6 +100,7 @@ pub mod Governor {
     use core::poseidon::{PoseidonTrait};
     use governance::call_trait::{HashSerializable, CallTrait};
     use governance::staker::{IStakerDispatcherTrait};
+    use starknet::storage_access::{Store, storage_base_address_from_felt252};
     use starknet::{
         get_block_timestamp, get_caller_address, get_contract_address,
         syscalls::{replace_class_syscall}, AccountContract, get_tx_info
@@ -159,7 +163,6 @@ pub mod Governor {
     #[storage]
     struct Storage {
         staker: IStakerDispatcher,
-        config: Config,
         config_versions: LegacyMap<u64, Config>,
         latest_config_version: u64,
         nonce: u64,
@@ -171,7 +174,9 @@ pub mod Governor {
     #[constructor]
     fn constructor(ref self: ContractState, staker: IStakerDispatcher, config: Config) {
         self.staker.write(staker);
-        self.config.write(config);
+
+        self.config_versions.write(0, config);
+        self.emit(Reconfigured { new_config: config, version: 0 });
     }
 
     pub fn get_proposal_id(address: ContractAddress, nonce: u64) -> felt252 {
@@ -405,11 +410,7 @@ pub mod Governor {
         }
 
         fn get_config_version(self: @ContractState, version: u64) -> Config {
-            if version.is_zero() {
-                self.config.read()
-            } else {
-                self.config_versions.read(version)
-            }
+            self.config_versions.read(version)
         }
 
         fn get_staker(self: @ContractState) -> IStakerDispatcher {
@@ -447,6 +448,29 @@ pub mod Governor {
 
             replace_class_syscall(class_hash).unwrap();
         }
+
+        fn _migrate_old_config_storage(ref self: ContractState) {
+            let old_config_storage_address = storage_base_address_from_felt252(selector!("config"));
+            let old_config: Config = Store::read(0, old_config_storage_address).unwrap();
+            // voting period of 0 is assumed to be invalid
+            assert(old_config.voting_period.is_non_zero(), 'NO_OLD_CONFIG');
+            self.config_versions.write(0, old_config);
+            self.emit(Reconfigured { version: 0, new_config: old_config });
+            Store::write(
+                0,
+                old_config_storage_address,
+                Config {
+                    voting_start_delay: 0,
+                    voting_period: 0,
+                    voting_weight_smoothing_duration: 0,
+                    quorum: 0,
+                    proposal_creation_threshold: 0,
+                    execution_delay: 0,
+                    execution_window: 0
+                }
+            )
+                .expect('FAILED_TO_DELETE');
+        }
     }
 
     // This implementation exists solely for the purpose of allowing simulation of calls from the governor with the flag to skip validation

diff --git a/src/governor_test.cairo b/src/governor_test.cairo
@@ -293,6 +293,7 @@ fn test_describe_proposal_successful() {
             id,
             "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
         );
+    pop_log::<Governor::Reconfigured>(governor.contract_address).unwrap();
     pop_log::<Governor::Proposed>(governor.contract_address).unwrap();
     assert_eq!(
         pop_log::<Governor::Described>(governor.contract_address).unwrap(),
@@ -320,6 +321,7 @@ fn test_propose_and_describe_successful() {
         );
     set_contract_address(address_before);
 
+    pop_log::<Governor::Reconfigured>(governor.contract_address).unwrap();
     pop_log::<Governor::Proposed>(governor.contract_address).unwrap();
     assert_eq!(
         pop_log::<Governor::Described>(governor.contract_address).unwrap(),
@@ -892,6 +894,7 @@ fn test_execute_emits_logs_from_data() {
     // both transfers suceeded
     assert_eq!(result, expected);
 
+    pop_log::<Governor::Reconfigured>(governor.contract_address).unwrap();
     pop_log::<Governor::Proposed>(governor.contract_address).unwrap();
     pop_log::<Governor::Voted>(governor.contract_address).unwrap();
     pop_log::<Governor::Voted>(governor.contract_address).unwrap();
@@ -1111,6 +1114,8 @@ fn test_reconfigure_succeeds_self_call() {
                 .span()
         );
 
+    // the first one is from constructor
+    pop_log::<Governor::Reconfigured>(governor.contract_address).unwrap();
     pop_log::<Governor::Proposed>(governor.contract_address).unwrap();
     pop_log::<Governor::Voted>(governor.contract_address).unwrap();
     let reconfigured = pop_log::<Governor::Reconfigured>(governor.contract_address).unwrap();