Skip to content

Commit

Permalink
Fix the mismatch between "State.Version.Consensus.App" and "State.Con…
Browse files Browse the repository at this point in the history
…sensusParams.Version.AppVersion" (#533)

* Add AppProtocol as the default value

* Set/Load ConsensusParams

* Add a test of the replay after rollback

* Update AppVersion

* Add the comment about checking "AppVersion" in "verifyApp"

* Set "State.Version" into "Block.Header.Version"

* (fixup) Update AppVersion
  • Loading branch information
tnasu authored Dec 21, 2022
1 parent 617efcd commit 67311dc
Show file tree
Hide file tree
Showing 37 changed files with 146 additions and 88 deletions.
2 changes: 1 addition & 1 deletion abci/example/kvstore/kvstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var (
stateKey = []byte("stateKey")
kvPairPrefixKey = []byte("kvPairKey:")

ProtocolVersion uint64 = 0x1
ProtocolVersion uint64 = 99
)

type State struct {
Expand Down
6 changes: 4 additions & 2 deletions blockchain/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"

bcproto "github.com/line/ostracon/proto/ostracon/blockchain"
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/types"
)

Expand Down Expand Up @@ -80,8 +81,9 @@ func TestBcStatusResponseMessageValidateBasic(t *testing.T) {

// nolint:lll // ignore line length in tests
func TestBlockchainMessageVectors(t *testing.T) {
block := types.MakeBlock(int64(3), []types.Tx{types.Tx("Hello World")}, nil, nil)
block := types.MakeBlock(int64(3), []types.Tx{types.Tx("Hello World")}, nil, nil, sm.InitStateVersion.Consensus)
block.Version.Block = 11 // overwrite updated protocol version
block.Version.App = 11 // overwrite updated protocol version

bpb, err := block.ToProto()
require.NoError(t, err)
Expand All @@ -97,7 +99,7 @@ func TestBlockchainMessageVectors(t *testing.T) {
BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}},
"0a0a08ffffffffffffffff7f"},
{"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a700a6e0a5b0a02080b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855120d0a0b48656c6c6f20576f726c641a00"},
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a720a700a5d0a04080b100b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855120d0a0b48656c6c6f20576f726c641a00"},
{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{
NoBlockResponse: &bcproto.NoBlockResponse{Height: 1}}}, "12020801"},
{"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{
Expand Down
3 changes: 2 additions & 1 deletion blockchain/v1/peer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/line/ostracon/libs/log"
tmrand "github.com/line/ostracon/libs/rand"
"github.com/line/ostracon/p2p"
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/types"
)

Expand Down Expand Up @@ -276,5 +277,5 @@ func checkByStoppingPeerTimer(t *testing.T, peer *BpPeer, running bool) {
}

func makeSmallBlock(height int) *types.Block {
return types.MakeBlock(int64(height), []types.Tx{types.Tx("foo")}, nil, nil)
return types.MakeBlock(int64(height), []types.Tx{types.Tx("foo")}, nil, nil, sm.InitStateVersion.Consensus)
}
15 changes: 9 additions & 6 deletions blockchain/v1/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/line/ostracon/libs/log"
"github.com/line/ostracon/p2p"
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/types"
)

