Add DeterministicError type for redactError

x/wasm/keeper/keeper.go

@@ -324,7 +324,7 @@ func (k Keeper) instantiate(
 		return nil, nil, errorsmod.Wrap(types.ErrVMError, err.Error())
 	}
 	if res.Err != "" {
-		return nil, nil, errorsmod.Wrap(types.ErrInstantiateFailed, res.Err)
+		return nil, nil, types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrInstantiateFailed, res.Err))
 	}
 
 	// persist instance first
@@ -408,7 +408,7 @@ func (k Keeper) execute(ctx context.Context, contractAddress, caller sdk.AccAddr
 		return nil, errorsmod.Wrap(types.ErrVMError, execErr.Error())
 	}
 	if res.Err != "" {
-		return nil, errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return nil, types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	sdkCtx.EventManager().EmitEvent(sdk.NewEvent(
@@ -485,7 +485,7 @@ func (k Keeper) migrate(
 		return nil, errorsmod.Wrap(types.ErrVMError, err.Error())
 	}
 	if res.Err != "" {
-		return nil, errorsmod.Wrap(types.ErrMigrationFailed, res.Err)
+		return nil, types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrMigrationFailed, res.Err))
 	}
 	// delete old secondary index entry
 	err = k.removeFromContractCodeSecondaryIndex(ctx, contractAddress, k.mustGetLastContractHistoryEntry(sdkCtx, contractAddress))
@@ -550,7 +550,7 @@ func (k Keeper) Sudo(ctx context.Context, contractAddress sdk.AccAddress, msg []
 		return nil, errorsmod.Wrap(types.ErrVMError, execErr.Error())
 	}
 	if res.Err != "" {
-		return nil, errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return nil, types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	sdkCtx.EventManager().EmitEvent(sdk.NewEvent(
@@ -590,7 +590,7 @@ func (k Keeper) reply(ctx sdk.Context, contractAddress sdk.AccAddress, reply was
 		return nil, errorsmod.Wrap(types.ErrVMError, execErr.Error())
 	}
 	if res.Err != "" {
-		return nil, errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return nil, types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	ctx.EventManager().EmitEvent(sdk.NewEvent(

x/wasm/keeper/msg_dispatcher.go

@@ -209,18 +209,19 @@ func redactError(err error) error {
 		return err
 	}
 
+	// If it is a DeterministicError, we can safely return it without redaction.
+	// We only check the top level error to avoid changes in the error chain becoming
+	// consensus-breaking.
+	if _, ok := err.(types.DeterministicError); ok {
+		return err
+	}
+
 	// FIXME: do we want to hardcode some constant string mappings here as well?
 	// Or better document them? (SDK error string may change on a patch release to fix wording)
 	// sdk/11 is out of gas
 	// sdk/5 is insufficient funds (on bank send)
 	// (we can theoretically redact less in the future, but this is a first step to safety)
 	codespace, code, _ := errorsmod.ABCIInfo(err, false)
-
-	// Also do not redact any errors that are coming from the contract,
-	// as they are always deterministic
-	if codespace == types.DefaultCodespace && contains(types.ContractErrorCodes, code) {
-		return err
-	}
 	return fmt.Errorf("codespace: %s, code: %d", codespace, code)
 }
 

x/wasm/keeper/recurse_test.go

@@ -264,7 +264,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) {
 			expectQueriesFromContract: 10,
 			expectOutOfGas:            false,
 			expectError:               "query wasm contract failed", // Error we get from the contract instance doing the failing query, not wasmd
-			expectedGas:               10*(GasWork2k+GasReturnHashed) + 3124,
+			expectedGas:               10*(GasWork2k+GasReturnHashed) - 249,
 		},
 	}
 

x/wasm/keeper/relay.go

@@ -76,7 +76,7 @@ func (k Keeper) OnConnectChannel(
 		return errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
 	}
 	if res.Err != "" {
-		return errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	return k.handleIBCBasicContractResponse(ctx, contractAddr, contractInfo.IBCPortID, res.Ok)
@@ -110,7 +110,7 @@ func (k Keeper) OnCloseChannel(
 		return errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
 	}
 	if res.Err != "" {
-		return errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	return k.handleIBCBasicContractResponse(ctx, contractAddr, contractInfo.IBCPortID, res.Ok)
@@ -201,7 +201,7 @@ func (k Keeper) OnAckPacket(
 		return errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
 	}
 	if res.Err != "" {
-		return errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	return k.handleIBCBasicContractResponse(ctx, contractAddr, contractInfo.IBCPortID, res.Ok)
@@ -232,7 +232,7 @@ func (k Keeper) OnTimeoutPacket(
 		return errorsmod.Wrap(types.ErrExecuteFailed, execErr.Error())
 	}
 	if res.Err != "" {
-		return errorsmod.Wrap(types.ErrExecuteFailed, res.Err)
+		return types.MarkErrorDeterministic(errorsmod.Wrap(types.ErrExecuteFailed, res.Err))
 	}
 
 	return k.handleIBCBasicContractResponse(ctx, contractAddr, contractInfo.IBCPortID, res.Ok)

x/wasm/types/errors.go

@@ -10,10 +10,6 @@ import (
 var (
 	DefaultCodespace = ModuleName
 
-	// ContractErrorCodes are the error codes for errors returned by the contract
-	// Since contract execution is deterministic, the errors are also deterministic
-	ContractErrorCodes = []uint32{InstantiateErrorCode, ExecuteErrorCode, QueryErrorCode, MigrateErrorCode}
-
 	// Note: never use code 1 for any errors - that is reserved for ErrInternal in the core cosmos sdk
 
 	// ErrCreateFailed error for wasm code that has already been uploaded or failed
@@ -23,10 +19,10 @@ var (
 	ErrAccountExists = errorsmod.Register(DefaultCodespace, 3, "contract account already exists")
 
 	// ErrInstantiateFailed error for rust instantiate contract failure
-	ErrInstantiateFailed = errorsmod.Register(DefaultCodespace, InstantiateErrorCode, "instantiate wasm contract failed")
+	ErrInstantiateFailed = errorsmod.Register(DefaultCodespace, 4, "instantiate wasm contract failed")
 
 	// ErrExecuteFailed error for rust execution contract failure
-	ErrExecuteFailed = errorsmod.Register(DefaultCodespace, ExecuteErrorCode, "execute wasm contract failed")
+	ErrExecuteFailed = errorsmod.Register(DefaultCodespace, 5, "execute wasm contract failed")
 
 	// ErrGasLimit error for out of gas
 	ErrGasLimit = errorsmod.Register(DefaultCodespace, 6, "insufficient gas")
@@ -38,13 +34,13 @@ var (
 	ErrNotFound = errorsmod.Register(DefaultCodespace, 8, "not found")
 
 	// ErrQueryFailed error for rust smart query contract failure
-	ErrQueryFailed = errorsmod.Register(DefaultCodespace, QueryErrorCode, "query wasm contract failed")
+	ErrQueryFailed = errorsmod.Register(DefaultCodespace, 9, "query wasm contract failed")
 
 	// ErrInvalidMsg error when we cannot process the error returned from the contract
 	ErrInvalidMsg = errorsmod.Register(DefaultCodespace, 10, "invalid CosmosMsg from the contract")
 
 	// ErrMigrationFailed error for rust execution contract failure
-	ErrMigrationFailed = errorsmod.Register(DefaultCodespace, MigrateErrorCode, "migrate wasm contract failed")
+	ErrMigrationFailed = errorsmod.Register(DefaultCodespace, 11, "migrate wasm contract failed")
 
 	// ErrEmpty error for empty content
 	ErrEmpty = errorsmod.Register(DefaultCodespace, 12, "empty")
@@ -95,14 +91,6 @@ var (
 	ErrVMError = errorsmod.Register(DefaultCodespace, 29, "wasmvm error")
 )
 
-// Error codes for wasm contract errors
-const (
-	InstantiateErrorCode = 4
-	ExecuteErrorCode     = 5
-	QueryErrorCode       = 9
-	MigrateErrorCode     = 11
-)
-
 // WasmVMErrorable mapped error type in wasmvm and are not redacted
 type WasmVMErrorable interface {
 	// ToWasmVMError convert instance to wasmvm friendly error if possible otherwise root cause. never nil
@@ -164,3 +152,32 @@ func (e WasmVMFlavouredError) Wrap(desc string) error { return errorsmod.Wrap(e,
 func (e WasmVMFlavouredError) Wrapf(desc string, args ...interface{}) error {
 	return errorsmod.Wrapf(e, desc, args...)
 }
+
+// DeterministicError is a wrapper type around an error that the creator guarantees to have
+// a deterministic error message.
+// This means that the `Error()` function must always return the same string on all nodes.
+// DeterministicErrors are not redacted when returned to a contract,
+// so not upholding this guarantee can lead to consensus failures.
+type DeterministicError struct {
+	error
+}
+
+var _ error = DeterministicError{}
+
+// MarkErrorDeterministic marks an error as deterministic.
+// Make sure to only do that if the error message is deterministic between systems.
+// See [DeterministicError] for more details.
+func MarkErrorDeterministic(e error) DeterministicError {
+	return DeterministicError{error: e}
+}
+
+// Unwrap implements the built-in errors.Unwrap
+func (e DeterministicError) Unwrap() error {
+	return e.error
+}
+
+// Cause is the same as unwrap but used by ABCIInfo
+// By returning the wrapped error here, we ensure
+func (e DeterministicError) Cause() error {
+	return e.Unwrap()
+}

x/wasm/types/errors_test.go

@@ -94,3 +94,19 @@ func TestWasmVMFlavouredError(t *testing.T) {
 		t.Run(name, spec.exec)
 	}
 }
+
+func TestDeterministicError(t *testing.T) {
+	inner := ErrInstantiateFailed
+	err := MarkErrorDeterministic(inner)
+
+	// behaves like a wrapper around inner error
+	assert.Equal(t, inner.Error(), err.Error())
+	assert.Equal(t, inner, err.Cause())
+	assert.Equal(t, inner, err.Unwrap())
+
+	// also works with ABCIInfo
+	codespace, code, _ := errorsmod.ABCIInfo(err, false)
+	innerCodeSpace, innerCode, _ := errorsmod.ABCIInfo(inner, false)
+	assert.Equal(t, innerCodeSpace, codespace)
+	assert.Equal(t, innerCode, code)
+}