Merge pull request #7930 from Agoric/mfig-bridge-no-error-stack

fix(cosmos): prevent Golang error wrapping stack frame divergence

.github/workflows/golangci-lint.yml

@@ -36,3 +36,10 @@ jobs:
           # only-new-issues: true
           only-new-issues: false
           working-directory: ./golang/cosmos
+      - name: forbid %w error-wrapping format specifier
+        run: |
+          set -e
+          if find ./golang/cosmos -name '*.go' ! -name '*_test.go' -print0 | xargs -0 grep '%w'; then
+            echo "Found %w in ./golang/cosmos; please use %s instead."
+            exit 1
+          fi

golang/cosmos/cmd/libdaemon/main_test.go

@@ -0,0 +1,36 @@
+package main_test
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+)
+
+func TestErrorStackTraces(t *testing.T) {
+	err := sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "my error %d", 123)
+	expected := "my error 123: insufficient fee"
+
+	// Check that sdkerrors.Wrapf(...).Error() does not leak stack.
+	got := err.Error()
+	if got != expected {
+		t.Fatalf("err.Error() %q should be %q", got, expected)
+	}
+
+	// Check that fmt.Errorf("... %s").Error() does not leak stack.
+	expected = "fail: " + expected
+	stringified := fmt.Errorf("fail: %s", err)
+	got = stringified.Error()
+	if got != expected {
+		t.Fatalf("stringified.Error() %q should be %q", got, expected)
+	}
+
+	// Check that fmt.Errorf("... %w").Error() leaks stack.
+	wrapped := fmt.Errorf("fail: %w", err)
+	got = wrapped.Error()
+	expectedAndStack := expected + " ["
+	if !strings.HasPrefix(got, expectedAndStack) {
+		t.Fatalf("wrapped.Error() %q should start with %q", got, expectedAndStack)
+	}
+}

golang/cosmos/daemon/cmd/genaccounts.go

@@ -61,15 +61,15 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 				info, err := kb.Key(args[0])
 				if err != nil {
-					return fmt.Errorf("failed to get address from Keybase: %w", err)
+					return fmt.Errorf("failed to get address from Keybase: %s", err)
 				}
 
 				addr = info.GetAddress()
 			}
 
 			coins, err := sdk.ParseCoinsNormalized(args[1])
 			if err != nil {
-				return fmt.Errorf("failed to parse coins: %w", err)
+				return fmt.Errorf("failed to parse coins: %s", err)
 			}
 
 			vestingStart, err := cmd.Flags().GetInt64(flagVestingStart)
@@ -87,7 +87,7 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 			vestingAmt, err := sdk.ParseCoinsNormalized(vestingAmtStr)
 			if err != nil {
-				return fmt.Errorf("failed to parse vesting amount: %w", err)
+				return fmt.Errorf("failed to parse vesting amount: %s", err)
 			}
 
 			// create concrete account type based on input parameters
@@ -119,20 +119,20 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 			}
 
 			if err := genAccount.Validate(); err != nil {
-				return fmt.Errorf("failed to validate new genesis account: %w", err)
+				return fmt.Errorf("failed to validate new genesis account: %s", err)
 			}
 
 			genFile := config.GenesisFile()
 			appState, genDoc, err := genutiltypes.GenesisStateFromGenFile(genFile)
 			if err != nil {
-				return fmt.Errorf("failed to unmarshal genesis state: %w", err)
+				return fmt.Errorf("failed to unmarshal genesis state: %s", err)
 			}
 
 			authGenState := authtypes.GetGenesisStateFromAppState(clientCtx.Codec, appState)
 
 			accs, err := authtypes.UnpackAccounts(authGenState.Accounts)
 			if err != nil {
-				return fmt.Errorf("failed to get accounts from any: %w", err)
+				return fmt.Errorf("failed to get accounts from any: %s", err)
 			}
 
 			if accs.Contains(addr) {
@@ -146,13 +146,13 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 			genAccs, err := authtypes.PackAccounts(accs)
 			if err != nil {
-				return fmt.Errorf("failed to convert accounts into any's: %w", err)
+				return fmt.Errorf("failed to convert accounts into any's: %s", err)
 			}
 			authGenState.Accounts = genAccs
 
 			authGenStateBz, err := clientCtx.Codec.MarshalJSON(&authGenState)
 			if err != nil {
-				return fmt.Errorf("failed to marshal auth genesis state: %w", err)
+				return fmt.Errorf("failed to marshal auth genesis state: %s", err)
 			}
 
 			appState[authtypes.ModuleName] = authGenStateBz
@@ -164,14 +164,14 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 			bankGenStateBz, err := clientCtx.Codec.MarshalJSON(bankGenState)
 			if err != nil {
-				return fmt.Errorf("failed to marshal bank genesis state: %w", err)
+				return fmt.Errorf("failed to marshal bank genesis state: %s", err)
 			}
 
 			appState[banktypes.ModuleName] = bankGenStateBz
 
 			appStateJSON, err := json.Marshal(appState)
 			if err != nil {
-				return fmt.Errorf("failed to marshal application genesis state: %w", err)
+				return fmt.Errorf("failed to marshal application genesis state: %s", err)
 			}
 
 			genDoc.AppState = appStateJSON

golang/cosmos/x/lien/lien.go

@@ -147,7 +147,7 @@ func (ch portHandler) handleGetStaking(ctx sdk.Context, msg portMessage) (string
 	}
 	bz, err := json.Marshal(&reply)
 	if err != nil {
-		return "", fmt.Errorf("cannot marshal %v: %w", reply, err)
+		return "", fmt.Errorf("cannot marshal %v: %s", reply, err)
 	}
 	return string(bz), nil
 }
