feat(proto)!: replace PubKey in abci.ValidatorUpdate with `PubKey…

…Type` and `PubKeyBytes` (cometbft#2843)

This PR deprecates `PubKey` in `abci.ValidatorUpdate` and adds two new
fields:

- string `PubKeyType`
- bytes `PubKeyBytes`

Co-authored by: @tac0turtle 

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Marko Baricevic <[email protected]>
Co-authored-by: Andy Nogueira <[email protected]>

.changelog/unreleased/breaking-changes/2843-abci-types-go-api.md

@@ -0,0 +1,2 @@
+ - [`abci/types`] Rename `UpdateValidator` to `NewValidatorUpdate`, remove
+   `Ed25519ValidatorUpdate` ([\#2843](https://github.com/cometbft/cometbft/pull/2843))

.changelog/unreleased/breaking-changes/2843-abci-types-validator-update-pubkey.md

@@ -0,0 +1,3 @@
+- [`abci/types`] Replace `ValidatorUpdate.PubKey` with `PubKeyType` and
+  `PubKeyBytes` to allow applications to avoid implementing `PubKey` interface.
+  ([\#2843](https://github.com/cometbft/cometbft/pull/2843))

.changelog/unreleased/breaking-changes/2843-proto-abci-v1-validator-update.md

@@ -0,0 +1,2 @@
+- [`proto`] Remove `abci.ValidatorUpdate.pub_key`, add `pub_key_type` and
+  `pub_key_bytes` ([\#2843](https://github.com/cometbft/cometbft/pull/2843))

abci/example/kvstore/helpers.go

@@ -7,18 +7,16 @@ import (
 	"strings"
 
 	"github.com/cometbft/cometbft/abci/types"
-	pbcrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
-	cryptoencoding "github.com/cometbft/cometbft/crypto/encoding"
+	"github.com/cometbft/cometbft/crypto/ed25519"
 	cmtrand "github.com/cometbft/cometbft/internal/rand"
 )
 
 // RandVal creates one random validator, with a key derived
 // from the input value.
 func RandVal() types.ValidatorUpdate {
-	pubkey := cmtrand.Bytes(32)
+	pubkey := ed25519.GenPrivKey().PubKey()
 	power := cmtrand.Uint16() + 1
-	v := types.UpdateValidator(pubkey, int64(power), "")
-	return v
+	return types.ValidatorUpdate{Power: int64(power), PubKeyType: pubkey.Type(), PubKeyBytes: pubkey.Bytes()}
 }
 
 // RandVals returns a list of cnt validators for initializing
@@ -48,13 +46,15 @@ func NewTx(key, value string) []byte {
 	return []byte(strings.Join([]string{key, value}, "="))
 }
 
+// NewRandomTx creates a new random transaction.
 func NewRandomTx(size int) []byte {
 	if size < 4 {
 		panic("random tx size must be greater than 3")
 	}
 	return NewTx(cmtrand.Str(2), cmtrand.Str(size-3))
 }
 
+// NewRandomTxs creates n transactions.
 func NewRandomTxs(n int) [][]byte {
 	txs := make([][]byte, n)
 	for i := 0; i < n; i++ {
@@ -63,18 +63,14 @@ func NewRandomTxs(n int) [][]byte {
 	return txs
 }
 
+// NewTxFromID creates a new transaction using the given ID.
 func NewTxFromID(i int) []byte {
 	return []byte(fmt.Sprintf("%d=%d", i, i))
 }
 
-// Create a transaction to add/remove/update a validator
+// MakeValSetChangeTx creates a transaction to add/remove/update a validator.
 // To remove, set power to 0.
-func MakeValSetChangeTx(pubkey pbcrypto.PublicKey, power int64) []byte {
-	pk, err := cryptoencoding.PubKeyFromProto(pubkey)
-	if err != nil {
-		panic(err)
-	}
-	pubStr := base64.StdEncoding.EncodeToString(pk.Bytes())
-	pubTypeStr := pk.Type()
-	return []byte(fmt.Sprintf("%s%s!%s!%d", ValidatorPrefix, pubTypeStr, pubStr, power))
+func MakeValSetChangeTx(v types.ValidatorUpdate) []byte {
+	pubStr := base64.StdEncoding.EncodeToString(v.PubKeyBytes)
+	return []byte(fmt.Sprintf("%s%s!%s!%d", ValidatorPrefix, v.PubKeyType, pubStr, v.Power))
 }

abci/example/kvstore/kvstore.go

@@ -12,8 +12,8 @@ import (
 
 	dbm "github.com/cometbft/cometbft-db"
 	"github.com/cometbft/cometbft/abci/types"
-	cryptoproto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
-	cryptoencoding "github.com/cometbft/cometbft/crypto/encoding"
+	"github.com/cometbft/cometbft/crypto"
+	cryptoenc "github.com/cometbft/cometbft/crypto/encoding"
 	"github.com/cometbft/cometbft/libs/log"
 	"github.com/cometbft/cometbft/version"
 )
@@ -43,7 +43,7 @@ type Application struct {
 
 	// validator set
 	valUpdates         []types.ValidatorUpdate
-	valAddrToPubKeyMap map[string]cryptoproto.PublicKey
+	valAddrToPubKeyMap map[string]crypto.PubKey
 
 	// If true, the app will generate block events in BeginBlock. Used to test the event indexer
 	// Should be false by default to avoid generating too much data.
@@ -55,7 +55,7 @@ func NewApplication(db dbm.DB) *Application {
 	return &Application{
 		logger:             log.NewNopLogger(),
 		state:              loadState(db),
-		valAddrToPubKeyMap: make(map[string]cryptoproto.PublicKey),
+		valAddrToPubKeyMap: make(map[string]crypto.PubKey),
 	}
 }
 
@@ -88,11 +88,11 @@ func (app *Application) Info(context.Context, *types.InfoRequest) (*types.InfoRe
 	if len(app.valAddrToPubKeyMap) == 0 && app.state.Height > 0 {
 		validators := app.getValidators()
 		for _, v := range validators {
-			pubkey, err := cryptoencoding.PubKeyFromProto(v.PubKey)
+			pubkey, err := cryptoenc.PubKeyFromTypeAndBytes(v.PubKeyType, v.PubKeyBytes)
 			if err != nil {
-				panic(fmt.Errorf("can't decode public key: %w", err))
+				panic(err)
 			}
-			app.valAddrToPubKeyMap[string(pubkey.Address())] = v.PubKey
+			app.valAddrToPubKeyMap[string(pubkey.Address())] = pubkey
 		}
 	}
 
@@ -205,8 +205,9 @@ func (app *Application) FinalizeBlock(_ context.Context, req *types.FinalizeBloc
 			//nolint:revive // this is a false positive from early-return
 			if pubKey, ok := app.valAddrToPubKeyMap[addr]; ok {
 				app.valUpdates = append(app.valUpdates, types.ValidatorUpdate{
-					PubKey: pubKey,
-					Power:  ev.Validator.Power - 1,
+					Power:       ev.Validator.Power - 1,
+					PubKeyType:  pubKey.Type(),
+					PubKeyBytes: pubKey.Bytes(),
 				})
 				app.logger.Info("Decreased val power by 1 because of the equivocation",
 					"val", addr)
@@ -223,7 +224,7 @@ func (app *Application) FinalizeBlock(_ context.Context, req *types.FinalizeBloc
 			if err != nil {
 				panic(err)
 			}
-			app.valUpdates = append(app.valUpdates, types.UpdateValidator(pubKey, power, keyType))
+			app.valUpdates = append(app.valUpdates, types.ValidatorUpdate{Power: power, PubKeyType: keyType, PubKeyBytes: pubKey})
 		} else {
 			app.stagedTxs = append(app.stagedTxs, tx)
 		}
@@ -446,9 +447,9 @@ func parseValidatorTx(tx []byte) (string, []byte, int64, error) {
 
 // add, update, or remove a validator.
 func (app *Application) updateValidator(v types.ValidatorUpdate) {
-	pubkey, err := cryptoencoding.PubKeyFromProto(v.PubKey)
+	pubkey, err := cryptoenc.PubKeyFromTypeAndBytes(v.PubKeyType, v.PubKeyBytes)
 	if err != nil {
-		panic(fmt.Errorf("can't decode public key: %w", err))
+		panic(err)
 	}
 	key := []byte(ValidatorPrefix + string(pubkey.Bytes()))
 
@@ -475,7 +476,7 @@ func (app *Application) updateValidator(v types.ValidatorUpdate) {
 		if err = app.state.db.Set(key, value.Bytes()); err != nil {
 			panic(err)
 		}
-		app.valAddrToPubKeyMap[string(pubkey.Address())] = v.PubKey
+		app.valAddrToPubKeyMap[string(pubkey.Address())] = pubkey
 	}
 }
 

abci/example/kvstore/kvstore_test.go

@@ -1,6 +1,7 @@
 package kvstore
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"sort"
@@ -162,8 +163,8 @@ func TestValUpdates(t *testing.T) {
 	// add some validators
 	v1, v2 = vals[nInit], vals[nInit+1]
 	diff := []types.ValidatorUpdate{v1, v2}
-	tx1 := MakeValSetChangeTx(v1.PubKey, v1.Power)
-	tx2 := MakeValSetChangeTx(v2.PubKey, v2.Power)
+	tx1 := MakeValSetChangeTx(v1)
+	tx2 := MakeValSetChangeTx(v2)
 
 	makeApplyBlock(ctx, t, kvstore, 1, diff, tx1, tx2)
 
@@ -176,9 +177,9 @@ func TestValUpdates(t *testing.T) {
 	v2.Power = 0
 	v3.Power = 0
 	diff = []types.ValidatorUpdate{v1, v2, v3}
-	tx1 = MakeValSetChangeTx(v1.PubKey, v1.Power)
-	tx2 = MakeValSetChangeTx(v2.PubKey, v2.Power)
-	tx3 := MakeValSetChangeTx(v3.PubKey, v3.Power)
+	tx1 = MakeValSetChangeTx(v1)
+	tx2 = MakeValSetChangeTx(v2)
+	tx3 := MakeValSetChangeTx(v3)
 
 	makeApplyBlock(ctx, t, kvstore, 2, diff, tx1, tx2, tx3)
 
@@ -194,7 +195,7 @@ func TestValUpdates(t *testing.T) {
 		v1.Power = 5
 	}
 	diff = []types.ValidatorUpdate{v1}
-	tx1 = MakeValSetChangeTx(v1.PubKey, v1.Power)
+	tx1 = MakeValSetChangeTx(v1)
 
 	makeApplyBlock(ctx, t, kvstore, 3, diff, tx1)
 
@@ -209,6 +210,7 @@ func TestCheckTx(t *testing.T) {
 	kvstore := NewInMemoryApplication()
 
 	val := RandVal()
+	val.Power = 10
 
 	testCases := []struct {
 		expCode uint32
@@ -222,7 +224,7 @@ func TestCheckTx(t *testing.T) {
 		{CodeTypeOK, []byte("a=b")},
 		{CodeTypeInvalidTxFormat, []byte("val=hello")},
 		{CodeTypeInvalidTxFormat, []byte("val=hi!5")},
-		{CodeTypeOK, MakeValSetChangeTx(val.PubKey, 10)},
+		{CodeTypeOK, MakeValSetChangeTx(val)},
 	}
 
 	for idx, tc := range testCases {
@@ -287,9 +289,11 @@ func valsEqual(t *testing.T, vals1, vals2 []types.ValidatorUpdate) {
 	sort.Sort(types.ValidatorUpdates(vals2))
 	for i, v1 := range vals1 {
 		v2 := vals2[i]
-		if !v1.PubKey.Equal(v2.PubKey) ||
+		if v1.PubKeyType != v2.PubKeyType ||
+			!bytes.Equal(v1.PubKeyBytes, v2.PubKeyBytes) ||
 			v1.Power != v2.Power {
-			t.Fatalf("vals dont match at index %d. got %X/%d , expected %X/%d", i, v2.PubKey, v2.Power, v1.PubKey, v1.Power)
+			t.Fatalf("vals dont match at index %d. got %s/%X/%d , expected %s/%X/%d", i,
+				v2.PubKeyType, v2.PubKeyBytes, v2.Power, v1.PubKeyType, v1.PubKeyBytes, v1.Power)
 		}
 	}
 }

abci/tests/server/client.go

@@ -17,7 +17,7 @@ func InitChain(ctx context.Context, client abcicli.Client) error {
 	for i := 0; i < total; i++ {
 		pubkey := cmtrand.Bytes(33)
 		power := cmtrand.Int()
-		vals[i] = types.UpdateValidator(pubkey, int64(power), "")
+		vals[i] = types.ValidatorUpdate{Power: int64(power), PubKeyType: "", PubKeyBytes: pubkey}
 	}
 	_, err := client.InitChain(ctx, &types.InitChainRequest{
 		Validators: vals,

abci/types/pubkey.go

@@ -1,44 +1,15 @@
 package types
 
 import (
-	"fmt"
-
-	"github.com/cometbft/cometbft/crypto/ed25519"
-	cryptoenc "github.com/cometbft/cometbft/crypto/encoding"
-	"github.com/cometbft/cometbft/crypto/secp256k1"
+	"github.com/cometbft/cometbft/crypto"
 )
 
-func Ed25519ValidatorUpdate(pk []byte, power int64) ValidatorUpdate {
-	pke := ed25519.PubKey(pk)
-
-	pkp, err := cryptoenc.PubKeyToProto(pke)
-	if err != nil {
-		panic(err)
-	}
-
+// NewValidatorUpdate creates a new ValidatorUpdate from the given public
+// key.
+func NewValidatorUpdate(pubKey crypto.PubKey, power int64) ValidatorUpdate {
 	return ValidatorUpdate{
-		// Address:
-		PubKey: pkp,
-		Power:  power,
-	}
-}
-
-func UpdateValidator(pk []byte, power int64, keyType string) ValidatorUpdate {
-	switch keyType {
-	case "", ed25519.KeyType:
-		return Ed25519ValidatorUpdate(pk, power)
-	case secp256k1.KeyType:
-		pke := secp256k1.PubKey(pk)
-		pkp, err := cryptoenc.PubKeyToProto(pke)
-		if err != nil {
-			panic(err)
-		}
-		return ValidatorUpdate{
-			// Address:
-			PubKey: pkp,
-			Power:  power,
-		}
-	default:
-		panic(fmt.Sprintf("key type %s not supported", keyType))
+		Power:       power,
+		PubKeyType:  pubKey.Type(),
+		PubKeyBytes: pubKey.Bytes(),
 	}
 }

abci/types/util.go

@@ -1,7 +1,9 @@
 package types
 
 import (
+	"bytes"
 	"sort"
+	"strings"
 )
 
 // ------------------------------------------------------------------------------
@@ -23,7 +25,7 @@ func (v ValidatorUpdates) Len() int {
 
 // XXX: doesn't distinguish same validator with different power.
 func (v ValidatorUpdates) Less(i, j int) bool {
-	return v[i].PubKey.Compare(v[j].PubKey) <= 0
+	return strings.Compare(v[i].PubKeyType, v[j].PubKeyType) <= 0 && bytes.Compare(v[i].PubKeyBytes, v[j].PubKeyBytes) <= 0
 }
 
 func (v ValidatorUpdates) Swap(i, j int) {