Use standard IBC errors (#3908)

* reverted errors to the ibc standard

* revision change

* updated dependencies

* fix rate limit errors

* fixed rate limit errors

* updated versions

* updated commit to after the merge

* convert new error

* new go mod

* increment error codes

* updated go mod to latest ibc-hooks

Co-authored-by: Dev Ojha <[email protected]>

go.mod

@@ -17,9 +17,9 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.16
 	github.com/ory/dockertest/v3 v3.9.1
 	github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
-	github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105180642-3de6477ab495
-	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495
-	github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105182447-580c779fa280
+	github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105183030-bccf5202f260
+	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99
+	github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230106090955-07448a793aaa
 	github.com/pkg/errors v0.9.1
 	github.com/rakyll/statik v0.1.7
 	github.com/spf13/cast v1.5.0

go.sum

@@ -853,12 +853,12 @@ github.com/osmosis-labs/cosmos-sdk v0.45.1-0.20221118211718-545aed73e94e h1:A3by
 github.com/osmosis-labs/cosmos-sdk v0.45.1-0.20221118211718-545aed73e94e/go.mod h1:rud0OaBIuq3+qOqtwT4SR7Q7iSzRp7w41fjninTjfnQ=
 github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3 h1:YlmchqTmlwdWSmrRmXKR+PcU96ntOd8u10vTaTZdcNY=
 github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3/go.mod h1:lV6KnqXYD/ayTe7310MHtM3I2q8Z6bBfMAi+bhwPYtI=
-github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105180642-3de6477ab495 h1:Bj0z6oLbTWwmuMg0xe0qLoL9BTPrH6NA7MSqc45Sj9o=
-github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105180642-3de6477ab495/go.mod h1:KrzYoNtnWUH75rj1XAsSR4nymlHFU7jeVOx7/1KMe0k=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495 h1:Fa2NwcMhnnVUjhjuHKWqTeLWrJTbU1jhlA31whspLbg=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105182447-580c779fa280 h1:kV0XKgKTalinoXzDwLyFlgsY2uqFIxN4gQ1NaxBOxzo=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105182447-580c779fa280/go.mod h1:zi4pcyqW2MF+ZLoGucoZzqbtUZJ53N2U3Q7nH9KaHmM=
+github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105183030-bccf5202f260 h1:+EbINXzHQyDtHje2CND357A22H2zUpceTtwJClC9IAM=
+github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105183030-bccf5202f260/go.mod h1:KrzYoNtnWUH75rj1XAsSR4nymlHFU7jeVOx7/1KMe0k=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99 h1:8yYGNa5u8MmFdh+FpZQ7/4jlGABd5XSDfPcENJE9HXs=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
+github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230106090955-07448a793aaa h1:UHGAZjmVpG51H9drRRUnQnvIQtP13C+wvGV/V1wJMJY=
+github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230106090955-07448a793aaa/go.mod h1:ey8gfcDaTYJMkj5d0FZ4+p3CPcXP6tbPQ/WyOww17Gc=
 github.com/osmosis-labs/wasmd v0.29.2-0.20221222131554-7c8ea36a6e30 h1:6uMi7HhPSwvKKU7j5NqljseFTEz4I7qHr+IPnnn42Ck=
 github.com/osmosis-labs/wasmd v0.29.2-0.20221222131554-7c8ea36a6e30/go.mod h1:5fDYJyMXBq6u2iuHqqOTYiZ5NitIOeIW5k7qEXqbwJE=
 github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=

go.work.sum

@@ -259,22 +259,8 @@ github.com/opencontainers/selinux v1.8.2/go.mod h1:MUIHuUEvKB1wtJjQdOyYRgOnLD2xA
 github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
 github.com/openzipkin/zipkin-go v0.2.5/go.mod h1:KpXfKdgRDnnhsxw4pNIH9Md5lyFqKUa4YDFlwRYAMyE=
 github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnhNrne+V0E6LAcBILJdPs=
-<<<<<<< HEAD
-github.com/osmosis-labs/osmosis/osmomath v0.0.3-0.20230105093646-e22b9c7884b7/go.mod h1:KrzYoNtnWUH75rj1XAsSR4nymlHFU7jeVOx7/1KMe0k=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105093646-e22b9c7884b7/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.2-flexible/go.mod h1:T7CCZKYhKWASnv5mRE8u3m0gst3NZ/sU16Brjmv4UPw=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105093646-e22b9c7884b7/go.mod h1:3u9IKKt6FXe6YcenRd4z8Kn22mnnNV4fAXxmPWfBcuI=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.3/go.mod h1:bASDMbNQ0vD+2YRsEqlKXg8NiEsJpQ8B8p3nR8kB+hA=
-||||||| parent of 64393a14e (Lifecycle completed without contracts (#3927))
-github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105100058-2ad8dafc6e4f/go.mod h1:KrzYoNtnWUH75rj1XAsSR4nymlHFU7jeVOx7/1KMe0k=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105100058-2ad8dafc6e4f/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105100058-2ad8dafc6e4f/go.mod h1:iHEtAvQf7wZb4sTxMRQZinu3k9QjRPWzFvkADwsfvFQ=
-=======
-github.com/osmosis-labs/osmosis/osmomath v0.0.0-20230105100058-2ad8dafc6e4f/go.mod h1:KrzYoNtnWUH75rj1XAsSR4nymlHFU7jeVOx7/1KMe0k=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105100058-2ad8dafc6e4f/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230105182447-580c779fa280/go.mod h1:zi4pcyqW2MF+ZLoGucoZzqbtUZJ53N2U3Q7nH9KaHmM=
-github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.3/go.mod h1:bASDMbNQ0vD+2YRsEqlKXg8NiEsJpQ8B8p3nR8kB+hA=
->>>>>>> 64393a14e (Lifecycle completed without contracts (#3927))
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105183030-bccf5202f260/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
+github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230106090955-07448a793aaa/go.mod h1:ey8gfcDaTYJMkj5d0FZ4+p3CPcXP6tbPQ/WyOww17Gc=
 github.com/otiai10/copy v1.7.0/go.mod h1:rmRl6QPdJj6EiUqXQ/4Nn2lLXoNQjFCQbbNrxgc/t3U=
 github.com/otiai10/mint v1.3.3 h1:7JgpsBaN0uMkyju4tbYHu0mnM55hNKVYLsXmwr15NQI=
 github.com/otiai10/mint v1.3.3/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc=

osmoutils/ibc.go

@@ -2,24 +2,29 @@ package osmoutils
 
 import (
 	"encoding/json"
-
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	transfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types"
 	ibcexported "github.com/cosmos/ibc-go/v4/modules/core/exported"
 )
 
-// NewStringErrorAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Error
-// type in the Response field. These errors differ from the IBC errors in that we use custom error strings.
-// 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 NewStringErrorAcknowledgement(err string) channeltypes.Acknowledgement {
-	// ToDo: Do we want to do this or do we want to use the IBC errors and emit the string?
-
-	return channeltypes.Acknowledgement{
-		Response: &channeltypes.Acknowledgement_Error{
-			Error: err,
-		},
+// NewEmitErrorAcknowledgement creates a new error acknowledgement after having emitted an event with the
+// details of the error.
+func NewEmitErrorAcknowledgement(ctx sdk.Context, err error, errorContexts ...string) channeltypes.Acknowledgement {
+	attributes := make([]sdk.Attribute, len(errorContexts)+1)
+	attributes[0] = sdk.NewAttribute("error", err.Error())
+	for i, s := range errorContexts {
+		attributes[i+1] = sdk.NewAttribute("error-context", s)
 	}
+
+	ctx.EventManager().EmitEvents(sdk.Events{
+		sdk.NewEvent(
+			"ibc-acknowledgement-error",
+			attributes...,
+		),
+	})
+
+	return channeltypes.NewErrorAcknowledgement(err)
 }
 
 // MustExtractDenomFromPacketOnRecv takes a packet with a valid ICS20 token data in the Data field and returns the

x/ibc-hooks/go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/cosmos/ibc-go/v4 v4.2.0
 	github.com/gorilla/mux v1.8.0
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
-	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495
+	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99
 	github.com/spf13/cobra v1.6.1
 	github.com/tendermint/tendermint v0.34.24
 )

x/ibc-hooks/go.sum

@@ -620,8 +620,8 @@ github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnh
 github.com/ory/dockertest v3.3.5+incompatible h1:iLLK6SQwIhcbrG783Dghaaa3WPzGc+4Emza6EbVUUGA=
 github.com/osmosis-labs/cosmos-sdk v0.45.1-0.20221118211718-545aed73e94e h1:A3byMZpvq21iI7yWJUNdHw0nf8jVAbVUsWY9twnXSXE=
 github.com/osmosis-labs/cosmos-sdk v0.45.1-0.20221118211718-545aed73e94e/go.mod h1:rud0OaBIuq3+qOqtwT4SR7Q7iSzRp7w41fjninTjfnQ=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495 h1:Fa2NwcMhnnVUjhjuHKWqTeLWrJTbU1jhlA31whspLbg=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105180642-3de6477ab495/go.mod h1:K4de+n3DtLdueen98dOzaRXZvqMd8JvigL8O1xW445o=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99 h1:8yYGNa5u8MmFdh+FpZQ7/4jlGABd5XSDfPcENJE9HXs=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230105184425-1e6fcd979d99/g
 github.com/osmosis-labs/wasmd v0.29.2-0.20221222131554-7c8ea36a6e30 h1:6uMi7HhPSwvKKU7j5NqljseFTEz4I7qHr+IPnnn42Ck=
 github.com/osmosis-labs/wasmd v0.29.2-0.20221222131554-7c8ea36a6e30/go.mod h1:5fDYJyMXBq6u2iuHqqOTYiZ5NitIOeIW5k7qEXqbwJE=
 github.com/otiai10/copy v1.7.0 h1:hVoPiN+t+7d2nzzwMiDHPSOogsWAStewq3TwU05+clE=

x/ibc-hooks/types/errors.go

@@ -1,8 +1,15 @@
 package types
 
+import sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
 var (
-	ErrBadPacketMetadataMsg = "cannot unmarshal metadata: '%v'. %s"
 	ErrBadMetadataFormatMsg = "wasm metadata not properly formatted for: '%v'. %s"
 	ErrBadExecutionMsg      = "cannot execute contract: %v"
-	ErrBadResponse          = "cannot create response: %v"
+
+	ErrMsgValidation = sdkerrors.Register("wasm-hooks", 2, "error in wasmhook message validation")
+	ErrMarshaling    = sdkerrors.Register("wasm-hooks", 3, "cannot marshal the ICS20 packet")
+	ErrInvalidPacket = sdkerrors.Register("wasm-hooks", 4, "invalid packet data")
+	ErrBadResponse   = sdkerrors.Register("wasm-hooks", 5, "cannot create response")
+	ErrWasmError     = sdkerrors.Register("wasm-hooks", 6, "wasm error")
+	ErrBadSender     = sdkerrors.Register("wasm-hooks", 7, "bad sender")
 )

x/ibc-hooks/wasm_hook.go

@@ -60,22 +60,22 @@ func (h WasmHooks) OnRecvPacketOverride(im IBCMiddleware, ctx sdk.Context, packe
 		return im.App.OnRecvPacket(ctx, packet, relayer)
 	}
 	if err != nil {
-		return osmoutils.NewStringErrorAcknowledgement(err.Error())
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrMsgValidation, err.Error())
 	}
 	if msgBytes == nil || contractAddr == nil { // This should never happen
-		return osmoutils.NewStringErrorAcknowledgement("error in wasmhook message validation")
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrMsgValidation)
 	}
 
 	// The funds sent on this packet need to be transferred to the wasm hooks module address/
 	// For this, we override the ICS20 packet's Receiver (essentially hijacking the funds for the module)
 	// and execute the underlying OnRecvPacket() call (which should eventually land on the transfer app's
-	// relay.go and send the sunds to the module.
+	// relay.go and send the funds to the module.
 	//
 	// If that succeeds, we make the contract call
 	data.Receiver = WasmHookModuleAccountAddr.String()
 	bz, err := json.Marshal(data)
 	if err != nil {
-		return osmoutils.NewStringErrorAcknowledgement(fmt.Sprintf("cannot marshal the ICS20 packet: %s", err.Error()))
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrMarshaling, err.Error())
 	}
 	packet.Data = bz
 
@@ -89,7 +89,7 @@ func (h WasmHooks) OnRecvPacketOverride(im IBCMiddleware, ctx sdk.Context, packe
 	if !ok {
 		// This should never happen, as it should've been caught in the underlaying call to OnRecvPacket,
 		// but returning here for completeness
-		return osmoutils.NewStringErrorAcknowledgement("Invalid packet data: Amount is not an int")
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrInvalidPacket, "Amount is not an int")
 	}
 
 	// The packet's denom is the denom in the sender chain. This needs to be converted to the local denom.
@@ -104,13 +104,13 @@ func (h WasmHooks) OnRecvPacketOverride(im IBCMiddleware, ctx sdk.Context, packe
 	}
 	response, err := h.execWasmMsg(ctx, &execMsg)
 	if err != nil {
-		return osmoutils.NewStringErrorAcknowledgement(err.Error())
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrWasmError, err.Error())
 	}
 
 	fullAck := ContractAck{ContractResult: response.Data, IbcAck: ack.Acknowledgement()}
 	bz, err = json.Marshal(fullAck)
 	if err != nil {
-		return osmoutils.NewStringErrorAcknowledgement(fmt.Sprintf(types.ErrBadResponse, err.Error()))
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrBadResponse, err.Error())
 	}
 
 	return channeltypes.NewResultAcknowledgement(bz)