@@ -157,11 +157,11 @@ func (ch portHandler) handleGetStaking(ctx sdk.Context, msg portMessage) (string
 func (ch portHandler) handleGetAccountState(ctx sdk.Context, msg portMessage) (string, error) {
 	addr, err := sdk.AccAddressFromBech32(msg.Address)
 	if err != nil {
-		return "", fmt.Errorf("cannot convert %s to address: %w", msg.Address, err)
+		return "", fmt.Errorf("cannot convert %s to address: %s", msg.Address, err)
 	}
 	denom := msg.Denom
 	if err = sdk.ValidateDenom(denom); err != nil {
-		return "", fmt.Errorf("invalid denom %s: %w", denom, err)
+		return "", fmt.Errorf("invalid denom %s: %s", denom, err)
 	}
 	state := ch.keeper.GetAccountState(ctx, addr)
 	reply := msgAccountState{
@@ -174,7 +174,7 @@ func (ch portHandler) handleGetAccountState(ctx sdk.Context, msg portMessage) (s
 	}
 	bz, err := json.Marshal(&reply)
 	if err != nil {
-		return "", fmt.Errorf("cannot marshal %v: %w", reply, err)
+		return "", fmt.Errorf("cannot marshal %v: %s", reply, err)
 	}
 	return string(bz), nil
 }
@@ -184,11 +184,11 @@ func (ch portHandler) handleGetAccountState(ctx sdk.Context, msg portMessage) (s
 func (ch portHandler) handleChangeLiened(ctx sdk.Context, msg portMessage) (string, error) {
 	addr, err := sdk.AccAddressFromBech32(msg.Address)
 	if err != nil {
-		return "", fmt.Errorf("cannot convert %s to address: %w", msg.Address, err)
+		return "", fmt.Errorf("cannot convert %s to address: %s", msg.Address, err)
 	}
 	denom := msg.Denom
 	if err = sdk.ValidateDenom(denom); err != nil {
-		return "", fmt.Errorf("invalid denom %s: %w", denom, err)
+		return "", fmt.Errorf("invalid denom %s: %s", denom, err)
 	}
 
 	newAmt, err := ch.keeper.ChangeLien(ctx, addr, denom, msg.Delta)
@@ -197,7 +197,7 @@ func (ch portHandler) handleChangeLiened(ctx sdk.Context, msg portMessage) (stri
 	}
 	bz, err := json.Marshal(&newAmt)
 	if err != nil {
-		return "", fmt.Errorf("cannot marshal %v: %w", newAmt, err)
+		return "", fmt.Errorf("cannot marshal %v: %s", newAmt, err)
 	}
 	return string(bz), nil
 }

golang/cosmos/x/lien/module.go

@@ -45,7 +45,7 @@ func (AppModuleBasic) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage {
 func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config client.TxEncodingConfig, data json.RawMessage) error {
 	var genesisState types.GenesisState
 	if err := cdc.UnmarshalJSON(data, &genesisState); err != nil {
-		return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err)
+		return fmt.Errorf("failed to unmarshal %s genesis state: %s", types.ModuleName, err)
 	}
 	return ValidateGenesis(genesisState)
 }

golang/cosmos/x/swingset/abci.go

@@ -115,7 +115,7 @@ func AfterCommitBlock(keeper Keeper) error {
 	// fmt.Fprintf(os.Stderr, "AFTER_COMMIT_BLOCK Returned from SwingSet: %s, %v\n", out, err)
 	if err != nil {
 		// Panic here, in the hopes that a replay from scratch will fix the problem.
-		panic(fmt.Errorf("AFTER_COMMIT_BLOCK failed: %w. Swingset is in an irrecoverable inconsistent state", err))
+		panic(fmt.Errorf("AFTER_COMMIT_BLOCK failed: %s. Swingset is in an irrecoverable inconsistent state", err))
 	}
 	return err
 }

golang/cosmos/x/vbank/vbank.go

@@ -135,10 +135,10 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
 	case "VBANK_GET_BALANCE":
 		addr, err := sdk.AccAddressFromBech32(msg.Address)
 		if err != nil {
-			return "", fmt.Errorf("cannot convert %s to address: %w", msg.Address, err)
+			return "", fmt.Errorf("cannot convert %s to address: %s", msg.Address, err)
 		}
 		if err = sdk.ValidateDenom(msg.Denom); err != nil {
-			return "", fmt.Errorf("invalid denom %s: %w", msg.Denom, err)
+			return "", fmt.Errorf("invalid denom %s: %s", msg.Denom, err)
 		}
 		coin := keeper.GetBalance(ctx.Context, addr, msg.Denom)
 		packet := coin.Amount.String()
@@ -152,18 +152,18 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
 	case "VBANK_GRAB":
 		addr, err := sdk.AccAddressFromBech32(msg.Sender)
 		if err != nil {
-			return "", fmt.Errorf("cannot convert %s to address: %w", msg.Sender, err)
+			return "", fmt.Errorf("cannot convert %s to address: %s", msg.Sender, err)
 		}
 		if err = sdk.ValidateDenom(msg.Denom); err != nil {
-			return "", fmt.Errorf("invalid denom %s: %w", msg.Denom, err)
+			return "", fmt.Errorf("invalid denom %s: %s", msg.Denom, err)
 		}
 		value, ok := sdk.NewIntFromString(msg.Amount)
 		if !ok {
 			return "", fmt.Errorf("cannot convert %s to int", msg.Amount)
 		}
 		coins := sdk.NewCoins(sdk.NewCoin(msg.Denom, value))
 		if err := keeper.GrabCoins(ctx.Context, addr, coins); err != nil {
-			return "", fmt.Errorf("cannot grab %s coins: %w", coins.Sort().String(), err)
+			return "", fmt.Errorf("cannot grab %s coins: %s", coins.Sort().String(), err)
 		}
 		addressToBalances := make(map[string]sdk.Coins, 1)
 		addressToBalances[msg.Sender] = sdk.NewCoins(sdk.NewInt64Coin(msg.Denom, 1))
@@ -180,18 +180,18 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
 	case "VBANK_GIVE":
 		addr, err := sdk.AccAddressFromBech32(msg.Recipient)
 		if err != nil {
-			return "", fmt.Errorf("cannot convert %s to address: %w", msg.Recipient, err)
+			return "", fmt.Errorf("cannot convert %s to address: %s", msg.Recipient, err)
 		}
 		if err = sdk.ValidateDenom(msg.Denom); err != nil {
-			return "", fmt.Errorf("invalid denom %s: %w", msg.Denom, err)
+			return "", fmt.Errorf("invalid denom %s: %s", msg.Denom, err)
 		}
 		value, ok := sdk.NewIntFromString(msg.Amount)
 		if !ok {
 			return "", fmt.Errorf("cannot convert %s to int", msg.Amount)
 		}
 		coins := sdk.NewCoins(sdk.NewCoin(msg.Denom, value))
 		if err := keeper.SendCoins(ctx.Context, addr, coins); err != nil {
-			return "", fmt.Errorf("cannot give %s coins: %w", coins.Sort().String(), err)
+			return "", fmt.Errorf("cannot give %s coins: %s", coins.Sort().String(), err)
 		}
 		addressToBalances := make(map[string]sdk.Coins, 1)
 		addressToBalances[msg.Recipient] = sdk.NewCoins(sdk.NewInt64Coin(msg.Denom, 1))
@@ -212,7 +212,7 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
 		}
 		coins := sdk.NewCoins(sdk.NewCoin(msg.Denom, value))
 		if err := keeper.StoreRewardCoins(ctx.Context, coins); err != nil {
-			return "", fmt.Errorf("cannot store reward %s coins: %w", coins.Sort().String(), err)
+			return "", fmt.Errorf("cannot store reward %s coins: %s", coins.Sort().String(), err)
 		}
 		if err != nil {
 			return "", err