mirrors/tendermint
1
0
Fork 0
mirror of https://github.com/tendermint/tendermint.git synced 2026-01-07 13:55:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Follow-up fixes to main PrepareProposal implementation (#9162)

Browse Source 
* -----start------

* [cherrypicked] state: panic on ResponsePrepareProposal validation error (#8145)

* state: panic on ResponsePrepareProposal validation error

* lint++

Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* [cherrypicked] abci++: remove CheckTx call from PrepareProposal flow (#8176)

* [cherrypicked] abci++: correct max-size check to only operate on added and unmodified (#8242)

* [cherrypicked] Remove `ModifiedTxStatus` from the spec and the code (#8210)

* Outstanding abci-gen changes to 'pb.go' files

* Removed modified_tx_status from spec and protobufs

* Fix sed for OSX

* Regenerated abci protobufs with 'abci-proto-gen'

* Code changes. UTs e2e tests passing

* Recovered UT: TestPrepareProposalModifiedTxStatusFalse

* Adapted UT

* Fixed UT

* Revert "Fix sed for OSX"

This reverts commit e576708c61.

* Update internal/state/execution_test.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* Update internal/state/execution_test.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Addressed some comments

* Added one test that tests error at the ABCI client + Fixed some mock calls

* Addressed remaining comments

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update abci/example/kvstore/kvstore.go

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Update spec/abci++/abci++_tmint_expected_behavior_002_draft.md

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

* Addressed William's latest comments

* Adressed Michael's comment

* Fixed UT

* Some md fixes

* More md fixes

* gofmt

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>

* make proto-gen

* Fixed testcase on PrepareProposal error

* mockery

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Co-authored-by: Sam Kleinman <garen@tychoish.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
This commit is contained in:
Sergio Mena
2022-08-03 19:39:17 +02:00
committed by GitHub
parent d268e56383
commit b2409b3345
18 changed files with 347 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files

182
state/execution_test.go
View File

@@ -2,6 +2,7 @@ package state_test
import (
"context"
"errors"
"testing"
"time"
@@ -9,6 +10,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
abciclientmocks "github.com/tendermint/tendermint/abci/client/mocks"
abci "github.com/tendermint/tendermint/abci/types"
abcimocks "github.com/tendermint/tendermint/abci/types/mocks"
"github.com/tendermint/tendermint/crypto"
@@ -20,6 +22,7 @@ import (
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmversion "github.com/tendermint/tendermint/proto/tendermint/version"
"github.com/tendermint/tendermint/proxy"
pmocks "github.com/tendermint/tendermint/proxy/mocks"
sm "github.com/tendermint/tendermint/state"
"github.com/tendermint/tendermint/state/mocks"
sf "github.com/tendermint/tendermint/state/test/factory"
@@ -496,7 +499,7 @@ func TestEndBlockValidatorUpdatesResultingInEmptySet(t *testing.T) {
func TestEmptyPrepareProposal(t *testing.T) {
const height = 2
app := &testApp{}
app := abcimocks.NewBaseMock()
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
@@ -532,6 +535,57 @@ func TestEmptyPrepareProposal(t *testing.T) {
require.NoError(t, err)
}
// TestPrepareProposalErrorOnNonExistingRemoved tests that the block creation logic returns
// an error if the ResponsePrepareProposal returned from the application marks
// a transaction as REMOVED that was not present in the original proposal.
func TestPrepareProposalErrorOnNonExistingRemoved(t *testing.T) {
const height = 2
state, stateDB, privVals := makeState(1, height)
stateStore := sm.NewStore(stateDB)
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs{})
// create an invalid ResponsePrepareProposal
rpp := abci.ResponsePrepareProposal{
TxRecords: []*abci.TxRecord{
{
Action: abci.TxRecord_REMOVED,
Tx: []byte("new tx"),
},
},
}
app := abcimocks.NewBaseMock()
app.On("PrepareProposal", mock.Anything).Return(rpp)
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
require.NoError(t, err)
defer proxyApp.Stop() //nolint:errcheck // ignore for tests
blockExec := sm.NewBlockExecutor(
stateStore,
log.TestingLogger(),
proxyApp.Consensus(),
mp,
evpool,
)
pa, _ := state.Validators.GetByIndex(0)
commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals)
require.NoError(t, err)
block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil)
require.Nil(t, block)
require.ErrorContains(t, err, "new transaction incorrectly marked as removed")
mp.AssertExpectations(t)
}
// TestPrepareProposalRemoveTxs tests that any transactions marked as REMOVED
// are not included in the block produced by CreateProposalBlock. The test also
// ensures that any transactions removed are also removed from the mempool.
@@ -544,7 +598,7 @@ func TestPrepareProposalRemoveTxs(t *testing.T) {
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
txs := factory.MakeTenTxs(height)
txs := factory.MakeNTxs(height, 10)
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs))
@@ -555,9 +609,8 @@ func TestPrepareProposalRemoveTxs(t *testing.T) {
app := abcimocks.NewBaseMock()
app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{
ModifiedTx: true,
TxRecords: trs,
}, nil)
TxRecords: trs,
})
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
@@ -587,8 +640,7 @@ func TestPrepareProposalRemoveTxs(t *testing.T) {
}
// TestPrepareProposalAddedTxsIncluded tests that any transactions marked as ADDED
// in the prepare proposal response are included in the block. The test also
// ensures that any transactions added are also checked into the mempool.
// in the prepare proposal response are included in the block.
func TestPrepareProposalAddedTxsIncluded(t *testing.T) {
const height = 2
@@ -598,10 +650,9 @@ func TestPrepareProposalAddedTxsIncluded(t *testing.T) {
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
txs := factory.MakeTenTxs(height)
txs := factory.MakeNTxs(height, 10)
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs[2:]))
mp.On("CheckTx", mock.Anything, mock.Anything, mock.Anything).Return(nil).Twice()
trs := txsToTxRecords(types.Txs(txs))
trs[0].Action = abci.TxRecord_ADDED
@@ -609,13 +660,13 @@ func TestPrepareProposalAddedTxsIncluded(t *testing.T) {
app := abcimocks.NewBaseMock()
app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{
ModifiedTx: true,
TxRecords: trs,
}, nil)
TxRecords: trs,
})
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
require.NoError(t, err)
defer proxyApp.Stop() //nolint:errcheck // ignore for tests
blockExec := sm.NewBlockExecutor(
stateStore,
@@ -634,8 +685,6 @@ func TestPrepareProposalAddedTxsIncluded(t *testing.T) {
require.Equal(t, txs[1], block.Data.Txs[1])
mp.AssertExpectations(t)
mp.AssertCalled(t, "CheckTx", types.Tx(trs[0].Tx), mock.Anything, mock.Anything)
mp.AssertCalled(t, "CheckTx", types.Tx(trs[1].Tx), mock.Anything, mock.Anything)
}
// TestPrepareProposalReorderTxs tests that CreateBlock produces a block with transactions
@@ -649,7 +698,7 @@ func TestPrepareProposalReorderTxs(t *testing.T) {
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
txs := factory.MakeTenTxs(height)
txs := factory.MakeNTxs(height, 10)
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs))
@@ -659,14 +708,14 @@ func TestPrepareProposalReorderTxs(t *testing.T) {
app := abcimocks.NewBaseMock()
app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{
ModifiedTx: true,
TxRecords: trs,
}, nil)
TxRecords: trs,
})
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
require.NoError(t, err)
defer proxyApp.Stop() //nolint:errcheck // ignore for tests
blockExec := sm.NewBlockExecutor(
stateStore,
@@ -688,6 +737,103 @@ func TestPrepareProposalReorderTxs(t *testing.T) {
}
// TestPrepareProposalErrorOnTooManyTxs tests that the block creation logic returns
// an error if the ResponsePrepareProposal returned from the application is invalid.
func TestPrepareProposalErrorOnTooManyTxs(t *testing.T) {
const height = 2
state, stateDB, privVals := makeState(1, height)
// limit max block size
state.ConsensusParams.Block.MaxBytes = 60 * 1024
stateStore := sm.NewStore(stateDB)
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
const nValidators = 1
var bytesPerTx int64 = 3
maxDataBytes := types.MaxDataBytes(state.ConsensusParams.Block.MaxBytes, 0, nValidators)
txs := factory.MakeNTxs(height, maxDataBytes/bytesPerTx+2) // +2 so that tx don't fit
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs))
trs := txsToTxRecords(types.Txs(txs))
app := abcimocks.NewBaseMock()
app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{
TxRecords: trs,
})
cc := proxy.NewLocalClientCreator(app)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
require.NoError(t, err)
defer proxyApp.Stop() //nolint:errcheck // ignore for tests
blockExec := sm.NewBlockExecutor(
stateStore,
log.NewNopLogger(),
proxyApp.Consensus(),
mp,
evpool,
)
pa, _ := state.Validators.GetByIndex(0)
commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals)
require.NoError(t, err)
block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil)
require.Nil(t, block)
require.ErrorContains(t, err, "transaction data size exceeds maximum")
mp.AssertExpectations(t)
}
// TestPrepareProposalErrorOnPrepareProposalError tests when the client returns an error
// upon calling PrepareProposal on it.
func TestPrepareProposalErrorOnPrepareProposalError(t *testing.T) {
const height = 2
state, stateDB, privVals := makeState(1, height)
stateStore := sm.NewStore(stateDB)
evpool := &mocks.EvidencePool{}
evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0))
txs := factory.MakeNTxs(height, 10)
mp := &mpmocks.Mempool{}
mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs))
cm := &abciclientmocks.Client{}
cm.On("SetLogger", mock.Anything).Return()
cm.On("Start").Return(nil)
cm.On("Quit").Return(nil)
cm.On("PrepareProposalSync", mock.Anything).Return(nil, errors.New("an injected error")).Once()
cm.On("Stop").Return(nil)
cc := &pmocks.ClientCreator{}
cc.On("NewABCIClient").Return(cm, nil)
proxyApp := proxy.NewAppConns(cc)
err := proxyApp.Start()
require.NoError(t, err)
defer proxyApp.Stop() //nolint:errcheck // ignore for tests
blockExec := sm.NewBlockExecutor(
stateStore,
log.NewNopLogger(),
proxyApp.Consensus(),
mp,
evpool,
)
pa, _ := state.Validators.GetByIndex(0)
commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals)
require.NoError(t, err)
block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil)
require.Nil(t, block)
require.ErrorContains(t, err, "an injected error")
mp.AssertExpectations(t)
}
func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.BlockID {
var (
h = make([]byte, tmhash.Size)