Skip to content

Commit

Permalink
fix: remove previous header in Prepare/Process Proposal + provide cha…
Browse files Browse the repository at this point in the history
…in id in baseapp + fix context for verifying txs (backport #15303) (#15377)

Co-authored-by: Facundo Medica <[email protected]>
Co-authored-by: Facundo Medica <[email protected]>
  • Loading branch information
3 people authored Mar 13, 2023
1 parent f3f4532 commit 828794e
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 34 deletions.
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))
}

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),
}
}
13 changes: 7 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,10 +290,11 @@ 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{
ChainId: SimAppChainID,
AppStateBytes: exported.AppState,
})

Expand Down Expand Up @@ -345,7 +346,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
4 changes: 3 additions & 1 deletion tests/integration/gov/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/simapp"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/server"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
Expand All @@ -25,7 +27,7 @@ func TestItCreatesModuleAccountOnInitBlock(t *testing.T) {
appOptions[flags.FlagHome] = simapp.DefaultNodeHome
appOptions[server.FlagInvCheckPeriod] = 5

app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, appOptions)
app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, appOptions, baseapp.SetChainID("test-chain-id"))

genesisState := simapp.GenesisStateWithSingleValidator(t, app)
stateBytes, err := tmjson.Marshal(genesisState)
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

0 comments on commit 828794e

Please sign in to comment.