[Bug]: ValidateVoteExtensions Expected Interface

[davidterpay]

Summary of Bug

ValidateVoteExtensions expects the staking keeper, however, the staking keeper interface has changed between versions so it is no longer able to be supported out of the box (users have to wrap the staking keeper).

Source code in question:

cosmos-sdk/baseapp/abci_utils.go

Lines 28 to 140 in 16ab635

	type (
	// Validator defines the interface contract require for verifying vote extension
	// signatures. Typically, this will be implemented by the x/staking module,
	// which has knowledge of the CometBFT public key.
	Validator interface {
	CmtConsPublicKey() (cmtprotocrypto.PublicKey, error)
	BondedTokens() math.Int
	}
	// ValidatorStore defines the interface contract require for verifying vote
	// extension signatures. Typically, this will be implemented by the x/staking
	// module, which has knowledge of the CometBFT public key.
	ValidatorStore interface {
	GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error)
	TotalBondedTokens(ctx sdk.Context) math.Int
	}
	// GasTx defines the contract that a transaction with a gas limit must implement.
	GasTx interface {
	GetGas() uint64
	}
	)
	// ValidateVoteExtensions defines a helper function for verifying vote extension
	// signatures that may be passed or manually injected into a block proposal from
	// a proposer in PrepareProposal. It returns an error if any signature is invalid
	// or if unexpected vote extensions and/or signatures are found or less than 2/3
	// power is received.
	func ValidateVoteExtensions(
	ctx sdk.Context,
	valStore ValidatorStore,
	currentHeight int64,
	chainID string,
	extCommit abci.ExtendedCommitInfo,
	) error {
	cp := ctx.ConsensusParams()
	extsEnabled := cp.Abci != nil && currentHeight >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
	marshalDelimitedFn := func(msg proto.Message) ([]byte, error) {
	var buf bytes.Buffer
	if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil {
	return nil, err
	}
	return buf.Bytes(), nil
	}
	sumVP := math.NewInt(0)
	for _, vote := range extCommit.Votes {
	if !extsEnabled {
	if len(vote.VoteExtension) > 0 {
	return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight)
	}
	if len(vote.ExtensionSignature) > 0 {
	return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight)
	}
	continue
	}
	if len(vote.ExtensionSignature) == 0 {
	return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight)
	}
	valConsAddr := cmtcrypto.Address(vote.Validator.Address)
	validator, err := valStore.GetValidatorByConsAddr(ctx, valConsAddr)
	if err != nil {
	return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err)
	}
	if validator == nil {
	return fmt.Errorf("validator %X not found", valConsAddr)
	}
	cmtPubKeyProto, err := validator.CmtConsPublicKey()
	if err != nil {
	return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err)
	}
	cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto)
	if err != nil {
	return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err)
	}
	cve := cmtproto.CanonicalVoteExtension{
	Extension: vote.VoteExtension,
	Height: currentHeight - 1, // the vote extension was signed in the previous height
	Round: int64(extCommit.Round),
	ChainId: chainID,
	}
	extSignBytes, err := marshalDelimitedFn(&cve)
	if err != nil {
	return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err)
	}
	if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) {
	return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr)
	}
	sumVP = sumVP.Add(validator.BondedTokens())
	}
	// Ensure we have at least 2/3 voting power that submitted valid vote
	// extensions.
	totalVP := valStore.TotalBondedTokens(ctx)
	percentSubmitted := math.LegacyNewDecFromInt(sumVP).Quo(math.LegacyNewDecFromInt(totalVP))
	if percentSubmitted.LT(VoteExtensionThreshold) {
	return fmt.Errorf("insufficient cumulative voting power received to verify vote extensions; got: %s, expected: >=%s", percentSubmitted, VoteExtensionThreshold)
	}
	return nil
	}

New staking keeper interface:

Version

The version we are working with is v0.50.0-beta.0.

[facundomedica]

We need to update that interface to TotalBondedTokens(context.Context) (math.Int, error) + add a test that uses staking keeper instead of the mock of that interface.

[alexanderbez]

Thanks @davidterpay!