Merge pull request #76 from datachainlab/audit

Audit

Author: yoshidan

.github/workflows/ci.yaml

@@ -41,3 +41,11 @@ jobs:
         with:
           command: test
           args: --release --features=std --manifest-path light-client/Cargo.toml
+      - uses: actions-rs/cargo@v1
+        name: unit-test-dev-test
+        with:
+          command: test
+          args: --release --features=dev --manifest-path light-client/Cargo.toml --lib test::dev_test
+        env:
+          MINIMUM_TIMESTAMP_SUPPORTED: 1731495592
+          MINIMUM_HEIGHT_SUPPORTED: 100

SPEC.md

@@ -140,7 +140,7 @@ interface ETHHeader {
 }
 ```
 
-## Misbehavior
+## misbehaviour
 
 The `Misbehaviour` type is used for detecting misbehaviour and freezing the client - to prevent further packet flow - if applicable. Parlia client `Misbehaviour` consists of two headers at the same height both of which the light client would have considered valid.
 
@@ -244,15 +244,15 @@ Primary verification according to BEP-126's finality rule involves:
 However, there may be cases where the VoteAttestation cannot directly determine the finality of the submitted header.
 In such cases, a valid descendant header is verified, which is included in the `headers` and can directly confirm its finality through VoteAttestation.
 
-## Misbehavior predicate
+## misbehaviour predicate
 
-The predicate will check if a submission contains evidence of Misbehavior.
+The predicate will check if a submission contains evidence of misbehaviour.
 If there are two different valid headers for the same height, the client will be frozen, preventing any further state updates.
 
 ```typescript
 function submitMisbehaviour(
     clientId: ClientId,
-    misbehaviour: Misbehavior
+    misbehaviour: misbehaviour
 ): ClientState {
     // assert heights are equal
     assert(misbehaviour.header1.getHeight() == misbehaviour.header2.getHeight())

light-client/build.rs

@@ -8,8 +8,24 @@ fn main() {
         writeln!(
             file,
             "pub const BLOCKS_PER_EPOCH: u64 = {};",
-            blocks_per_epoch
+            blocks_per_epoch,
         )
         .unwrap();
     }
+
+    {
+        use std::io::Write;
+        let mut file = std::fs::File::create("src/header/hardfork.rs").unwrap();
+        let minimum_time_stamp_supported =
+            std::env::var("MINIMUM_TIMESTAMP_SUPPORTED").unwrap_or_else(|_| "0".to_string());
+        let minimum_height_supported =
+            std::env::var("MINIMUM_HEIGHT_SUPPORTED").unwrap_or_else(|_| "0".to_string());
+        writeln!(
+            file,
+            "pub const MINIMUM_TIMESTAMP_SUPPORTED: u64 = {};\npub const MINIMUM_HEIGHT_SUPPORTED: u64 = {};",
+            minimum_time_stamp_supported,
+            minimum_height_supported
+        )
+            .unwrap();
+    }
 }

light-client/src/client.rs

@@ -11,6 +11,7 @@ use parlia_ibc_proto::ibc::lightclients::parlia::v1::ClientState as RawClientSta
 use crate::commitment::resolve_account;
 use crate::consensus_state::ConsensusState;
 use crate::errors::Error;
+use crate::header::hardfork::{MINIMUM_HEIGHT_SUPPORTED, MINIMUM_TIMESTAMP_SUPPORTED};
 use crate::header::Header;
 use crate::misbehaviour::Misbehaviour;
 use crate::misc::{new_height, Address, ChainId, Hash};
