From e42d0d2768f87305395933939fab6324613b1226 Mon Sep 17 00:00:00 2001
From: DimitrisJim <d.f.hilliard@gmail.com>
Date: Wed, 14 Feb 2024 15:43:08 +0200
Subject: [PATCH] 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 <cian@interchain.io>
---
 .../core/04-channel/types/acknowledgement.go  | 20 ++++++++++++
 .../04-channel/types/acknowledgement_test.go  | 32 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/modules/core/04-channel/types/acknowledgement.go b/modules/core/04-channel/types/acknowledgement.go
index e8db340bfeb..34d615a235a 100644
--- a/modules/core/04-channel/types/acknowledgement.go
+++ b/modules/core/04-channel/types/acknowledgement.go
@@ -24,6 +24,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 1db3476ad47..9b8c93c80aa 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())
+		})
+	}
+}