Skip to content

Commit

Permalink
added packet timeouts to wasm hooks (#3862)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolaslara authored Dec 29, 2022
1 parent 2ec476b commit 574443a
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 1 deletion.
Binary file modified tests/ibc-hooks/bytecode/counter.wasm
Binary file not shown.
34 changes: 34 additions & 0 deletions tests/ibc-hooks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"testing"
"time"

wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"

Expand Down Expand Up @@ -460,6 +461,39 @@ func (suite *HooksTestSuite) TestAcks() {

}

func (suite *HooksTestSuite) TestTimeouts() {
suite.chainA.StoreContractCode(&suite.Suite, "./bytecode/counter.wasm")
addr := suite.chainA.InstantiateContract(&suite.Suite, `{"count": 0}`, 1)

// Generate swap instructions for the contract
callbackMemo := fmt.Sprintf(`{"ibc_callback":"%s"}`, addr)
// Send IBC transfer with the memo with crosschain-swap instructions
transferMsg := NewMsgTransfer(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000)), suite.chainA.SenderAccount.GetAddress().String(), addr.String(), callbackMemo)
transferMsg.TimeoutTimestamp = uint64(suite.coordinator.CurrentTime.Add(time.Minute).UnixNano())
sendResult, err := suite.chainA.SendMsgsNoCheck(transferMsg)
suite.Require().NoError(err)

packet, err := ibctesting.ParsePacketFromEvents(sendResult.GetEvents())
suite.Require().NoError(err)

// Move chainB forward one block
suite.chainB.NextBlock()
// One month later
suite.coordinator.IncrementTimeBy(time.Hour)
err = suite.path.EndpointA.UpdateClient()
suite.Require().NoError(err)

err = suite.path.EndpointA.TimeoutPacket(packet)
suite.Require().NoError(err)

// The test contract will increment the counter for itself by 10 when a packet times out
state := suite.chainA.QueryContract(
&suite.Suite, addr,
[]byte(fmt.Sprintf(`{"get_count": {"addr": "%s"}}`, addr)))
suite.Require().Equal(`{"count":10}`, state)

}

func (suite *HooksTestSuite) TestSendWithoutMemo() {
// Sending a packet without memo to ensure that the ibc_callback middleware doesn't interfere with a regular send
transferMsg := NewMsgTransfer(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000)), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), "")
Expand Down
17 changes: 17 additions & 0 deletions tests/ibc-hooks/testutils/contracts/counter/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ pub fn sudo(deps: DepsMut, env: Env, msg: SudoMsg) -> Result<Response, ContractE
ack: _,
success,
} => sudo::receive_ack(deps, env.contract.address, success),
SudoMsg::IBCTimeout {
channel: _,
sequence: _,
} => sudo::ibc_timeout(deps, env.contract.address),
}
}

Expand All @@ -141,6 +145,19 @@ pub mod sudo {
)?;
Ok(Response::new().add_attribute("action", "ack"))
}

pub(crate) fn ibc_timeout(deps: DepsMut, contract: Addr) -> Result<Response, ContractError> {
utils::update_counter(
deps,
contract,
&|counter| match counter {
None => 10,
Some(counter) => counter.count + 10,
},
&|_counter| vec![],
)?;
Ok(Response::new().add_attribute("action", "timeout"))
}
}

pub fn naive_add_coins(lhs: &Vec<Coin>, rhs: &Vec<Coin>) -> Vec<Coin> {
Expand Down
2 changes: 2 additions & 0 deletions tests/ibc-hooks/testutils/contracts/counter/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,6 @@ pub enum SudoMsg {
ack: String,
success: bool,
},
#[serde(rename = "ibc_timeout")]
IBCTimeout { channel: String, sequence: u64 },
}
41 changes: 40 additions & 1 deletion x/ibc-hooks/wasm_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func (h WasmHooks) OnAcknowledgementPacketOverride(im IBCMiddleware, ctx sdk.Con

contractAddr, err := sdk.AccAddressFromBech32(contract)
if err != nil {
return sdkerrors.Wrap(err, "Ack callback error") // The callback configured is not a beck32. Error out
return sdkerrors.Wrap(err, "Ack callback error") // The callback configured is not a bech32. Error out
}

success := "false"
Expand All @@ -326,8 +326,47 @@ func (h WasmHooks) OnAcknowledgementPacketOverride(im IBCMiddleware, ctx sdk.Con
_, err = h.ContractKeeper.Sudo(ctx, contractAddr, sudoMsg)
if err != nil {
// error processing the callback
// ToDo: Open Question: Should we also delete the callback here?
return sdkerrors.Wrap(err, "Ack callback error")
}
h.ibcHooksKeeper.DeletePacketCallback(ctx, packet.GetSourceChannel(), packet.GetSequence())
return nil
}

func (h WasmHooks) OnTimeoutPacketOverride(im IBCMiddleware, ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error {
err := im.App.OnTimeoutPacket(ctx, packet, relayer)
if err != nil {
return err
}

if !h.ProperlyConfigured() {
// Not configured. Return from the underlying implementation
return nil
}

contract := h.ibcHooksKeeper.GetPacketCallback(ctx, packet.GetSourceChannel(), packet.GetSequence())
if contract == "" {
// No callback configured
return nil
}

contractAddr, err := sdk.AccAddressFromBech32(contract)
if err != nil {
return sdkerrors.Wrap(err, "Timeout callback error") // The callback configured is not a bech32. Error out
}

sudoMsg := []byte(fmt.Sprintf(
`{"ibc_timeout": {"channel": "%s", "sequence": %d}}`,
packet.SourceChannel, packet.Sequence))
_, err = h.ContractKeeper.Sudo(ctx, contractAddr, sudoMsg)
if err != nil {
// error processing the callback. This could be because the contract doesn't implement the message type to
// process the callback. Retrying this will not help, so we delete the callback from storage.
// Since the packet has timed out, we don't expect any other responses that may trigger the callback.
h.ibcHooksKeeper.DeletePacketCallback(ctx, packet.GetSourceChannel(), packet.GetSequence())
return sdkerrors.Wrap(err, "Timeout callback error")
}
//
h.ibcHooksKeeper.DeletePacketCallback(ctx, packet.GetSourceChannel(), packet.GetSequence())
return nil
}

0 comments on commit 574443a

Please sign in to comment.