From cdb50010bbd823fb2f56b5cf231b65479db8cb56 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Tue, 18 Feb 2020 18:17:46 +0900 Subject: [PATCH 1/4] Add encoding/decoding tests for Connection datagrams --- core/src/ibc/transaction_handler/datagrams.rs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/core/src/ibc/transaction_handler/datagrams.rs b/core/src/ibc/transaction_handler/datagrams.rs index 751f6c4537..c0adcb067e 100644 --- a/core/src/ibc/transaction_handler/datagrams.rs +++ b/core/src/ibc/transaction_handler/datagrams.rs @@ -251,3 +251,49 @@ impl Decodable for Datagram { } } } + +#[cfg(test)] +mod tests { + use super::*; + use rlp::{self, rlp_encode_and_decode_test}; + + #[test] + fn conn_open_init() { + let conn_open_init = Datagram::ConnOpenInit { + identifier: "identifier".to_owned(), + desired_counterparty_connection_identifier: "desired_counterparty_connection_identifier".to_owned(), + counterparty_prefix: "counterparty_prefix".to_owned(), + client_identifier: "client_identifier".to_owned(), + counterparty_client_identifier: "counterparty_client_identifier".to_owned(), + }; + rlp_encode_and_decode_test!(conn_open_init); + } + + #[test] + fn conn_open_try() { + let conn_open_try = Datagram::ConnOpenTry { + desired_identifier: "desired_identifier".to_owned(), + counterparty_connection_identifier: "counterparty_connection_identifier".to_owned(), + counterparty_prefix: "counterparty_prefix".to_owned(), + counterparty_client_identifier: "counterparty_client_identifier".to_owned(), + client_identifier: "client_identifier".to_owned(), + proof_init: b"proof_init".to_vec(), + proof_consensus: b"proof_consensus".to_vec(), + proof_height: 1, + consensus_height: 2, + }; + rlp_encode_and_decode_test!(conn_open_try); + } + + #[test] + fn conn_open_ack() { + let conn_open_ack = Datagram::ConnOpenAck { + identifier: "identifier".to_owned(), + proof_try: b"proof_try".to_vec(), + proof_consensus: b"proof_consensus".to_vec(), + proof_height: 1, + consensus_height: 2, + }; + rlp_encode_and_decode_test!(conn_open_ack); + } +} From f63af7b9e046ad2586664542de1f1befc805b2e7 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Thu, 20 Feb 2020 17:34:22 +0900 Subject: [PATCH 2/4] Do not ignore verification result in the Connection code --- core/src/ibc/connection_03/manager.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/core/src/ibc/connection_03/manager.rs b/core/src/ibc/connection_03/manager.rs index fad03393de..ef4d8a53f9 100644 --- a/core/src/ibc/connection_03/manager.rs +++ b/core/src/ibc/connection_03/manager.rs @@ -95,7 +95,9 @@ impl<'a> Manager<'a> { counterparty_client_identifier: counterparty_client_identifier.clone(), }; - self.verify_connection_state(&connection, proof_height, proof_init, desired_identifier.clone(), &expected); + if !self.verify_connection_state(&connection, proof_height, proof_init, desired_identifier.clone(), &expected) { + return Err(format!("Counterparty chain's connection state verification fail. expected: {:?}", expected)) + } if let Some(previous_connection_end) = self.query(&desired_identifier) { let expected_init = ConnectionEnd { @@ -147,7 +149,14 @@ impl<'a> Manager<'a> { client_identifier: connection.counterparty_client_identifier.clone(), counterparty_client_identifier: connection.client_identifier.clone(), }; - self.verify_connection_state(&connection, proof_height, proof_try, identifier.clone(), &expected_connection); + + if !self.verify_connection_state(&connection, proof_height, proof_try, identifier.clone(), &expected_connection) + { + return Err(format!( + "Counterparty chain's connection state verification fail. expected: {:?}", + expected_connection + )) + } connection.state = ConnectionState::OPEN; let kv_store = self.ctx.get_kv_store_mut(); From e693dd5621053b4d84d06171136a62ed65e07a22 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Tue, 18 Feb 2020 19:35:33 +0900 Subject: [PATCH 3/4] Implement ConnOpenConfirm datagram --- core/src/ibc/connection_03/manager.rs | 33 ++++++++++++++++ core/src/ibc/transaction_handler/datagrams.rs | 39 +++++++++++++++++++ core/src/ibc/transaction_handler/mod.rs | 10 +++++ 3 files changed, 82 insertions(+) diff --git a/core/src/ibc/connection_03/manager.rs b/core/src/ibc/connection_03/manager.rs index ef4d8a53f9..2350b0c660 100644 --- a/core/src/ibc/connection_03/manager.rs +++ b/core/src/ibc/connection_03/manager.rs @@ -166,6 +166,39 @@ impl<'a> Manager<'a> { Ok(()) } + pub fn handle_open_confirm( + &mut self, + identifier: Identifier, + proof_ack: Vec, + proof_height: u64, + ) -> Result<(), String> { + let mut connection = self + .query(&identifier) + .ok_or_else(|| format!("Cannot find connection with the identifier: {}", identifier))?; + if connection.state != ConnectionState::TRYOPEN { + return Err(format!("Invalid connection state expected TRYOPEN but found {:?}", connection.state)) + } + + let expected = ConnectionEnd { + state: ConnectionState::OPEN, + counterparty_connection_identifier: identifier.clone(), + counterparty_prefix: get_commiment_prefix(), + client_identifier: connection.counterparty_client_identifier.clone(), + counterparty_client_identifier: connection.client_identifier.clone(), + }; + + if !self.verify_connection_state(&connection, proof_height, proof_ack, identifier.clone(), &expected) { + return Err(format!("Counterparty chain's connection state verification fail. expected: {:?}", expected)) + } + + connection.state = ConnectionState::OPEN; + let kv_store = self.ctx.get_kv_store_mut(); + let path = connection_path(&identifier); + kv_store.set(&path, &connection.rlp_bytes()); + + Ok(()) + } + fn query(&mut self, identifier: &str) -> Option { let kv_store = self.ctx.get_kv_store(); diff --git a/core/src/ibc/transaction_handler/datagrams.rs b/core/src/ibc/transaction_handler/datagrams.rs index c0adcb067e..626e21148f 100644 --- a/core/src/ibc/transaction_handler/datagrams.rs +++ b/core/src/ibc/transaction_handler/datagrams.rs @@ -24,6 +24,7 @@ enum DatagramTag { ConnOpenInit = 3, ConnOpenTry = 4, ConnOpenAck = 5, + ConnOpenConfirm = 6, } impl Encodable for DatagramTag { @@ -41,6 +42,7 @@ impl Decodable for DatagramTag { 3 => Ok(DatagramTag::ConnOpenInit), 4 => Ok(DatagramTag::ConnOpenTry), 5 => Ok(DatagramTag::ConnOpenAck), + 6 => Ok(DatagramTag::ConnOpenConfirm), _ => Err(DecoderError::Custom("Unexpected DatagramTag Value")), } } @@ -83,6 +85,11 @@ pub enum Datagram { proof_height: u64, consensus_height: u64, }, + ConnOpenConfirm { + identifier: String, + proof_ack: Vec, + proof_height: u64, + }, } impl Encodable for Datagram { @@ -160,6 +167,14 @@ impl Encodable for Datagram { .append(proof_height) .append(consensus_height); } + Datagram::ConnOpenConfirm { + identifier, + proof_ack, + proof_height, + } => { + s.begin_list(4); + s.append(&DatagramTag::ConnOpenConfirm).append(identifier).append(proof_ack).append(proof_height); + } }; } } @@ -248,6 +263,20 @@ impl Decodable for Datagram { consensus_height: rlp.val_at(5)?, }) } + DatagramTag::ConnOpenConfirm => { + let item_count = rlp.item_count()?; + if item_count != 4 { + return Err(DecoderError::RlpInvalidLength { + expected: 4, + got: item_count, + }) + } + Ok(Datagram::ConnOpenConfirm { + identifier: rlp.val_at(1)?, + proof_ack: rlp.val_at(2)?, + proof_height: rlp.val_at(3)?, + }) + } } } } @@ -296,4 +325,14 @@ mod tests { }; rlp_encode_and_decode_test!(conn_open_ack); } + + #[test] + fn conn_open_confirm() { + let conn_open_confirm = Datagram::ConnOpenConfirm { + identifier: "identifier".to_owned(), + proof_ack: b"proof_ack".to_vec(), + proof_height: 1, + }; + rlp_encode_and_decode_test!(conn_open_confirm); + } } diff --git a/core/src/ibc/transaction_handler/mod.rs b/core/src/ibc/transaction_handler/mod.rs index 2b05895a26..cd3fd8e1cb 100644 --- a/core/src/ibc/transaction_handler/mod.rs +++ b/core/src/ibc/transaction_handler/mod.rs @@ -108,6 +108,16 @@ pub fn execute( .handle_open_ack(identifier, proof_try, proof_consensus, proof_height, consensus_height) .map_err(|err| RuntimeError::IBC(format!("ConnOpenAck: {}", err)).into()) } + Datagram::ConnOpenConfirm { + identifier, + proof_ack, + proof_height, + } => { + let mut connection_manager = ibc_connection::Manager::new(&mut context); + connection_manager + .handle_open_confirm(identifier, proof_ack, proof_height) + .map_err(|err| RuntimeError::IBC(format!("ConnOpenConfirm: {}", err)).into()) + } } } From 07edf6401a74db0442f7f01182e71236c2e3036a Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Fri, 21 Feb 2020 14:47:01 +0900 Subject: [PATCH 4/4] Use Bytes type instead of Vec in the IBC code --- core/src/ibc/connection_03/manager.rs | 13 +++++++------ core/src/ibc/context.rs | 4 ++-- core/src/ibc/kv_store/mod.rs | 4 ++-- core/src/ibc/transaction_handler/datagrams.rs | 17 +++++++++-------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/core/src/ibc/connection_03/manager.rs b/core/src/ibc/connection_03/manager.rs index 2350b0c660..74b8171376 100644 --- a/core/src/ibc/connection_03/manager.rs +++ b/core/src/ibc/connection_03/manager.rs @@ -20,6 +20,7 @@ use crate::ibc; use crate::ibc::commitment_23::types::{get_commiment_prefix, CommitmentPrefix, CommitmentProof}; use crate::ibc::connection_03::client_connections_path; use crate::ibc::connection_03::types::{ConnectionEnd, ConnectionIdentifiersInClient, ConnectionState}; +use primitives::Bytes; use rlp::{Encodable, Rlp}; pub struct Manager<'a> { @@ -67,8 +68,8 @@ impl<'a> Manager<'a> { counterparty_prefix: CommitmentPrefix, counterparty_client_identifier: Identifier, client_identifier: Identifier, - proof_init: Vec, - proof_consensus: Vec, + proof_init: Bytes, + proof_consensus: Bytes, proof_height: u64, consensus_height: u64, ) -> Result<(), String> { @@ -123,8 +124,8 @@ impl<'a> Manager<'a> { pub fn handle_open_ack( &mut self, identifier: Identifier, - proof_try: Vec, - proof_consensus: Vec, + proof_try: Bytes, + proof_consensus: Bytes, proof_height: u64, consensus_height: u64, ) -> Result<(), String> { @@ -169,7 +170,7 @@ impl<'a> Manager<'a> { pub fn handle_open_confirm( &mut self, identifier: Identifier, - proof_ack: Vec, + proof_ack: Bytes, proof_height: u64, ) -> Result<(), String> { let mut connection = self @@ -216,7 +217,7 @@ impl<'a> Manager<'a> { &mut self, connection: &ConnectionEnd, proof_height: u64, - proof: Vec, + proof: Bytes, connection_identifier: Identifier, connection_end: &ConnectionEnd, ) -> bool { diff --git a/core/src/ibc/context.rs b/core/src/ibc/context.rs index ffda7544b6..0e38a36a2e 100644 --- a/core/src/ibc/context.rs +++ b/core/src/ibc/context.rs @@ -17,7 +17,7 @@ use super::kv_store::{KVStore, Path}; use ccrypto::blake256; use cstate::{TopLevelState, TopState, TopStateView}; use merkle_trie::proof::{CryptoProof, CryptoProofUnit}; -use primitives::H256; +use primitives::{Bytes, H256}; use rlp::RlpStream; pub trait Context { @@ -70,7 +70,7 @@ impl<'a> TopLevelKVStore<'a> { } impl<'a> KVStore for TopLevelKVStore<'a> { - fn get(&self, path: Path) -> Vec { + fn get(&self, path: Path) -> Bytes { let key = TopLevelKVStore::key(path); self.state.ibc_data(&key).expect("Get key").expect("Data empty").into() } diff --git a/core/src/ibc/kv_store/mod.rs b/core/src/ibc/kv_store/mod.rs index d1daab58f3..01010fd5e1 100644 --- a/core/src/ibc/kv_store/mod.rs +++ b/core/src/ibc/kv_store/mod.rs @@ -15,13 +15,13 @@ // along with this program. If not, see . use merkle_trie::proof::{CryptoProof, CryptoProofUnit}; -use primitives::H256; +use primitives::{Bytes, H256}; pub type Path<'a> = &'a str; // An abstraction of state db that will be provided as a environment for the ICS handler. pub trait KVStore { - fn get(&self, path: Path) -> Vec; + fn get(&self, path: Path) -> Bytes; fn has(&self, path: Path) -> bool; fn set(&mut self, path: Path, value: &[u8]); fn delete(&mut self, path: Path); diff --git a/core/src/ibc/transaction_handler/datagrams.rs b/core/src/ibc/transaction_handler/datagrams.rs index 626e21148f..7addc95ece 100644 --- a/core/src/ibc/transaction_handler/datagrams.rs +++ b/core/src/ibc/transaction_handler/datagrams.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use primitives::Bytes; use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream}; #[repr(u8)] @@ -53,12 +54,12 @@ pub enum Datagram { CreateClient { id: String, kind: u8, - consensus_state: Vec, - data: Vec, + consensus_state: Bytes, + data: Bytes, }, UpdateClient { id: String, - header: Vec, + header: Bytes, }, ConnOpenInit { identifier: String, @@ -73,21 +74,21 @@ pub enum Datagram { counterparty_prefix: String, counterparty_client_identifier: String, client_identifier: String, - proof_init: Vec, - proof_consensus: Vec, + proof_init: Bytes, + proof_consensus: Bytes, proof_height: u64, consensus_height: u64, }, ConnOpenAck { identifier: String, - proof_try: Vec, - proof_consensus: Vec, + proof_try: Bytes, + proof_consensus: Bytes, proof_height: u64, consensus_height: u64, }, ConnOpenConfirm { identifier: String, - proof_ack: Vec, + proof_ack: Bytes, proof_height: u64, }, }