chg: address PR comments

client/keys/parse.go

@@ -16,10 +16,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/bech32"
 )
 
-func bech32Prefixes(config *sdk.Config) []string {
-	return []string{}
-}
-
 type hexOutput struct {
 	Human string `json:"human"`
 	Bytes string `json:"bytes"`
@@ -38,18 +34,14 @@ type bech32Output struct {
 }
 
 func newBech32Output(config *sdk.Config, bs []byte) bech32Output {
-	bech32Prefixes := bech32Prefixes(config)
-	out := bech32Output{Formats: make([]string, len(bech32Prefixes))}
-
-	for i, prefix := range bech32Prefixes {
-		bech32Addr, err := bech32.ConvertAndEncode(prefix, bs)
-		if err != nil {
-			panic(err)
-		}
-
-		out.Formats[i] = bech32Addr
+	out := bech32Output{Formats: make([]string, 1)}
+	bech32Addr, err := bech32.ConvertAndEncode("", bs)
+	if err != nil {
+		panic(err)
 	}
 
+	out.Formats[0] = bech32Addr
+
 	return out
 }
 

client/keys/show.go

@@ -107,7 +107,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
 		return errors.New("cannot use --output with --address or --pubkey")
 	}
 
-	bechKeyOut, err := getHexKeyOut("")
+	hexKeyOut, err := getHexKeyOut("")
 	if err != nil {
 		return err
 	}
@@ -118,7 +118,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
 
 	switch {
 	case isShowAddr, isShowPubKey:
-		ko, err := bechKeyOut(k)
+		ko, err := hexKeyOut(k)
 		if err != nil {
 			return err
 		}
@@ -131,7 +131,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
 			return err
 		}
 	default:
-		if err := printKeyringRecord(cmd.OutOrStdout(), k, bechKeyOut, outputFormat); err != nil {
+		if err := printKeyringRecord(cmd.OutOrStdout(), k, hexKeyOut, outputFormat); err != nil {
 			return err
 		}
 	}
@@ -192,7 +192,7 @@ func validateMultisigThreshold(k, nKeys int) error {
 	return nil
 }
 
-func getHexKeyOut(bechPrefix string) (bechKeyOutFn, error) {
+func getHexKeyOut(bechPrefix string) (hexKeyOutFn, error) {
 	if bechPrefix == "" {
 		return MkAccKeyOutput, nil
 	}

client/keys/show_test.go

@@ -187,28 +187,23 @@ func Test_validateMultisigThreshold(t *testing.T) {
 	}
 }
 
-// TODO HV2: removed as irrelevant to Heimdall
-/*
-func Test_getBechKeyOut(t *testing.T) {
+func Test_getHexKeyOut(t *testing.T) {
 	type args struct {
-		bechPrefix string
+		hexPrefix string
 	}
 	tests := []struct {
 		name    string
 		args    args
-		want    bechKeyOutFn
+		want    hexKeyOutFn
 		wantErr bool
 	}{
-		{"empty", args{""}, nil, true},
+		{"empty", args{""}, nil, false},
 		{"wrong", args{"???"}, nil, true},
-		{"acc", args{sdk.PrefixAccount}, MkAccKeyOutput, false},
-		{"val", args{sdk.PrefixValidator}, MkValKeyOutput, false},
-		{"cons", args{sdk.PrefixConsensus}, MkConsKeyOutput, false},
 	}
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := getBechKeyOut(tt.args.bechPrefix)
+			got, err := getHexKeyOut(tt.args.hexPrefix)
 			if tt.wantErr {
 				require.Error(t, err)
 			} else {
@@ -218,4 +213,3 @@ func Test_getBechKeyOut(t *testing.T) {
 		})
 	}
 }
-*/

client/keys/utils.go

@@ -11,10 +11,10 @@ import (
 	cryptokeyring "github.com/cosmos/cosmos-sdk/crypto/keyring"
 )
 
-type bechKeyOutFn func(k *cryptokeyring.Record) (KeyOutput, error)
+type hexKeyOutFn func(k *cryptokeyring.Record) (KeyOutput, error)
 
-func printKeyringRecord(w io.Writer, k *cryptokeyring.Record, bechKeyOut bechKeyOutFn, output string) error {
-	ko, err := bechKeyOut(k)
+func printKeyringRecord(w io.Writer, k *cryptokeyring.Record, hexKeyOut hexKeyOutFn, output string) error {
+	ko, err := hexKeyOut(k)
 	if err != nil {
 		return err
 	}

simapp/ante.go

@@ -31,6 +31,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 		return nil, errors.New("sign mode handler is required for ante builder")
 	}
 
+	// TODO HV2: choose here the decorators that fit heimdall needs
 	anteDecorators := []sdk.AnteDecorator{
 		ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
 		circuitante.NewCircuitBreakerDecorator(options.CircuitKeeper),

x/auth/ante/ante.go

@@ -45,10 +45,10 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 		NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
 		NewValidateBasicDecorator(),
 		// NewTxTimeoutHeightDecorator(), // TODO HV2: this is not present in heimdall. Is it safe to remove?
-		NewValidateMemoDecorator(options.AccountKeeper), // TODO HV2: can we keep this despite we don't support Memo? Or is it better/safer to remove it?
-		// NewConsumeGasForTxSizeDecorator(options.AccountKeeper), // TODO HV2: this was removed in heimdall's `auth/ante.go` (original ancestor's method `newCtx.GasMeter().ConsumeGas`)
+		NewValidateMemoDecorator(options.AccountKeeper),                                                                // TODO HV2: can we keep this despite we don't support Memo? Or is it better/safer to remove it?
+		NewConsumeGasForTxSizeDecorator(options.AccountKeeper),                                                         // TODO HV2: removed `ConsumeGas` in it as done in heimdall's `auth/ante.go` (original ancestor's method `newCtx.GasMeter().ConsumeGas`)
 		NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker), // TODO HV2: heavily changed, double check
-		NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
+		NewSetPubKeyDecorator(options.AccountKeeper),                                                                   // SetPubKeyDecorator must be called before all signature verification decorators
 		// NewValidateSigCountDecorator(options.AccountKeeper), // TODO HV2: this was removed in heimdall's `auth/ante.go` (original ancestor's method `ValidateSigCount`)
 		NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer),
 		NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),

x/auth/ante/ante_test.go

@@ -652,7 +652,6 @@ func TestAnteHandlerFees(t *testing.T) {
 
 // Test logic around memo gas consumption.
 func TestAnteHandlerMemoGas(t *testing.T) {
-	t.Skip("skipping test as not relevant to Heimdall (no memo)")
 	testCases := []TestCase{
 		{
 			"tx does not have enough gas",
@@ -666,8 +665,8 @@ func TestAnteHandlerMemoGas(t *testing.T) {
 			},
 			false,
 			false,
-			sdkerrors.ErrInvalidGasLimit,
-			true,
+			sdkerrors.ErrOutOfGas,
+			false,
 		},
 		{
 			"tx with memo doesn't have enough gas",
@@ -684,7 +683,7 @@ func TestAnteHandlerMemoGas(t *testing.T) {
 			false,
 			false,
 			sdkerrors.ErrOutOfGas,
-			true,
+			false,
 		},
 		{
 			"memo too large",
@@ -701,13 +700,15 @@ func TestAnteHandlerMemoGas(t *testing.T) {
 			false,
 			false,
 			sdkerrors.ErrMemoTooLarge,
-			true,
+			false,
 		},
 		{
 			"tx with memo has enough gas",
 			func(suite *AnteTestSuite) TestCaseArgs {
 				accs := suite.CreateTestAccounts(1)
 				suite.txBuilder.SetMemo(strings.Repeat("0123456789", 10))
+				// TODO HV2 in heimdall the fee is taken from params.GetTxFees() (hence not zero here), so we expect a call to SendCoinsFromAccountToModule
+				suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
 				return TestCaseArgs{
 					feeAmount: sdk.NewCoins(sdk.NewInt64Coin("matic", 0)),
 					gasLimit:  60000,
@@ -717,7 +718,7 @@ func TestAnteHandlerMemoGas(t *testing.T) {
 			false,
 			true,
 			nil,
-			true,
+			false,
 		},
 	}
 

x/auth/ante/basic.go

@@ -98,7 +98,7 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
 	}
 	params := cgts.ak.GetParams(ctx)
 
-	ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*storetypes.Gas(len(ctx.TxBytes())), "txSize")
+	// ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*storetypes.Gas(len(ctx.TxBytes())), "txSize")
 
 	// simulate gas cost for signatures in simulate mode
 	if simulate {
@@ -146,7 +146,7 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
 				cost *= params.TxSigLimit
 			}
 
-			ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize")
+			// ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize")
 		}
 	}
 

x/auth/ante/basic_test.go

@@ -147,9 +147,10 @@ func TestConsumeGasForTxSize(t *testing.T) {
 			suite.ctx, err = antehandler(suite.ctx, tx, false)
 			require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err)
 
+			// TODO HV2: removed as `ConsumeGas` is not used in heimdall
 			// require that decorator consumes expected amount of gas
-			consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas
-			require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas")
+			// consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas
+			// require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas")
 
 			// simulation must not underestimate gas of this decorator even with nil signatures
 			txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx)
@@ -165,15 +166,16 @@ func TestConsumeGasForTxSize(t *testing.T) {
 			// Set suite.ctx with smaller simulated TxBytes manually
 			suite.ctx = suite.ctx.WithTxBytes(simTxBytes)
 
-			beforeSimGas := suite.ctx.GasMeter().GasConsumed()
+			// beforeSimGas := suite.ctx.GasMeter().GasConsumed()
 
 			// run antehandler with simulate=true
 			suite.ctx, err = antehandler(suite.ctx, tx, true)
-			consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas
+			// consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas
 
 			// require that antehandler passes and does not underestimate decorator cost
 			require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err)
-			require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas)
+			// TODO HV2: removed as `ConsumeGas` is not used in heimdall
+			// require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas)
 		})
 	}
 }

