LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix: ValidatorStore interface doesn't match StakingKeeper (backport #17164) (#17283)

Browse Source 
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
This commit is contained in:
mergify[bot] 2023-08-03 23:13:13 +00:00 committed by GitHub
parent 8375a67cff
commit 69db56b49b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 174 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/group, x/gov) [#17220](https://github.com/cosmos/cosmos-sdk/pull/17220) Add `--skip-metadata` flag in `draft-proposal` to skip metadata prompt.
* (x/group, x/gov) [#17109](https://github.com/cosmos/cosmos-sdk/pull/17109) Let proposal summary be 40x longer than metadata limit.
* (version) [#17096](https://github.com/cosmos/cosmos-sdk/pull/17096) Improve `getSDKVersion()` to handle module replacements.
* (x/staking) [#17164](https://github.com/cosmos/cosmos-sdk/pull/17164) Add `BondedTokensAndPubKeyByConsAddr` to the keeper to enable vote extension verification.
### Bug Fixes

10
baseapp/abci_test.go
View File

@ -1785,13 +1785,9 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) {
},
}
val1 := mock.NewMockValidator(ctrl)
val1.EXPECT().BondedTokens().Return(math.NewInt(667))
val1.EXPECT().CmtConsPublicKey().Return(tmPk, nil).AnyTimes()
consAddr := sdk.ConsAddress(addr.String())
valStore.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr.Bytes()).Return(val1, nil).AnyTimes()
valStore.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(1000)).AnyTimes()
valStore.EXPECT().BondedTokensAndPubKeyByConsAddr(gomock.Any(), consAddr.Bytes()).Return(math.NewInt(667), tmPk, nil)
valStore.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(1000), nil).AnyTimes()
// set up baseapp
prepareOpt := func(bapp *baseapp.BaseApp) {
@ -1879,7 +1875,7 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) {
require.Equal(t, 1, len(resPrepareProposal.Txs))
// now vote extensions but our sole voter doesn't reach majority
val1.EXPECT().BondedTokens().Return(math.NewInt(666))
valStore.EXPECT().BondedTokensAndPubKeyByConsAddr(gomock.Any(), consAddr.Bytes()).Return(math.NewInt(666), tmPk, nil)
resPrepareProposal, err = suite.baseApp.PrepareProposal(&reqPrepareProposal)
require.NoError(t, err)
require.Equal(t, 0, len(resPrepareProposal.Txs))

39
baseapp/abci_utils.go
View File

@ -2,11 +2,11 @@ package baseapp
import (
"bytes"
"context"
"fmt"
"github.com/cockroachdb/errors"
abci "github.com/cometbft/cometbft/abci/types"
cmtcrypto "github.com/cometbft/cometbft/crypto"
cryptoenc "github.com/cometbft/cometbft/crypto/encoding"
cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
@ -15,7 +15,6 @@ import (
"cosmossdk.io/math"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/mempool"
)
@ -26,20 +25,12 @@ import (
var VoteExtensionThreshold = math.LegacyNewDecWithPrec(667, 3)
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
TotalBondedTokens(ctx context.Context) (math.Int, error)
BondedTokensAndPubKeyByConsAddr(context.Context, sdk.ConsAddress) (math.Int, cmtprotocrypto.PublicKey, error)
}
// GasTx defines the contract that a transaction with a gas limit must implement.
@ -62,7 +53,6 @@ func ValidateVoteExtensions(
) 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 {
@ -89,19 +79,10 @@ func ValidateVoteExtensions(
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)
valConsAddr := sdk.ConsAddress(vote.Validator.Address)
bondedTokens, cmtPubKeyProto, err := valStore.BondedTokensAndPubKeyByConsAddr(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)
return fmt.Errorf("failed to get validator %X info (bonded tokens and public key): %w", valConsAddr, err)
}
cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto)
@ -125,12 +106,16 @@ func ValidateVoteExtensions(
return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr)
}
sumVP = sumVP.Add(validator.BondedTokens())
sumVP = sumVP.Add(bondedTokens)
}
// Ensure we have at least 2/3 voting power that submitted valid vote
// extensions.
totalVP := valStore.TotalBondedTokens(ctx)
totalVP, err := valStore.TotalBondedTokens(ctx)
if err != nil {
return fmt.Errorf("failed to get total bonded tokens: %w", err)
}
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)

87
baseapp/testutil/mock/mocks.go
View File

