From dc509e64d561016735202ea606177c7dc8edf9aa Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 14 Feb 2024 22:35:55 +0100 Subject: [PATCH] feat: add NewErrorAcknowledgementWithCodespace to allow codespaces in ack errors (backport #5788) (#5842) * feat: Add NewErrorAcknowledgementWithCodespace. (#5788) This new constructor comes with an implicit requirement that people do not change a given err's codespace between patch/non-state machine breaking versions. The codespace is inserted into the returned error to assist in debugging since the error code on its own is insufficient to debug the source of the error. Co-authored-by: Cian Hatton (cherry picked from commit e42d0d2768f87305395933939fab6324613b1226) * add changelog --------- Co-authored-by: DimitrisJim Co-authored-by: Carlos Rodriguez --- CHANGELOG.md | 16 ++++++++++ .../core/04-channel/types/acknowledgement.go | 20 ++++++++++++ .../04-channel/types/acknowledgement_test.go | 32 +++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53ead4c1095..1027c833d2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,22 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [Unreleased] + +### Dependencies + +### API Breaking + +### State Machine Breaking + +### Improvements + +### Features + +* [\#5788](https://github.com/cosmos/ibc-go/pull/5788) Add `NewErrorAcknowledgementWithCodespace` to allow codespaces in ack errors. + +### Bug Fixes + ## [v8.1.0](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0) - 2024-01-31 ### Dependencies diff --git a/modules/core/04-channel/types/acknowledgement.go b/modules/core/04-channel/types/acknowledgement.go index 27617d391f5..bfe7247f9cc 100644 --- a/modules/core/04-channel/types/acknowledgement.go +++ b/modules/core/04-channel/types/acknowledgement.go @@ -26,6 +26,26 @@ func NewResultAcknowledgement(result []byte) Acknowledgement { } } +// NewErrorAcknowledgementWithCodespace returns a new instance of Acknowledgement using an Acknowledgement_Error +// type in the Response field. +// NOTE: The error includes the ABCI codespace and code in the error string to provide more information about the module +// that generated the error. This is useful for debugging but can potentially introduce non-determinism if care is +// not taken to ensure the codespace doesn't change in non state-machine breaking versions. +func NewErrorAcknowledgementWithCodespace(err error) Acknowledgement { + // The ABCI code is included in the abcitypes.ResponseDeliverTx hash + // constructed in Tendermint and is therefore deterministic. + // However, a code without codespace is incomplete information (e.g. sdk/5 and wasm/5 are + // different errors). We add this codespace here, in oder to provide a meaningful error + // identifier which means changing the codespace of an error becomes a consensus breaking change. + codespace, code, _ := errorsmod.ABCIInfo(err, false) + + return Acknowledgement{ + Response: &Acknowledgement_Error{ + Error: fmt.Sprintf("ABCI error: %s/%d: %s", codespace, code, ackErrorString), + }, + } +} + // 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 diff --git a/modules/core/04-channel/types/acknowledgement_test.go b/modules/core/04-channel/types/acknowledgement_test.go index 71234065321..e6d30873deb 100644 --- a/modules/core/04-channel/types/acknowledgement_test.go +++ b/modules/core/04-channel/types/acknowledgement_test.go @@ -141,3 +141,35 @@ func (suite *TypesTestSuite) TestAcknowledgementError() { suite.Require().Equal(ack, ackSameABCICode) suite.Require().NotEqual(ack, ackDifferentABCICode) } + +func (suite TypesTestSuite) TestAcknowledgementWithCodespace() { //nolint:govet // this is a test, we are okay with copying locks + testCases := []struct { + name string + ack types.Acknowledgement + expBytes []byte + }{ + { + "valid failed ack", + types.NewErrorAcknowledgementWithCodespace(ibcerrors.ErrInsufficientFunds), + []byte(`{"error":"ABCI error: ibc/3: error handling packet: see events for details"}`), + }, + { + "unknown error", + types.NewErrorAcknowledgementWithCodespace(fmt.Errorf("unknown error")), + []byte(`{"error":"ABCI error: undefined/1: error handling packet: see events for details"}`), + }, + { + "nil error", + types.NewErrorAcknowledgementWithCodespace(nil), + []byte(`{"error":"ABCI error: /0: error handling packet: see events for details"}`), + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.Require().Equal(tc.expBytes, tc.ack.Acknowledgement()) + }) + } +}