imp(contracts): added guardrails to SP1ICS07Tendermint.sol

contracts/src/SP1ICS07Tendermint.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.13;
 
 import {ICS07Tendermint} from "ibc-lite-shared/ics07-tendermint/ICS07Tendermint.sol";
 import {ISP1Verifier} from "@sp1-contracts/ISP1Verifier.sol";
+import "forge-std/console.sol";
 
 /// @title SP1ICS07Tendermint
 /// @author srdtrk
@@ -59,19 +60,48 @@ contract SP1ICS07Tendermint {
             clientState.is_frozen == false,
             "SP1ICS07Tendermint: client is frozen"
         );
+        // TODO: Make sure this timestamp check is correct
         require(
             block.timestamp * 1e9 <= output.env.now + ALLOWED_SP1_CLOCK_DRIFT,
             "SP1ICS07Tendermint: invalid timestamp"
         );
+        require(
+            keccak256(bytes(output.env.chain_id)) ==
+                keccak256(bytes(clientState.chain_id)),
+            "SP1ICS07Tendermint: chain ID mismatch"
+        );
+        require(
+            output.env.trust_threshold.numerator ==
+                clientState.trust_level.numerator &&
+                output.env.trust_threshold.denominator ==
+                clientState.trust_level.denominator,
+            "SP1ICS07Tendermint: trust threshold mismatch"
+        );
+        require(
+            output.env.trusting_period == clientState.trusting_period,
+            "SP1ICS07Tendermint: trusting period mismatch"
+        );
+        require(
+            output.env.trusting_period == clientState.unbonding_period,
+            "SP1ICS07Tendermint: unbonding period mismatch"
+        );
+        require(
+            keccak256(
+                abi.encode(
+                    consensusStates[output.trusted_height.revision_height]
+                )
+            ) == keccak256(abi.encode(output.trusted_consensus_state)),
+            "SP1ICS07Tendermint: trusted consensus state mismatch"
+        );
+        // TODO: Make sure that we don't need more checks here.
 
-        // TODO: verify that the client state and the saved consensus state match the public values.
-        // More checks need to be made here get the trusted consensus clientState and etc
-
+        // TODO: Make sure that other checks have been made in the proof verification
+        // such as the consensus state not being outside the trusting period.
         verifier.verifyProof(ics07UpdateClientProgramVkey, publicValues, proof);
 
         // adding the new consensus state to the mapping
         clientState.latest_height = output.new_height;
-        consensusStates[output.new_consensus_state.timestamp] = output
+        consensusStates[output.new_height.revision_height] = output
             .new_consensus_state;
     }
 

contracts/test/SP1ICS07Tendermint.t.sol

@@ -4,12 +4,13 @@ pragma solidity ^0.8.13;
 import "forge-std/console.sol";
 import {Test} from "forge-std/Test.sol";
 import {stdJson} from "forge-std/StdJson.sol";
+import {stdError} from "forge-std/StdError.sol";
 import {ICS07Tendermint} from "ibc-lite-shared/ics07-tendermint/ICS07Tendermint.sol";
 import {SP1ICS07Tendermint} from "../src/SP1ICS07Tendermint.sol";
 import {SP1Verifier} from "@sp1-contracts/SP1Verifier.sol";
 import {SP1MockVerifier} from "@sp1-contracts/SP1MockVerifier.sol";
 
