Implement IBC Callbacks Adapter #160

NotJeremyLiu · 2025-03-03T14:41:24Z

This PR

Creates a Skip2 package which provides types and methods using Cosmwasm 2
Adds an IBC Callbacks adapter that can receive IBC destination and source callbacks, and dispatch IBC transfers with multiple encodings
Adds logic in the entrypoint to distribute and emit events when handling Eureka fees

Highlights on interfaces / interactions

Events emitted on eureka fee:

        response = response
            .add_message(eureka_fee_bank_send)
            .add_attribute("action", "dispatch_eureka_fee_bank_send")
            .add_attribute("eureka_fee_denom", eureka_fee.coin.denom.clone())
            .add_attribute("eureka_fee_amount", eureka_fee.coin.amount)
            .add_attribute("eureka_fee_receiver", eureka_fee.receiver.clone());

Adds an optional encoding and EurekaFee to the IbcInfo:

#[cw_serde]
#[derive(Default)]
pub struct EurekaFee {
    pub coin: Coin,
    pub receiver: String,
    pub timeout_timestamp: u64,
}

// The IbcInfo struct defines the information for an IBC transfer standardized across all IBC Transfer Adapter contracts.
#[cw_serde]
pub struct IbcInfo {
    pub source_channel: String,
    pub receiver: String,
    pub fee: Option<IbcFee>, // Only used in Neutron
    pub memo: String,
    pub recover_address: String,
    pub encoding: Option<String>,
    pub eureka_fee: Option<EurekaFee>,
}

Note

Deprecated fields are allowed in the ibc callbacks adapter so that it is portable to both cw 1 and cw 2 chains

TODOs

Add unit tests (can be in a follow-up PR)

which is skip types and methods for cosmwasm 2

required to compile cosmwasm 2 deps

- a copy of ibc hooks adapter

- also removes unused code in skip2 pkg

dhfang

really good work, thank you sir 🙏

dhfang · 2025-03-04T07:13:47Z

contracts/adapters/ibc/ibc-callbacks/src/contract.rs

+    let resp: MsgTransferResponse = MsgTransferResponse::decode(
+        sub_msg_response
+            .data
+            .ok_or(ContractError::MissingResponseData)?


what conditions could this happen in? should we clear IN_PROGRESS_RECOVER_ADDRESS and IN_PROGRESS_CHANNEL_ID?

This is a chain failure if there's no data in the sub_msg_response. The contract is expecting this data field to always be here.

The MsgTransferResponse data is just the sequence number back from the transfer ibc moduke, so no data here = no sequence given back which is a transfer module critical failure

dhfang · 2025-03-04T07:47:30Z

contracts/adapters/ibc/ibc-callbacks/src/contract.rs

+    let recv_denom = get_recv_denom(msg.packet, packet_data.denom.clone());
+
+    // Decode the memo to get the contract address and message to execute
+    let (contract_addr, msg) = get_contract_addr_and_msg_from_ibc_hooks_memo(packet_data.memo)?;


So iiuc, on packet receipt the callback handler on the entrypoint contract gets invoked, which then executes the wasm hooks payload at the target address, which in our case will also be the entrypoint contract?

So this callbacks adapter is the receiver of the destination callback (which is this adapter contract).

In the wasm field in the memo the contract addr is the entrypoint addr (which is already what we do in msg building), so this destination callback logic on the callbacks adapter will decode + call the entry point contract with the payload + tokens.

Then from the entrypoint contract it all looks normal

dhfang · 2025-03-04T08:01:14Z

contracts/adapters/ibc/ibc-callbacks/src/contract.rs

+    ACK_ID_TO_RECOVER_ADDRESS.remove(deps.storage, ack_id);
+
+    // Get all coins from contract's balance, which will be the
+    // failed ibc transfer coin and any leftover dust on the contract


if a previous user's funds are on the contract due to a failure in the source callback to refund, this refund logic seems like it gives another user an avenue to extract those funds. the source callback is not that complex right now that i'm super worried about this, but I could see us making it more complex in the future. or some other developer extends the contract in a way that funds are stored on the contract without full context here.

this is why i dont really like that the application can't revert the timeout or acknowledgement transaction. it makes it harder to reason about the state transitions of the application. perhaps make sense to store the amount and denom along with the other recovery information?

Yeah so rn this logic is mirror'd across all of our ibc adapters right now. So this is feature parity with the way we handle other timeouts/errors

I remember now why we did it this way, it was because on neutron when there's ibc fees as well we refund those so it's multiple denoms so we wanted to send back multiple denoms

Addressed here: a2e7202

dhfang · 2025-03-04T08:09:40Z

contracts/adapters/ibc/ibc-callbacks/src/contract.rs

+    );
+
+    // Require that the packet was successfully received
+    // TODO: This fails due to the msg.ack.data having extra bytes then just the


