2118 improving state updates tests related to state history retention period

contract-bindings/src/light_client_mock.rs

@@ -712,6 +712,44 @@ pub mod light_client_mock {
                         state_mutability: ::ethers::core::abi::ethabi::StateMutability::NonPayable,
                     },],
                 ),
+                (
+                    ::std::borrow::ToOwned::to_owned("updateLCStateHistory"),
+                    ::std::vec![::ethers::core::abi::ethabi::Function {
+                        name: ::std::borrow::ToOwned::to_owned("updateLCStateHistory",),
+                        inputs: ::std::vec![
+                            ::ethers::core::abi::ethabi::Param {
+                                name: ::std::borrow::ToOwned::to_owned("blockNumber"),
+                                kind: ::ethers::core::abi::ethabi::ParamType::Uint(64usize),
+                                internal_type: ::core::option::Option::Some(
+                                    ::std::borrow::ToOwned::to_owned("uint64"),
+                                ),
+                            },
+                            ::ethers::core::abi::ethabi::Param {
+                                name: ::std::borrow::ToOwned::to_owned("blockTimestamp"),
+                                kind: ::ethers::core::abi::ethabi::ParamType::Uint(64usize),
+                                internal_type: ::core::option::Option::Some(
+                                    ::std::borrow::ToOwned::to_owned("uint64"),
+                                ),
+                            },
+                            ::ethers::core::abi::ethabi::Param {
+                                name: ::std::borrow::ToOwned::to_owned("state"),
+                                kind: ::ethers::core::abi::ethabi::ParamType::Tuple(::std::vec![
+                                    ::ethers::core::abi::ethabi::ParamType::Uint(64usize),
+                                    ::ethers::core::abi::ethabi::ParamType::Uint(64usize),
+                                    ::ethers::core::abi::ethabi::ParamType::Uint(256usize),
+                                ],),
+                                internal_type: ::core::option::Option::Some(
+                                    ::std::borrow::ToOwned::to_owned(
+                                        "struct LightClient.LightClientState",
+                                    ),
+                                ),
+                            },
+                        ],
+                        outputs: ::std::vec![],
+                        constant: ::core::option::Option::None,
+                        state_mutability: ::ethers::core::abi::ethabi::StateMutability::NonPayable,
+                    },],
+                ),
                 (
                     ::std::borrow::ToOwned::to_owned("upgradeToAndCall"),
                     ::std::vec![::ethers::core::abi::ethabi::Function {
@@ -1295,6 +1333,17 @@ pub mod light_client_mock {
                 .method_hash([242, 253, 227, 139], new_owner)
                 .expect("method not found (this should never happen)")
         }
+        ///Calls the contract's `updateLCStateHistory` (0x7dc792bf) function
+        pub fn update_lc_state_history(
+            &self,
+            block_number: u64,
+            block_timestamp: u64,
+            state: LightClientState,
+        ) -> ::ethers::contract::builders::ContractCall<M, ()> {
+            self.0
+                .method_hash([125, 199, 146, 191], (block_number, block_timestamp, state))
+                .expect("method not found (this should never happen)")
+        }
         ///Calls the contract's `upgradeToAndCall` (0x4f1ef286) function
         pub fn upgrade_to_and_call(
             &self,
@@ -2782,6 +2831,28 @@ pub mod light_client_mock {
     pub struct TransferOwnershipCall {
         pub new_owner: ::ethers::core::types::Address,
     }
+    ///Container type for all input parameters for the `updateLCStateHistory` function with signature `updateLCStateHistory(uint64,uint64,(uint64,uint64,uint256))` and selector `0x7dc792bf`
+    #[derive(
+        Clone,
+        ::ethers::contract::EthCall,
+        ::ethers::contract::EthDisplay,
+        serde::Serialize,
+        serde::Deserialize,
+        Default,
+        Debug,
+        PartialEq,
+        Eq,
+        Hash,
+    )]
+    #[ethcall(
+        name = "updateLCStateHistory",
+        abi = "updateLCStateHistory(uint64,uint64,(uint64,uint64,uint256))"
+    )]
+    pub struct UpdateLCStateHistoryCall {
+        pub block_number: u64,
+        pub block_timestamp: u64,
+        pub state: LightClientState,
+    }
     ///Container type for all input parameters for the `upgradeToAndCall` function with signature `upgradeToAndCall(address,bytes)` and selector `0x4f1ef286`
     #[derive(
         Clone,
@@ -2829,6 +2900,7 @@ pub mod light_client_mock {
         StateHistoryFirstIndex(StateHistoryFirstIndexCall),
         StateHistoryRetentionPeriod(StateHistoryRetentionPeriodCall),
         TransferOwnership(TransferOwnershipCall),
+        UpdateLCStateHistory(UpdateLCStateHistoryCall),
         UpgradeToAndCall(UpgradeToAndCallCall),
     }
     impl ::ethers::core::abi::AbiDecode for LightClientMockCalls {
@@ -2957,6 +3029,11 @@ pub mod light_client_mock {
             {
                 return Ok(Self::TransferOwnership(decoded));
             }
+            if let Ok(decoded) =
+                <UpdateLCStateHistoryCall as ::ethers::core::abi::AbiDecode>::decode(data)
+            {
+                return Ok(Self::UpdateLCStateHistory(decoded));
+            }
             if let Ok(decoded) =
                 <UpgradeToAndCallCall as ::ethers::core::abi::AbiDecode>::decode(data)
             {
@@ -3022,6 +3099,9 @@ pub mod light_client_mock {
                     ::ethers::core::abi::AbiEncode::encode(element)
                 }
                 Self::TransferOwnership(element) => ::ethers::core::abi::AbiEncode::encode(element),
+                Self::UpdateLCStateHistory(element) => {
+                    ::ethers::core::abi::AbiEncode::encode(element)
+                }
                 Self::UpgradeToAndCall(element) => ::ethers::core::abi::AbiEncode::encode(element),
             }
         }
@@ -3059,6 +3139,7 @@ pub mod light_client_mock {
                 Self::StateHistoryFirstIndex(element) => ::core::fmt::Display::fmt(element, f),
                 Self::StateHistoryRetentionPeriod(element) => ::core::fmt::Display::fmt(element, f),
                 Self::TransferOwnership(element) => ::core::fmt::Display::fmt(element, f),
+                Self::UpdateLCStateHistory(element) => ::core::fmt::Display::fmt(element, f),
                 Self::UpgradeToAndCall(element) => ::core::fmt::Display::fmt(element, f),
             }
         }
@@ -3193,6 +3274,11 @@ pub mod light_client_mock {
             Self::TransferOwnership(value)
         }
     }
