Skip to content

Commit

Permalink
test: ResponsePrepareProposal.app_version set to required
Browse files Browse the repository at this point in the history
  • Loading branch information
lklimek committed Mar 27, 2024
1 parent aaa4f5e commit 686c087
Show file tree
Hide file tree
Showing 18 changed files with 109 additions and 81 deletions.
3 changes: 2 additions & 1 deletion abci/client/routed_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func TestRouting(t *testing.T) {
consensusApp, consensusSocket := startApp(ctx, t, logger, "consensus")
defer consensusApp.AssertExpectations(t)
consensusApp.On("PrepareProposal", mock.Anything, mock.Anything).Return(&types.ResponsePrepareProposal{
AppHash: []byte("apphash"),
AppHash: []byte("apphash"),
AppVersion: 1,
}, nil).Once()
consensusApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&types.ResponseFinalizeBlock{
RetainHeight: 1,
Expand Down
1 change: 1 addition & 0 deletions abci/example/counter/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (app *Application) PrepareProposal(_ context.Context, req *types.RequestPre
AppHash: make([]byte, tmcrypto.DefaultAppHashSize),
CoreChainLockUpdate: app.lastCoreChainLock.ToProto(),
TxResults: app.lastTxResults,
AppVersion: 1,
}
return &resp, nil
}
Expand Down
2 changes: 1 addition & 1 deletion abci/example/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestKVStore(t *testing.T) {
logger := log.NewNopLogger()

t.Log("### Testing KVStore")
app, err := kvstore.NewMemoryApp(kvstore.WithLogger(logger), kvstore.WithEnforceVersionToHeight())
app, err := kvstore.NewMemoryApp(kvstore.WithLogger(logger), kvstore.WithAppVersion(0))
require.NoError(t, err)
testBulk(ctx, t, logger, app)
}
Expand Down
64 changes: 33 additions & 31 deletions abci/example/kvstore/kvstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ type Application struct {
offerSnapshot *offerSnapshot

shouldCommitVerify bool
// shouldEnforceVersionToHeight set to true will cause the application to enforce the app version to be equal to the height
shouldEnforceVersionToHeight bool
// appVersion returned in ResponsePrepareProposal.
// Special value of 0 means that it will be always set to current height.
appVersion uint64
}

// WithCommitVerification enables commit verification
Expand All @@ -99,11 +100,11 @@ func WithCommitVerification() OptFunc {
}
}