@@ -97,12 +98,23 @@ impl ClientState {
     }
 
     fn check_header(&self, now: Time, cs: &ConsensusState, header: &Header) -> Result<(), Error> {
+        // Ensure header has supported timestamp
+        let ts = header.timestamp()?;
+
+        #[allow(clippy::absurd_extreme_comparisons)]
+        if ts.as_unix_timestamp_secs() < MINIMUM_TIMESTAMP_SUPPORTED {
+            return Err(Error::UnsupportedMinimumTimestamp(ts));
+        }
+        #[allow(clippy::absurd_extreme_comparisons)]
+        if header.height().revision_height() < MINIMUM_HEIGHT_SUPPORTED {
+            return Err(Error::UnsupportedMinimumHeight(header.height()));
+        }
         // Ensure last consensus state is within the trusting period
         validate_within_trusting_period(
             now,
             self.trusting_period,
             self.max_clock_drift,
-            header.timestamp()?,
+            ts,
             cs.timestamp,
         )?;
 
@@ -159,6 +171,13 @@ impl TryFrom<RawClientState> for ClientState {
 
         let chain_id = ChainId::new(value.chain_id);
 
+        if chain_id.version() != raw_latest_height.revision_number {
+            return Err(Error::UnexpectedLatestHeightRevision(
+                chain_id.version(),
+                raw_latest_height.revision_number,
+            ));
+        }
+
         let latest_height = new_height(
             raw_latest_height.revision_number,
             raw_latest_height.revision_height,
@@ -489,6 +508,19 @@ mod test {
             err => unreachable!("{:?}", err),
         }
 
+        cs.latest_height = Some(Height {
+            revision_number: 1,
+            revision_height: 0,
+        });
+        let err = ClientState::try_from(cs.clone()).unwrap_err();
+        match err {
+            Error::UnexpectedLatestHeightRevision(e1, e2) => {
+                assert_eq!(e1, 0);
+                assert_eq!(e2, 1);
+            }
+            err => unreachable!("{:?}", err),
+        }
+
         cs.latest_height = Some(Height::default());
         let err = ClientState::try_from(cs.clone()).unwrap_err();
         match err {
@@ -673,4 +705,69 @@ mod test {
             panic!("expected error");
         }
     }
+    #[cfg(feature = "dev")]
+    mod dev_test {
+        use crate::client_state::ClientState;
+        use crate::consensus_state::ConsensusState;
+        use crate::errors::Error;
+        use crate::fixture::localnet;
+        use crate::header::eth_headers::ETHHeaders;
+        use crate::header::hardfork::{MINIMUM_HEIGHT_SUPPORTED, MINIMUM_TIMESTAMP_SUPPORTED};
+        use crate::header::Header;
+        use crate::misc::new_timestamp;
+        use parlia_ibc_proto::ibc::core::client::v1::Height;
+
+        #[test]
+        fn test_supported_timestamp() {
+            let header = Header::new(
+                vec![1],
+                ETHHeaders {
+                    target: localnet().previous_epoch_header(),
+                    all: vec![],
+                },
+                Height::default(),
+                localnet().previous_epoch_header().epoch.unwrap(),
+                localnet().epoch_header().epoch.unwrap(),
+            );
+            let cs = ClientState::default();
+            let cons_state = ConsensusState::default();
+            let err = cs
+                .check_header(new_timestamp(0).unwrap(), &cons_state, &header)
+                .unwrap_err();
+            match err {
+                Error::UnsupportedMinimumTimestamp(e1) => {
+                    assert_eq!(e1, header.timestamp().unwrap());
+                }
+                err => unreachable!("{:?}", err),
+            }
+        }
+
+        #[test]
+        fn test_supported_height() {
+            let mut header = Header::new(
+                vec![1],
+                ETHHeaders {
+                    target: localnet().previous_epoch_header(),
+                    all: vec![],
+                },
+                Height::default(),
+                localnet().previous_epoch_header().epoch.unwrap(),
+                localnet().epoch_header().epoch.unwrap(),
+            );
+            header.eth_header_mut().target.timestamp = MINIMUM_TIMESTAMP_SUPPORTED;
+            header.eth_header_mut().target.number = MINIMUM_HEIGHT_SUPPORTED - 1;
+
+            let cs = ClientState::default();
+            let cons_state = ConsensusState::default();
+            let err = cs
+                .check_header(new_timestamp(0).unwrap(), &cons_state, &header)
+                .unwrap_err();
+            match err {
+                Error::UnsupportedMinimumHeight(e1) => {
+                    assert_eq!(e1, header.height());
+                }
+                err => unreachable!("{:?}", err),
+            }
+        }
+    }
 }

light-client/src/client_state.rs

@@ -32,6 +32,7 @@ pub enum Error {
     UnexpectedCommitmentSlot(Vec<u8>),
     ClientFrozen(ClientId),
     UnexpectedProofHeight(Height, Height),
+    UnexpectedRevisionHeight(u64),
 
     // ConsensusState error
     AccountNotFound(Address),
@@ -50,6 +51,8 @@ pub enum Error {
     UnexpectedValidatorsHashSize(Vec<u8>),
 
     // Header error
+    UnsupportedMinimumTimestamp(Time),
+    UnsupportedMinimumHeight(Height),
     MissingPreviousValidators(BlockNumber),
     MissingCurrentValidators(BlockNumber),
     OutOfTrustingPeriod(Time, Time),
@@ -60,6 +63,7 @@ pub enum Error {
     UnexpectedTrustedHeight(BlockNumber, BlockNumber),
     EmptyHeader,
     UnexpectedHeaderRevision(u64, u64),
+    UnexpectedLatestHeightRevision(u64, u64),
     UnexpectedSignature(BlockNumber, signature::Error),
     MissingVanityInExtraData(BlockNumber, usize, usize),
     MissingSignatureInExtraData(BlockNumber, usize, usize),
@@ -80,11 +84,10 @@ pub enum Error {
     MissingTurnLengthInEpochBlock(BlockNumber),
     MissingEpochInfoInEpochBlock(BlockNumber),
     MissingNextValidatorSet(BlockNumber),
-    MissingCurrentValidatorSet(BlockNumber),
     UnexpectedPreviousValidatorsHash(Height, Height, Hash, Hash),
     UnexpectedCurrentValidatorsHash(Height, Height, Hash, Hash),
     InvalidVerifyingHeaderLength(BlockNumber, usize),
-    InsufficientTrustedValidatorsInUntrustedValidators(Hash, usize, usize),
+    InsufficientHonestValidator(Hash, usize, usize),
     MissingValidatorToVerifySeal(BlockNumber),
     MissingValidatorToVerifyVote(BlockNumber),
     UnexpectedNextCheckpointHeader(BlockNumber, BlockNumber),
@@ -94,6 +97,7 @@ pub enum Error {
     UnexpectedDifficultyNoTurn(BlockNumber, u64, usize),
     UnexpectedUntrustedValidatorsHashInEpoch(Height, Height, Hash, Hash),
     UnexpectedCurrentValidatorsHashInEpoch(Height, Height, Hash, Hash),
+    UnexpectedUntrustedValidators(BlockNumber, BlockNumber),
 
     // Vote attestation
     UnexpectedTooManyHeadersToFinalize(BlockNumber, usize),
@@ -163,6 +167,9 @@ impl core::fmt::Display for Error {
             Error::UnexpectedHeaderRevision(e1, e2) => {
                 write!(f, "UnexpectedHeaderRevision: {} {}", e1, e2)
             }
+            Error::UnexpectedLatestHeightRevision(e1, e2) => {
+                write!(f, "UnexpectedLatestHeightRevision: {} {}", e1, e2)
+            }
             Error::UnexpectedSignature(e1, e2) => write!(f, "UnexpectedSignature: {} {}", e1, e2),
             Error::MissingVanityInExtraData(e1, e2, e3) => {
                 write!(f, "MissingVanityInExtraData: {} {} {}", e1, e2, e3)
@@ -309,19 +316,12 @@ impl core::fmt::Display for Error {
             Error::UnexpectedVoteRelation(e1, e2, e3) => {
                 write!(f, "UnexpectedVoteRelation : {} {} {:?}", e1, e2, e3)
             }
-            Error::InsufficientTrustedValidatorsInUntrustedValidators(e1, e2, e3) => {
-                write!(
-                    f,
-                    "InsufficientTrustedValidatorsInUntrustedValidators : {:?} {} {}",
-                    e1, e2, e3
-                )
+            Error::InsufficientHonestValidator(e1, e2, e3) => {
+                write!(f, "InsufficientHonestValidator : {:?} {} {}", e1, e2, e3)
             }
             Error::MissingNextValidatorSet(e1) => {
                 write!(f, "MissingNextValidatorSet : {}", e1)
             }
-            Error::MissingCurrentValidatorSet(e1) => {
-                write!(f, "MissingCurrentValidatorSet : {}", e1)
-            }
             Error::MissingValidatorToVerifySeal(e1) => {
                 write!(f, "MissingValidatorToVerifySeal : {:?}", e1)
             }
@@ -372,6 +372,18 @@ impl core::fmt::Display for Error {
                     e1, e2, e3, e4
                 )
             }
+            Error::UnexpectedUntrustedValidators(e1, e2) => {
+                write!(f, "UnexpectedUntrustedValidators : {} {}", e1, e2)
+            }
+            Error::UnsupportedMinimumTimestamp(e1) => {
+                write!(f, "UnsupportedMinimumTimestamp : {:?}", e1)
+            }
+            Error::UnsupportedMinimumHeight(e1) => {
+                write!(f, "UnsupportedMinimumHeight : {:?}", e1)
+            }
+            Error::UnexpectedRevisionHeight(e1) => {
+                write!(f, "UnexpectedRevisionHeight : {}", e1)
+            }
         }
     }
 }