Skip to content

Commit

Permalink
feat(abci): proposer-based app version (#769)
Browse files Browse the repository at this point in the history
* feat(abci)!: proposer-based app version

Proposer, during CreateProposal, can propose app version to be included in the current block, replacing the one determined based on consensus params.

* chore: bump abci version requirement

* test(state): remove invalid tests from header/block validation

* doc(proto): minor improvements

* test(kvstore): enforce version to be equal to height

* test(e2e): verify that enforcing version to be equal to height in kvstore works

* test(kvstore): fix tests

* chore: RequestPrepareProposal.app_version is REQUIRED

* refactor(proto)!: deprecate ConsensusParams.AppVersion

* test:  ResponsePrepareProposal.app_version set to required

* test: fix tests for app version

* fix(abci-cli): add version
  • Loading branch information
lklimek authored Mar 27, 2024
1 parent bc91645 commit 39c275a
Show file tree
Hide file tree
Showing 31 changed files with 558 additions and 331 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
12 changes: 7 additions & 5 deletions abci/cmd/abci-cli/abci-cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/dashpay/tenderdash/libs/log"
"github.com/dashpay/tenderdash/proto/tendermint/crypto"
tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
pbversion "github.com/dashpay/tenderdash/proto/tendermint/version"
"github.com/dashpay/tenderdash/version"
)