@ -5,68 +5,15 @@
package mock
import (
context "context"
reflect "reflect"
math "cosmossdk.io/math"
crypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
baseapp "github.com/cosmos/cosmos-sdk/baseapp"
types "github.com/cosmos/cosmos-sdk/crypto/types"
types0 "github.com/cosmos/cosmos-sdk/types"
types "github.com/cosmos/cosmos-sdk/types"
gomock "github.com/golang/mock/gomock"
)
// MockValidator is a mock of Validator interface.
type MockValidator struct {
ctrl *gomock.Controller
recorder *MockValidatorMockRecorder
}
// MockValidatorMockRecorder is the mock recorder for MockValidator.
type MockValidatorMockRecorder struct {
mock *MockValidator
}
// NewMockValidator creates a new mock instance.
func NewMockValidator(ctrl *gomock.Controller) *MockValidator {
mock := &MockValidator{ctrl: ctrl}
mock.recorder = &MockValidatorMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockValidator) EXPECT() *MockValidatorMockRecorder {
return m.recorder
}
// BondedTokens mocks base method.
func (m *MockValidator) BondedTokens() math.Int {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "BondedTokens")
ret0, _ := ret[0].(math.Int)
return ret0
}
// BondedTokens indicates an expected call of BondedTokens.
func (mr *MockValidatorMockRecorder) BondedTokens() *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondedTokens", reflect.TypeOf((*MockValidator)(nil).BondedTokens))
}
// CmtConsPublicKey mocks base method.
func (m *MockValidator) CmtConsPublicKey() (crypto.PublicKey, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "CmtConsPublicKey")
ret0, _ := ret[0].(crypto.PublicKey)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// CmtConsPublicKey indicates an expected call of CmtConsPublicKey.
func (mr *MockValidatorMockRecorder) CmtConsPublicKey() *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CmtConsPublicKey", reflect.TypeOf((*MockValidator)(nil).CmtConsPublicKey))
}
// MockValidatorStore is a mock of ValidatorStore interface.
type MockValidatorStore struct {
ctrl *gomock.Controller
@ -90,27 +37,29 @@ func (m *MockValidatorStore) EXPECT() *MockValidatorStoreMockRecorder {
return m.recorder
}
// GetValidatorByConsAddr mocks base method.
func (m *MockValidatorStore) GetValidatorByConsAddr(arg0 types0.Context, arg1 types.Address) (baseapp.Validator, error) {
// BondedTokensAndPubKeyByConsAddr mocks base method.
func (m *MockValidatorStore) BondedTokensAndPubKeyByConsAddr(arg0 context.Context, arg1 types.ConsAddress) (math.Int, crypto.PublicKey, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetValidatorByConsAddr", arg0, arg1)
ret0, _ := ret[0].(baseapp.Validator)
ret1, _ := ret[1].(error)
return ret0, ret1
ret := m.ctrl.Call(m, "BondedTokensAndPubKeyByConsAddr", arg0, arg1)
ret0, _ := ret[0].(math.Int)
ret1, _ := ret[1].(crypto.PublicKey)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
}
// GetValidatorByConsAddr indicates an expected call of GetValidatorByConsAddr.
func (mr *MockValidatorStoreMockRecorder) GetValidatorByConsAddr(arg0, arg1 interface{}) *gomock.Call {
// BondedTokensAndPubKeyByConsAddr indicates an expected call of BondedTokensAndPubKeyByConsAddr.
func (mr *MockValidatorStoreMockRecorder) BondedTokensAndPubKeyByConsAddr(arg0, arg1 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetValidatorByConsAddr", reflect.TypeOf((*MockValidatorStore)(nil).GetValidatorByConsAddr), arg0, arg1)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondedTokensAndPubKeyByConsAddr", reflect.TypeOf((*MockValidatorStore)(nil).BondedTokensAndPubKeyByConsAddr), arg0, arg1)
}
// TotalBondedTokens mocks base method.
func (m *MockValidatorStore) TotalBondedTokens(ctx types0.Context) math.Int {
func (m *MockValidatorStore) TotalBondedTokens(ctx context.Context) (math.Int, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "TotalBondedTokens", ctx)
ret0, _ := ret[0].(math.Int)
return ret0
ret1, _ := ret[1].(error)
return ret0, ret1
}
// TotalBondedTokens indicates an expected call of TotalBondedTokens.
@ -180,7 +129,7 @@ func (m *MockProposalTxVerifier) EXPECT() *MockProposalTxVerifierMockRecorder {
}
// PrepareProposalVerifyTx mocks base method.
func (m *MockProposalTxVerifier) PrepareProposalVerifyTx(tx types0.Tx) ([]byte, error) {
func (m *MockProposalTxVerifier) PrepareProposalVerifyTx(tx types.Tx) ([]byte, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "PrepareProposalVerifyTx", tx)
ret0, _ := ret[0].([]byte)
@ -195,10 +144,10 @@ func (mr *MockProposalTxVerifierMockRecorder) PrepareProposalVerifyTx(tx interfa
}
// ProcessProposalVerifyTx mocks base method.
func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types0.Tx, error) {
func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types.Tx, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ProcessProposalVerifyTx", txBz)
ret0, _ := ret[0].(types0.Tx)
ret0, _ := ret[0].(types.Tx)
ret1, _ := ret[1].(error)
return ret0, ret1
}

15
docs/architecture/adr-064-abci-2.0.md
View File

@ -218,7 +218,8 @@ a default signature verification method which applications can use:
```go
type ValidatorStore interface {
GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (cryptotypes.PubKey, error)
TotalBondedTokens(ctx context.Context) (math.Int, error)
BondedTokensAndPubKeyByConsAddr(context.Context, sdk.ConsAddress) (math.Int, cmtprotocrypto.PublicKey, error)
}
// ValidateVoteExtensions is a function that an application can execute in
@ -229,16 +230,10 @@ func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64,
continue
}
valConsAddr := cmtcrypto.Address(vote.Validator.Address)
validator, err := app.validatorStore.GetValidatorByConsAddr(ctx, valConsAddr)
valConsAddr := sdk.ConsAddress(vote.Validator.Address)
bondedTokens, cmtPubKey, err := valStore.BondedTokensAndPubKeyByConsAddr(ctx, valConsAddr)
if err != nil {
return fmt.Errorf("failed to get validator %s for vote extension", valConsAddr)
}
cmtPubKey, err := validator.CmtConsPublicKey()
if err != nil {
return fmt.Errorf("failed to convert public key: %w", err)
return fmt.Errorf("failed to get bonded tokens and public key for validator %s: %w", valConsAddr, err)
}
if len(vote.ExtensionSignature) == 0 {

96
tests/integration/staking/keeper/vote_extensions_test.go Normal file
View File

@ -0,0 +1,96 @@
package keeper_test
import (
"bytes"
"testing"
abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
protoio "github.com/cosmos/gogoproto/io"
"github.com/cosmos/gogoproto/proto"
"gotest.tools/v3/assert"
"cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/baseapp"
ed25519 "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/testutil"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)
func TestValidateVoteExtensions(t *testing.T) {
t.Parallel()
f := initFixture(t)
// enable vote extensions
cp := simtestutil.DefaultConsensusParams
cp.Abci = &cmtproto.ABCIParams{VoteExtensionsEnableHeight: 1}
f.sdkCtx = f.sdkCtx.WithConsensusParams(*cp).WithBlockHeight(2)
// setup the validators
numVals := 3
privKeys := []cryptotypes.PrivKey{}
for i := 0; i < numVals; i++ {
privKeys = append(privKeys, ed25519.GenPrivKey())
}
vals := []stakingtypes.Validator{}
for _, v := range privKeys {
valAddr := sdk.ValAddress(v.PubKey().Address())
simtestutil.AddTestAddrsFromPubKeys(f.bankKeeper, f.stakingKeeper, f.sdkCtx, []cryptotypes.PubKey{v.PubKey()}, math.NewInt(100000000000))
vals = append(vals, testutil.NewValidator(t, valAddr, v.PubKey()))
}
votes := []abci.ExtendedVoteInfo{}
for i, v := range vals {
v.Tokens = math.NewInt(1000000)
v.Status = stakingtypes.Bonded
assert.NilError(t, f.stakingKeeper.SetValidator(f.sdkCtx, v))
assert.NilError(t, f.stakingKeeper.SetValidatorByConsAddr(f.sdkCtx, v))
assert.NilError(t, f.stakingKeeper.SetNewValidatorByPowerIndex(f.sdkCtx, v))
_, err := f.stakingKeeper.Delegate(f.sdkCtx, sdk.AccAddress(privKeys[i].PubKey().Address()), v.Tokens, stakingtypes.Unbonded, v, true)
assert.NilError(t, err)
// each val produces a vote
voteExt := []byte("something" + v.OperatorAddress)
cve := cmtproto.CanonicalVoteExtension{
Extension: voteExt,
Height: f.sdkCtx.BlockHeight() - 1, // the vote extension was signed in the previous height
Round: 0,
ChainId: "chain-id-123",
}
extSignBytes, err := mashalVoteExt(&cve)
assert.NilError(t, err)
sig, err := privKeys[i].Sign(extSignBytes)
assert.NilError(t, err)
ve := abci.ExtendedVoteInfo{
Validator: abci.Validator{
Address: v.GetOperator(),
Power: v.ConsensusPower(sdk.DefaultPowerReduction),
},
VoteExtension: voteExt,
ExtensionSignature: sig,
BlockIdFlag: cmtproto.BlockIDFlagCommit,
}
votes = append(votes, ve)
}
err := baseapp.ValidateVoteExtensions(f.sdkCtx, f.stakingKeeper, f.sdkCtx.BlockHeight(), "chain-id-123", abci.ExtendedCommitInfo{Round: 0, Votes: votes})
assert.NilError(t, err)
}
func mashalVoteExt(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
}

16
x/staking/keeper/validator.go
View File

@ -7,6 +7,7 @@ import (
"fmt"
"time"
cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
gogotypes "github.com/cosmos/gogoproto/types"
corestore "cosmossdk.io/core/store"
@ -601,3 +602,18 @@ func (k Keeper) IsValidatorJailed(ctx context.Context, addr sdk.ConsAddress) (bo
return v.Jailed, nil
}
// BondedTokensAndPubKeyByConsAddr returns the consensus public key and bonded tokens by consensus address
func (k Keeper) BondedTokensAndPubKeyByConsAddr(ctx context.Context, addr sdk.ConsAddress) (math.Int, cmtprotocrypto.PublicKey, error) {
v, err := k.GetValidatorByConsAddr(ctx, addr)
if err != nil {
return math.ZeroInt(), cmtprotocrypto.PublicKey{}, err
}
pubkey, err := v.CmtConsPublicKey()
if err != nil {
return math.ZeroInt(), cmtprotocrypto.PublicKey{}, err
}
return v.BondedTokens(), pubkey, nil
}

17
x/staking/testutil/expected_keepers_mocks.go
View File

@ -10,6 +10,7 @@ import (
address "cosmossdk.io/core/address"
math "cosmossdk.io/math"
crypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
types "github.com/cosmos/cosmos-sdk/types"
types0 "github.com/cosmos/cosmos-sdk/x/staking/types"
gomock "github.com/golang/mock/gomock"
@ -290,6 +291,22 @@ func (m *MockValidatorSet) EXPECT() *MockValidatorSetMockRecorder {
return m.recorder
}
// BondedTokensAndPubKeyByConsAddr mocks base method.
func (m *MockValidatorSet) BondedTokensAndPubKeyByConsAddr(arg0 context.Context, arg1 types.ConsAddress) (math.Int, crypto.PublicKey, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "BondedTokensAndPubKeyByConsAddr", arg0, arg1)
ret0, _ := ret[0].(math.Int)
ret1, _ := ret[1].(crypto.PublicKey)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
}
// BondedTokensAndPubKeyByConsAddr indicates an expected call of BondedTokensAndPubKeyByConsAddr.
func (mr *MockValidatorSetMockRecorder) BondedTokensAndPubKeyByConsAddr(arg0, arg1 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondedTokensAndPubKeyByConsAddr", reflect.TypeOf((*MockValidatorSet)(nil).BondedTokensAndPubKeyByConsAddr), arg0, arg1)
}
// Delegation mocks base method.
func (m *MockValidatorSet) Delegation(arg0 context.Context, arg1 types.AccAddress, arg2 types.ValAddress) (types0.DelegationI, error) {
m.ctrl.T.Helper()

6
x/staking/types/expected_keepers.go
View File

@ -3,6 +3,8 @@ package types
import (
context "context"
cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
"cosmossdk.io/core/address"
"cosmossdk.io/math"
@ -70,6 +72,10 @@ type ValidatorSet interface {
// MaxValidators returns the maximum amount of bonded validators
MaxValidators(context.Context) (uint32, error)
// BondedTokensAndPubKeyByConsAddr returns the bonded tokens and consensus public key for a validator.
// Used in vote extension validation.
BondedTokensAndPubKeyByConsAddr(context.Context, sdk.ConsAddress) (math.Int, cmtprotocrypto.PublicKey, error)
}
// DelegationSet expected properties for the set of all delegations for a particular (noalias)