-struct SP1TendermintFixtureJson {
+struct SP1ICS07TendermintFixtureJson {
     bytes trustedClientState;
     bytes trustedConsensusState;
     bytes targetConsensusState;
@@ -19,7 +20,7 @@ struct SP1TendermintFixtureJson {
     bytes proof;
 }
 
-contract SP1TendermintTest is Test {
+contract SP1ICS07TendermintTest is Test {
     using stdJson for string;
 
     // TODO: Test non-mock ics07Tendermint, once we have a way to generate the fixture.
@@ -38,7 +39,7 @@ contract SP1TendermintTest is Test {
         );
         */
 
-        SP1TendermintFixtureJson memory mockFixture = loadFixture(
+        SP1ICS07TendermintFixtureJson memory mockFixture = loadFixture(
             "mock_fixture.json"
         );
         SP1MockVerifier mockVerifier = new SP1MockVerifier();
@@ -48,11 +49,39 @@ contract SP1TendermintTest is Test {
             mockFixture.trustedClientState,
             mockFixture.trustedConsensusState
         );
+
+        (
+            string memory chain_id,
+            ICS07Tendermint.TrustThreshold memory trust_level,
+            ICS07Tendermint.Height memory latest_height,
+            uint64 trusting_period,
+            uint64 unbonding_period,
+            bool is_frozen
+        ) = mockIcs07Tendermint.clientState();
+
+        assert(keccak256(bytes(chain_id)) == keccak256(bytes("mocha-4")));
+        assert(trust_level.numerator == 1);
+        assert(trust_level.denominator == 3);
+        assert(latest_height.revision_number == 4);
+        assert(latest_height.revision_height == 2110658);
+        assert(trusting_period == 1_209_600_000_000_000);
+        assert(unbonding_period == 1_209_600_000_000_000);
+        assert(is_frozen == false);
+
+        (
+            uint64 timestamp,
+            bytes memory root,
+            bytes memory next_validators_hash
+        ) = mockIcs07Tendermint.consensusStates(2110658);
+
+        assert(timestamp > 0);
+        assert(root.length > 0);
+        assert(next_validators_hash.length > 0);
     }
 
     function loadFixture(
         string memory fileName
-    ) public view returns (SP1TendermintFixtureJson memory) {
+    ) public view returns (SP1ICS07TendermintFixtureJson memory) {
         string memory root = vm.projectRoot();
         string memory path = string.concat(root, "/fixtures/", fileName);
         string memory json = vm.readFile(path);
@@ -68,25 +97,31 @@ contract SP1TendermintTest is Test {
         bytes memory publicValues = json.readBytes(".publicValues");
         bytes memory proof = json.readBytes(".proof");
 
-        SP1TendermintFixtureJson memory fixture = SP1TendermintFixtureJson({
-            trustedClientState: trustedClientState,
-            trustedConsensusState: trustedConsensusState,
-            targetConsensusState: targetConsensusState,
-            targetHeight: targetHeight,
-            vkey: vkey,
-            publicValues: publicValues,
-            proof: proof
-        });
+        SP1ICS07TendermintFixtureJson
+            memory fixture = SP1ICS07TendermintFixtureJson({
+                trustedClientState: trustedClientState,
+                trustedConsensusState: trustedConsensusState,
+                targetConsensusState: targetConsensusState,
+                targetHeight: targetHeight,
+                vkey: vkey,
+                publicValues: publicValues,
+                proof: proof
+            });
 
         return fixture;
     }
 
     // Confirm that submitting an empty proof passes the mock verifier.
     function test_ValidMockTendermint() public {
-        SP1TendermintFixtureJson memory fixture = loadFixture(
+        SP1ICS07TendermintFixtureJson memory fixture = loadFixture(
             "mock_fixture.json"
         );
 
+        mockIcs07Tendermint.verifyIcs07UpdateClientProof(
+            bytes(""),
+            fixture.publicValues
+        );
+
         (
             string memory chain_id,
             ICS07Tendermint.TrustThreshold memory trust_level,
@@ -100,30 +135,33 @@ contract SP1TendermintTest is Test {
         assert(trust_level.numerator == 1);
         assert(trust_level.denominator == 3);
         assert(latest_height.revision_number == 4);
-        assert(latest_height.revision_height == 2110658);
+        assert(latest_height.revision_height == 2110668);
         assert(trusting_period == 1_209_600_000_000_000);
         assert(unbonding_period == 1_209_600_000_000_000);
         assert(is_frozen == false);
 
-        mockIcs07Tendermint.verifyIcs07UpdateClientProof(
-            bytes(""),
-            fixture.publicValues
-        );
+        (
+            uint64 timestamp,
+            bytes memory root,
+            bytes memory next_validators_hash
+        ) = mockIcs07Tendermint.consensusStates(2110668);
+
+        assert(timestamp > 0);
+        assert(root.length > 0);
+        assert(next_validators_hash.length > 0);
     }
 
     // Confirm that submitting a non-empty proof with the mock verifier fails. This typically
     // indicates that the user has passed in a real proof to the mock verifier.
-    function testFail_Invalid_MockTendermint() public {
-        SP1TendermintFixtureJson memory fixture = loadFixture(
+    function test_Invalid_MockTendermint() public {
+        SP1ICS07TendermintFixtureJson memory fixture = loadFixture(
             "mock_fixture.json"
         );
 
+        vm.expectRevert();
         mockIcs07Tendermint.verifyIcs07UpdateClientProof(
             bytes("aa"),
             fixture.publicValues
         );
-
-        // assert(mockIcs07Tendermint.latestHeader() == fixture.targetHeaderHash);
-        // assert(mockIcs07Tendermint.latestHeight() == fixture.targetHeight);
     }
 }