Crate

IBC

Summary of Bug

Almost all verification functions in ibc::ics07_tendermint::client_def::TendermintClient are not implemented.

Version

0.6.0

Steps to Reproduce

Call verification functions such as ibc::ics03_connection::handler::verify::verify_proofs with a TendermintClient

Acceptance Criteria

All verification functions are implemented

• [x] informalsystems/ibc-rs-tmp#71
• [ ] verify_client_consensus_state
• [ ] verify_connection_state
• [ ] verify_channel_state
• [ ] verify_client_full_state

For Admin Use

• [X] Not duplicate issue
• [x] Appropriate labels applied
• [x] Appropriate milestone (priority) applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

Summary

Create an ADR for redesigning the modules' API in a way that is more robust and usable for our customers.

Requirements

• async - The API must be usable in an async context.
• Usability - The API must be usable with a variety of execution models and must avoid making assumptions about the host blockchain env.
• Composability - The API must be flexible and extensible by its users. It should additionally provide building blocks that are composable.

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate milestone (priority) applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

In Beefy protocol, there's one data structure, MMR root, "one level higher" than block header. The MMR root of chainA is generated and broadcasted once every N new blocks are generated. After ChainA's light client on ChainB receives a tx containing a new MMR root, it will verify the MMR root, and then verify any of the N headers based on the verified MMR root asynchronously.

From the architectural perspective, is it a good idea to do the 2 asynchronous verifications in the existing function check_header_and_update_state?

Kindly reply if any comments. thank you.

Hello,

I have a question about the usage of the function Writing acknowledgments.

In the life cycle of a packet, there's a functionWriting acknowledgements.

After reading the PR about it, no suggestion on the integration was found.

Is there any suggested place to call the function Writing acknowledgments? How about ibc-rs provides an option to call the function after this line?

Or it's suggested to call the function on the IBC app?

Thank you

Closes: #214

Description

This is my implementation of the abci_tag proco marco implementation of the feeling that there is still a toy Everyone see what other improvements, this implementation is to solve the problems raised in this issue.

PR author checklist:

• [ ] Added changelog entry, using unclog.
• [ ] Added tests.
• [ ] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Closes: #259

Description

PR author checklist:

• [ ] Added changelog entry, using unclog.
• [ ] Added tests.
• [ ] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Summary

## this for codec 
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
scale-info = { version = "2.1.2", default-features = false, features = ["derive"] }
sp-runtime = {  version = "7.0.0", default-features = false }
## this for borsh
borsh = {version = "0.9.3", default-features = false }

example case ClientId:

use serde::{Deserialize, Serialize};

use codec::{Decode, Encode};
use scale_info::TypeInfo;
use sp_runtime::RuntimeDebug;
use borsh::{BorshSerialize, BorshDeserialize};


#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
pub struct ClientId(pub String);

#[cfg(feature = "codec")]
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize, Encode, Decode, RuntimeDebug, TypeInfo)]
pub struct ClientId(pub String);

#[cfg(feature = "borsh")]
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize, BorshSerialize, BorshDeserialize)]
pub struct ClientId(pub String);

Problem Definition

Proposal

Acceptance Criteria

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate milestone (priority) applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

Closes: #164

Description

PR author checklist:

• [ ] Added changelog entry, using unclog.
• [ ] Added tests.
• [ ] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Crate

ibc

Summary

