Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allow VerifyVoteExtensions without ExtendVote #17251

Merged
merged 5 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,27 +548,27 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (
// Always reset state given that ExtendVote and VerifyVoteExtension can timeout
// and be called again in a subsequent round.
emptyHeader := cmtproto.Header{ChainID: app.chainID, Height: req.Height}
app.setState(execModeVoteExtension, emptyHeader)
ms := app.cms.CacheMultiStore()
ctx := sdk.NewContext(ms, emptyHeader, false, app.logger).WithStreamingManager(app.streamingManager)

if app.extendVote == nil {
return nil, errors.New("application ExtendVote handler not set")
}

// If vote extensions are not enabled, as a safety precaution, we return an
// error.
cp := app.GetConsensusParams(app.voteExtensionState.ctx)
cp := app.GetConsensusParams(ctx)

extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
if !extsEnabled {
return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height)
}

app.voteExtensionState.ctx = app.voteExtensionState.ctx.
ctx = ctx.
WithConsensusParams(cp).
WithBlockGasMeter(storetypes.NewInfiniteGasMeter()).
WithBlockHeight(req.Height).
WithHeaderHash(req.Hash).
WithExecMode(sdk.ExecModeVoteExtension).
WithHeaderInfo(coreheader.Info{
ChainID: app.chainID,
Height: req.Height,
Expand All @@ -588,7 +588,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (
}
}()

resp, err = app.extendVote(app.voteExtensionState.ctx, req)
resp, err = app.extendVote(ctx, req)
if err != nil {
app.logger.Error("failed to extend vote", "height", req.Height, "error", err)
return &abci.ResponseExtendVote{VoteExtension: []byte{}}, nil
Expand All @@ -608,9 +608,13 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r
return nil, errors.New("application VerifyVoteExtension handler not set")
}

emptyHeader := cmtproto.Header{ChainID: app.chainID, Height: req.Height}
ms := app.cms.CacheMultiStore()
ctx := sdk.NewContext(ms, emptyHeader, false, app.logger).WithStreamingManager(app.streamingManager)

// If vote extensions are not enabled, as a safety precaution, we return an
// error.
cp := app.GetConsensusParams(app.voteExtensionState.ctx)
cp := app.GetConsensusParams(ctx)
Comment on lines 609 to +618
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).VerifyVoteExtension (baseapp/abci.go:606)


extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
if !extsEnabled {
Expand All @@ -631,7 +635,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r
}
}()

resp, err = app.verifyVoteExt(app.voteExtensionState.ctx, req)
resp, err = app.verifyVoteExt(ctx, req)
if err != nil {
app.logger.Error("failed to verify vote extension", "height", req.Height, "error", err)
return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil
Expand Down
57 changes: 57 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,63 @@ func TestABCI_ExtendVote(t *testing.T) {
require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status)
}

// TestABCI_OnlyVerifyVoteExtension makes sure we can call VerifyVoteExtension
// without having called ExtendVote before.
func TestABCI_OnlyVerifyVoteExtension(t *testing.T) {
name := t.Name()
db := dbm.NewMemDB()
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)

app.SetVerifyVoteExtensionHandler(func(ctx sdk.Context, req *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) {
// do some kind of verification here
expectedVoteExt := "foo" + hex.EncodeToString(req.Hash) + strconv.FormatInt(req.Height, 10)
if !bytes.Equal(req.VoteExtension, []byte(expectedVoteExt)) {
return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil
}

return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_ACCEPT}, nil
})

app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
_, err := app.InitChain(
&abci.RequestInitChain{
InitialHeight: 1,
ConsensusParams: &cmtproto.ConsensusParams{
Abci: &cmtproto.ABCIParams{
VoteExtensionsEnableHeight: 200,
},
},
},
)
require.NoError(t, err)

// Verify Vote Extensions
_, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 123, VoteExtension: []byte("1234567")})
require.ErrorContains(t, err, "vote extensions are not enabled")

// First vote on the first enabled height
vres, err := app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 200, Hash: []byte("thehash"), VoteExtension: []byte("foo74686568617368200")})
require.NoError(t, err)
require.Equal(t, abci.ResponseVerifyVoteExtension_ACCEPT, vres.Status)

vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 1000, Hash: []byte("thehash"), VoteExtension: []byte("foo746865686173681000")})
require.NoError(t, err)
require.Equal(t, abci.ResponseVerifyVoteExtension_ACCEPT, vres.Status)

// Reject because it's just some random bytes
vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 201, Hash: []byte("thehash"), VoteExtension: []byte("12345678")})
require.NoError(t, err)
require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status)

// Reject because the verification failed (no error)
app.SetVerifyVoteExtensionHandler(func(ctx sdk.Context, req *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) {
return nil, errors.New("some error")
})
vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 201, Hash: []byte("thehash"), VoteExtension: []byte("12345678")})
require.NoError(t, err)
require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status)
}

func TestABCI_GRPCQuery(t *testing.T) {
grpcQueryOpt := func(bapp *baseapp.BaseApp) {
testdata.RegisterQueryServer(
Expand Down
10 changes: 0 additions & 10 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ const (
execModeSimulate // Simulate a transaction
execModePrepareProposal // Prepare a block proposal
execModeProcessProposal // Process a block proposal
execModeVoteExtension // Extend or verify a pre-commit vote
execModeFinalize // Finalize a block proposal
)

Expand Down Expand Up @@ -104,11 +103,6 @@ type BaseApp struct {
// previous block's state. This state is never committed. In case of multiple
// consensus rounds, the state is always reset to the previous block's state.
//
// - voteExtensionState: Used for ExtendVote and VerifyVoteExtension, which is
// set based on the previous block's state. This state is never committed. In
// case of multiple rounds, the state is always reset to the previous block's
// state.
//
// - processProposalState: Used for ProcessProposal, which is set based on the
// the previous block's state. This state is never committed. In case of
// multiple rounds, the state is always reset to the previous block's state.
Expand All @@ -118,7 +112,6 @@ type BaseApp struct {
checkState *state
prepareProposalState *state
processProposalState *state
voteExtensionState *state
finalizeBlockState *state

// An inter-block write-through cache provided to the context during the ABCI
Expand Down Expand Up @@ -475,9 +468,6 @@ func (app *BaseApp) setState(mode execMode, header cmtproto.Header) {
case execModeProcessProposal:
app.processProposalState = baseState

case execModeVoteExtension:
app.voteExtensionState = baseState

case execModeFinalize:
app.finalizeBlockState = baseState

Expand Down
1 change: 0 additions & 1 deletion types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const (
ExecModeSimulate
ExecModePrepareProposal
ExecModeProcessProposal
ExecModeVoteExtension
ExecModeFinalize
)

Expand Down