From 2e51bab0086a8f114a1514100cabd2b7a0caaa46 Mon Sep 17 00:00:00 2001
From: Damian Nolan <damiannolan@gmail.com>
Date: Wed, 2 Feb 2022 14:56:14 +0100
Subject: [PATCH 1/3] chore: replace error string in transfer acks with const
(#818)
* fix: adding ack error string const for transfer
* updating godoc
* adding warning note to godoc in 04-channel
* updating to include abci error code, and copy tests from ica
* adding changelog entry
(cherry picked from commit ac46ac06084f586a460b092b2b293a321b7c43d6)
# Conflicts:
# modules/apps/27-interchain-accounts/host/types/ack.go
# modules/apps/transfer/ibc_module.go
---
CHANGELOG.md | 2 +
.../27-interchain-accounts/host/types/ack.go | 27 ++
modules/apps/transfer/ibc_module.go | 280 ++++++++++++++++++
modules/apps/transfer/types/ack.go | 27 ++
modules/apps/transfer/types/ack_test.go | 101 +++++++
.../core/04-channel/types/acknowledgement.go | 2 +
6 files changed, 439 insertions(+)
create mode 100644 modules/apps/27-interchain-accounts/host/types/ack.go
create mode 100644 modules/apps/transfer/ibc_module.go
create mode 100644 modules/apps/transfer/types/ack.go
create mode 100644 modules/apps/transfer/types/ack_test.go
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ed1bea6378..111714cd3ad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
## [v2.0.3](https://github.com/cosmos/ibc-go/releases/tag/v2.0.2) - 2022-02-03
+* (transfer) [\#818](https://github.com/cosmos/ibc-go/pull/818) Error acknowledgements returned from Transfer `OnRecvPacket` now include a deterministic ABCI code and error message.
+
### Improvements
* (channel) [\#692](https://github.com/cosmos/ibc-go/pull/692) Minimize channel logging by only emitting the packet sequence, source port/channel, destination port/channel upon packet receives, acknowledgements and timeouts.
diff --git a/modules/apps/27-interchain-accounts/host/types/ack.go b/modules/apps/27-interchain-accounts/host/types/ack.go
new file mode 100644
index 00000000000..202404fff3a
--- /dev/null
+++ b/modules/apps/27-interchain-accounts/host/types/ack.go
@@ -0,0 +1,27 @@
+package types
+
+import (
+ "fmt"
+
+ sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+ channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+)
+
+const (
+ // ackErrorString defines a string constant included in error acknowledgements
+ // NOTE: Changing this const is state machine breaking as acknowledgements are written into state
+ ackErrorString = "error handling packet on host chain: see events for details"
+)
+
+// NewErrorAcknowledgement returns a deterministic error string which may be used in
+// the packet acknowledgement.
+func NewErrorAcknowledgement(err error) channeltypes.Acknowledgement {
+ // the ABCI code is included in the abcitypes.ResponseDeliverTx hash
+ // constructed in Tendermint and is therefore determinstic
+ _, code, _ := sdkerrors.ABCIInfo(err, false) // discard non-deterministic codespace and log values
+
+ errorString := fmt.Sprintf("ABCI code: %d: %s", code, ackErrorString)
+
+ return channeltypes.NewErrorAcknowledgement(errorString)
+}
diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go
new file mode 100644
index 00000000000..26f1c533434
--- /dev/null
+++ b/modules/apps/transfer/ibc_module.go
@@ -0,0 +1,280 @@
+package transfer
+
+import (
+ "fmt"
+ "math"
+
+ sdk "github.com/cosmos/cosmos-sdk/types"
+ sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+ capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+
+ "github.com/cosmos/ibc-go/v3/modules/apps/transfer/keeper"
+ "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
+ channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+ porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types"
+ host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
+ ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
+)
+
+// IBCModule implements the ICS26 interface for transfer given the transfer keeper.
+type IBCModule struct {
+ keeper keeper.Keeper
+}
+
+// NewIBCModule creates a new IBCModule given the keeper
+func NewIBCModule(k keeper.Keeper) IBCModule {
+ return IBCModule{
+ keeper: k,
+ }
+}
+
+// ValidateTransferChannelParams does validation of a newly created transfer channel. A transfer
+// channel must be UNORDERED, use the correct port (by default 'transfer'), and use the current
+// supported version. Only 2^32 channels are allowed to be created.
+func ValidateTransferChannelParams(
+ ctx sdk.Context,
+ keeper keeper.Keeper,
+ order channeltypes.Order,
+ portID string,
+ channelID string,
+) error {
+ // NOTE: for escrow address security only 2^32 channels are allowed to be created
+ // Issue: https://github.com/cosmos/cosmos-sdk/issues/7737
+ channelSequence, err := channeltypes.ParseChannelSequence(channelID)
+ if err != nil {
+ return err
+ }
+ if channelSequence > uint64(math.MaxUint32) {
+ return sdkerrors.Wrapf(types.ErrMaxTransferChannels, "channel sequence %d is greater than max allowed transfer channels %d", channelSequence, uint64(math.MaxUint32))
+ }
+ if order != channeltypes.UNORDERED {
+ return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s ", channeltypes.UNORDERED, order)
+ }
+
+ // Require portID is the portID transfer module is bound to
+ boundPort := keeper.GetPort(ctx)
+ if boundPort != portID {
+ return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s, expected %s", portID, boundPort)
+ }
+
+ return nil
+}
+
+// OnChanOpenInit implements the IBCModule interface
+func (im IBCModule) OnChanOpenInit(
+ ctx sdk.Context,
+ order channeltypes.Order,
+ connectionHops []string,
+ portID string,
+ channelID string,
+ chanCap *capabilitytypes.Capability,
+ counterparty channeltypes.Counterparty,
+ version string,
+) error {
+ if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
+ return err
+ }
+
+ if version != types.Version {
+ return sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version)
+ }
+
+ // Claim channel capability passed back by IBC module
+ if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
+ return err
+ }
+
+ return nil
+}
+
+// OnChanOpenTry implements the IBCModule interface.
+func (im IBCModule) OnChanOpenTry(
+ ctx sdk.Context,
+ order channeltypes.Order,
+ connectionHops []string,
+ portID,
+ channelID string,
+ chanCap *capabilitytypes.Capability,
+ counterparty channeltypes.Counterparty,
+ counterpartyVersion string,
+) (string, error) {
+ if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
+ return "", err
+ }
+
+ if counterpartyVersion != types.Version {
+ return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: got: %s, expected %s", counterpartyVersion, types.Version)
+ }
+
+ // Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos
+ // (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry)
+ // If module can already authenticate the capability then module already owns it so we don't need to claim
+ // Otherwise, module does not have channel capability and we must claim it from IBC
+ if !im.keeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) {
+ // Only claim channel capability passed back by IBC module if we do not already own it
+ if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
+ return "", err
+ }
+ }
+
+ return types.Version, nil
+}
+
+// OnChanOpenAck implements the IBCModule interface
+func (im IBCModule) OnChanOpenAck(
+ ctx sdk.Context,
+ portID,
+ channelID string,
+ counterpartyVersion string,
+) error {
+ if counterpartyVersion != types.Version {
+ return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: %s, expected %s", counterpartyVersion, types.Version)
+ }
+ return nil
+}
+
+// OnChanOpenConfirm implements the IBCModule interface
+func (im IBCModule) OnChanOpenConfirm(
+ ctx sdk.Context,
+ portID,
+ channelID string,
+) error {
+ return nil
+}
+
+// OnChanCloseInit implements the IBCModule interface
+func (im IBCModule) OnChanCloseInit(
+ ctx sdk.Context,
+ portID,
+ channelID string,
+) error {
+ // Disallow user-initiated channel closing for transfer channels
+ return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
+}
+
+// OnChanCloseConfirm implements the IBCModule interface
+func (im IBCModule) OnChanCloseConfirm(
+ ctx sdk.Context,
+ portID,
+ channelID string,
+) error {
+ return nil
+}
+
+// OnRecvPacket implements the IBCModule interface. A successful acknowledgement
+// is returned if the packet data is succesfully decoded and the receive application
+// logic returns without error.
+func (im IBCModule) OnRecvPacket(
+ ctx sdk.Context,
+ packet channeltypes.Packet,
+ relayer sdk.AccAddress,
+) ibcexported.Acknowledgement {
+ ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
+
+ var data types.FungibleTokenPacketData
+ if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+ ack = channeltypes.NewErrorAcknowledgement("cannot unmarshal ICS-20 transfer packet data")
+ }
+
+ // only attempt the application logic if the packet data
+ // was successfully decoded
+ if ack.Success() {
+ err := im.keeper.OnRecvPacket(ctx, packet, data)
+ if err != nil {
+ ack = types.NewErrorAcknowledgement(err)
+ }
+ }
+
+ ctx.EventManager().EmitEvent(
+ sdk.NewEvent(
+ types.EventTypePacket,
+ sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+ sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
+ sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
+ sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
+ sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())),
+ ),
+ )
+
+ // NOTE: acknowledgement will be written synchronously during IBC handler execution.
+ return ack
+}
+
+// OnAcknowledgementPacket implements the IBCModule interface
+func (im IBCModule) OnAcknowledgementPacket(
+ ctx sdk.Context,
+ packet channeltypes.Packet,
+ acknowledgement []byte,
+ relayer sdk.AccAddress,
+) error {
+ var ack channeltypes.Acknowledgement
+ if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
+ return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
+ }
+ var data types.FungibleTokenPacketData
+ if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+ return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
+ }
+
+ if err := im.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil {
+ return err
+ }
+
+ ctx.EventManager().EmitEvent(
+ sdk.NewEvent(
+ types.EventTypePacket,
+ sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+ sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
+ sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
+ sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
+ sdk.NewAttribute(types.AttributeKeyAck, ack.String()),
+ ),
+ )
+
+ switch resp := ack.Response.(type) {
+ case *channeltypes.Acknowledgement_Result:
+ ctx.EventManager().EmitEvent(
+ sdk.NewEvent(
+ types.EventTypePacket,
+ sdk.NewAttribute(types.AttributeKeyAckSuccess, string(resp.Result)),
+ ),
+ )
+ case *channeltypes.Acknowledgement_Error:
+ ctx.EventManager().EmitEvent(
+ sdk.NewEvent(
+ types.EventTypePacket,
+ sdk.NewAttribute(types.AttributeKeyAckError, resp.Error),
+ ),
+ )
+ }
+
+ return nil
+}
+
+// OnTimeoutPacket implements the IBCModule interface
+func (im IBCModule) OnTimeoutPacket(
+ ctx sdk.Context,
+ packet channeltypes.Packet,
+ relayer sdk.AccAddress,
+) error {
+ var data types.FungibleTokenPacketData
+ if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+ return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
+ }
+ // refund tokens
+ if err := im.keeper.OnTimeoutPacket(ctx, packet, data); err != nil {
+ return err
+ }
+
+ ctx.EventManager().EmitEvent(
+ sdk.NewEvent(
+ types.EventTypeTimeout,
+ sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+ sdk.NewAttribute(types.AttributeKeyRefundReceiver, data.Sender),
+ sdk.NewAttribute(types.AttributeKeyRefundDenom, data.Denom),
+ sdk.NewAttribute(types.AttributeKeyRefundAmount, data.Amount),
+ ),
+ )
+
+ return nil
+}
diff --git a/modules/apps/transfer/types/ack.go b/modules/apps/transfer/types/ack.go
new file mode 100644
index 00000000000..6512f2e8371
--- /dev/null
+++ b/modules/apps/transfer/types/ack.go
@@ -0,0 +1,27 @@
+package types
+
+import (
+ "fmt"
+
+ sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+ channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+)
+
+const (
+ // ackErrorString defines a string constant included in error acknowledgements
+ // NOTE: Changing this const is state machine breaking as acknowledgements are written into state
+ ackErrorString = "error handling packet on destination chain: see events for details"
+)
+
+// NewErrorAcknowledgement returns a deterministic error string which may be used in
+// the packet acknowledgement.
+func NewErrorAcknowledgement(err error) channeltypes.Acknowledgement {
+ // the ABCI code is included in the abcitypes.ResponseDeliverTx hash
+ // constructed in Tendermint and is therefore deterministic
+ _, code, _ := sdkerrors.ABCIInfo(err, false) // discard non-determinstic codespace and log values
+
+ errorString := fmt.Sprintf("ABCI code: %d: %s", code, ackErrorString)
+
+ return channeltypes.NewErrorAcknowledgement(errorString)
+}
diff --git a/modules/apps/transfer/types/ack_test.go b/modules/apps/transfer/types/ack_test.go
new file mode 100644
index 00000000000..bc4e2d07afc
--- /dev/null
+++ b/modules/apps/transfer/types/ack_test.go
@@ -0,0 +1,101 @@
+package types_test
+
+import (
+ "testing"
+
+ sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+ "github.com/stretchr/testify/suite"
+ abcitypes "github.com/tendermint/tendermint/abci/types"
+ tmprotostate "github.com/tendermint/tendermint/proto/tendermint/state"
+ tmstate "github.com/tendermint/tendermint/state"
+
+ "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host/types"
+ ibctesting "github.com/cosmos/ibc-go/v3/testing"
+)
+
+const (
+ gasUsed = uint64(100)
+ gasWanted = uint64(100)
+)
+
+type TypesTestSuite struct {
+ suite.Suite
+
+ coordinator *ibctesting.Coordinator
+
+ chainA *ibctesting.TestChain
+ chainB *ibctesting.TestChain
+}
+
+func (suite *TypesTestSuite) SetupTest() {
+ suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+
+ suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
+ suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+}
+
+func TestTypesTestSuite(t *testing.T) {
+ suite.Run(t, new(TypesTestSuite))
+}
+
+// The safety of including ABCI error codes in the acknowledgement rests
+// on the inclusion of these ABCI error codes in the abcitypes.ResposneDeliverTx
+// hash. If the ABCI codes get removed from consensus they must no longer be used
+// in the packet acknowledgement.
+//
+// This test acts as an indicator that the ABCI error codes may no longer be deterministic.
+func (suite *TypesTestSuite) TestABCICodeDeterminism() {
+ // same ABCI error code used
+ err := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 1")
+ errSameABCICode := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 2")
+
+ // different ABCI error code used
+ errDifferentABCICode := sdkerrors.ErrNotFound
+
+ deliverTx := sdkerrors.ResponseDeliverTx(err, gasUsed, gasWanted, false)
+ responses := tmprotostate.ABCIResponses{
+ DeliverTxs: []*abcitypes.ResponseDeliverTx{
+ &deliverTx,
+ },
+ }
+
+ deliverTxSameABCICode := sdkerrors.ResponseDeliverTx(errSameABCICode, gasUsed, gasWanted, false)
+ responsesSameABCICode := tmprotostate.ABCIResponses{
+ DeliverTxs: []*abcitypes.ResponseDeliverTx{
+ &deliverTxSameABCICode,
+ },
+ }
+
+ deliverTxDifferentABCICode := sdkerrors.ResponseDeliverTx(errDifferentABCICode, gasUsed, gasWanted, false)
+ responsesDifferentABCICode := tmprotostate.ABCIResponses{
+ DeliverTxs: []*abcitypes.ResponseDeliverTx{
+ &deliverTxDifferentABCICode,
+ },
+ }
+
+ hash := tmstate.ABCIResponsesResultsHash(&responses)
+ hashSameABCICode := tmstate.ABCIResponsesResultsHash(&responsesSameABCICode)
+ hashDifferentABCICode := tmstate.ABCIResponsesResultsHash(&responsesDifferentABCICode)
+
+ suite.Require().Equal(hash, hashSameABCICode)
+ suite.Require().NotEqual(hash, hashDifferentABCICode)
+}
+
+// TestAcknowledgementError will verify that only a constant string and
+// ABCI error code are used in constructing the acknowledgement error string
+func (suite *TypesTestSuite) TestAcknowledgementError() {
+ // same ABCI error code used
+ err := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 1")
+ errSameABCICode := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 2")
+
+ // different ABCI error code used
+ errDifferentABCICode := sdkerrors.ErrNotFound
+
+ ack := types.NewErrorAcknowledgement(err)
+ ackSameABCICode := types.NewErrorAcknowledgement(errSameABCICode)
+ ackDifferentABCICode := types.NewErrorAcknowledgement(errDifferentABCICode)
+
+ suite.Require().Equal(ack, ackSameABCICode)
+ suite.Require().NotEqual(ack, ackDifferentABCICode)
+
+}
diff --git a/modules/core/04-channel/types/acknowledgement.go b/modules/core/04-channel/types/acknowledgement.go
index cfc088ab0c9..b46de2b981d 100644
--- a/modules/core/04-channel/types/acknowledgement.go
+++ b/modules/core/04-channel/types/acknowledgement.go
@@ -20,6 +20,8 @@ func NewResultAcknowledgement(result []byte) Acknowledgement {
// NewErrorAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Error
// type in the Response field.
+// NOTE: Acknowledgements are written into state and thus, changes made to error strings included in packet acknowledgements
+// risk an app hash divergence when nodes in a network are running different patch versions of software.
func NewErrorAcknowledgement(err string) Acknowledgement {
return Acknowledgement{
Response: &Acknowledgement_Error{
From d333aa71b4b8db651cecc96aea8441bf6eeeb947 Mon Sep 17 00:00:00 2001
From: Carlos Rodriguez <crodveg@gmail.com>
Date: Thu, 24 Feb 2022 22:16:38 +0100
Subject: [PATCH 2/3] fix merge conflicts
---
CHANGELOG.md | 10 +
.../27-interchain-accounts/host/types/ack.go | 27 --
modules/apps/transfer/ibc_module.go | 280 ------------------
modules/apps/transfer/module.go | 2 +-
modules/apps/transfer/types/ack.go | 2 +-
modules/apps/transfer/types/ack_test.go | 8 +-
6 files changed, 16 insertions(+), 313 deletions(-)
delete mode 100644 modules/apps/27-interchain-accounts/host/types/ack.go
delete mode 100644 modules/apps/transfer/ibc_module.go
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 111714cd3ad..b3618d023a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,16 @@ Ref: https://keepachangelog.com/en/1.0.0/
# Changelog
+## [v2.1.0](https://github.com/cosmos/ibc-go/releases/tag/v2.1.0) - 2022-03-01
+
+### State Machine Breaking
+
+* (transfer) [\#818](https://github.com/cosmos/ibc-go/pull/818) Error acknowledgements returned from Transfer `OnRecvPacket` now include a deterministic ABCI code and error message.
+
+### Features
+
+* [\#679](https://github.com/cosmos/ibc-go/pull/679) New CLI command `query ibc-transfer denom-hash <denom trace>` to get the denom hash for a denom trace; this might be useful for debug
+
## [v2.0.3](https://github.com/cosmos/ibc-go/releases/tag/v2.0.2) - 2022-02-03
* (transfer) [\#818](https://github.com/cosmos/ibc-go/pull/818) Error acknowledgements returned from Transfer `OnRecvPacket` now include a deterministic ABCI code and error message.
diff --git a/modules/apps/27-interchain-accounts/host/types/ack.go b/modules/apps/27-interchain-accounts/host/types/ack.go
deleted file mode 100644
index 202404fff3a..00000000000
--- a/modules/apps/27-interchain-accounts/host/types/ack.go
+++ /dev/null
@@ -1,27 +0,0 @@
-package types
-
-import (
- "fmt"
-
- sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-
- channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
-)
-
-const (
- // ackErrorString defines a string constant included in error acknowledgements
- // NOTE: Changing this const is state machine breaking as acknowledgements are written into state
- ackErrorString = "error handling packet on host chain: see events for details"
-)
-
-// NewErrorAcknowledgement returns a deterministic error string which may be used in
-// the packet acknowledgement.
-func NewErrorAcknowledgement(err error) channeltypes.Acknowledgement {
- // the ABCI code is included in the abcitypes.ResponseDeliverTx hash
- // constructed in Tendermint and is therefore determinstic
- _, code, _ := sdkerrors.ABCIInfo(err, false) // discard non-deterministic codespace and log values
-
- errorString := fmt.Sprintf("ABCI code: %d: %s", code, ackErrorString)
-
- return channeltypes.NewErrorAcknowledgement(errorString)
-}
diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go
deleted file mode 100644
index 26f1c533434..00000000000
--- a/modules/apps/transfer/ibc_module.go
+++ /dev/null
@@ -1,280 +0,0 @@
-package transfer
-
-import (
- "fmt"
- "math"
-
- sdk "github.com/cosmos/cosmos-sdk/types"
- sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
- capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
-
- "github.com/cosmos/ibc-go/v3/modules/apps/transfer/keeper"
- "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
- channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
- porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types"
- host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
- ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
-)
-
-// IBCModule implements the ICS26 interface for transfer given the transfer keeper.
-type IBCModule struct {
- keeper keeper.Keeper
-}
-
-// NewIBCModule creates a new IBCModule given the keeper
-func NewIBCModule(k keeper.Keeper) IBCModule {
- return IBCModule{
- keeper: k,
- }
-}
-
-// ValidateTransferChannelParams does validation of a newly created transfer channel. A transfer
-// channel must be UNORDERED, use the correct port (by default 'transfer'), and use the current
-// supported version. Only 2^32 channels are allowed to be created.
-func ValidateTransferChannelParams(
- ctx sdk.Context,
- keeper keeper.Keeper,
- order channeltypes.Order,
- portID string,
- channelID string,
-) error {
- // NOTE: for escrow address security only 2^32 channels are allowed to be created
- // Issue: https://github.com/cosmos/cosmos-sdk/issues/7737
- channelSequence, err := channeltypes.ParseChannelSequence(channelID)
- if err != nil {
- return err
- }
- if channelSequence > uint64(math.MaxUint32) {
- return sdkerrors.Wrapf(types.ErrMaxTransferChannels, "channel sequence %d is greater than max allowed transfer channels %d", channelSequence, uint64(math.MaxUint32))
- }
- if order != channeltypes.UNORDERED {
- return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s ", channeltypes.UNORDERED, order)
- }
-
- // Require portID is the portID transfer module is bound to
- boundPort := keeper.GetPort(ctx)
- if boundPort != portID {
- return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid port: %s, expected %s", portID, boundPort)
- }
-
- return nil
-}
-
-// OnChanOpenInit implements the IBCModule interface
-func (im IBCModule) OnChanOpenInit(
- ctx sdk.Context,
- order channeltypes.Order,
- connectionHops []string,
- portID string,
- channelID string,
- chanCap *capabilitytypes.Capability,
- counterparty channeltypes.Counterparty,
- version string,
-) error {
- if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
- return err
- }
-
- if version != types.Version {
- return sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version)
- }
-
- // Claim channel capability passed back by IBC module
- if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
- return err
- }
-
- return nil
-}
-
-// OnChanOpenTry implements the IBCModule interface.
-func (im IBCModule) OnChanOpenTry(
- ctx sdk.Context,
- order channeltypes.Order,
- connectionHops []string,
- portID,
- channelID string,
- chanCap *capabilitytypes.Capability,
- counterparty channeltypes.Counterparty,
- counterpartyVersion string,
-) (string, error) {
- if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
- return "", err
- }
-
- if counterpartyVersion != types.Version {
- return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: got: %s, expected %s", counterpartyVersion, types.Version)
- }
-
- // Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos
- // (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry)
- // If module can already authenticate the capability then module already owns it so we don't need to claim
- // Otherwise, module does not have channel capability and we must claim it from IBC
- if !im.keeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) {
- // Only claim channel capability passed back by IBC module if we do not already own it
- if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
- return "", err
- }
- }
-
- return types.Version, nil
-}
-
-// OnChanOpenAck implements the IBCModule interface
-func (im IBCModule) OnChanOpenAck(
- ctx sdk.Context,
- portID,
- channelID string,
- counterpartyVersion string,
-) error {
- if counterpartyVersion != types.Version {
- return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: %s, expected %s", counterpartyVersion, types.Version)
- }
- return nil
-}
-
-// OnChanOpenConfirm implements the IBCModule interface
-func (im IBCModule) OnChanOpenConfirm(
- ctx sdk.Context,
- portID,
- channelID string,
-) error {
- return nil
-}
-
-// OnChanCloseInit implements the IBCModule interface
-func (im IBCModule) OnChanCloseInit(
- ctx sdk.Context,
- portID,
- channelID string,
-) error {
- // Disallow user-initiated channel closing for transfer channels
- return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
-}
-
-// OnChanCloseConfirm implements the IBCModule interface
-func (im IBCModule) OnChanCloseConfirm(
- ctx sdk.Context,
- portID,
- channelID string,
-) error {
- return nil
-}
-
-// OnRecvPacket implements the IBCModule interface. A successful acknowledgement
-// is returned if the packet data is succesfully decoded and the receive application
-// logic returns without error.
-func (im IBCModule) OnRecvPacket(
- ctx sdk.Context,
- packet channeltypes.Packet,
- relayer sdk.AccAddress,
-) ibcexported.Acknowledgement {
- ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
-
- var data types.FungibleTokenPacketData
- if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
- ack = channeltypes.NewErrorAcknowledgement("cannot unmarshal ICS-20 transfer packet data")
- }
-
- // only attempt the application logic if the packet data
- // was successfully decoded
- if ack.Success() {
- err := im.keeper.OnRecvPacket(ctx, packet, data)
- if err != nil {
- ack = types.NewErrorAcknowledgement(err)
- }
- }
-
- ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- types.EventTypePacket,
- sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
- sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
- sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
- sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
- sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())),
- ),
- )
-
- // NOTE: acknowledgement will be written synchronously during IBC handler execution.
- return ack
-}
-
-// OnAcknowledgementPacket implements the IBCModule interface
-func (im IBCModule) OnAcknowledgementPacket(
- ctx sdk.Context,
- packet channeltypes.Packet,
- acknowledgement []byte,
- relayer sdk.AccAddress,
-) error {
- var ack channeltypes.Acknowledgement
- if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
- return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
- }
- var data types.FungibleTokenPacketData
- if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
- return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
- }
-
- if err := im.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil {
- return err
- }
-
- ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- types.EventTypePacket,
- sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
- sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
- sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
- sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
- sdk.NewAttribute(types.AttributeKeyAck, ack.String()),
- ),
- )
-
- switch resp := ack.Response.(type) {
- case *channeltypes.Acknowledgement_Result:
- ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- types.EventTypePacket,
- sdk.NewAttribute(types.AttributeKeyAckSuccess, string(resp.Result)),
- ),
- )
- case *channeltypes.Acknowledgement_Error:
- ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- types.EventTypePacket,
- sdk.NewAttribute(types.AttributeKeyAckError, resp.Error),
- ),
- )
- }
-
- return nil
-}
-
-// OnTimeoutPacket implements the IBCModule interface
-func (im IBCModule) OnTimeoutPacket(
- ctx sdk.Context,
- packet channeltypes.Packet,
- relayer sdk.AccAddress,
-) error {
- var data types.FungibleTokenPacketData
- if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
- return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
- }
- // refund tokens
- if err := im.keeper.OnTimeoutPacket(ctx, packet, data); err != nil {
- return err
- }
-
- ctx.EventManager().EmitEvent(
- sdk.NewEvent(
- types.EventTypeTimeout,
- sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
- sdk.NewAttribute(types.AttributeKeyRefundReceiver, data.Sender),
- sdk.NewAttribute(types.AttributeKeyRefundDenom, data.Denom),
- sdk.NewAttribute(types.AttributeKeyRefundAmount, data.Amount),
- ),
- )
-
- return nil
-}
diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go
index 7af82060149..941214a84ee 100644
--- a/modules/apps/transfer/module.go
+++ b/modules/apps/transfer/module.go
@@ -337,7 +337,7 @@ func (am AppModule) OnRecvPacket(
if ack.Success() {
err := am.keeper.OnRecvPacket(ctx, packet, data)
if err != nil {
- ack = channeltypes.NewErrorAcknowledgement(err.Error())
+ ack = types.NewErrorAcknowledgement(err)
}
}
diff --git a/modules/apps/transfer/types/ack.go b/modules/apps/transfer/types/ack.go
index 6512f2e8371..a063ac426d8 100644
--- a/modules/apps/transfer/types/ack.go
+++ b/modules/apps/transfer/types/ack.go
@@ -5,7 +5,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
- channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
+ channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
)
const (
diff --git a/modules/apps/transfer/types/ack_test.go b/modules/apps/transfer/types/ack_test.go
index bc4e2d07afc..96214a84ea9 100644
--- a/modules/apps/transfer/types/ack_test.go
+++ b/modules/apps/transfer/types/ack_test.go
@@ -9,8 +9,8 @@ import (
tmprotostate "github.com/tendermint/tendermint/proto/tendermint/state"
tmstate "github.com/tendermint/tendermint/state"
- "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host/types"
- ibctesting "github.com/cosmos/ibc-go/v3/testing"
+ "github.com/cosmos/ibc-go/v2/modules/apps/transfer/types"
+ ibctesting "github.com/cosmos/ibc-go/v2/testing"
)
const (
@@ -30,8 +30,8 @@ type TypesTestSuite struct {
func (suite *TypesTestSuite) SetupTest() {
suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
- suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
- suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+ suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0))
+ suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1))
}
func TestTypesTestSuite(t *testing.T) {
From 3d289a7d7bd16f130f11738904f684ef31c663c3 Mon Sep 17 00:00:00 2001
From: Carlos Rodriguez <crodveg@gmail.com>
Date: Thu, 24 Feb 2022 22:22:11 +0100
Subject: [PATCH 3/3] fix changelog
---
CHANGELOG.md | 2 --
1 file changed, 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b3618d023a5..90944394068 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,8 +46,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
## [v2.0.3](https://github.com/cosmos/ibc-go/releases/tag/v2.0.2) - 2022-02-03
-* (transfer) [\#818](https://github.com/cosmos/ibc-go/pull/818) Error acknowledgements returned from Transfer `OnRecvPacket` now include a deterministic ABCI code and error message.
-
### Improvements
* (channel) [\#692](https://github.com/cosmos/ibc-go/pull/692) Minimize channel logging by only emitting the packet sequence, source port/channel, destination port/channel upon packet receives, acknowledgements and timeouts.