Don't use arbitrary error message as the acknowledgement

[yihuang]

Summary

currently, the transfer app uses whatever error message is returned as acknowledgment (code), which makes the error message a part of consensus state, so the error message change will be a consensus breaking change, which may not align with people's intuition, making it easier to bring in consensus breaking changes in minor upgrades.

Problem Definition

Proposal

Use static errors, suggested by @leejw51crypto

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[damiannolan]

Hi @yihuang, thanks for opening this issue. The code which you link is calling types.NewErrorAcknowledgement(err) from the transfer types pkg. See here.

This function extracts the abci error code and then includes a static ack error string (string constant).

// NewErrorAcknowledgement returns a deterministic error string which may be used in
// the packet acknowledgement.
func NewErrorAcknowledgement(err error) channeltypes.Acknowledgement {
	// the ABCI code is included in the abcitypes.ResponseDeliverTx hash
	// constructed in Tendermint and is therefore deterministic
	_, code, _ := sdkerrors.ABCIInfo(err, false) // discard non-determinstic codespace and log values

	errorString := fmt.Sprintf("ABCI code: %d: %s", code, ackErrorString)

	return channeltypes.NewErrorAcknowledgement(errorString)
}

I believe this is the desired behaviour which you propose. Please review this and feel free to close the issue or raise any other concerns. Many thanks!

edit: this will be included in the v3 release

[yihuang]

I see, I checked that in v2 and v3, it's indeed as you described, the issue I described is only in v1. I'll close this.

EDIT: this issue is fixed in v2.1.x and above, #993.