Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBC implementation idea #27

Closed
hashedone opened this issue Mar 9, 2023 · 5 comments
Closed

IBC implementation idea #27

hashedone opened this issue Mar 9, 2023 · 5 comments
Labels
IBC IBC support
Milestone

Comments

@hashedone
Copy link
Contributor

Implementing entry points

The first step is to implement all IBC-related entry points for contracts. What has to be done to add them to Contract trait and update the ContractWrapper. I would prefer the #26 first, but technically ordering is less relevant. Entry points I have written down to add are:

#[entry_point]
fn ibc_channel_open(
	deps: DepsMut,
	env: Env,
	msg: IbcChannelOpenMsg, // From `cosmwasm_std`
) -> Result<(), ContractError> {
	todo!()
}

#[entry_point]
fn ibc_channel_connect(
	deps: DepsMut,
	env: Env,
	msg: IbcChannelConnectMsg, // From `cosmwasm_std`
) -> Result<IbcBasicResponse, ContractError> {
	todo!()
}

#[entry_point]
fn ibc_channel_close(
	deps: DepsMut,
	env: Env,
	channel: IbcChannelCloseMsg, // From `cosmwasm_std`
)

#[entry_point]
fn ibc_packet_receive(
	deps: DepsMut,
	env: Env,
	msg: IbcPacketReceiveMsg, // From `cosmwasm_std`
) -> Result<IbcReceiveResponse, ()> {
	// This entry point shuold never return error - instead,
	// it should set the `ack` on the response to failure.
	todo!()
}

#[entry_point]
fn ibc_packet_ack(
	deps: DepsMut,
	env: Env,
	msg: IbcPacketAckMsg, // From `cosmwasm_std`
) -> Result<IbcBasicResponse, ContractError> {
	todo!()
}

#[entry_point]
fn ibc_packet_timeout(
	deps: DepsMut,
	env: Env,
	msg: IbcPacketTimeoutMsg, // From `cosmwasm_std`
)

Creating IBC Keeper

The second step is to create an IBC Keeper. Its purpose is to handle IBC-related messages:

pub enum IbcMsg {
    Transfer {
        channel_id: String,
        to_address: String,
        amount: Coin,
        timeout: IbcTimeout,
    },
    SendPacket {
        channel_id: String,
        data: Binary,
        timeout: IbcTimeout,
    },
    CloseChannel {
        channel_id: String,
    },
}

#[non_exhaustive]
pub enum IbcQuery {
    PortId {},
    ListChannels {
        port_id: Option,
    },
    Channel {
        channel_id: String,
        port_id: Option<String>,
    },
}

I feel like all the messages should behave similarly: there should be some internal queue in the keeper, which contains messages to be relayed. The message type would look like this:

enum IbcRelayable {
  Packet {
    data: Binary,
    // We have `channel_id` from the `Transfer` msg, `port` is to be deduced from to which one is the contract
    // bound. I am pretty sure this deduction should be made by `WasmKeeper` when parsing `Response`,
    // or `WasmKeeper` has to pass the contract address to `IbcKeeper` so it can do it.
    // I might confuse the `src` and `dst` sides now, so it might be a case that `src` as actually on the queue and `dst` is
    // added by relayer.
    dst: IbcEndpoint,
    timeout: IbcTimeout,
  },
  CloseChannel {
    channel_id: String,
  },
  Ack {
    acknowledgement: IbcAcknowledgment,
    original_packet: IbcPacket,
  }
}

Making the App into Rc<RefCell<App>>

I don't like solutions like Rc<RefCell<T>>, but at this point, most arguments against it don't apply (we are on testing env, and leaks or performance does not matter). The problem is that the app has to be continually passed all over the place while it could be just fixed into some types using it later. We already use this approach in the Sylvia framework, but here it is even more important - you will need to relay messages at some point, and you don't want to pass app instances every time, as it puts you in the risk of messing up the order of app in the relayer (the whole idea is to have single app per chain simulated).

The easiest way to go here is to make an App into AppImpl, and then create an additional thin wrapper:

struct App(AppImpl);

Having all the inner functions of AppImpl, but taking self always by the shared borrow. Then App can be copied all over the place cheaply, and all the API are kept (as they got only more generic).

Relayer

Next in the queue is the IbcRelayer type. It would be something like that (probably with more data I miss here). As we want to allow later maybe redefinition of Relayer for custom cases, I will start with defining the trait:

trait Relayer {
  // We need it - it will be used by the `App` later
  fn relay(&mut self, msgs: &mut Vec<IbcRelayable);
}

Then we create a relayer type:

struct Relayer<App1, App2> {
  // Both NOT public! We don't want to mess with `A`/`B` all the time
  chain_a: App1,
  chain_b: App2,
}

So relayer is just keeping some two chains for future use. We want to make it useful. First, let things about creation:

#[derive(Clone, Copy, Debug)]
struct ChainHandle(usize);

impl<App1, App2> Relayer<App1, App2>
where:
  // We define this `RelayableApp` to ensure that `IbcT` in the `App` is type exposing everything we need - for eg.
  // its our particular type, or it implements some traits. Also, if we want anything more for relaying, we can
  // put it in the trait.
  App1: RelayableApp,
  App2: RelayableApp,
{
  fn new(chain_a: App1, chain_b: App2) -> (Self, ChainHandle, ChainHandle) {
    let relayer = Self {
      chain_a,
      chain_b
    };
    
    (relayer, ChainHandle(0), ChainHandle(0))
  }
}

