Set attestation lastUpdated based on block number

Update Scorer interface

Update tests

Changes after CR

Changes after CR

Changes after CR

Author: Filip-L

abis/SLIOracle.json

@@ -266,41 +266,29 @@
         "internalType": "CommonTypes.FilActorId"
       },
       {
-        "name": "attestation",
+        "name": "slis",
         "type": "tuple",
-        "internalType": "struct SLITypes.Attestation",
+        "internalType": "struct SLITypes.SLIThresholds",
         "components": [
           {
-            "name": "lastUpdate",
-            "type": "uint256",
-            "internalType": "uint256"
+            "name": "retrievabilityPct",
+            "type": "uint8",
+            "internalType": "uint8"
           },
           {
-            "name": "slis",
-            "type": "tuple",
-            "internalType": "struct SLITypes.SLIThresholds",
-            "components": [
-              {
-                "name": "retrievabilityPct",
-                "type": "uint8",
-                "internalType": "uint8"
-              },
-              {
-                "name": "bandwidthMbps",
-                "type": "uint16",
-                "internalType": "uint16"
-              },
-              {
-                "name": "latencyMs",
-                "type": "uint16",
-                "internalType": "uint16"
-              },
-              {
-                "name": "indexingPct",
-                "type": "uint8",
-                "internalType": "uint8"
-              }
-            ]
+            "name": "bandwidthMbps",
+            "type": "uint16",
+            "internalType": "uint16"
+          },
+          {
+            "name": "latencyMs",
+            "type": "uint16",
+            "internalType": "uint16"
+          },
+          {
+            "name": "indexingPct",
+            "type": "uint8",
+            "internalType": "uint8"
           }
         ]
       }
@@ -443,10 +431,16 @@
         "indexed": true,
         "internalType": "CommonTypes.FilActorId"
       },