oh are you trying to address this pre audit?

potentially, i need to figure out this extra bytes issue, so if I can resolve today then down to add this logic in today for the audit

Addressed here: 19c94ba

Discussed with IBC team that for v2 it gets proto marshaled, leading to the extra bytes. So keeping the string comparison as the check for now

dhfang · 2025-03-04T08:09:50Z

contracts/adapters/ibc/ibc-callbacks/src/contract.rs

+        amount: Uint128::from_str(&packet_data.amount)?,
+    };
+
+    // @NotJeremyLiu TODO: Turn this into a sub msg and figure out what the recovery case is here


are you trying to address this pre audit?

Yeah so rn the logic is that if the underlying call fails, the tx will fail.

So we only want to impl this sub_msg logic if we want a failure to do something else (like refund to a given address on the chain). But we have a case where if we can't pull a recovery addr from the memo some failure case anyways.

NotJeremyLiu added 16 commits February 23, 2025 18:20

create a skip2 package

e0e7447

which is skip types and methods for cosmwasm 2

add skip2 to workspace cargo.toml

32514bd

update makefile to use 15.1 optimizer

5a72034

required to compile cosmwasm 2 deps

add ibc-callbacks scaffolding

1792b02

- a copy of ibc hooks adapter

update skip and skip2 packages with eureka fee

fd5a337

- also removes unused code in skip2 pkg

update makefile with latest cw optimizer

398eb0b

update cargo toml and cargo lock

656b1d9

add ibc callbacks adapter

6db328b

update entrypoint contract with eureka fee logic

31bbed6

update other ibc adapters for updated type

8ceb621

Merge branch 'main' into jl/ibc-callbacks-adapter

1309ae1

Update Cargo.lock

b915923

run make schema

5506a6f

allow large enum variant

c083cdc

fix mantra clippy

b13a5a5

allow deprecated fields / variants in ibc callbacks adapter

35b671c

NotJeremyLiu changed the title ~~IBC Callbacks Adapter~~ Implement IBC Callbacks Adapter Mar 3, 2025

NotJeremyLiu added 2 commits March 3, 2025 12:46

fix typo commited

3f9f8e2

hex encode upper on the ibc denom

7f470ea

dhfang approved these changes Mar 4, 2025

View reviewed changes

dhfang reviewed Mar 4, 2025

View reviewed changes

NotJeremyLiu added 6 commits March 4, 2025 09:47

check success on dest callback

19c94ba

run make fmt

add6386

add error

d14da5c

refund only the coin that was ibc transferred on err and timeout

a2e7202

clarify comment

55b7d71

run make fmt

f179e58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement IBC Callbacks Adapter #160

Implement IBC Callbacks Adapter #160

NotJeremyLiu commented Mar 3, 2025 •

edited

Loading

dhfang left a comment

dhfang Mar 4, 2025

NotJeremyLiu Mar 4, 2025

NotJeremyLiu Mar 4, 2025

dhfang Mar 4, 2025

NotJeremyLiu Mar 4, 2025

dhfang Mar 4, 2025 •

edited

Loading

NotJeremyLiu Mar 4, 2025

NotJeremyLiu Mar 4, 2025

NotJeremyLiu Mar 5, 2025

dhfang Mar 4, 2025

NotJeremyLiu Mar 4, 2025

NotJeremyLiu Mar 5, 2025

dhfang Mar 4, 2025

NotJeremyLiu Mar 4, 2025

Implement IBC Callbacks Adapter #160

Are you sure you want to change the base?

Implement IBC Callbacks Adapter #160

Conversation

NotJeremyLiu commented Mar 3, 2025 • edited Loading

This PR

Highlights on interfaces / interactions

Note

TODOs

dhfang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhfang Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NotJeremyLiu commented Mar 3, 2025 •

edited

Loading

dhfang Mar 4, 2025 •

edited

Loading