// WithEnforceVersionToHeight enables the application to enforce the app version to be equal to the height using app_version
// field in the RequestPrepareProposal
func WithEnforceVersionToHeight() OptFunc {
// WithAppVersion enables the application to enforce the app version to be equal to provided value.
// Special value of `0` means that app version will be always set to current block version.
func WithAppVersion(version uint64) OptFunc {
return func(app *Application) error {
app.shouldEnforceVersionToHeight = true
app.appVersion = version
return nil
}
}
Expand Down Expand Up @@ -212,20 +213,20 @@ func newApplication(stateStore StoreFactory, opts ...OptFunc) (*Application, err
var err error

app := &Application{
logger: log.NewNopLogger(),
LastCommittedState: NewKvState(dbm.NewMemDB(), initialHeight), // initial state to avoid InitChain() in unit tests
roundStates: map[string]State{},
preparedProposals: map[int32]bool{},
processedProposals: map[int32]bool{},
validatorSetUpdates: map[int64]abci.ValidatorSetUpdate{},
consensusParamsUpdates: map[int64]types1.ConsensusParams{},
initialHeight: initialHeight,
store: stateStore,
prepareTxs: prepareTxs,
verifyTx: verifyTx,
execTx: execTx,
shouldCommitVerify: false,
shouldEnforceVersionToHeight: false,
logger: log.NewNopLogger(),
LastCommittedState: NewKvState(dbm.NewMemDB(), initialHeight), // initial state to avoid InitChain() in unit tests
roundStates: map[string]State{},
preparedProposals: map[int32]bool{},
processedProposals: map[int32]bool{},
validatorSetUpdates: map[int64]abci.ValidatorSetUpdate{},
consensusParamsUpdates: map[int64]types1.ConsensusParams{},
initialHeight: initialHeight,
store: stateStore,
prepareTxs: prepareTxs,
verifyTx: verifyTx,
execTx: execTx,
shouldCommitVerify: false,
appVersion: ProtocolVersion,
}

for _, opt := range opts {
Expand Down Expand Up @@ -360,10 +361,7 @@ func (app *Application) PrepareProposal(_ context.Context, req *abci.RequestPrep
ConsensusParamUpdates: app.getConsensusParamsUpdate(req.Height),
CoreChainLockUpdate: coreChainLock,
ValidatorSetUpdate: app.getValidatorSetUpdate(req.Height),
}

if app.shouldEnforceVersionToHeight {
resp.AppVersion = uint64(roundState.GetHeight())
AppVersion: app.appVersionForHeight(req.Height),
}

if app.cfg.PrepareProposalDelayMS != 0 {
Expand Down Expand Up @@ -392,9 +390,9 @@ func (app *Application) ProcessProposal(_ context.Context, req *abci.RequestProc
}, err
}

if app.shouldEnforceVersionToHeight && req.Version.App != uint64(roundState.GetHeight()) {
app.logger.Error("app version mismatch, kvstore expects it to be equal to the current height",
"version", req.Version, "height", roundState.GetHeight())
if req.Version.App != app.appVersionForHeight(req.Height) {
app.logger.Error("app version mismatch in process proposal request",
"version", req.Version.App, "expected", app.appVersionForHeight(req.Height), "height", roundState.GetHeight())
return &abci.ResponseProcessProposal{
Status: abci.ResponseProcessProposal_REJECT,
}, nil
Expand Down Expand Up @@ -583,6 +581,13 @@ func (app *Application) ApplySnapshotChunk(_ context.Context, req *abci.RequestA
app.logger.Debug("ApplySnapshotChunk", "resp", resp)
return resp, nil
}
func (app *Application) appVersionForHeight(height int64) uint64 {
if app.appVersion == 0 {
return uint64(height)
}

return app.appVersion
}

func (app *Application) createSnapshot() error {
height := app.LastCommittedState.GetHeight()
Expand All @@ -608,11 +613,8 @@ func (app *Application) Info(_ context.Context, req *abci.RequestInfo) (*abci.Re
app.mu.Lock()
defer app.mu.Unlock()
appHash := app.LastCommittedState.GetAppHash()
appVersion := app.appVersionForHeight(app.LastCommittedState.GetHeight() + 1) // we set app version to CURRENT height

appVersion := ProtocolVersion
if app.shouldEnforceVersionToHeight {
appVersion = uint64(app.LastCommittedState.GetHeight()) + 1 // we set app version to CURRENT height
}
resp := &abci.ResponseInfo{
Data: fmt.Sprintf("{\"appHash\":\"%s\"}", appHash.String()),
Version: version.ABCIVersion,
Expand Down
12 changes: 6 additions & 6 deletions abci/example/kvstore/kvstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestPersistentKVStoreKV(t *testing.T) {

kvstore, err := NewPersistentApp(DefaultConfig(dir),
WithLogger(logger.With("module", "kvstore")),
WithEnforceVersionToHeight())
WithAppVersion(0))
require.NoError(t, err)

key := testKey
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestPersistentKVStoreInfo(t *testing.T) {
dir := t.TempDir()
logger := log.NewTestingLogger(t).With("module", "kvstore")

kvstore, err := NewPersistentApp(DefaultConfig(dir), WithLogger(logger), WithEnforceVersionToHeight())
kvstore, err := NewPersistentApp(DefaultConfig(dir), WithLogger(logger), WithAppVersion(0))
require.NoError(t, err)
defer kvstore.Close()

Expand Down Expand Up @@ -245,7 +245,7 @@ func TestValUpdates(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

kvstore, err := NewMemoryApp(WithEnforceVersionToHeight())
kvstore, err := NewMemoryApp(WithAppVersion(0))
require.NoError(t, err)

// init with some validators
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestClientServer(t *testing.T) {
// set up socket app
kvstore, err := NewMemoryApp(
WithLogger(logger.With("module", "app")),
WithEnforceVersionToHeight())
WithAppVersion(0))
require.NoError(t, err)

client, server, err := makeSocketClientServer(ctx, t, logger, kvstore, "kvstore-socket")
Expand All @@ -387,7 +387,7 @@ func TestClientServer(t *testing.T) {
runClientTests(ctx, t, client)

// set up grpc app
kvstore, err = NewMemoryApp(WithEnforceVersionToHeight())
kvstore, err = NewMemoryApp(WithAppVersion(0))
require.NoError(t, err)

gclient, gserver, err := makeGRPCClientServer(ctx, t, logger, kvstore, "/tmp/kvstore-grpc")
Expand Down Expand Up @@ -530,7 +530,7 @@ func newKvApp(ctx context.Context, t *testing.T, genesisHeight int64, opts ...Op
WithValidatorSetUpdates(map[int64]types.ValidatorSetUpdate{
genesisHeight: RandValidatorSetUpdate(1),
}),
WithEnforceVersionToHeight(),
WithAppVersion(0),
}
app, err := NewMemoryApp(append(defaultOpts, opts...)...)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions abci/types/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ func (BaseApplication) PrepareProposal(_ context.Context, req *RequestPreparePro
})
}
return &ResponsePrepareProposal{TxRecords: trs,
AppHash: make([]byte, crypto.DefaultAppHashSize),
TxResults: txResults(req.Txs),
AppHash: make([]byte, crypto.DefaultAppHashSize),
TxResults: txResults(req.Txs),
AppVersion: 1,
}, nil
}

Expand Down
5 changes: 3 additions & 2 deletions dash/quorum/validator_conn_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,9 @@ func (app *testApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP
}

return &abci.ResponsePrepareProposal{
AppHash: resultsHash,
TxResults: results,
AppHash: resultsHash,
TxResults: results,
AppVersion: 1,
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/consensus/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ func makeConsensusState(
walDir := filepath.Dir(thisConfig.Consensus.WalFile())
ensureDir(t, walDir, 0700)

app, err := kvstore.NewMemoryApp()
app, err := kvstore.NewMemoryApp(kvstore.WithLogger(logger))
require.NoError(t, err)
t.Cleanup(func() { _ = app.Close() })

Expand Down
7 changes: 4 additions & 3 deletions internal/consensus/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,10 @@ func (app *CounterApplication) PrepareProposal(_ context.Context, req *abci.Requ
})
}
return &abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
TxRecords: trs,
TxResults: app.txResults(req.Txs),
AppHash: make([]byte, crypto.DefaultAppHashSize),
TxRecords: trs,
TxResults: app.txResults(req.Txs),
AppVersion: 1,
}, nil
}

Expand Down
13 changes: 7 additions & 6 deletions internal/consensus/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2025,7 +2025,7 @@ func TestProcessProposalAccept(t *testing.T) {
Status: status,
}, nil)
m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil).Maybe()

cs1, _ := makeState(ctx, t, makeStateArgs{config: config, application: m})
Expand Down Expand Up @@ -2080,7 +2080,7 @@ func TestFinalizeBlockCalled(t *testing.T) {
Status: abci.ResponseProcessProposal_ACCEPT,
}, nil)
m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil)
// We only expect VerifyVoteExtension to be called on non-nil precommits.
// https://github.com/tendermint/tendermint/issues/8487
Expand Down Expand Up @@ -2152,7 +2152,7 @@ func TestExtendVote(t *testing.T) {
Status: abci.ResponseProcessProposal_ACCEPT,
}, nil)
m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil)
m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
VoteExtensions: []*abci.ExtendVoteExtension{
Expand Down Expand Up @@ -2311,7 +2311,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
Status: abci.ResponseProcessProposal_ACCEPT,
}, nil)
m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil)
m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
VoteExtensions: voteExtensions.ToExtendProto(),
Expand Down Expand Up @@ -2434,7 +2434,7 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
assert.EqualValues(t, voteExtensions.GetExtensions(), extensions)

})).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil)