x/ibc-rate-limit/ibc_middleware_test.go

@@ -9,8 +9,6 @@ import (
 
 	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
 
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	transfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types"
@@ -140,7 +138,7 @@ func (suite *MiddlewareTestSuite) TestInvalidReceiver() {
 	_, ack, _ := suite.FullSendBToA(msg)
 	suite.Require().Contains(string(ack), "error",
 		"acknowledgment is not an error")
-	suite.Require().Contains(string(ack), sdkerrors.ErrInvalidAddress.Error(),
+	suite.Require().Contains(string(ack), fmt.Sprintf("ABCI code: %d", types.ErrBadMessage.ABCICode()),
 		"acknowledgment error is not of the right type")
 }
 
@@ -214,7 +212,7 @@ func (suite *MiddlewareTestSuite) AssertReceive(success bool, msg sdk.Msg) (stri
 	} else {
 		suite.Require().Contains(string(ack), "error",
 			"acknowledgment is not an error")
-		suite.Require().Contains(string(ack), types.ErrRateLimitExceeded.Error(),
+		suite.Require().Contains(string(ack), fmt.Sprintf("ABCI code: %d", types.ErrRateLimitExceeded.ABCICode()),
 			"acknowledgment error is not of the right type")
 	}
 	return ack, err

x/ibc-rate-limit/ibc_module.go

@@ -125,7 +125,7 @@ func (im *IBCModule) OnRecvPacket(
 	relayer sdk.AccAddress,
 ) exported.Acknowledgement {
 	if err := ValidateReceiverAddress(packet); err != nil {
-		return osmoutils.NewStringErrorAcknowledgement(err.Error())
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrBadMessage, err.Error())
 	}
 
 	contract := im.ics4Middleware.GetParams(ctx)
@@ -137,10 +137,10 @@ func (im *IBCModule) OnRecvPacket(
 	err := CheckAndUpdateRateLimits(ctx, im.ics4Middleware.ContractKeeper, "recv_packet", contract, packet)
 	if err != nil {
 		if strings.Contains(err.Error(), "rate limit exceeded") {
-			return osmoutils.NewStringErrorAcknowledgement(types.ErrRateLimitExceeded.Error())
+			return osmoutils.NewEmitErrorAcknowledgement(ctx, types.ErrRateLimitExceeded)
 		}
 		fullError := sdkerrors.Wrap(types.ErrContractError, err.Error())
-		return osmoutils.NewStringErrorAcknowledgement(fullError.Error())
+		return osmoutils.NewEmitErrorAcknowledgement(ctx, fullError)
 	}
 
 	// if this returns an Acknowledgement that isn't successful, all state changes are discarded