+    impl ::core::convert::From<UpdateLCStateHistoryCall> for LightClientMockCalls {
+        fn from(value: UpdateLCStateHistoryCall) -> Self {
+            Self::UpdateLCStateHistory(value)
+        }
+    }
     impl ::core::convert::From<UpgradeToAndCallCall> for LightClientMockCalls {
         fn from(value: UpgradeToAndCallCall) -> Self {
             Self::UpgradeToAndCall(value)

contracts/src/LightClient.sol

@@ -305,30 +305,30 @@ contract LightClient is Initializable, OwnableUpgradeable, UUPSUpgradeable {
         }
     }
 
-    /// @notice updates the stateHistoryCommitments array each time a new
-    /// finalized state is added to the LightClient contract.
-    /// Ensures that the time difference between the most recent and oldest
-    /// elements in this array does not exceed the stateHistoryRetentionPeriod (in seconds).
+    /// @notice Updates the `stateHistoryCommitments` array when a new finalized state is added
+    /// and ensures that the array does not retain states older than the
+    /// `stateHistoryRetentionPeriod`.
     /// @dev the block timestamp is used to determine if the stateHistoryCommitments array
-    /// should be pruned, based on the stateHistoryRetentionPeriod.
+    /// should be pruned, based on the stateHistoryRetentionPeriod (seconds).
     /// @dev a FIFO approach is used to delete elements from the start of the array,
-    /// ensuring that only the most recent states are retained within the
-    /// stateHistoryRetentionPeriod
+    /// ensuring that only the most recent states that only recent states are kept within
+    /// the retention window.
     /// @dev the `delete` method does not reduce the array length but resets the value at the
-    /// specified index to zero.
-    /// the stateHistoryFirstIndex variable acts as an offset to indicate the starting point for
-    /// reading the array,
-    /// since the length of the array is not reduced even after deletion.
+    /// specified index to zero. the stateHistoryFirstIndex variable acts as an offset to indicate
+    /// the starting point for reading the array, since the length of the array is not reduced
+    /// even after deletion.
+    /// @param blockNumber The block number of the new finalized state.
+    /// @param blockTimestamp The block timestamp used to check the retention period.
+    /// @param state The new `LightClientState` being added to the array.
     function updateStateHistory(
         uint64 blockNumber,
         uint64 blockTimestamp,
         LightClientState memory state
     ) internal {
         if (
             stateHistoryCommitments.length != 0
-                && stateHistoryCommitments[stateHistoryCommitments.length - 1].l1BlockTimestamp
-                    - stateHistoryCommitments[stateHistoryFirstIndex].l1BlockTimestamp
-                    >= stateHistoryRetentionPeriod
+                && blockTimestamp - stateHistoryCommitments[stateHistoryFirstIndex].l1BlockTimestamp
+                    > stateHistoryRetentionPeriod
         ) {
             // The stateHistoryCommitments array has reached the maximum retention period
             // delete the oldest (first) non-empty element to maintain the FIFO structure.
@@ -370,23 +370,23 @@ contract LightClient is Initializable, OwnableUpgradeable, UUPSUpgradeable {
 
         uint256 prevBlock;
         bool prevUpdateFound;
-
         uint256 i = updatesCount - 1;
         while (!prevUpdateFound) {
-            if (stateHistoryCommitments[i].l1BlockHeight <= blockNumber) {
-                prevUpdateFound = true;
-                prevBlock = stateHistoryCommitments[i].l1BlockHeight;
-            }
-
             // We don't consider the lag time for the first two updates
             if (i < 2) {
                 break;
             }
 
             // We've reached the first recorded block
-            if (i == stateHistoryFirstIndex) {
+            if (i < stateHistoryFirstIndex) {
                 break;
             }
+
+            if (stateHistoryCommitments[i].l1BlockHeight <= blockNumber) {
+                prevUpdateFound = true;
+                prevBlock = stateHistoryCommitments[i].l1BlockHeight;
+            }
+
             i--;
         }
 
@@ -395,7 +395,6 @@ contract LightClient is Initializable, OwnableUpgradeable, UUPSUpgradeable {
         if (!prevUpdateFound) {
             revert InsufficientSnapshotHistory();
         }
-
         return blockNumber - prevBlock > blockThreshold;
     }