Expand Down Expand Up @@ -54,7 +55,7 @@ func RootCmmand(logger log.Logger) *cobra.Command {
Use: "abci-cli",
Short: "the ABCI CLI tool wraps an ABCI client",
Long: "the ABCI CLI tool wraps an ABCI client and is used for testing ABCI servers",
PersistentPreRunE: func(cmd *cobra.Command, args []string) (err error) {
PersistentPreRunE: func(cmd *cobra.Command, _args []string) (err error) {

switch cmd.Use {
case "kvstore", "version":
Expand Down Expand Up @@ -223,7 +224,7 @@ var versionCmd = &cobra.Command{
Short: "print ABCI console version",
Long: "print ABCI console version",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
RunE: func(_cmd *cobra.Command, _args []string) error {
fmt.Println(version.ABCIVersion)
return nil
},
Expand Down Expand Up @@ -697,8 +698,9 @@ func cmdProcessProposal(cmd *cobra.Command, args []string) error {
panic(err)
}
res, err := client.ProcessProposal(cmd.Context(), &types.RequestProcessProposal{
Height: height,
Txs: txsBytesArray,
Height: height,
Txs: txsBytesArray,
Version: &pbversion.Consensus{Block: version.BlockProtocol, App: kvstore.ProtocolVersion},
})
if err != nil {
return err
Expand All @@ -711,7 +713,7 @@ func cmdProcessProposal(cmd *cobra.Command, args []string) error {
}

func makeKVStoreCmd(logger log.Logger) func(*cobra.Command, []string) error {
return func(cmd *cobra.Command, args []string) error {
return func(cmd *cobra.Command, _args []string) error {
// Create the application - in memory or persisted to disk
var (
app types.Application
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
9 changes: 7 additions & 2 deletions abci/example/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/dashpay/tenderdash/abci/types"
"github.com/dashpay/tenderdash/libs/log"
tmnet "github.com/dashpay/tenderdash/libs/net"
"github.com/dashpay/tenderdash/proto/tendermint/version"
)

func TestKVStore(t *testing.T) {
Expand All @@ -28,7 +29,7 @@ func TestKVStore(t *testing.T) {
logger := log.NewNopLogger()

t.Log("### Testing KVStore")
app, err := kvstore.NewMemoryApp(kvstore.WithLogger(logger))
app, err := kvstore.NewMemoryApp(kvstore.WithLogger(logger), kvstore.WithAppVersion(0))
require.NoError(t, err)
testBulk(ctx, t, logger, app)
}
Expand Down Expand Up @@ -73,7 +74,11 @@ func testBulk(ctx context.Context, t *testing.T, logger log.Logger, app types.Ap
require.NoError(t, err)

// Construct request
rpp := &types.RequestProcessProposal{Height: 1, Txs: make([][]byte, numDeliverTxs)}
rpp := &types.RequestProcessProposal{
Height: 1,
Txs: make([][]byte, numDeliverTxs),
Version: &version.Consensus{App: 1},
}
for counter := 0; counter < numDeliverTxs; counter++ {
rpp.Txs[counter] = []byte("test")
}
Expand Down
39 changes: 36 additions & 3 deletions abci/example/kvstore/kvstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ import (
"github.com/dashpay/tenderdash/version"
)

const ProtocolVersion uint64 = 0x12345678
// ProtocolVersion defines initial protocol (app) version.
// App version is incremented on every block, to match current height.
const ProtocolVersion uint64 = 1

//---------------------------------------------------

Expand Down Expand Up @@ -85,6 +87,9 @@ type Application struct {
offerSnapshot *offerSnapshot

shouldCommitVerify 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 @@ -95,6 +100,15 @@ func WithCommitVerification() 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.appVersion = version
return nil
}
}

// WithValidatorSetUpdates defines initial validator set when creating Application
func WithValidatorSetUpdates(validatorSetUpdates map[int64]abci.ValidatorSetUpdate) OptFunc {
return func(app *Application) error {
Expand Down Expand Up @@ -212,6 +226,7 @@ func newApplication(stateStore StoreFactory, opts ...OptFunc) (*Application, err
verifyTx: verifyTx,
execTx: execTx,
shouldCommitVerify: false,
appVersion: ProtocolVersion,
}

for _, opt := range opts {
Expand Down Expand Up @@ -287,7 +302,7 @@ func (app *Application) InitChain(_ context.Context, req *abci.RequestInitChain)
if !ok {
consensusParams = types1.ConsensusParams{
Version: &types1.VersionParams{
AppVersion: ProtocolVersion,
AppVersion: uint64(app.LastCommittedState.GetHeight()) + 1,
},
}
}
Expand Down Expand Up @@ -346,6 +361,7 @@ func (app *Application) PrepareProposal(_ context.Context, req *abci.RequestPrep
ConsensusParamUpdates: app.getConsensusParamsUpdate(req.Height),
CoreChainLockUpdate: coreChainLock,
ValidatorSetUpdate: app.getValidatorSetUpdate(req.Height),
AppVersion: app.appVersionForHeight(req.Height),
}

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

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
}

resp := &abci.ResponseProcessProposal{
Status: abci.ResponseProcessProposal_ACCEPT,
AppHash: roundState.GetAppHash(),
Expand Down Expand Up @@ -555,6 +579,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 @@ -580,10 +611,12 @@ 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

resp := &abci.ResponseInfo{
Data: fmt.Sprintf("{\"appHash\":\"%s\"}", appHash.String()),
Version: version.ABCIVersion,
AppVersion: ProtocolVersion,
AppVersion: appVersion,
LastBlockHeight: app.LastCommittedState.GetHeight(),
LastBlockAppHash: app.LastCommittedState.GetAppHash(),
}
Expand Down
39 changes: 24 additions & 15 deletions abci/example/kvstore/kvstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/dashpay/tenderdash/libs/log"
"github.com/dashpay/tenderdash/libs/service"
tmproto "github.com/dashpay/tenderdash/proto/tendermint/types"
pbversion "github.com/dashpay/tenderdash/proto/tendermint/version"
tmtypes "github.com/dashpay/tenderdash/types"
"github.com/dashpay/tenderdash/version"
)
Expand Down Expand Up @@ -48,8 +49,9 @@ func testKVStore(ctx context.Context, t *testing.T, app types.Application, tx []
require.ErrorContains(t, err, "duplicate PrepareProposal call")

reqProcess := &types.RequestProcessProposal{
Txs: [][]byte{tx},
Height: height,
Txs: [][]byte{tx},
Height: height,
Version: &pbversion.Consensus{App: uint64(height)},
}
respProcess, err := app.ProcessProposal(ctx, reqProcess)
require.NoError(t, err)
Expand Down Expand Up @@ -122,7 +124,9 @@ func TestPersistentKVStoreKV(t *testing.T) {
dir := t.TempDir()
logger := log.NewNopLogger()

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

key := testKey
Expand Down Expand Up @@ -157,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))
kvstore, err := NewPersistentApp(DefaultConfig(dir), WithLogger(logger), WithAppVersion(0))
require.NoError(t, err)
defer kvstore.Close()

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

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

// init with some validators
Expand Down Expand Up @@ -283,9 +287,10 @@ func makeApplyBlock(
}

respProcessProposal, err := kvstore.ProcessProposal(ctx, &types.RequestProcessProposal{
Hash: hash,
Height: height,
Txs: txs,
Hash: hash,
Height: height,
Txs: txs,
Version: &pbversion.Consensus{App: uint64(height)},
})
require.NoError(t, err)
require.NotZero(t, respProcessProposal)
Expand Down Expand Up @@ -368,7 +373,9 @@ func TestClientServer(t *testing.T) {
logger := log.NewTestingLogger(t)

// set up socket app
kvstore, err := NewMemoryApp(WithLogger(logger.With("module", "app")))
kvstore, err := NewMemoryApp(
WithLogger(logger.With("module", "app")),
WithAppVersion(0))
require.NoError(t, err)

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

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

gclient, gserver, err := makeGRPCClientServer(ctx, t, logger, kvstore, "/tmp/kvstore-grpc")
Expand All @@ -406,8 +413,9 @@ func runClientTests(ctx context.Context, t *testing.T, client abciclient.Client)

func testClient(ctx context.Context, t *testing.T, app abciclient.Client, height int64, tx []byte, key, value string) {
rpp, err := app.ProcessProposal(ctx, &types.RequestProcessProposal{
Txs: [][]byte{tx},
Height: height,
Txs: [][]byte{tx},
Height: height,
Version: &pbversion.Consensus{App: uint64(height)},
})
require.NoError(t, err)
require.NotZero(t, rpp)
Expand Down Expand Up @@ -522,6 +530,7 @@ func newKvApp(ctx context.Context, t *testing.T, genesisHeight int64, opts ...Op
WithValidatorSetUpdates(map[int64]types.ValidatorSetUpdate{
genesisHeight: RandValidatorSetUpdate(1),
}),
WithAppVersion(0),
}
app, err := NewMemoryApp(append(defaultOpts, opts...)...)
require.NoError(t, err)
Expand All @@ -536,17 +545,17 @@ func newKvApp(ctx context.Context, t *testing.T, genesisHeight int64, opts ...Op
return app
}

func assertRespInfo(t *testing.T, expectHeight int64, expectAppHash tmbytes.HexBytes, actual types.ResponseInfo, msgs ...interface{}) {
func assertRespInfo(t *testing.T, expectLastBlockHeight int64, expectAppHash tmbytes.HexBytes, actual types.ResponseInfo, msgs ...interface{}) {
t.Helper()

if expectAppHash == nil {
expectAppHash = make(tmbytes.HexBytes, tmcrypto.DefaultAppHashSize)
}
expected := types.ResponseInfo{
LastBlockHeight: expectHeight,
LastBlockHeight: expectLastBlockHeight,
LastBlockAppHash: expectAppHash,
Version: version.ABCIVersion,
AppVersion: ProtocolVersion,
AppVersion: uint64(expectLastBlockHeight + 1),
Data: fmt.Sprintf(`{"appHash":"%s"}`, expectAppHash.String()),
}

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
3 changes: 3 additions & 0 deletions abci/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,9 @@ func (m *ResponsePrepareProposal) Validate() error {
if !isValidApphash(m.AppHash) {
return fmt.Errorf("apphash (%X) of size %d is invalid", m.AppHash, len(m.AppHash))
}
if m.AppVersion == 0 {
return fmt.Errorf("app version cannot be 0")
}

return nil
}
Expand Down
Loading

0 comments on commit 39c275a

Please sign in to comment.