Here is a nice trick to use. What drives me crazy in ts-relayer is remembering which chain is which. Now I can do the following:

let (relayer, wasmh, osmoh) = Relayer::new();

All the types of structures returned by the relayer would have a function like fn chain(&self, ChainHandle) -> ChainSpecificData), so I pass the well-named handle here.

Some essential functions to be callable on the relayer are, for example, relay_all(&self) - which relates messages until there is nothing to relay, relay_once(&self, ChainHandle), which relays all messages pending on the passed chain, or even relay_single(&self, ChainHandle) - I am committing the return type as it is just a structure describing all events happened while relaying. Also, create_channel would create a channel on a given port.

One other thing I have in mind is to add function(s) like relay_with(&self, impl Fn<(ChainHandle, IbcRelayable)> -> RelayMethod), which allows controlling the relayer with the RelayMethod type:

enum RelayMethod {
  Relay, // Message would be relayed.
  Keep, // Message will not be relayed but left in the queue to dispatch it later.
  Drop, // Message will not be relayed, and it will be removed from the queue - for testing timing out.
  Fail(String), // Message will not be relayed; it would send `ack` with the given failure error and will be removed from the queue - for testing unusual errors.
}

Access to the queue

We could expose the queue for everyone, delivering a function on an App:

impl AppImpl<...> {
  // This actually could (probably should) be a part of `RelayableApp` trait - but it is a detail
  fn ibc_queue(&mut self) -> &mut Vec<...>
  where
    Self: RelayableApp
  {
    todo!()
  }
}

However, I wouldn't say I like it as it exposes some internals too easily. Instead, I want to provide a function like this:

impl AppImpl<...> {
  // Again - in the `RelayableApp`, probably
  fn relay<R>(&mut self, relayer: &mut R) -> &mut Vec<_>
  where
    R: Relayer
  {
    relayer.relay(self.router.ibc.queue())
  }
}

Now in all the relaying function (like relay_all), the relayer will call the proper app.relay(self) to get access to underlying functions. Another thing the relayer has to do is to execute the entry points, and it leads to...

Updating WasmKeeper

I am not 100% about that, but it is how it feels natural to me. WasmKeeper is an entity which keeps the code of the contract. Therefore it is the best one to execute its entry points - including IBC ones. I will add flow to it, which is just like execute, but it would be an execute_ibc, and instead of WasmMsg, it would take our newly crafted type like:

enum IbcCallMsg {
  ChannelConnect { ... },
  ChannelOpen { ... },
  ChannelClose { ... },
  PacketReceive { ... },
  PacketAck { ... },
  PacketTimeout { ... },
}

Now, it should be possible to schedule those messages from app level, so there is an execution loop performed on Wasm.

Built-in IBC capabilities

Note that the whole relayer works on the RelayableApp trait providing access to the relay queue. That means that the App itself could have some additional functionalities allowing it to perform some "building" actions - either on its IbcT module or by some other functions which enqueue messages (additionally do whatever they want to do). One can also create a custom RelayableApp, which he uses only to push IBC messages on the queue, without any bank or wasm capabilities.

Timing out

Timing out should be handled without involving the relayer (so it is guaranteed to happen). The best would be to do it automatically when update_block is called. Unfortunately, it leads to an issue that IbcT is generic, so it might not contain the required functionality. We could trait-bound it, but it would be very backwards-incompatible, and I know there are chains already implementing IbcT, so they would be strongly affected by this.

Instead, I would add the ibc_timeout function on App, enabled (by trait-bounds) only when our App implements RelayableApp. It would query IbcKeeper for timed-out packages waiting (it would immediately drop them), and then App would send proper IbcCallMsg::PacketTimeout { ... } messages.

Ports creation

The easiest thing here is to provide a function like bind_ibc(port_id: &str, contract: &str), which binds the contract to the given port on the chain (creates a new port if it does not exist).

Events, attributes, all the things

I think the most challenging part will be to emit all the events and attributes properly. On the one hand - it is not this relevant for testing purposes; on the other hand - one might find it suitable. It would require tedious work to ensure that the IbcKeeper and WasmKeeper are emitting all the events specific to IBC-related messages.

@hashedone hashedone added enhancement New feature or request idea labels Mar 9, 2023
@hashedone
Copy link
Contributor Author

@njerschow
Copy link

Any update on this issue?

@njerschow
Copy link

Still curious if this is something that will be added to cw-multi-test at somepoint

@ethanfrey
Copy link
Member

I think a simple first step, is just to ignore any IBC messages returned.

This won't allow testing IBC, but will at least allow testing some non-ibc related functionality in said contracts (as they usually become completely untestable as soon as they have IBC Messages)

@hashedone hashedone added this to the 3.0.0 milestone Feb 21, 2024
@DariuszDepta DariuszDepta added IBC IBC support and removed enhancement New feature or request idea labels Jul 3, 2024
@DariuszDepta
Copy link
Member

I am closing this issue, as IBC support will be implemented as in #184.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IBC IBC support
Projects
None yet
Development

No branches or pull requests

4 participants