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 <[email protected]>
(cherry picked from commit e42d0d2)

* add changelog

---------

Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

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

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

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())
+		})
+	}
+}