chore: remove _lastValidateTime initialization in constructor (#493)

* chore: remove _lastValidateTime initialization in constructor * chore: update stateful tests * chore: small update * chore: reintroduce nzKey * chore: fix flakyness
chainflip-io · Feb 12, 2024 · faaedad · faaedad
1 parent 3813b43
commit faaedad
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 36 deletions.
diff --git a/contracts/KeyManager.sol b/contracts/KeyManager.sol
@@ -32,7 +32,6 @@ contract KeyManager is SchnorrSECP256K1, Shared, IKeyManager {
         _aggKey = initialAggKey;
         _govKey = initialGovKey;
         _commKey = initialCommKey;
-        _lastValidateTime = block.timestamp;
     }
 
     //////////////////////////////////////////////////////////////

diff --git a/tests/integration/keyManager/test_setKey_setKey.py b/tests/integration/keyManager/test_setKey_setKey.py
@@ -30,15 +30,6 @@ def test_setAggKeyWithAggKey_setAggKeyWithAggKey(cf):
 
 
 def test_setGovKeyWithGovKey_setAggKeyWithGovKey(cf):
-    # Changing the agg key with the gov key should fail if the delay hasn't been long enough yet
-    # No time has passed since the constructor has set the first value of _lastValidateTime
-    with reverts(REV_MSG_DELAY):
-        cf.keyManager.setAggKeyWithGovKey(
-            AGG_SIGNER_2.getPubData(), {"from": cf.GOVERNOR_2}
-        )
-
-    chain.sleep(AGG_KEY_TIMEOUT)
-
     # Change the gov key
     setGovKeyWithGovKey_test(cf)
 
@@ -63,15 +54,6 @@ def test_setGovKeyWithGovKey_setAggKeyWithGovKey(cf):
 
 # Check that _withAggKey calls update the _lastValidateTime
 def test_setAggKeyWithGovKey_setKeyWithAggKey(cf):
-    # Changing the agg key with the gov key should fail if the delay hasn't been long enough yet
-    # No time has passed since the constructor has set the first value of _lastValidateTime
-    with reverts(REV_MSG_DELAY):
-        cf.keyManager.setAggKeyWithGovKey(
-            AGG_SIGNER_2.getPubData(), {"from": cf.GOVERNOR}
-        )
-
-    chain.sleep(AGG_KEY_TIMEOUT)
-
     setAggKeyWithAggKey_test(cf)
 
     # Reverts due to setAggKeyWithAggKey updating the _lastValidateTime

diff --git a/tests/stateful/test_all.py b/tests/stateful/test_all.py
@@ -235,7 +235,7 @@ def setup(self):
             self.v_suspended = self.v.getSuspendedState()
 
             # KeyManager
-            self.lastValidateTime = self.deployerContract.tx.timestamp
+            self.lastValidateTime = 0
             self.keyIDToCurKeys = {AGG: AGG_SIGNER_1}
             self.allKeys = [*self.keyIDToCurKeys.values()] + (
                 [Signer.gen_signer(None, {})]
@@ -2123,7 +2123,7 @@ def rule_upgrade_keyManager(self, st_sender):
 
                 self._updateBalancesOnUpgrade(self.km, newKeyManager)
                 self.km = newKeyManager
-                self.lastValidateTime = self.km.tx.timestamp
+                self.lastValidateTime = 0
 
         # Deploys a new Vault and transfers the funds from the old Vault to the new one
         def rule_upgrade_Vault(self, st_sender, st_sleep_time):

diff --git a/tests/stateful/test_keyManager.py b/tests/stateful/test_keyManager.py
@@ -31,7 +31,7 @@ def __init__(cls, a, cfDeploy):
 
         # Reset the local versions of state to compare the contract to after every run
         def setup(self):
-            self.lastValidateTime = self.deployerContract.tx.timestamp
+            self.lastValidateTime = 0
             self.keyIDToCurKeys = {AGG: AGG_SIGNER_1}
             self.allKeys = [*self.keyIDToCurKeys.values()] + (
                 [Signer.gen_signer(None, {})]

diff --git a/tests/stateful/test_upgradability.py b/tests/stateful/test_upgradability.py
@@ -50,7 +50,7 @@ def setup(self):
             self.v = self.orig_v
             self.km = self.orig_km
 
-            self.lastValidateTime = self.deployerContract.tx.timestamp
+            self.lastValidateTime = 0
             self.numTxsTested = 0
 
             # StateChainGateway
@@ -104,7 +104,7 @@ def rule_upgrade_keyManager(self, st_sender):
                 assert aggKeyNonceConsumer.getKeyManager() == newKeyManager
 
             self.km = newKeyManager
-            self.lastValidateTime = self.km.tx.timestamp
+            self.lastValidateTime = 0
 
         # Deploys a new Vault and transfers the funds from the old Vault to the new one
         def rule_upgrade_Vault(

diff --git a/tests/unit/keyManager/test_keyManager_constructor.py b/tests/unit/keyManager/test_keyManager_constructor.py
@@ -6,4 +6,4 @@
 def test_constructor(a, cf):
     assert cf.keyManager.getAggregateKey() == AGG_SIGNER_1.getPubDataWith0x()
     assert cf.keyManager.getGovernanceKey() == cf.GOVERNOR
-    assert cf.keyManager.getLastValidateTime() == cf.deployerContract.tx.timestamp
+    assert cf.keyManager.getLastValidateTime() == 0
diff --git a/tests/unit/keyManager/test_setAggKeyWithGovKey.py b/tests/unit/keyManager/test_setAggKeyWithGovKey.py
@@ -6,15 +6,12 @@
 
 
 def test_setAggKeyWithGovKey(cf):
-    chain.sleep(AGG_KEY_TIMEOUT)
     setAggKeyWithGovKey_test(cf)
 
 
-def test_setAggKeyWithGovKey_rev_time(cf):
-    with reverts(REV_MSG_DELAY):
-        cf.keyManager.setAggKeyWithGovKey(
-            cf.keyManager.getAggregateKey(), {"from": cf.GOVERNOR}
-        )
+def test_setAggKeyWithGovKey_delay_success(cf):
+    chain.sleep(AGG_KEY_TIMEOUT)
+    setAggKeyWithGovKey_test(cf)
 
 
 def test_setAggKeyWithGovKey_rev_governor(cf):

diff --git a/tests/unit/vault/test_executeActions.py b/tests/unit/vault/test_executeActions.py
@@ -424,8 +424,8 @@ def test_executeActions_revgas(cf, multicall):
     # Gas limit that doesn't allow the Multicall to execute the actions but leaves
     # enough gas to trigger "Vault: gasMulticall too low". Succesfull tx according
     # to gas test is ~140k but it doesn't succeed until gas_limit is not at least
-    # 180k (without the gas check). Then from 190k to 210k, when adding the gas check,
-    # it reverts with "Vault: gasMulticall too low". After 210k it will succeed as normal
+    # 180k (without the gas check). Then from 190k to 220k, when adding the gas check,
+    # it reverts with "Vault: gasMulticall too low". After 220k it will succeed as normal
     gas_limit = 200000
 
     # Reverted with empty revert string is to catch the invalid opcode
@@ -437,7 +437,7 @@ def test_executeActions_revgas(cf, multicall):
             {"from": cf.DENICE, "gas_limit": gas_limit},
         )
 
-    gas_limit = 210000
+    gas_limit = 220000
 
     tx = cf.vault.executeActions(
         sigData,
@@ -448,7 +448,7 @@ def test_executeActions_revgas(cf, multicall):
 
 
 @given(
-    st_gas_limit=strategy("uint256", min_value=80000, max_value=250000),
+    st_gas_limit=strategy("uint256", min_value=100000, max_value=250000),
 )
 def test_executeActions_gas(cf, multicall, st_gas_limit):
     print("gas limit", st_gas_limit)
@@ -473,7 +473,7 @@ def test_executeActions_gas(cf, multicall, st_gas_limit):
 
     # Exact gas limit that makes the transaction have enough gas to pass the
     # gas check, execute the actions and succeeed.
-    cutoff_gas_limit = 202288
+    cutoff_gas_limit = 219452
 
     # On low gas_limit values it will revert with not enough gas error and other
     # error such as no reason string. Arbitrary 80k under the cutoff gas limit