Refactor processSign()

x/auth/ante.go

@@ -147,7 +147,6 @@ func validateBasic(tx StdTx) (err sdk.Error) {
 
 // verify the signature and increment the sequence.
 // if the account doesn't have a pubkey, set it.
-// nolint: gocyclo
 func processSig(
 	ctx sdk.Context, am AccountMapper,
 	addr sdk.AccAddress, sig StdSignature, signBytes []byte, simulate bool) (
@@ -178,28 +177,15 @@ func processSig(
 	}
 	// If pubkey is not known for account,
 	// set it from the StdSignature.
-	pubKey := acc.GetPubKey()
-	if !simulate && (pubKey == nil) {
-		pubKey = sig.PubKey
-		if pubKey == nil {
-			return nil, sdk.ErrInvalidPubKey("PubKey not found").Result()
-		}
-		if !bytes.Equal(pubKey.Address(), addr) {
-			return nil, sdk.ErrInvalidPubKey(
-				fmt.Sprintf("PubKey does not match Signer address %v", addr)).Result()
-		}
-	}
-	if simulate && (pubKey == nil) {
-		// In case pubkey is nil, both signature verification and gasKVStore.Set()
-		// will consume the largest amount
-		pubKey = secp256k1.GenPrivKey().PubKey()
+	pubKey, res := processPubKey(acc, sig, simulate)
+	if !res.IsOK() {
+		return nil, res
 	}
 	err = acc.SetPubKey(pubKey)
 	if err != nil {
 		return nil, sdk.ErrInternal("setting PubKey on signer's account").Result()
 	}
 
-	// Check sig.
 	consumeSignatureVerificationGas(ctx.GasMeter(), pubKey)
 	if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) {
 		return nil, sdk.ErrUnauthorized("signature verification failed").Result()
@@ -208,6 +194,33 @@ func processSig(
 	return
 }
 
+func processPubKey(acc Account, sig StdSignature, simulate bool) (crypto.PubKey, sdk.Result) {
+	// If pubkey is not known for account,
+	// set it from the StdSignature.
+	pubKey := acc.GetPubKey()
+	if simulate {
+		// In simulate mode the transaction comes with no signatures, thus
+		// if the account's pubkey is nil, both signature verification
+		// and gasKVStore.Set() shall consume the largest amount, i.e.
+		// it takes more gas to verifiy secp256k1 keys than ed25519 ones.
+		if pubKey == nil {
+			return secp256k1.GenPrivKey().PubKey(), sdk.Result{}
+		}
+		return pubKey, sdk.Result{}
+	}
+	if pubKey == nil {
+		pubKey = sig.PubKey
+		if pubKey == nil {
+			return nil, sdk.ErrInvalidPubKey("PubKey not found").Result()
+		}
+		if !bytes.Equal(pubKey.Address(), acc.GetAddress()) {
+			return nil, sdk.ErrInvalidPubKey(
+				fmt.Sprintf("PubKey does not match Signer address %v", acc.GetAddress())).Result()
+		}
+	}
+	return pubKey, sdk.Result{}
+}
+
 func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) {
 	switch pubkey.(type) {
 	case ed25519.PubKeyEd25519:

x/auth/ante_test.go

@@ -4,14 +4,14 @@ import (
 	"fmt"
 	"testing"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	wire "github.com/cosmos/cosmos-sdk/wire"
 	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/crypto/secp256k1"
 	"github.com/tendermint/tendermint/libs/log"
-
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	wire "github.com/cosmos/cosmos-sdk/wire"
 )
 
 func newTestMsg(addrs ...sdk.AccAddress) *sdk.TestMsg {
@@ -567,3 +567,63 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
 	acc2 = mapper.GetAccount(ctx, addr2)
 	require.Nil(t, acc2.GetPubKey())
 }
+
+func TestProcessPubKey(t *testing.T) {
+	ms, capKey, _ := setupMultiStore()
+	cdc := wire.NewCodec()
+	RegisterBaseAccount(cdc)
+	mapper := NewAccountMapper(cdc, capKey, ProtoBaseAccount)
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, log.NewNopLogger())
+	// keys
+	_, addr1 := privAndAddr()
+	priv2, _ := privAndAddr()
+	acc1 := mapper.NewAccountWithAddress(ctx, addr1)
+	type args struct {
+		acc      Account
+		sig      StdSignature
+		simulate bool
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"no sigs, simulate off", args{acc1, StdSignature{}, false}, true},
+		{"no sigs, simulate on", args{acc1, StdSignature{}, true}, false},
+		{"pubkey doesn't match addr, simulate off", args{acc1, StdSignature{PubKey: priv2.PubKey()}, false}, true},
+		{"pubkey doesn't match addr, simulate on", args{acc1, StdSignature{PubKey: priv2.PubKey()}, true}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := processPubKey(tt.args.acc, tt.args.sig, tt.args.simulate)
+			require.Equal(t, tt.wantErr, !err.IsOK())
+		})
+	}
+}
+
+func TestConsumeSignatureVerificationGas(t *testing.T) {
+	type args struct {
+		meter  sdk.GasMeter
+		pubkey crypto.PubKey
+	}
+	tests := []struct {
+		name        string
+		args        args
+		gasConsumed int64
+		wantPanic   bool
+	}{
+		{"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), ed25519.GenPrivKey().PubKey()}, ed25519VerifyCost, false},
+		{"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), secp256k1.GenPrivKey().PubKey()}, secp256k1VerifyCost, false},
+		{"unknown key", args{sdk.NewInfiniteGasMeter(), nil}, 0, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.wantPanic {
+				require.Panics(t, func() { consumeSignatureVerificationGas(tt.args.meter, tt.args.pubkey) })
+			} else {
+				consumeSignatureVerificationGas(tt.args.meter, tt.args.pubkey)
+				require.Equal(t, tt.args.meter.GasConsumed(), tt.gasConsumed)
+			}
+		})
+	}
+}