fix: address tmnt 156

Author: LHerskind

l1-contracts/src/governance/libraries/AddressSnapshotLib.sol

@@ -121,22 +121,19 @@ library AddressSnapshotLib {
     // then update Charlie in addressToCurrentIndex to reflect the new index of 1.
 
     uint224 lastIndex = currentSize - 1;
-    address lastValidator = address(_self.indexToAddressHistory[lastIndex].latest().toUint160());
-
     uint32 key = block.timestamp.toUint32();
 
-    // If we are removing the last item, we cannot swap it with anything
-    // so we append a new address of zero for this timestamp
-    // And since we are removing it, we set the location to 0
-    if (lastIndex == _index) {
-      _self.indexToAddressHistory[_index].push(key, uint224(0));
-    } else {
-      // Otherwise, we swap the last item with the item we are removing
-      // and update the location of the last item
+    // If not removing the last item, swap the value of the last item into the `_index` to remove
+    if (lastIndex != _index) {
+      address lastValidator = address(_self.indexToAddressHistory[lastIndex].latest().toUint160());
+
       _self.addressToCurrentIndex[lastValidator] = Index({exists: true, index: _index.toUint224()});
       _self.indexToAddressHistory[_index].push(key, uint160(lastValidator).toUint224());
     }
 
+    // Then "pop" the last index by setting the value to `address(0)`
+    _self.indexToAddressHistory[lastIndex].push(key, uint224(0));
+
     // Finally, we update the size to reflect the new size of the set.
     _self.size.push(key, (lastIndex).toUint224());
     return true;

l1-contracts/test/governance/governance/address_snapshots/AddressSnapshotsBase.t.sol

@@ -50,6 +50,10 @@ contract AddressSetWrapper {
   function valuesAtTimestamp(uint32 _timestamp) public view returns (address[] memory) {
     return validatorSet.valuesAtTimestamp(_timestamp);
   }
+
+  function unsafeGetRecentAddressFromIndexAtTimestamp(uint256 _index, uint32 _timestamp) public view returns (address) {
+    return validatorSet.unsafeGetRecentAddressFromIndexAtTimestamp(_index, _timestamp);
+  }
 }
 
 contract AddressSnapshotsBase is Test {

l1-contracts/test/governance/governance/address_snapshots/remove.t.sol

@@ -98,6 +98,10 @@ contract AddressSnapshotRemoveTest is AddressSnapshotsBase {
     assertEq(validatorSet.getAddressFromIndexAtTimestamp(0, ts2), address(1));
     assertEq(validatorSet.getAddressFromIndexAtTimestamp(1, ts2), address(3));
 
+    // Ensure that the values read outside the boundary are `address(0)`
+    assertEq(validatorSet.unsafeGetRecentAddressFromIndexAtTimestamp(2, ts2), address(0));
+    assertEq(validatorSet.unsafeGetRecentAddressFromIndexAtTimestamp(2000, ts2), address(0));
+
     assertEq(validatorSet.getAddressFromIndexAtTimestamp(0, ts3), address(3));
 
     vals = validatorSet.valuesAtTimestamp(ts);