Skip to content
31 changes: 17 additions & 14 deletions core/src/ibc/channel_04/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,17 @@ impl<'a> Manager<'a> {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "Acked-by: Park Juhyung [email protected]" in your commit message? ('Fix connection hop to use the same chain's identifier')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it by mistake. I'll remove it.


// Utility functions
fn check_connection_opened(&self, id: IdentifierSlice) -> Result<Identifier, String> {
fn query_connection(&self, connection_id: IdentifierSlice) -> Result<ConnectionEnd, String> {
let kv_store = self.ctx.get_kv_store();
let connection_end: ConnectionEnd =
rlp::decode(&kv_store.get(&connection_path(&id)).ok_or_else(|| "Connection doesn't exist".to_owned())?)
.expect("Illformed ConnectionEnd stored in the DB");
let connection_end: ConnectionEnd = rlp::decode(
&kv_store.get(&connection_path(&connection_id)).ok_or_else(|| "Connection doesn't exist".to_owned())?,
)
.expect("Illformed ConnectionEnd stored in the DB");
Ok(connection_end)
}

fn check_connection_opened(&self, id: IdentifierSlice) -> Result<Identifier, String> {
let connection_end = self.query_connection(id)?;
if connection_end.state != ConnectionState::OPEN {
return Err("Connection not opened".to_owned())
}
Expand Down Expand Up @@ -203,14 +208,15 @@ impl<'a> Manager<'a> {
}

let client_identifier = self.check_connection_opened(&connection)?;
let connection_end = self.query_connection(&connection)?;

let expected = ChannelEnd {
state: ChannelState::INIT,
ordering: order,
counterparty_port_identifier: DEFAULT_PORT.to_string(),
counterparty_channel_identifier: channel_identifier.clone(),
// Note: the array should be reversed in the future where `connection` becomes an array.
connection_hops: vec![connection.clone()],
connection_hops: vec![connection_end.counterparty_connection_identifier],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that ICS javascript example uses just connection (given as an argument) here. It's ok not to change if you think this is not important.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the word "connection" was used in the channel/manager.rs code for the connection identifier, I used the term "connection_end".

We need a consistent naming rule. What do you prefer? Shall we use connection for the identifier? Shall we use connection for the ConnectionEnd?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was misunderstanding something. The name connection is already used in the function parameter, which represents connectionHops in javascript. And I misunderstood that we use previous here.
About your point, yes you're right but I think we can just stick to our convention (connection for id and connection_end for struct).

version: counterparty_version,
};

Expand Down Expand Up @@ -284,9 +290,8 @@ impl<'a> Manager<'a> {
counterparty_port_identifier: DEFAULT_PORT.to_string(),
counterparty_channel_identifier: channel_identifier.clone(),
connection_hops: {
let mut x = previous.connection_hops.clone();
x.reverse();
x
let connection_end = self.query_connection(&previous.connection_hops[0])?;
vec![connection_end.counterparty_connection_identifier]
},
version: counterparty_version.clone(),
};
Expand Down Expand Up @@ -330,9 +335,8 @@ impl<'a> Manager<'a> {
counterparty_port_identifier: DEFAULT_PORT.to_string(),
counterparty_channel_identifier: channel_identifier.clone(),
connection_hops: {
let mut x = previous.connection_hops.clone();
x.reverse();
x
let connection_end = self.query_connection(&previous.connection_hops[0])?;
vec![connection_end.counterparty_connection_identifier]
},
version: previous.version.clone(),
};
Expand Down Expand Up @@ -392,9 +396,8 @@ impl<'a> Manager<'a> {
counterparty_port_identifier: DEFAULT_PORT.to_string(),
counterparty_channel_identifier: channel_identifier.clone(),
connection_hops: {
let mut x = previous.connection_hops.clone();
x.reverse();
x
let connection_end = self.query_connection(&previous.connection_hops[0])?;
vec![connection_end.counterparty_connection_identifier]
},
version: previous.version.clone(),
};
Expand Down
7 changes: 6 additions & 1 deletion core/src/ibc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,13 @@ impl<'a> KVStore for TopLevelKVStore<'a> {
}

fn insert(&mut self, path: Path, value: &[u8]) -> Option<Bytes> {
// FIXME: the update_ibc_data returns the previous data.
// When the previous data is empty, it should return None.
// Currently it is returning an empty RLP array.
let prev = self.get(path);
let key = TopLevelKVStore::key(path);
self.state.update_ibc_data(&key, value.to_vec()).expect("Set in IBC KVStore").map(Bytes::from)
self.state.update_ibc_data(&key, value.to_vec()).expect("Set in IBC KVStore");
prev
}

fn remove(&mut self, path: Path) -> Option<Bytes> {
Expand Down
51 changes: 38 additions & 13 deletions core/src/ibc/transaction_handler/datagrams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::ibc::channel_04::types::ChannelOrder;
use crate::ibc::channel_04::types::Packet;
use primitives::Bytes;
use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream};

#[repr(u8)]
#[derive(Clone, Copy)]
enum DatagramTag {
#[derive(Clone, Copy, PartialEq, Debug)]
pub enum DatagramTag {
CreateClient = 1,
UpdateClient = 2,
ConnOpenInit = 3,
Expand Down Expand Up @@ -72,15 +73,17 @@ impl Decodable for DatagramTag {
// This is because of RLP macro's weak support.
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanOpenInit {
pub order: u8,
pub tag: DatagramTag,
pub order: ChannelOrder,
pub connection: String,
pub channel_identifier: String,
pub counterparty_channel_identifier: String,
pub version: String,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanOpenTry {
pub order: u8,
pub tag: DatagramTag,
pub order: ChannelOrder,
pub connection: String,
pub channel_identifier: String,
pub counterparty_channel_identifier: String,
Expand All @@ -91,40 +94,47 @@ pub struct ChanOpenTry {
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanOpenAck {
pub tag: DatagramTag,
pub channel_identifier: String,
pub counterparty_version: String,
pub proof_try: Bytes,
pub proof_height: u64,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanOpenConfirm {
pub tag: DatagramTag,
pub channel_identifier: String,
pub proof_ack: Bytes,
pub proof_height: u64,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanCloseInit {
pub tag: DatagramTag,
pub channel_identifier: String,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct ChanCloseConfirm {
pub tag: DatagramTag,
pub channel_identifier: String,
pub proof_init: Bytes,
pub proof_height: u64,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct SendPacket {
pub tag: DatagramTag,
pub packet: Packet,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct RecvPacket {
pub tag: DatagramTag,
pub packet: Packet,
pub proof: Bytes,
pub proof_height: u64,
pub ack: Bytes,
}
#[derive(RlpEncodable, RlpDecodable, PartialEq, Debug)]
pub struct AcknowledgePacket {
pub tag: DatagramTag,
pub packet: Packet,
pub ack: Bytes,
pub proof: Bytes,
Expand Down Expand Up @@ -288,47 +298,47 @@ impl Encodable for Datagram {
Datagram::ChanOpenInit {
raw,
} => {
s.append(&DatagramTag::ChanOpenInit).append(raw);
s.append_single_value(raw);
}
Datagram::ChanOpenTry {
raw,
} => {
s.append(&DatagramTag::ChanOpenTry).append(raw);
s.append_single_value(raw);
}
Datagram::ChanOpenAck {
raw,
} => {
s.append(&DatagramTag::ChanOpenAck).append(raw);
s.append_single_value(raw);
}
Datagram::ChanOpenConfirm {
raw,
} => {
s.append(&DatagramTag::ChanOpenConfirm).append(raw);
s.append_single_value(raw);
}
Datagram::ChanCloseInit {
raw,
} => {
s.append(&DatagramTag::ChanCloseInit).append(raw);
s.append_single_value(raw);
}
Datagram::ChanCloseConfirm {
raw,
} => {
s.append(&DatagramTag::ChanCloseConfirm).append(raw);
s.append_single_value(raw);
}
Datagram::SendPacket {
raw,
} => {
s.append(&DatagramTag::SendPacket).append(raw);
s.append_single_value(raw);
}
Datagram::RecvPacket {
raw,
} => {
s.append(&DatagramTag::RecvPacket).append(raw);
s.append_single_value(raw);
}
Datagram::AcknowledgePacket {
raw,
} => {
s.append(&DatagramTag::AcknowledgePacket).append(raw);
s.append_single_value(raw);
}
};
}
Expand Down Expand Up @@ -517,4 +527,19 @@ mod tests {
};
rlp_encode_and_decode_test!(conn_open_confirm);
}

#[test]
fn chann_open_init() {
let chan_open_init = Datagram::ChanOpenInit {
raw: ChanOpenInit {
tag: DatagramTag::ChanOpenInit,
order: ChannelOrder::ORDERED,
connection: "connection".to_owned(),
channel_identifier: "channel".to_owned(),
counterparty_channel_identifier: "counterparty_channel".to_owned(),
version: "version".to_owned(),
},
};
rlp_encode_and_decode_test!(chan_open_init);
}
}
40 changes: 19 additions & 21 deletions core/src/ibc/transaction_handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,22 @@ use ibc::context as ibc_context;
use rlp::{Decodable, Rlp};

pub fn execute(
bytes: &[u8],
state: &mut TopLevelState,
fee_payer: &Address,
sender_public: &Public,
current_block_number: u64,
) -> StateResult<()> {
let result = execute_inner(bytes, state, fee_payer, sender_public, current_block_number);

if let Err(err) = &result {
cwarn!(IBC, "Executing datagram failed: {:?}", err);
}

result
}

fn execute_inner(
bytes: &[u8],
state: &mut TopLevelState,
_fee_payer: &Address,
Expand All @@ -49,7 +65,7 @@ pub fn execute(
}
}

let result = match datagram {
match datagram {
Datagram::CreateClient {
id,
kind,
Expand Down Expand Up @@ -151,13 +167,7 @@ pub fn execute(
let mut channel_manager = ibc_channel::Manager::new(&mut context);
channel_manager
.chan_open_init(
{
if raw.order == 1 {
ibc::channel_04::types::ChannelOrder::ORDERED
} else {
ibc::channel_04::types::ChannelOrder::UNORDERED
}
},
raw.order,
raw.connection,
raw.channel_identifier,
raw.counterparty_channel_identifier,
Expand All @@ -172,13 +182,7 @@ pub fn execute(
let mut channel_manager = ibc_channel::Manager::new(&mut context);
channel_manager
.chan_open_try(
{
if raw.order == 1 {
ibc::channel_04::types::ChannelOrder::ORDERED
} else {
ibc::channel_04::types::ChannelOrder::UNORDERED
}
},
raw.order,
raw.connection,
raw.channel_identifier,
raw.counterparty_channel_identifier,
Expand Down Expand Up @@ -260,11 +264,5 @@ pub fn execute(
.map_err(|err| RuntimeError::IBC(format!("AcknowledgePacket: {}", err)))?;
Ok(())
}
};

if let Err(err) = &result {
cwarn!(IBC, "Executing datagram failed: {:?}", err);
}

result
}
12 changes: 11 additions & 1 deletion ibc.ts/src/common/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { IBC } from "./foundry/transaction";
import { delay } from "./util";
import Debug from "debug";
import { ClientState } from "./foundry/types";
import { IBCHeader, IBCQueryResult, ConnectionEnd } from "./types";
import { IBCHeader, IBCQueryResult, ConnectionEnd, ChannelEnd } from "./types";

const debug = Debug("common:tx");

Expand Down Expand Up @@ -126,6 +126,16 @@ export class Chain {
blockNumber
]);
}

public async queryChannel(
blockNumber?: number
): Promise<IBCQueryResult<ChannelEnd> | null> {
return this.sdk.rpc.sendRpcRequest("ibc_query_channel_end", [
"DEFAULT_PORT",
this.counterpartyIdentifiers.channel,
blockNumber
]);
}
}

async function waitForTx(sdk: SDK, txHash: H256) {
Expand Down
39 changes: 39 additions & 0 deletions ibc.ts/src/common/datagram/chanOpenAck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const RLP = require("rlp");

export class ChanOpenAckDatagram {
private channelIdentifier: string;
private counterpartyVersion: string;
private proofTry: Buffer;
private proofHeight: number;

public constructor({
channelIdentifier,
counterpartyVersion,
proofTry,
proofHeight
}: {
channelIdentifier: string;
counterpartyVersion: string;
proofTry: Buffer;
proofHeight: number;
}) {
this.channelIdentifier = channelIdentifier;
this.counterpartyVersion = counterpartyVersion;
this.proofTry = proofTry;
this.proofHeight = proofHeight;
}

public rlpBytes(): Buffer {
return RLP.encode(this.toEncodeObject());
}

public toEncodeObject(): any[] {
return [
9,
this.channelIdentifier,
this.counterpartyVersion,
this.proofTry,
this.proofHeight
];
}
}
Loading