Expand Down Expand Up @@ -81,7 +82,9 @@ func makeBlockPool(bcr *testBcR, height int64, peers []BpPeer, blocks map[int64]
bPool.peers[p.id].RequestSent(h)
if p.create {
// simulate that a block at height h has been received
_ = bPool.peers[p.id].AddBlock(types.MakeBlock(h, txs, nil, nil), 100)
_ = bPool.peers[p.id].AddBlock(
types.MakeBlock(h, txs, nil, nil, sm.InitStateVersion.Consensus),
100)
}
}
return bPool
Expand Down Expand Up @@ -392,7 +395,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
pool: makeBlockPool(testBcR, 10, []BpPeer{{ID: "P1", Height: 100}}, map[int64]tPBlocks{}),
args: args{
peerID: "P2",
block: types.MakeBlock(int64(10), txs, nil, nil),
block: types.MakeBlock(int64(10), txs, nil, nil, sm.InitStateVersion.Consensus),
blockSize: 100,
},
poolWanted: makeBlockPool(testBcR, 10, []BpPeer{{ID: "P1", Height: 100}}, map[int64]tPBlocks{}),
Expand All @@ -404,7 +407,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
map[int64]tPBlocks{10: {"P1", false}}),
args: args{
peerID: "P1",
block: types.MakeBlock(int64(11), txs, nil, nil),
block: types.MakeBlock(int64(11), txs, nil, nil, sm.InitStateVersion.Consensus),
blockSize: 100,
},
poolWanted: makeBlockPool(testBcR, 10,
Expand All @@ -418,7 +421,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
map[int64]tPBlocks{10: {"P1", true}, 11: {"P1", false}}),
args: args{
peerID: "P1",
block: types.MakeBlock(int64(10), txs, nil, nil),
block: types.MakeBlock(int64(10), txs, nil, nil, sm.InitStateVersion.Consensus),
blockSize: 100,
},
poolWanted: makeBlockPool(testBcR, 10,
Expand All @@ -432,7 +435,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
map[int64]tPBlocks{10: {"P1", false}}),
args: args{
peerID: "P2",
block: types.MakeBlock(int64(10), txs, nil, nil),
block: types.MakeBlock(int64(10), txs, nil, nil, sm.InitStateVersion.Consensus),
blockSize: 100,
},
poolWanted: makeBlockPool(testBcR, 10,
Expand All @@ -446,7 +449,7 @@ func TestBlockPoolAddBlock(t *testing.T) {
map[int64]tPBlocks{10: {"P1", false}}),
args: args{
peerID: "P1",
block: types.MakeBlock(int64(10), txs, nil, nil),
block: types.MakeBlock(int64(10), txs, nil, nil, sm.InitStateVersion.Consensus),
blockSize: 100,
},
poolWanted: makeBlockPool(testBcR, 10,
Expand Down
5 changes: 3 additions & 2 deletions blockchain/v1/reactor_fsm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
tmmath "github.com/line/ostracon/libs/math"
tmrand "github.com/line/ostracon/libs/rand"
"github.com/line/ostracon/p2p"
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/types"
)

Expand Down Expand Up @@ -142,7 +143,7 @@ func sBlockRespEv(current, expected string, peerID p2p.ID, height int64, prevBlo
data: bReactorEventData{
peerID: peerID,
height: height,
block: types.MakeBlock(height, txs, nil, nil),
block: types.MakeBlock(height, txs, nil, nil, sm.InitStateVersion.Consensus),
length: 100},
wantState: expected,
wantNewBlocks: append(prevBlocks, height),
Expand All @@ -159,7 +160,7 @@ func sBlockRespEvErrored(current, expected string,
data: bReactorEventData{
peerID: peerID,
height: height,
block: types.MakeBlock(height, txs, nil, nil),
block: types.MakeBlock(height, txs, nil, nil, sm.InitStateVersion.Consensus),
length: 100},
wantState: expected,
wantErr: wantErr,
Expand Down
10 changes: 9 additions & 1 deletion consensus/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func (h *Handshaker) Handshake(proxyApp proxy.AppConns) error {

// Only set the version if there is no existing state.
if h.initialState.LastBlockHeight == 0 {
h.initialState.ConsensusParams.Version.AppVersion = res.AppVersion
h.initialState.Version.Consensus.App = res.AppVersion
}

Expand Down Expand Up @@ -487,6 +488,7 @@ func (h *Handshaker) replayBlocks(

if mutateState {
// sync the final block
h.logger.Info("Replaying final block using real app", "height", storeBlockHeight)
state, err = h.replayBlock(state, storeBlockHeight, proxyApp.Consensus())
if err != nil {
return nil, err
Expand All @@ -502,13 +504,19 @@ func (h *Handshaker) replayBlocks(
func (h *Handshaker) replayBlock(state sm.State, height int64, proxyApp proxy.AppConnConsensus) (sm.State, error) {
block := h.store.LoadBlock(height)
meta := h.store.LoadBlockMeta(height)
var err error
consensusParams, err := h.stateStore.LoadConsensusParams(height)
if err != nil {
return sm.State{}, err
}
state.ConsensusParams = consensusParams
state.Version.Consensus.App = consensusParams.Version.AppVersion

// Use stubs for both mempool and evidence pool since no transactions nor
// evidence are needed here - block already exists.
blockExec := sm.NewBlockExecutor(h.stateStore, h.logger, proxyApp, emptyMempool{}, sm.EmptyEvidencePool{})
blockExec.SetEventBus(h.eventBus)

var err error
state, _, err = blockExec.ApplyBlock(state, meta.BlockID, block, nil)
if err != nil {
return sm.State{}, err
Expand Down
33 changes: 23 additions & 10 deletions consensus/replay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/line/ostracon/proxy"
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/types"
"github.com/line/ostracon/version"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -351,7 +352,8 @@ var (
// 1 - saved block but app and state are behind
// 2 - save block and committed but state is behind
// 3 - save block and committed with truncated block store and state behind
var modes = []uint{0, 1, 2, 3}
// 4 - save block and committed with rollback state and state behind
var modes = []uint{0, 1, 2, 3, 4}

func getProposerIdx(state *State, height int64, round int32) (int32, *types.Validator) {
proposer := state.Validators.SelectProposer(state.state.LastProofHash, height, round)
Expand Down Expand Up @@ -682,6 +684,8 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin
TestSimulateValidatorsChange(t)
}
genesisState = sim.GenesisState
genesisState.ConsensusParams.Version.AppVersion = kvstore.ProtocolVersion
genesisState.Version.Consensus.App = kvstore.ProtocolVersion
config = sim.Config
chain = append([]*types.Block{}, sim.Chain...) // copy chain
commits = sim.Commits
Expand Down Expand Up @@ -746,10 +750,17 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin
require.EqualValues(t, 1, pruned)
expectError = int64(nBlocks) < 2
}
if mode == 4 {
rollbackHeight, rollbackAppHash, err := sm.Rollback(store, stateStore)
require.NoError(t, err)
require.EqualValues(t, state.LastBlockHeight, rollbackHeight)
require.EqualValues(t, state.AppHash, rollbackAppHash)
}

// now start the app using the handshake - it should sync
t.Log("####: now start the app using the handshake - it should sync")
genDoc, _ := sm.MakeGenesisDocFromFile(config.GenesisFile())
handshaker := NewHandshaker(stateStore, state, store, genDoc)
handshaker.SetLogger(log.TestingLogger())
proxyApp := proxy.NewAppConns(clientCreator2)
if err := proxyApp.Start(); err != nil {
t.Fatalf("Error starting proxy app connections: %v", err)
Expand Down Expand Up @@ -815,7 +826,8 @@ func buildAppStateFromChain(proxyApp proxy.AppConns, stateStore sm.Store,
}
defer proxyApp.Stop() //nolint:errcheck // ignore

state.Version.Consensus.App = kvstore.ProtocolVersion // simulate handshake, receive app version
state.ConsensusParams.Version.AppVersion = kvstore.ProtocolVersion // simulate handshake, receive app version
state.Version.Consensus.App = kvstore.ProtocolVersion // simulate handshake, receive app version
validators := types.OC2PB.ValidatorUpdates(state.Validators)
if _, err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{
Validators: validators,
Expand All @@ -831,13 +843,13 @@ func buildAppStateFromChain(proxyApp proxy.AppConns, stateStore sm.Store,
block := chain[i]
state = applyBlock(stateStore, state, block, proxyApp)
}
case 1, 2, 3:
case 1, 2, 3, 4:
for i := 0; i < nBlocks-1; i++ {
block := chain[i]
state = applyBlock(stateStore, state, block, proxyApp)
}

if mode == 2 || mode == 3 {
if mode == 2 || mode == 3 || mode == 4 {
// update the kvstore height and apphash
// as if we ran commit but not
state = applyBlock(stateStore, state, chain[nBlocks-1], proxyApp)
Expand Down Expand Up @@ -865,7 +877,8 @@ func buildOCStateFromChain(
}
defer proxyApp.Stop() //nolint:errcheck

state.Version.Consensus.App = kvstore.ProtocolVersion // simulate handshake, receive app version
state.ConsensusParams.Version.AppVersion = kvstore.ProtocolVersion // simulate handshake, receive app version
state.Version.Consensus.App = kvstore.ProtocolVersion // simulate handshake, receive app version
validators := types.OC2PB.ValidatorUpdates(state.Validators)
if _, err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{
Validators: validators,
Expand All @@ -882,7 +895,7 @@ func buildOCStateFromChain(
state = applyBlock(stateStore, state, block, proxyApp)
}

case 1, 2, 3:
case 1, 2, 3, 4:
// sync up to the penultimate as if we stored the block.
// whether we commit or not depends on the appHash
for _, block := range chain[:len(chain)-1] {
Expand All @@ -907,10 +920,9 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) {
config := ResetConfig("handshake_test_")
defer os.RemoveAll(config.RootDir)
privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
const appVersion = 0x0
pubKey, err := privVal.GetPubKey()
require.NoError(t, err)
stateDB, state, store := stateAndStore(config, pubKey, appVersion)
stateDB, state, store := stateAndStore(config, pubKey, version.AppProtocol)
stateStore := sm.NewStore(stateDB)
genDoc, _ := sm.MakeGenesisDocFromFile(config.GenesisFile())
state.LastVoters = state.Voters.Copy()
Expand Down Expand Up @@ -1173,6 +1185,7 @@ func stateAndStore(
stateDB := dbm.NewMemDB()
stateStore := sm.NewStore(stateDB)
state, _ := sm.MakeGenesisStateFromFile(config.GenesisFile())
state.ConsensusParams.Version.AppVersion = appVersion
state.Version.Consensus.App = appVersion
store := newMockBlockStore(config, state.ConsensusParams)
if err := stateStore.Save(state); err != nil {
Expand Down Expand Up @@ -1247,7 +1260,7 @@ func TestHandshakeUpdatesValidators(t *testing.T) {
privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
pubKey, err := privVal.GetPubKey()
require.NoError(t, err)
stateDB, state, store := stateAndStore(config, pubKey, 0x0)
stateDB, state, store := stateAndStore(config, pubKey, version.AppProtocol)
stateStore := sm.NewStore(stateDB)

oldValAddr := state.Validators.Validators[0].Address
Expand Down
4 changes: 2 additions & 2 deletions consensus/wal_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
sm "github.com/line/ostracon/state"
"github.com/line/ostracon/store"
"github.com/line/ostracon/types"
"github.com/pkg/errors"
)

// WALGenerateNBlocks generates a consensus WAL. It does this by spinning up a
Expand All @@ -44,7 +43,7 @@ func WALGenerateNBlocks(t *testing.T, wr io.Writer, numBlocks int) (err error) {
privKeyType := config.PrivValidatorKeyType()
privValidator, err := privval.LoadOrGenFilePV(privValidatorKeyFile, privValidatorStateFile, privKeyType)
if err != nil {
return errors.Wrap(err, "failed to create a private key")
return fmt.Errorf("failed to load FilePV: %w", err)
}
genDoc, err := types.GenesisDocFromFile(config.GenesisFile())
if err != nil {
Expand All @@ -57,6 +56,7 @@ func WALGenerateNBlocks(t *testing.T, wr io.Writer, numBlocks int) (err error) {
if err != nil {
return fmt.Errorf("failed to make genesis state: %w", err)
}
state.ConsensusParams.Version.AppVersion = kvstore.ProtocolVersion
state.Version.Consensus.App = kvstore.ProtocolVersion
if err = stateStore.Save(state); err != nil {
t.Error(err)
Expand Down
13 changes: 6 additions & 7 deletions evidence/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@ import (
"testing"
"time"

"github.com/line/ostracon/crypto"
"github.com/line/ostracon/crypto/bls"
"github.com/line/ostracon/crypto/composite"
"github.com/line/ostracon/crypto/ed25519"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

dbm "github.com/tendermint/tm-db"

"github.com/line/ostracon/crypto"
"github.com/line/ostracon/crypto/bls"
"github.com/line/ostracon/crypto/composite"
"github.com/line/ostracon/crypto/ed25519"
"github.com/line/ostracon/evidence"
"github.com/line/ostracon/evidence/mocks"
"github.com/line/ostracon/libs/log"
Expand Down Expand Up @@ -211,7 +210,7 @@ func TestEvidencePoolUpdate(t *testing.T) {
ev := types.NewMockDuplicateVoteEvidenceWithValidator(height, defaultEvidenceTime.Add(21*time.Minute),
val, evidenceChainID)
lastCommit := makeCommit(height, val.PrivKey.PubKey().Address())
block := types.MakeBlock(height+1, []types.Tx{}, lastCommit, []types.Evidence{ev})
block := types.MakeBlock(height+1, []types.Tx{}, lastCommit, []types.Evidence{ev}, sm.InitStateVersion.Consensus)
// update state (partially)
state.LastBlockHeight = height + 1
state.LastBlockTime = defaultEvidenceTime.Add(22 * time.Minute)
Expand Down Expand Up @@ -436,7 +435,7 @@ func initializeBlockStore(db dbm.DB, state sm.State, valPrivkey crypto.PrivKey)
block, _ := state.MakeBlock(i, []types.Tx{}, lastCommit, nil,
state.Validators.SelectProposer(proof, i, round).Address, round, proof)
block.Header.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute)
block.Header.Version = tmversion.Consensus{Block: version.BlockProtocol, App: 1}
block.Header.Version = tmversion.Consensus{Block: version.BlockProtocol, App: version.AppProtocol}
const parts = 1
partSet := block.MakePartSet(parts)

Expand Down
5 changes: 2 additions & 3 deletions evidence/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"testing"
"time"

"github.com/line/ostracon/light"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -19,6 +17,7 @@ import (
"github.com/line/ostracon/evidence"
"github.com/line/ostracon/evidence/mocks"
"github.com/line/ostracon/libs/log"
"github.com/line/ostracon/light"
tmproto "github.com/line/ostracon/proto/ostracon/types"
tmversion "github.com/line/ostracon/proto/ostracon/version"
sm "github.com/line/ostracon/state"
Expand Down Expand Up @@ -725,7 +724,7 @@ func makeVote(

func makeHeaderRandom(height int64) *types.Header {
return &types.Header{
Version: tmversion.Consensus{Block: version.BlockProtocol, App: 1},
Version: tmversion.Consensus{Block: version.BlockProtocol, App: version.AppProtocol},
ChainID: evidenceChainID,
Height: height,
Time: defaultEvidenceTime,
Expand Down
2 changes: 1 addition & 1 deletion light/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func genHeader(chainID string, height int64, bTime time.Time, txs types.Txs,
proof tmbytes.HexBytes) *types.Header {

return &types.Header{
Version: tmversion.Consensus{Block: version.BlockProtocol, App: 0},
Version: tmversion.Consensus{Block: version.BlockProtocol, App: version.AppProtocol},
ChainID: chainID,
Height: height,
Time: bTime,
Expand Down
Loading

0 comments on commit 67311dc

Please sign in to comment.