Add codespace to NewErrorAcknowledgement

[webmaster128]

Right now, NewErrorAcknowledgement only stored an error code without the corresponsing codespace. But the codespace is an incomplete identifier for an error since e.g. sdk/5 is a different error than wasm/5. Having this codespace available would be a huge step forward without creating significant risk of non-determinism. We'd only require that a codespace of an error does not change in a patch release, which is not too much to ask IMO.

So what I would like to propose is this diff:

--- a/modules/core/04-channel/types/acknowledgement.go
+++ b/modules/core/04-channel/types/acknowledgement.go
@@ -29,13 +29,16 @@ func NewResultAcknowledgement(result []byte) Acknowledgement {
 // 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 error) Acknowledgement {
-       // the ABCI code is included in the abcitypes.ResponseDeliverTx hash
-       // constructed in Tendermint and is therefore deterministic
-       _, code, _ := errorsmod.ABCIInfo(err, false) // discard non-deterministic codespace and log values
+       // 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) // discard non-deterministic log values
 
        return Acknowledgement{
                Response: &Acknowledgement_Error{
-                       Error: fmt.Sprintf("ABCI code: %d: %s", code, ackErrorString),
+                       Error: fmt.Sprintf("ABCI error: %s/%d: %s", codespace, code, ackErrorString),
                },
        }
 }

This diff is a compromise between the error code alone and the full error message.

Some related discussions around that matter:

• chore: replace error string in transfer acks with const #818
• update NewErrorAcknowledgement #4749
• NewErrorAcknowledgement misses codespace in error message #3390
• Feature request: show text along with codespace numbers CosmWasm/wasmd#1740
• Add codespace into the determinsticExecTxResult value  cometbft/cometbft#844

Furthermore I'd like to promote <codespace>/<code> as a notation for error codes to highlight that the number is namespaced under the codespace.

Please let me know if I got something wrong here.

[DimitrisJim]

Discussed this and decided to add a new NewErrorAcknowledgementWithCodespace to allow opt-in by people who need it and are aware of addition of non-deterministic code-space. This would also allow a cleaner backport. Removing needs discussion tag.

[webmaster128]

Sounds good to me, thank you. We'd use that in wasmd as soon as it is available.

[crodriguezvega]

@webmaster128 We are going to release this in v8.2.0 (in Q2, most likely first half of the quarter, but will give a more accurate timeline later on).

[webmaster128]

Thanks, no rush. I think I can even copy the function if needed in the meantime

[crodriguezvega]

@webmaster128 We have released this addition in v8.3.0.