+      {
+        "name": "lastUpdate",
+        "type": "uint256",
+        "indexed": false,
+        "internalType": "uint256"
+      },
       {
         "name": "slis",
         "type": "tuple",
-        "indexed": true,
+        "indexed": false,
         "internalType": "struct SLITypes.SLIThresholds",
         "components": [
           {
@@ -540,11 +534,21 @@
     "name": "FailedCall",
     "inputs": []
   },
+  {
+    "type": "error",
+    "name": "InvalidAdmin",
+    "inputs": []
+  },
   {
     "type": "error",
     "name": "InvalidInitialization",
     "inputs": []
   },
+  {
+    "type": "error",
+    "name": "InvalidOracle",
+    "inputs": []
+  },
   {
     "type": "error",
     "name": "NotInitializing",

src/SLIOracle.sol

@@ -14,6 +14,16 @@ import {SLITypes} from "./types/SLITypes.sol";
  * @notice
  */
 contract SLIOracle is ISLIOracle, Initializable, AccessControlUpgradeable, UUPSUpgradeable, MulticallUpgradeable {
+    /**
+     * @notice Thrown when an invalid oracle address is provided
+     */
+    error InvalidOracle();
+
+    /**
+     * @notice Thrown when an invalid admin address is provided
+     */
+    error InvalidAdmin();
+
     /**
      * @notice Upgradable role which allows for contract upgrades
      */
@@ -24,12 +34,12 @@ contract SLIOracle is ISLIOracle, Initializable, AccessControlUpgradeable, UUPSU
      */
     bytes32 public constant ORACLE_ROLE = keccak256("ORACLE_ROLE");
 
-    // @custom:storage-location erc7201:slioracle.storage.SLIOracleStorage
+    /// @custom:storage-location erc7201:slioracle.storage.SLIOracleStorage
     struct SLIOracleStorage {
         mapping(CommonTypes.FilActorId provider => SLITypes.Attestation attestation) attestations;
     }
 
-    /// @custom:storage-location erc7201:slioracle.storage.SLIOracleStorage
+    // keccak256(abi.encode(uint256(keccak256("slioracle.storage.SLIOracleStorage")) - 1)) & ~bytes32(uint256(0xff))
     bytes32 private constant SLIORACLE_STORAGE_LOCATION =
         0x0e62b9471a0c1cf5755a696ad58de1a3aec2ce3013fbd703781a8b7b7bd90100;
 
@@ -49,9 +59,12 @@ contract SLIOracle is ISLIOracle, Initializable, AccessControlUpgradeable, UUPSU
     /**
      * @notice Emitted when SLI values are updated for a provider
      * @param provider ID of the provider
+     * @param lastUpdate The block numer of last update
      * @param slis New SLI values
      */
-    event SLIAttestationUpdate(CommonTypes.FilActorId indexed provider, SLITypes.SLIThresholds indexed slis);
+    event SLIAttestationUpdate(
+        CommonTypes.FilActorId indexed provider, uint256 lastUpdate, SLITypes.SLIThresholds slis
+    );
 
     /**
      * @notice Disabled constructor (proxy pattern)
@@ -66,6 +79,8 @@ contract SLIOracle is ISLIOracle, Initializable, AccessControlUpgradeable, UUPSU
      * @param oracle Address that will get ORACLE_ROLE
      */
     function initialize(address admin, address oracle) external initializer {
+        if (admin == address(0)) revert InvalidAdmin();
+        if (oracle == address(0)) revert InvalidOracle();
         __AccessControl_init();
         __Multicall_init();
         _grantRole(DEFAULT_ADMIN_ROLE, admin);
@@ -76,15 +91,15 @@ contract SLIOracle is ISLIOracle, Initializable, AccessControlUpgradeable, UUPSU
     /**
      * @notice Sets SLI values for a provider
      * @param provider ID of the provider
-     * @param attestation New attestation values
+     * @param slis New slis values for a provider
      */
-    function setSLI(CommonTypes.FilActorId provider, SLITypes.Attestation calldata attestation)
+    function setSLI(CommonTypes.FilActorId provider, SLITypes.SLIThresholds calldata slis)
         external
         onlyRole(ORACLE_ROLE)
     {
-        emit SLIAttestationUpdate(provider, attestation.slis);
-        SLIOracleStorage storage $ = s();
-        $.attestations[provider] = attestation;
+        uint256 currentEpoch = block.number;
+        s().attestations[provider] = SLITypes.Attestation({lastUpdate: currentEpoch, slis: slis});
+        emit SLIAttestationUpdate(provider, currentEpoch, slis);
     }
 
     /**

src/SLIScorer.sol

@@ -7,25 +7,35 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/U
 import {CommonTypes} from "filecoin-solidity/v0.8/types/CommonTypes.sol";
 import {SLIOracle} from "./SLIOracle.sol";
 import {SLITypes} from "./types/SLITypes.sol";
+import {ISLIScorer} from "./interfaces/ISLIScorer.sol";
 
 /**
  * @title SLA Registry
  * @notice Upgradeable contract for managing SLA deals with role-based access control
  */
-contract SLIScorer is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
-    /// @notice Thrown when an invalid oracle address is provided
+contract SLIScorer is ISLIScorer, Initializable, AccessControlUpgradeable, UUPSUpgradeable {
+    /**
+     * @notice Thrown when an invalid oracle address is provided
+     */
     error InvalidOracle();
 
-    /// @notice Thrown when an invalid admin address is provided
+    /**
+     *@notice Thrown when an invalid admin address is provided
+     */
     error InvalidAdmin();
 
-    /// @notice Thrown when no attestation exists for the given provider
-    /// @param provider The FilActor ID of the provider without attestation
+    /**
+     *@notice Thrown when no attestation exists for the given provider
+     * @param provider The FilActor ID of the provider without attestation
+     */
     error NoAttestation(CommonTypes.FilActorId provider);
 
-    /// @notice Thrown when an attestation has expired for the given provider
-    /// @param provider The FilActor ID of the provider with expired attestation
+    /**
+     *@notice Thrown when an attestation has expired for the given provider
+     *@param provider The FilActor ID of the provider with expired attestation
+     */
     error AttestationExpired(CommonTypes.FilActorId provider);
+
     /**
      * @notice Upgradable role which allows for contract upgrades
      */
@@ -37,12 +47,12 @@ contract SLIScorer is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
      */
     uint256 private constant EPOCHS_IN_MONTH = 86_400;
 
-    // @custom:storage-location erc7201:sliscorer.storage.SLIScorerStorage
+    /// @custom:storage-location erc7201:sliscorer.storage.SLIScorerStorage
     struct SLIScorerStorage {
         SLIOracle oracle;
     }
 
-    /// @custom:storage-location erc7201:sliscorer.storage.SLIScorerStorage
+    // keccak256(abi.encode(uint256(keccak256("sliscorer.storage.SLIScorerStorage")) - 1)) & ~bytes32(uint256(0xff))
     bytes32 private constant SLISCORER_STORAGE_LOCATION =
         0xfc214f7b8d05a80223ac984f4c5d514cbee885916c0eb499aae1223022938a00;
 
@@ -126,7 +136,7 @@ contract SLIScorer is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
 
         if (required.latencyMs != 0) {
             slasDefined++;
-            if (required.latencyMs <= attestation.slis.latencyMs) slasMet++;
+            if (required.latencyMs >= attestation.slis.latencyMs) slasMet++;
         }
 
         if (required.indexingPct != 0) {

src/interfaces/ISLIOracle.sol

@@ -6,13 +6,20 @@ import {SLITypes} from "../types/SLITypes.sol";
 
 /**
  * @title ISLIOracle
- * @notice Interface for retrieving measured SLI values for storage providers
+ * @notice Interface for managing and retrieving SLI values for storage providers
  */
 interface ISLIOracle {
     /**
-     * @notice Get measured SLI values for a provider
-     * @param provider The storage provider actor ID
-     * @return attestation for a provider
+     * @notice Sets SLI values for a provider
+     * @param provider ID of the provider
+     * @param slis New slis values for a provider
+     */
+    function setSLI(CommonTypes.FilActorId provider, SLITypes.SLIThresholds calldata slis) external;
+
+    /**
+     * @notice Retrieves the SLI values for a provider
+     * @param provider ID of the provider
+     * @return attestation The attestation values for the provider
      */
     function getAttestation(CommonTypes.FilActorId provider)
         external

test/SLIOracle.t.sol

@@ -13,14 +13,12 @@ contract SLIOracleTest is Test {
     SLIOracle public sliOracle;
     address public oracle = address(0x123);
     CommonTypes.FilActorId public provider = CommonTypes.FilActorId.wrap(1000);
-    SLITypes.Attestation public attestation = SLITypes.Attestation({
-        lastUpdate: block.number,
-        slis: SLITypes.SLIThresholds({retrievabilityPct: 80, bandwidthMbps: 500, latencyMs: 200, indexingPct: 90})
-    });
+    SLITypes.SLIThresholds public slis =
+        SLITypes.SLIThresholds({retrievabilityPct: 80, bandwidthMbps: 500, latencyMs: 200, indexingPct: 90});
 
     function setUp() public {
         SLIOracle impl = new SLIOracle();
-        bytes memory initData = abi.encodeWithSignature("initialize(address,address)", address(this), oracle);
+        bytes memory initData = abi.encodeCall(SLIOracle.initialize, (address(this), oracle));
         ERC1967Proxy proxy = new ERC1967Proxy(address(impl), initData);
         sliOracle = SLIOracle(address(proxy));
     }
@@ -49,23 +47,34 @@ contract SLIOracleTest is Test {
 
     function testSLIAttestationEvent() public {
         vm.expectEmit(true, true, false, false);
-        emit SLIOracle.SLIAttestationUpdate(provider, attestation.slis);
+        emit SLIOracle.SLIAttestationUpdate(provider, block.number, slis);
         vm.prank(oracle);
-        sliOracle.setSLI(provider, attestation);
+        sliOracle.setSLI(provider, slis);
     }
 
     function testSetSLIAttestation() public {
         vm.prank(oracle);
-        sliOracle.setSLI(provider, attestation);
+        sliOracle.setSLI(provider, slis);
 
         SLITypes.Attestation memory storedAttestation = sliOracle.getAttestation(provider);
-        // Compare lastUpdate is set correctly in storage
-        assertEq(storedAttestation.lastUpdate, attestation.lastUpdate);
-        // Compare that last update is set to current block number
         assertEq(storedAttestation.lastUpdate, block.number);
-        assertEq(storedAttestation.slis.retrievabilityPct, attestation.slis.retrievabilityPct);
-        assertEq(storedAttestation.slis.latencyMs, attestation.slis.latencyMs);
-        assertEq(storedAttestation.slis.indexingPct, attestation.slis.indexingPct);
-        assertEq(storedAttestation.slis.bandwidthMbps, attestation.slis.bandwidthMbps);
+        assertEq(storedAttestation.slis.retrievabilityPct, slis.retrievabilityPct);
+        assertEq(storedAttestation.slis.latencyMs, slis.latencyMs);
+        assertEq(storedAttestation.slis.indexingPct, slis.indexingPct);
+        assertEq(storedAttestation.slis.bandwidthMbps, slis.bandwidthMbps);
+    }
+
+    function testInitializeRevertInvalidAdmin() public {
+        SLIOracle impl = new SLIOracle();
+        bytes memory initData = abi.encodeCall(SLIOracle.initialize, (address(0), oracle));
+        vm.expectRevert(abi.encodeWithSelector(SLIOracle.InvalidAdmin.selector));
+        new ERC1967Proxy(address(impl), initData);
+    }
+
+    function testInitializeRevertInvalidOracle() public {
+        SLIOracle impl = new SLIOracle();
+        bytes memory initData = abi.encodeCall(SLIOracle.initialize, (address(this), address(0)));
+        vm.expectRevert(abi.encodeWithSelector(SLIOracle.InvalidOracle.selector));
+        new ERC1967Proxy(address(impl), initData);
     }
 }

test/SLIScorer.t.sol

@@ -60,15 +60,15 @@ contract SLIScorerTest is Test {
 
     function testCalculateScoreIsNonZeroForSLI() public {
         vm.prank(client);
-        oracle.setAttestations(1000, 100, 100, 80, 80);
+        oracle.setAttestations(1000, 100, 100, 100, 80);
         vm.roll(1001);
         uint256 score = sliScorer.calculateScore(provider, sliParams);
         assertEq(score, 50);
     }
 
     function testCalculateScore() public {
         vm.prank(client);
-        oracle.setAttestations(1000, 100, 100, 100, 100);
+        oracle.setAttestations(1000, 100, 100, 95, 100);
         vm.roll(1001);
         uint256 score = sliScorer.calculateScore(provider, sliParams);
         assertEq(score, 100);