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: remove previous header in Prepare/Process Proposal + provide chain id in baseapp + fix context for verifying txs (backport #15303) #15377

Merged
merged 7 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
59 changes: 39 additions & 20 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ const (
// InitChain implements the ABCI interface. It runs the initialization logic
// directly on the CommitMultiStore.
func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain) {
if req.ChainId != app.chainID {
panic(fmt.Sprintf("invalid chain-id on InitChain; expected: %s, got: %s", app.chainID, req.ChainId))
}

// On a new chain, we consider the init chain block height as 0, even though
// req.InitialHeight is 1 by default.
initHeader := tmproto.Header{ChainID: req.ChainId, Time: req.Time}
Expand All @@ -54,8 +58,14 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
// initialize states with a correct header
app.setState(runTxModeDeliver, initHeader)
app.setState(runTxModeCheck, initHeader)
app.setState(runTxPrepareProposal, initHeader)
app.setState(runTxProcessProposal, initHeader)

// Use an empty header for prepare and process proposal states. The header
// will be overwritten for the first block (see getContextForProposal()) and
// cleaned up on every Commit(). Only the ChainID is needed so it's set in
// the context.
emptyHeader := tmproto.Header{ChainID: req.ChainId}
app.setState(runTxPrepareProposal, emptyHeader)
app.setState(runTxProcessProposal, emptyHeader)

// Store the consensus params in the BaseApp's paramstore. Note, this must be
// done after the deliver state and context have been set as it's persisted
Expand Down Expand Up @@ -148,6 +158,10 @@ func (app *BaseApp) FilterPeerByID(info string) abci.ResponseQuery {

// BeginBlock implements the ABCI application interface.
func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
if req.Header.ChainID != app.chainID {
panic(fmt.Sprintf("invalid chain-id on BeginBlock; expected: %s, got: %s", app.chainID, req.Header.ChainID))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

path flow from Begin/EndBlock to a panic call
}

if app.cms.TracingEnabled() {
app.cms.SetTracingContext(sdk.TraceContext(
map[string]interface{}{"blockHeight": req.Header.Height},
Expand Down Expand Up @@ -250,15 +264,15 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
panic("PrepareProposal called with invalid height")
}

gasMeter := app.getBlockGasMeter(app.prepareProposalState.ctx)
ctx := app.getContextForProposal(app.prepareProposalState.ctx, req.Height)

ctx = ctx.WithVoteInfos(app.voteInfos).
app.prepareProposalState.ctx = app.getContextForProposal(app.prepareProposalState.ctx, req.Height).
WithVoteInfos(app.voteInfos).
WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithProposer(req.ProposerAddress).
WithConsensusParams(app.GetConsensusParams(ctx)).
WithBlockGasMeter(gasMeter)
WithProposer(req.ProposerAddress)

app.prepareProposalState.ctx = app.prepareProposalState.ctx.
WithConsensusParams(app.GetConsensusParams(app.prepareProposalState.ctx)).
WithBlockGasMeter(app.getBlockGasMeter(app.prepareProposalState.ctx))

defer func() {
if err := recover(); err != nil {
Expand All @@ -273,7 +287,7 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
}
}()

resp = app.prepareProposal(ctx, req)
resp = app.prepareProposal(app.prepareProposalState.ctx, req)
return resp
}

Expand All @@ -297,17 +311,16 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.
panic("app.ProcessProposal is not set")
}

gasMeter := app.getBlockGasMeter(app.processProposalState.ctx)
ctx := app.getContextForProposal(app.processProposalState.ctx, req.Height)

ctx = ctx.
app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height).
WithVoteInfos(app.voteInfos).
WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithHeaderHash(req.Hash).
WithProposer(req.ProposerAddress).
WithConsensusParams(app.GetConsensusParams(ctx)).
WithBlockGasMeter(gasMeter)
WithProposer(req.ProposerAddress)

app.processProposalState.ctx = app.processProposalState.ctx.
WithConsensusParams(app.GetConsensusParams(app.processProposalState.ctx)).
WithBlockGasMeter(app.getBlockGasMeter(app.processProposalState.ctx))

defer func() {
if err := recover(); err != nil {
Expand All @@ -322,7 +335,7 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.
}
}()

resp = app.processProposal(ctx, req)
resp = app.processProposal(app.processProposalState.ctx, req)
return resp
}

Expand Down Expand Up @@ -436,8 +449,12 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
// NOTE: This is safe because Tendermint holds a lock on the mempool for
// Commit. Use the header from this latest block.
app.setState(runTxModeCheck, header)
app.setState(runTxPrepareProposal, header)
app.setState(runTxProcessProposal, header)

// Reset state to the latest committed but with an empty header to avoid
// leaking the header from the last block.
emptyHeader := tmproto.Header{ChainID: app.chainID}
app.setState(runTxPrepareProposal, emptyHeader)
app.setState(runTxProcessProposal, emptyHeader)

// empty/reset the deliver state
app.deliverState = nil
Expand Down Expand Up @@ -955,6 +972,8 @@ func SplitABCIQueryPath(requestPath string) (path []string) {
func (app *BaseApp) getContextForProposal(ctx sdk.Context, height int64) sdk.Context {
if height == 1 {
ctx, _ = app.deliverState.ctx.CacheContext()
// clear all context data set during InitChain to avoid inconsistent behavior
ctx = ctx.WithBlockHeader(tmproto.Header{})
return ctx
}
return ctx
Expand Down
9 changes: 7 additions & 2 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestABCI_InitChain(t *testing.T) {
name := t.Name()
db := dbm.NewMemDB()
logger := defaultLogger()
app := baseapp.NewBaseApp(name, logger, db, nil)
app := baseapp.NewBaseApp(name, logger, db, nil, baseapp.SetChainID("test-chain-id"))

capKey := sdk.NewKVStoreKey("main")
capKey2 := sdk.NewKVStoreKey("key2")
Expand All @@ -62,8 +62,13 @@ func TestABCI_InitChain(t *testing.T) {
Data: key,
}

// initChain is nil and chain ID is wrong - panics
require.Panics(t, func() {
app.InitChain(abci.RequestInitChain{ChainId: "wrong-chain-id"})
})

// initChain is nil - nothing happens
app.InitChain(abci.RequestInitChain{})
app.InitChain(abci.RequestInitChain{ChainId: "test-chain-id"})
res := app.Query(query)
require.Equal(t, 0, len(res.Value))

Expand Down
9 changes: 6 additions & 3 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/cometbft/cometbft/crypto/tmhash"
"github.com/cometbft/cometbft/libs/log"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/gogoproto/proto"
"golang.org/x/exp/maps"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/snapshots"
"github.com/cosmos/cosmos-sdk/store"
Expand All @@ -19,8 +22,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/mempool"
"github.com/cosmos/gogoproto/proto"
"golang.org/x/exp/maps"
)

type (
Expand Down Expand Up @@ -142,6 +143,8 @@ type BaseApp struct { //nolint: maligned
// abciListeners for hooking into the ABCI message processing of the BaseApp
// and exposing the requests and responses to external consumers
abciListeners []ABCIListener

chainID string
}

// NewBaseApp returns a reference to an initialized BaseApp. It accepts a
Expand Down Expand Up @@ -349,7 +352,7 @@ func (app *BaseApp) Init() error {
panic("cannot call initFromMainStore: baseapp already sealed")
}

emptyHeader := tmproto.Header{}
emptyHeader := tmproto.Header{ChainID: app.chainID}

// needed for the export command which inits from store but never calls initchain
app.setState(runTxModeCheck, emptyHeader)
Expand Down
5 changes: 5 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ func SetMempool(mempool mempool.Mempool) func(*BaseApp) {
return func(app *BaseApp) { app.SetMempool(mempool) }
}

// SetChainID sets the chain ID in BaseApp.
func SetChainID(chainID string) func(*BaseApp) {
return func(app *BaseApp) { app.chainID = chainID }
}

func (app *BaseApp) SetName(name string) {
if app.sealed {
panic("SetName() on sealed BaseApp")
Expand Down
3 changes: 2 additions & 1 deletion server/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import (
"fmt"

tmcmd "github.com/cometbft/cometbft/cmd/cometbft/commands"
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/server/types"
"github.com/spf13/cobra"
)

// NewRollbackCmd creates a command to rollback tendermint and multistore state by one height.
Expand Down
16 changes: 15 additions & 1 deletion server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
tmcli "github.com/cometbft/cometbft/libs/cli"
tmflags "github.com/cometbft/cometbft/libs/cli/flags"
tmlog "github.com/cometbft/cometbft/libs/log"
tmtypes "github.com/cometbft/cometbft/types"
"github.com/spf13/cast"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -435,7 +436,19 @@ func DefaultBaseappOptions(appOpts types.AppOptions) []func(*baseapp.BaseApp) {
panic(err)
}

snapshotDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data", "snapshots")
homeDir := cast.ToString(appOpts.Get(flags.FlagHome))
chainID := cast.ToString(appOpts.Get(flags.FlagChainID))
if chainID == "" {
// fallback to genesis chain-id
appGenesis, err := tmtypes.GenesisDocFromFile(filepath.Join(homeDir, "config", "genesis.json"))
if err != nil {
panic(err)
}

chainID = appGenesis.ChainID
}

snapshotDir := filepath.Join(homeDir, "data", "snapshots")
if err = os.MkdirAll(snapshotDir, os.ModePerm); err != nil {
panic(fmt.Errorf("failed to create snapshots directory: %w", err))
}
Expand Down Expand Up @@ -472,5 +485,6 @@ func DefaultBaseappOptions(appOpts types.AppOptions) []func(*baseapp.BaseApp) {
),
),
baseapp.SetIAVLLazyLoading(cast.ToBool(appOpts.Get(FlagIAVLLazyLoading))),
baseapp.SetChainID(chainID),
}
}
12 changes: 6 additions & 6 deletions simapp/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestFullAppSimulation(t *testing.T) {
appOptions[flags.FlagHome] = DefaultNodeHome
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
require.Equal(t, "SimApp", app.Name())

// run randomized simulation
Expand Down Expand Up @@ -129,7 +129,7 @@ func TestAppImportExport(t *testing.T) {
appOptions[flags.FlagHome] = DefaultNodeHome
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestAppImportExport(t *testing.T) {
require.NoError(t, os.RemoveAll(newDir))
}()

newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, appOptions, fauxMerkleModeOpt)
newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
require.Equal(t, "SimApp", newApp.Name())

var genesisState GenesisState
Expand Down Expand Up @@ -245,7 +245,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
appOptions[flags.FlagHome] = DefaultNodeHome
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt)
app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
require.NoError(t, os.RemoveAll(newDir))
}()

newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, appOptions, fauxMerkleModeOpt)
newApp := NewSimApp(log.NewNopLogger(), newDB, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
require.Equal(t, "SimApp", newApp.Name())

newApp.InitChain(abci.RequestInitChain{
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestAppStateDeterminism(t *testing.T) {
}

db := dbm.NewMemDB()
app := NewSimApp(logger, db, nil, true, appOptions, interBlockCacheOpt())
app := NewSimApp(logger, db, nil, true, appOptions, interBlockCacheOpt(), baseapp.SetChainID(SimAppChainID))

fmt.Printf(
"running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n",
Expand Down
1 change: 1 addition & 0 deletions simapp/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ func NewTestNetworkFixture() network.TestFixture {
simtestutil.NewAppOptionsWithFlagHome(val.GetCtx().Config.RootDir),
bam.SetPruning(pruningtypes.NewPruningOptionsFromString(val.GetAppConfig().Pruning)),
bam.SetMinGasPrices(val.GetAppConfig().MinGasPrices),
bam.SetChainID(val.GetCtx().Viper.GetString(flags.FlagChainID)),
)
}

Expand Down
1 change: 1 addition & 0 deletions tests/e2e/params/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func (s *E2ETestSuite) SetupSuite() {
nil,
baseapp.SetPruning(pruningtypes.NewPruningOptionsFromString(val.GetAppConfig().Pruning)),
baseapp.SetMinGasPrices(val.GetAppConfig().MinGasPrices),
baseapp.SetChainID(s.cfg.ChainID),
)

s.Require().NoError(app.Load(false))
Expand Down
7 changes: 7 additions & 0 deletions testutil/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ import (

"cosmossdk.io/math"
tmlog "github.com/cometbft/cometbft/libs/log"

"github.com/cosmos/cosmos-sdk/testutil/configurator"
"github.com/cosmos/cosmos-sdk/testutil/testdata"

"cosmossdk.io/depinject"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -184,6 +187,7 @@ func DefaultConfigWithAppConfig(appConfig depinject.Config) (Config, error) {
nil,
baseapp.SetPruning(pruningtypes.NewPruningOptionsFromString(val.GetAppConfig().Pruning)),
baseapp.SetMinGasPrices(val.GetAppConfig().MinGasPrices),
baseapp.SetChainID(cfg.ChainID),
)

testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{})
Expand Down Expand Up @@ -546,6 +550,9 @@ func New(l Logger, baseDir string, cfg Config) (*Network, error) {
WithTxConfig(cfg.TxConfig).
WithAccountRetriever(cfg.AccountRetriever)

// Provide ChainID here since we can't modify it in the Comet config.
ctx.Viper.Set(flags.FlagChainID, cfg.ChainID)

network.Validators[i] = &Validator{
AppConfig: appCfg,
ClientCtx: clientCtx,
Expand Down