Since we started working on ICS04 handlers(https://github.com/informalsystems/ibc-rs/pull/452) we noticed that there is a recurring need to encapsulate the tuple (PortId, ChannelId) as a type. This tuple is needed in error types:

https://github.com/informalsystems/ibc-rs/blob/d6250014e8d8fcddde651f9e3ceb6be5c93cbeae/modules/src/application/ics20_fungible_token_transfer/error.rs#L19-L20

or as parameters to structures:

https://github.com/informalsystems/ibc-rs/blob/71d9baa6a2eed5a9286f9fe88f1cfacbd51e19d8/modules/src/mock/context.rs#L86-L86

or as parameters to methods:

https://github.com/informalsystems/ibc-rs/blob/71d9baa6a2eed5a9286f9fe88f1cfacbd51e19d8/modules/src/mock/context.rs#L279-L282

So there's ample reasons to introduce a new type for capturing the two.

Proposal

The new type should probably reside in ICS04. Potential names:

pub struct PortChannel(PortId, ChannelId);
pub struct BoundChannel(PortId, ChannelId);
pub struct AuthorizedChannel(PortId, ChannelId);

For Admin Use

• [X] Not duplicate issue
• [x] Appropriate labels applied
• [x] Appropriate milestone (priority) applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

Problem Definition

In order for this library to be used in smart contract and blockchain environments, as i believe is the intention, crypto (sign, hash, verify) operations should be instead delegated to trait implementations. Reason is crypto operations are very expensive and slow in wasm. And for this reason the substrate runtime exposes host functions for hashing:

https://github.com/paritytech/substrate/blob/9461b2de04210c6c193726a745c3ec6552b4ce9f/primitives/io/src/lib.rs#L993-L1033

as well as signature verification and digital signature schemes:

https://github.com/paritytech/substrate/blob/9461b2de04210c6c193726a745c3ec6552b4ce9f/primitives/io/src/lib.rs#L616-L989

Using a trait will allow delegating these operations to pre-compiles (aka host functions)

cc @adizere

Summary

Should Ics20Context implement Module so ChannelKeeper and ChannelReader don't need to be implemented?

Problem Definition

The current ICS20 logic looks a bit like a standalone IBC module, which means it doesn't interop nicely with the Module system proposed by ICS26. It also leads to a lot of extra boilerplate code due to having to implement ChannelKeeper and ChannelReader that could be delegated to a routing module that implements ICS26.

Proposal

Have Ics20Context implement Module, and update the interface of Ics20Context so only the BankKeeper and Ics20Reader functions need to be implemented.

Acceptance Criteria

The changes proposed are made to Ics20Context and the other related traits.

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate milestone (priority) applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

Closes: #33

PR author checklist:

• [X] Added changelog entry, using unclog.
• [ ] Added tests.
• [X] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Closes: https://github.com/cosmos/ibc-rs/issues/293

Description

This PR builds on top of @DaviRain-Su's PR #301 - I was unable to push on their fork so opened this PR.

PR author checklist:

• [ ] Added changelog entry, using unclog.
• [ ] Added tests.
• [ ] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Supersedes: #318 Blocked on: https://github.com/informalsystems/tendermint-rs/issues/1258

We should have the mocks feature not require std since there's no fundamental reason why it should. This involves:

1. Don't use Timestamp::now() which reads time from the OS; implement logic to advance time everytime a "mock now()" is called instead.
2. Use the no_std version of tendermint-testgen when available

Closes: #324

Description

PR author checklist:

• [ ] Added changelog entry, using unclog.
• [ ] Added tests.
• [ ] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Closes: #20

Side effect

Bringing ChannelEnd fields along with including proofs inside the struct of messages, adds #allow(clippy::too_many_arguments)] for MsgChannelOpenTry

PR author checklist:

• [X] Added changelog entry, using unclog.
• [ ] Added tests.
• [X] Linked to GitHub issue.
• [ ] Updated code comments and documentation (e.g., docs/).
• [X] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• [ ] Reviewed Files changed in the GitHub PR explorer.
• [ ] Manually tested (in case integration/unit/mock tests are absent).

Comes out of a comment by @Farhad-Shabani and ensuing sync discussion. We still need to figure out the details, but a main point of confusion was that validate() was in the Context trait; it's not clear from a first look that all the methods in the Context are used in the validate() function. We can also think of different ways to separate the methods into a few different traits if that helps with readability, and doesn't duplicate methods.

Blocked on #221.

Below is the original comment:

Originally posted by @Farhad-Shabani in https://github.com/cosmos/ibc-rs/issues/221#issuecomment-1371403478

Is it right that we have a tailored validation and execution function for each of the messages like MsgCreateClient, MsgUpdateClient, etc.? Couldn’t we use the following pattern (as a general concept) and implement them on each message type?

impl validator<Ctx: RouterContext> {
	pub fn validate(&self, ctx: &mut Ctx) -> Result<(), Error>;
}

impl executor<Ctx: RouterContext> {
	pub fn execute(&self, ctx: &mut Ctx) -> Result<(), Error>;
}

// consider it for MsgCreateClient 
impl validator<MockContext> for MsgCreateClient {...}
impl executor<MockContext> for MsgCreateClient {...}

It looks more modular, maintainable, and extensible if it works! (please direct me to the places if I am missing something. tnx)