Skip to content

Commit

Permalink
removed freeze state machine in Ismp host trait (#218)
Browse files Browse the repository at this point in the history
  • Loading branch information
MrishoLukamba authored May 16, 2024
1 parent 08682ba commit 8fad8b6
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 123 deletions.
7 changes: 1 addition & 6 deletions modules/ismp/core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
//! ISMP error definitions

use crate::{
consensus::{ConsensusClientId, ConsensusStateId, StateMachineHeight, StateMachineId},
consensus::{ConsensusClientId, ConsensusStateId, StateMachineHeight},
events::Meta,
};
use alloc::{string::String, vec::Vec};
Expand Down Expand Up @@ -56,11 +56,6 @@ pub enum Error {
/// The consensus client identifier
consensus_state_id: ConsensusStateId,
},
/// The given state machine has been frozen
FrozenStateMachine {
/// The given state machine id
id: StateMachineId,
},
/// The given request was not found
RequestCommitmentNotFound {
/// The request metadata
Expand Down
8 changes: 3 additions & 5 deletions modules/ismp/core/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ where

/// This function does the preliminary checks for a request or response message
/// - It ensures the consensus client is not frozen
/// - It ensures the state machine is not frozen
/// - Checks that the delay period configured for the state machine has elaspsed.
/// - Checks for frozen state machine is deprecated and malicious state machine commitment will be
/// deleted instead
/// - Checks that the delay period configured for the state machine has elapsed.
pub fn validate_state_machine<H>(
host: &H,
proof_height: StateMachineHeight,
Expand All @@ -105,9 +106,6 @@ where
// Ensure client is not frozen
host.is_consensus_client_frozen(proof_height.id.consensus_state_id)?;

// Ensure state machine is not frozen
host.is_state_machine_frozen(proof_height.id)?;

// Ensure delay period has elapsed
if !verify_delay_passed(host, &proof_height)? {
return Err(Error::ChallengePeriodNotElapsed {
Expand Down
4 changes: 0 additions & 4 deletions modules/ismp/core/src/handlers/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ where
let mut last_commitment_height = None;
for commitment_height in commitment_heights.iter() {
let state_height = StateMachineHeight { id, height: commitment_height.height };
// If a state machine is frozen, we skip it
if host.is_state_machine_frozen(id).is_err() {
continue;
}

// Only allow heights greater than latest height
if previous_latest_height > commitment_height.height {
Expand Down
7 changes: 0 additions & 7 deletions modules/ismp/core/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ pub trait IsmpHost: Keccak256 {
/// Should return the current timestamp on the host
fn timestamp(&self) -> Duration;

/// Checks if a state machine is frozen, should return Ok(()) if it isn't
/// or [`Error::FrozenStateMachine`] if it is.
fn is_state_machine_frozen(&self, machine: StateMachineId) -> Result<(), Error>;

/// Checks if a consensus state is frozen should return Ok(()) if it isn't
/// or [`Error::FrozenConsensusClient`] if it is.
fn is_consensus_client_frozen(&self, consensus_state_id: ConsensusStateId)
Expand Down Expand Up @@ -147,9 +143,6 @@ pub trait IsmpHost: Keccak256 {
/// Freeze a consensus state with the given identifier
fn freeze_consensus_client(&self, consensus_state_id: ConsensusStateId) -> Result<(), Error>;

/// Freeze a consensus state with the given identifier
fn freeze_state_machine_client(&self, state_machine: StateMachineId) -> Result<(), Error>;

/// Store latest height for a state machine
fn store_latest_commitment_height(&self, height: StateMachineHeight) -> Result<(), Error>;

Expand Down
1 change: 0 additions & 1 deletion modules/ismp/pallets/pallet/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ impl From<ismp::error::Error> for HandlingError {
HandlingError::StateCommitmentNotFound { height },
IsmpError::FrozenConsensusClient { consensus_state_id } =>
HandlingError::FrozenConsensusClient { id: consensus_state_id },
IsmpError::FrozenStateMachine { id } => HandlingError::FrozenStateMachine { id },
IsmpError::RequestCommitmentNotFound { meta } =>
HandlingError::RequestCommitmentNotFound { meta },
IsmpError::RequestVerificationFailed { meta } =>
Expand Down
18 changes: 2 additions & 16 deletions modules/ismp/pallets/pallet/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use crate::{
dispatcher::{RefundingRouter, RequestMetadata},
utils::{ConsensusClientProvider, ResponseReceipt},
ChallengePeriod, Config, ConsensusClientUpdateTime, ConsensusStateClient, ConsensusStates,
FrozenConsensusClients, FrozenStateMachine, LatestStateMachineHeight, Nonce, Pallet, Responded,
StateCommitments, StateMachineUpdateTime, UnbondingPeriod,
FrozenConsensusClients, LatestStateMachineHeight, Nonce, Pallet, Responded, StateCommitments,
StateMachineUpdateTime, UnbondingPeriod,
};
use alloc::{format, string::ToString};
use codec::{Decode, Encode};
Expand Down Expand Up @@ -90,15 +90,6 @@ impl<T: Config> IsmpHost for Pallet<T> {
<T::TimestampProvider as UnixTime>::now()
}

fn is_state_machine_frozen(&self, machine: StateMachineId) -> Result<(), Error> {
if let Some(frozen) = FrozenStateMachine::<T>::get(machine) {
if frozen {
Err(Error::FrozenStateMachine { id: machine })?
}
}
Ok(())
}

fn is_consensus_client_frozen(&self, client: ConsensusStateId) -> Result<(), Error> {
if FrozenConsensusClients::<T>::get(client) {
Err(Error::FrozenConsensusClient { consensus_state_id: client })?
Expand Down Expand Up @@ -292,11 +283,6 @@ impl<T: Config> IsmpHost for Pallet<T> {
Box::new(RefundingRouter::<T>::new(Box::new(T::Router::default())))
}

fn freeze_state_machine_client(&self, state_machine: StateMachineId) -> Result<(), Error> {
FrozenStateMachine::<T>::insert(state_machine, true);
Ok(())
}

fn store_request_commitment(&self, req: &Request, meta: Vec<u8>) -> Result<(), Error> {
let hash = hash_request::<Self>(req);
let leaf_meta = RequestMetadata::<T>::decode(&mut &*meta)
Expand Down
7 changes: 0 additions & 7 deletions modules/ismp/pallets/pallet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,6 @@ pub mod pallet {
pub type ConsensusStates<T: Config> =
StorageMap<_, Twox64Concat, ConsensusClientId, Vec<u8>, OptionQuery>;

/// Holds a map of state machines to the height at which they've been frozen due to byzantine
/// behaviour
#[pallet::storage]
#[pallet::getter(fn frozen_state_machine)]
pub type FrozenStateMachine<T: Config> =
StorageMap<_, Blake2_128Concat, StateMachineId, bool, OptionQuery>;

/// A mapping of consensus state identifier to it's associated consensus client identifier
#[pallet::storage]
pub type ConsensusStateClient<T: Config> =
Expand Down
32 changes: 0 additions & 32 deletions modules/ismp/testsuite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,38 +211,6 @@ pub fn frozen_consensus_client_check<H: IsmpHost>(host: &H) -> Result<(), &'stat
Ok(())
}

pub fn frozen_state_machine_check<H: IsmpHost>(host: &H) -> Result<(), &'static str> {
let intermediate_state = setup_mock_client(host);
// Set the previous update time
let challenge_period = host.challenge_period(mock_consensus_state_id()).unwrap();
let previous_update_time = host.timestamp() - (challenge_period * 2);
host.store_consensus_update_time(mock_consensus_state_id(), previous_update_time)
.unwrap();
host.store_state_machine_update_time(intermediate_state.height, previous_update_time)
.unwrap();
host.freeze_state_machine_client(intermediate_state.height.id).unwrap();

let post = Post {
source: intermediate_state.height.id.state_id,
dest: host.host_state_machine(),
nonce: 0,
from: vec![0u8; 32],
to: vec![0u8; 32],
timeout_timestamp: 0,
data: vec![0u8; 64],
};
// Request message handling check
let request_message = Message::Request(RequestMessage {
requests: vec![post.clone()],
proof: Proof { height: intermediate_state.height, proof: vec![] },
signer: vec![],
});

let res = handle_incoming_message(host, request_message);
assert!(matches!(res, Err(Error::FrozenStateMachine { .. })));
Ok(())
}

/// Missing state commitments
pub fn missing_state_commitment_check<H: IsmpHost>(host: &H) -> Result<(), &'static str> {
let intermediate_state = setup_mock_client(host);
Expand Down
40 changes: 12 additions & 28 deletions modules/ismp/testsuite/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ pub struct Host {
consensus_states: Rc<RefCell<HashMap<ConsensusStateId, Vec<u8>>>>,
state_commitments: Rc<RefCell<HashMap<StateMachineHeight, StateCommitment>>>,
consensus_update_time: Rc<RefCell<HashMap<ConsensusStateId, Duration>>>,
frozen_state_machines: Rc<RefCell<HashMap<StateMachineId, bool>>>,
frozen_consensus_clients: Rc<RefCell<HashMap<ConsensusStateId, bool>>>,
latest_state_height: Rc<RefCell<HashMap<StateMachineId, u64>>>,
nonce: Rc<RefCell<u64>>,
Expand Down Expand Up @@ -208,16 +207,6 @@ impl IsmpHost for Host {
SystemTime::now().duration_since(UNIX_EPOCH).unwrap()
}

fn is_state_machine_frozen(&self, machine: StateMachineId) -> Result<(), Error> {
let binding = self.frozen_state_machines.borrow();
let val = binding.get(&machine).unwrap_or(&false);
if *val {
Err(Error::FrozenStateMachine { id: machine })?;
}

Ok(())
}

fn is_consensus_client_frozen(&self, _client: ConsensusStateId) -> Result<(), Error> {
let binding = self.frozen_consensus_clients.borrow();
let val = binding.get(&_client).unwrap_or(&false);
Expand Down Expand Up @@ -359,6 +348,18 @@ impl IsmpHost for Host {
Ok(())
}

fn store_request_commitment(&self, req: &Request, _meta: Vec<u8>) -> Result<(), Error> {
let hash = hash_request::<Self>(req);
self.requests.borrow_mut().insert(hash);
Ok(())
}

fn store_response_commitment(&self, res: &PostResponse, _meta: Vec<u8>) -> Result<(), Error> {
let hash = hash_request::<Self>(&Request::Post(res.post.clone()));
self.responses.borrow_mut().insert(hash);
Ok(())
}

fn consensus_clients(&self) -> Vec<Box<dyn ConsensusClient>> {
vec![Box::new(MockClient), Box::new(MockProxyClient)]
}
Expand Down Expand Up @@ -386,23 +387,6 @@ impl IsmpHost for Host {
fn ismp_router(&self) -> Box<dyn IsmpRouter> {
Box::new(MockRouter(self.clone()))
}

fn freeze_state_machine_client(&self, state_machine: StateMachineId) -> Result<(), Error> {
self.frozen_state_machines.borrow_mut().insert(state_machine, true);
Ok(())
}

fn store_request_commitment(&self, req: &Request, _meta: Vec<u8>) -> Result<(), Error> {
let hash = hash_request::<Self>(req);
self.requests.borrow_mut().insert(hash);
Ok(())
}

fn store_response_commitment(&self, res: &PostResponse, _meta: Vec<u8>) -> Result<(), Error> {
let hash = hash_request::<Self>(&Request::Post(res.post.clone()));
self.responses.borrow_mut().insert(hash);
Ok(())
}
}

impl Keccak256 for Host {
Expand Down
12 changes: 3 additions & 9 deletions modules/ismp/testsuite/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use ismp::host::{Ethereum, StateMachine};

use crate::{
check_challenge_period, check_client_expiry, check_request_source_and_destination,
check_response_source, frozen_consensus_client_check, frozen_state_machine_check,
missing_state_commitment_check, mocks::Host, post_request_timeout_check,
post_response_timeout_check, prevent_request_processing_on_proxy_with_known_state_machine,
check_response_source, frozen_consensus_client_check, missing_state_commitment_check,
mocks::Host, post_request_timeout_check, post_response_timeout_check,
prevent_request_processing_on_proxy_with_known_state_machine,
prevent_request_timeout_on_proxy_with_known_state_machine,
prevent_response_timeout_on_proxy_with_known_state_machine, write_outgoing_commitments,
};
Expand Down Expand Up @@ -35,12 +35,6 @@ fn should_reject_messages_for_frozen_consensus_clients() {
frozen_consensus_client_check(&host).unwrap()
}

#[test]
fn should_reject_messages_for_frozen_state_machine_clients() {
let host = Host::default();
frozen_state_machine_check(&host).unwrap()
}

#[test]
fn should_reject_expired_check_clients() {
let host = Host::default();
Expand Down
8 changes: 0 additions & 8 deletions tesseract/evm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ impl ismp::host::IsmpHost for Host {
todo!()
}

fn is_state_machine_frozen(&self, _machine: StateMachineId) -> Result<(), Error> {
todo!()
}

fn is_consensus_client_frozen(
&self,
_consensus_state_id: ConsensusStateId,
Expand Down Expand Up @@ -204,10 +200,6 @@ impl ismp::host::IsmpHost for Host {
todo!()
}

fn freeze_state_machine_client(&self, _state_machine: StateMachineId) -> Result<(), Error> {
todo!()
}

fn store_request_commitment(&self, _req: &Request, _meta: Vec<u8>) -> Result<(), Error> {
todo!()
}
Expand Down

0 comments on commit 8fad8b6

Please sign in to comment.