m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_ACCEPT}, nil)
Expand Down Expand Up @@ -3157,6 +3157,7 @@ func TestStateTryAddCommitCallsProcessProposal(t *testing.T) {

block, err := sf.MakeBlock(css0StateData.state, 1, &types.Commit{}, kvstore.ProtocolVersion)
require.NoError(t, err)
require.NotZero(t, block.Version.App)
block.CoreChainLockedHeight = 1

commit, err := factory.MakeCommit(
Expand Down Expand Up @@ -3322,7 +3323,7 @@ func mockProposerApplicationCalls(t *testing.T, m *abcimocks.Application, round
})

m.On("PrepareProposal", mock.Anything, roundMatcher).Return(&abci.ResponsePrepareProposal{
AppHash: make([]byte, crypto.DefaultAppHashSize),
AppHash: make([]byte, crypto.DefaultAppHashSize), AppVersion: 1,
}, nil).Once()

m.On("ProcessProposal", mock.Anything, roundMatcher).Return(&abci.ResponseProcessProposal{
Expand Down
6 changes: 3 additions & 3 deletions internal/state/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,12 @@ func (blockExec *BlockExecutor) ProcessProposal(
if resp.IsStatusUnknown() {
return CurrentRoundState{}, fmt.Errorf("ProcessProposal responded with status %s", resp.Status.String())
}
if err := resp.Validate(); err != nil {
return CurrentRoundState{}, fmt.Errorf("ProcessProposal responded with invalid response: %w", err)
}
if !resp.IsAccepted() {
return CurrentRoundState{}, ErrBlockRejected
}
if err := resp.Validate(); err != nil {
return CurrentRoundState{}, fmt.Errorf("ProcessProposal responded with invalid response: %w", err)
}
if err := validateExecTxResults(resp.TxResults, block.Data.Txs); err != nil {
return CurrentRoundState{}, fmt.Errorf("invalid tx results: %w", err)
}
Expand Down
Loading

0 comments on commit 686c087

Please sign in to comment.