x/auth/ante/validator_tx_fee.go

@@ -23,7 +23,7 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx, params type
 	gas := params.GetMaxTxGas()
 	feeCoins := sdk.Coins{sdk.Coin{Denom: types.FeeToken, Amount: amount}}
 
-	// TODO HV2: removed as not present in heimdall
+	// TODO HV2: removed because it had also been removed in heimdall
 	// Ensure that the provided fees meet a minimum threshold for the validator,
 	// if this is a CheckTx. This is only for local mempool purposes, and thus
 	// is only ran on check tx.

x/auth/keeper/genesis.go

@@ -22,19 +22,20 @@ func (ak AccountKeeper) InitGenesis(ctx sdk.Context, data types.GenesisState, pr
 
 	// Set the accounts and make sure the global account number matches the largest account number (even if zero).
 	var lastAccNum *uint64
-	for _, acc := range accounts {
-		accNum := acc.GetAccountNumber()
+	for _, gacc := range accounts {
+		accNum := gacc.GetAccountNumber()
 		for lastAccNum == nil || *lastAccNum < accNum {
 			n := ak.NextAccountNumber(ctx)
 			lastAccNum = &n
 		}
 
-		// TODO HV2: this is imported from heimdall
+		// TODO HV2: this is imported (and modified) from heimdall
+		acc := sdk.AccountI(gacc) //nolint
 		baseAcc := types.NewBaseAccount(acc.GetAddress(), acc.GetPubKey(), accNum, acc.GetSequence())
 
 		// execute account processors
 		for _, p := range processors {
-			acc = p(&acc, baseAcc) //nolint
+			acc = p(&gacc, baseAcc) //nolint
 		}
 		ak.SetAccount(ctx, acc)
 	}