From 3df3084d0cd7760370a4e88a7ec6401393dde77a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 9 Apr 2023 10:00:41 -0400 Subject: [PATCH 01/32] updates --- baseapp/baseapp.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7b4beb7a8067..ce7d4d9ec050 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -62,17 +62,20 @@ type BaseApp struct { //nolint: maligned txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx txEncoder sdk.TxEncoder // marshal sdk.Tx into []byte - mempool mempool.Mempool // application side mempool - anteHandler sdk.AnteHandler // ante handler for fee and auth - postHandler sdk.PostHandler // post handler, optional, e.g. for tips - initChainer sdk.InitChainer // initialize state with validators and state blob - beginBlocker sdk.LegacyBeginBlocker // logic to run before any txs - processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal - prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal - endBlocker sdk.LegacyEndBlocker // logic to run after all txs, and to determine valset changes - addrPeerFilter sdk.PeerFilter // filter peers by address and port - idPeerFilter sdk.PeerFilter // filter peers by node ID - fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. + mempool mempool.Mempool // application side mempool + anteHandler sdk.AnteHandler // ante handler for fee and auth + postHandler sdk.PostHandler // post handler, optional, e.g. for tips + initChainer sdk.InitChainer // initialize state with validators and state blob + beginBlocker sdk.LegacyBeginBlocker // (legacy ABCI) BeginBlock handler + endBlocker sdk.LegacyEndBlocker // (legacy ABCI) EndBlock handler + processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler + prepareProposal sdk.PrepareProposalHandler // ABCI PrepareProposal + extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler + verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler + + addrPeerFilter sdk.PeerFilter // filter peers by address and port + idPeerFilter sdk.PeerFilter // filter peers by node ID + fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. // manages snapshots, i.e. dumps of app state at certain intervals snapshotManager *snapshots.Manager From 8019c2e262eb35e046f790a2e40c4037ea8c04d9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 9 Apr 2023 10:02:12 -0400 Subject: [PATCH 02/32] updates --- baseapp/baseapp.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index ce7d4d9ec050..e64b45bd8ff2 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -62,10 +62,11 @@ type BaseApp struct { //nolint: maligned txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx txEncoder sdk.TxEncoder // marshal sdk.Tx into []byte - mempool mempool.Mempool // application side mempool - anteHandler sdk.AnteHandler // ante handler for fee and auth - postHandler sdk.PostHandler // post handler, optional, e.g. for tips - initChainer sdk.InitChainer // initialize state with validators and state blob + mempool mempool.Mempool // application side mempool + anteHandler sdk.AnteHandler // ante handler for fee and auth + postHandler sdk.PostHandler // post handler, optional, e.g. for tips + + initChainer sdk.InitChainer // ABCI InitChain handler beginBlocker sdk.LegacyBeginBlocker // (legacy ABCI) BeginBlock handler endBlocker sdk.LegacyEndBlocker // (legacy ABCI) EndBlock handler processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler From 5a069e747d0511312f84099ed91bd132a0bb6f6a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 09:37:48 -0400 Subject: [PATCH 03/32] updates --- baseapp/baseapp.go | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index e64b45bd8ff2..93eb3b1daf56 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -86,15 +86,32 @@ type BaseApp struct { //nolint: maligned // - checkState is set on InitChain and reset on Commit // - finalizeBlockState is set on InitChain and FinalizeBlock and set to nil // on Commit. - checkState *state // for CheckTx - processProposalState *state // for ProcessProposal - prepareProposalState *state // for PrepareProposal - finalizeBlockState *state // for FinalizeBlock - - // an inter-block write-through cache provided to the context during deliverState + // + // - checkState: Used for CheckTx, which is set based on the previous block's + // state. This state is never committed. + // - prepareProposalState: Used for PrepareProposal, which is set based on the + // previous block's state. This state is never committed. In case of multiple + // consensus rounds, the state is always reset to the previous block's state. + // - voteExtensionState: Used for ExtendVote and VerifyVoteExtension, which is + // set based on the previous block's state. This state is never committed. In + // case of multiple rounds, the state is always reset to the previous block's + // state. + // processProposalState: Used for ProcessProposal, which is set based on the + // the previous block's state. This state is never committed. In case of + // multiple rounds, the state is always reset to the previous block's state. + // finalizeBlockState: Used for FinalizeBlock, which is set based on the + // previous block's state. This state is committed. + checkState *state + prepareProposalState *state + processProposalState *state + voteExtensionState *state + finalizeBlockState *state + + // An inter-block write-through cache provided to the context during the ABCI + // FinalizeBlock call. interBlockCache storetypes.MultiStorePersistentCache - // absent validators from begin block + // absent validators from FinalizeBlock voteInfos []abci.VoteInfo // paramStore is used to query for ABCI consensus parameters from an @@ -105,7 +122,7 @@ type BaseApp struct { //nolint: maligned // transaction. This is mainly used for DoS and spam prevention. minGasPrices sdk.DecCoins - // initialHeight is the initial height at which we start the baseapp + // initialHeight is the initial height at which we start the BaseApp initialHeight int64 // flag for sealing options and parameters to a BaseApp From 7c1174ffc023c02fbbf5a108f54a1d63d7dd431c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:04:58 -0400 Subject: [PATCH 04/32] updates --- baseapp/abci.go | 101 ++++++++++++++++++++++++++++++++++++---- baseapp/baseapp.go | 67 +++++++++++++------------- baseapp/test_helpers.go | 4 +- 3 files changed, 128 insertions(+), 44 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2e0597a7536d..ca0c0abbc495 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -60,8 +60,8 @@ func (app *BaseApp) InitChain(_ context.Context, req *abci.RequestInitChain) (*a } // initialize states with a correct header - app.setState(runTxModeFinalize, initHeader) - app.setState(runTxModeCheck, initHeader) + app.setState(execModeFinalize, initHeader) + app.setState(execModeCheck, initHeader) // Store the consensus params in the BaseApp's param store. Note, this must be // done after the finalizeBlockState and context have been set as it's persisted @@ -407,14 +407,14 @@ func (app *BaseApp) legacyEndBlock(req *abci.RequestFinalizeBlock) sdk.LegacyRes // will contain relevant error information. Regardless of tx execution outcome, // the ResponseCheckTx will contain relevant gas execution context. func (app *BaseApp) CheckTx(_ context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTx, error) { - var mode runTxMode + var mode execMode switch { case req.Type == abci.CheckTxType_New: - mode = runTxModeCheck + mode = execModeCheck case req.Type == abci.CheckTxType_Recheck: - mode = runTxModeReCheck + mode = execModeReCheck default: return nil, fmt.Errorf("unknown RequestCheckTx type: %s", req.Type) @@ -454,7 +454,7 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP // always reset state given that PrepareProposal can timeout and be called again emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(runTxPrepareProposal, emptyHeader) + app.setState(execModePrepareProposal, emptyHeader) // CometBFT must never call PrepareProposal with a height of 0. // Ref: https://github.com/cometbft/cometbft/blob/059798a4f5b0c9f52aa8655fa619054a0154088c/spec/core/state.md?plain=1#L37-L38 @@ -521,7 +521,7 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // always reset state given that ProcessProposal can timeout and be called again emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(runTxProcessProposal, emptyHeader) + app.setState(execModeProcessProposal, emptyHeader) app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -555,6 +555,87 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP return resp, nil } +// ExtendVote implements the ExtendVote ABCI method and returns a ResponseExtendVote. +// It calls the application's ExtendVote handler which is responsible for performing +// application-specific business logic when sending a pre-commit for the NEXT +// block height. The extensions response may be non-deterministic but must always +// be returned, even if empty. +// +// Agreed upon vote extensions are made available to the proposer of the next +// height and are committed in subsequent height, i.e. H+2. An error is returned +// if vote extensions are not enabled or if extendVote fails or panics. +func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { + // Always reset state given that ExtendVote and VerifyVoteExtension can timeout + // and be called again in a subsequent round. + emptyHeader := cmtproto.Header{ChainID: app.chainID, Height: req.Height} + app.setState(execModeVoteExtension, emptyHeader) + + // If vote extensions are not enabled, as a safety precaution, we return an + // error. + cp := app.GetConsensusParams(app.voteExtensionState.ctx) + if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) + } + + // add a deferred recover handler in case extendVote panics + defer func() { + if err := recover(); err != nil { + app.logger.Error( + "panic recovered in ExtendVote", + "height", req.Height, + "hash", fmt.Sprintf("%X", req.Hash), + "panic", err, + ) + err = fmt.Errorf("recovered panic in ExtendVote: %w", err) + } + }() + + resp, err = app.extendVote(app.voteExtensionState.ctx, req) + if err != nil { + app.logger.Error("failed to extend vote", "height", req.Height, "error", err) + return nil, fmt.Errorf("failed to extend vote: %w", err) + } + + return resp, err +} + +// VerifyVoteExtension implements the VerifyVoteExtension ABCI method and returns +// a ResponseVerifyVoteExtension. It calls the applications' VerifyVoteExtension +// handler which is responsible for performing application-specific business +// logic in verifying a vote extension from another validator during the pre-commit +// phase. The response MUST be deterministic. An error is returned if vote +// extensions are not enabled or if verifyVoteExt fails or panics. +func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVerifyVoteExtension) (resp *abci.ResponseVerifyVoteExtension, err error) { + // If vote extensions are not enabled, as a safety precaution, we return an + // error. + cp := app.GetConsensusParams(app.voteExtensionState.ctx) + if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) + } + + // add a deferred recover handler in case verifyVoteExt panics + defer func() { + if err := recover(); err != nil { + app.logger.Error( + "panic recovered in VerifyVoteExtension", + "height", req.Height, + "hash", fmt.Sprintf("%X", req.Hash), + "validator", fmt.Sprintf("%X", req.ValidatorAddress), + "panic", err, + ) + err = fmt.Errorf("recovered panic in VerifyVoteExtension: %w", err) + } + }() + + resp, err = app.verifyVoteExt(app.voteExtensionState.ctx, req) + if err != nil { + app.logger.Error("failed to verify vote extension", "height", req.Height, "error", err) + return nil, fmt.Errorf("failed to verify vote extension: %w", err) + } + + return resp, err +} + func (app *BaseApp) legacyDeliverTx(tx []byte) *abci.ExecTxResult { gInfo := sdk.GasInfo{} resultStr := "successful" @@ -583,7 +664,7 @@ func (app *BaseApp) legacyDeliverTx(tx []byte) *abci.ExecTxResult { telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, anteEvents, err := app.runTx(runTxModeFinalize, tx) + gInfo, result, anteEvents, err := app.runTx(execModeFinalize, tx) if err != nil { resultStr = "failed" resp = sdkerrors.ResponseExecTxResultWithEvents( @@ -632,7 +713,7 @@ func (app *BaseApp) FinalizeBlock(_ context.Context, req *abci.RequestFinalizeBl // already be initialized in InitChain. Otherwise app.finalizeBlockState will be // nil, since it is reset on Commit. if app.finalizeBlockState == nil { - app.setState(runTxModeFinalize, header) + app.setState(execModeFinalize, header) } else { // In the first block, app.finalizeBlockState.ctx will already be initialized // by InitChain. Context is now updated with Header information. @@ -718,7 +799,7 @@ func (app *BaseApp) Commit(_ context.Context, _ *abci.RequestCommit) (*abci.Resp // // NOTE: This is safe because CometBFT holds a lock on the mempool for // Commit. Use the header from this latest block. - app.setState(runTxModeCheck, header) + app.setState(execModeCheck, header) app.finalizeBlockState = nil diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 93eb3b1daf56..7502af9c7901 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -26,8 +26,7 @@ import ( ) type ( - // Enum mode for app.runTx - runTxMode uint8 + execMode uint8 // StoreLoader defines a customizable function to control how we load the CommitMultiStore // from disk. This is useful for state migration, when loading a datastore written with @@ -37,12 +36,13 @@ type ( ) const ( - runTxModeCheck runTxMode = iota // Check a transaction - runTxModeReCheck // Recheck a (pending) transaction after a commit - runTxModeSimulate // Simulate a transaction - runTxModeFinalize // Finalize a block proposal - runTxPrepareProposal // Prepare a block proposal - runTxProcessProposal // Process a block proposal + execModeCheck execMode = iota // Check a transaction + execModeReCheck // Recheck a (pending) transaction after a commit + execModeSimulate // Simulate a transaction + execModePrepareProposal // Prepare a block proposal + execModeProcessProposal // Process a block proposal + execModeVoteExtension // Extend or verify a pre-commit vote + execModeFinalize // Finalize a block proposal ) var _ abci.Application = (*BaseApp)(nil) @@ -375,7 +375,7 @@ func (app *BaseApp) Init() error { emptyHeader := cmtproto.Header{ChainID: app.chainID} // needed for the export command which inits from store but never calls initchain - app.setState(runTxModeCheck, emptyHeader) + app.setState(execModeCheck, emptyHeader) app.Seal() if app.cms == nil { @@ -426,7 +426,7 @@ func (app *BaseApp) IsSealed() bool { return app.sealed } // setState sets the BaseApp's state for the corresponding mode with a branched // multi-store (i.e. a CacheMultiStore) and a new Context with the same // multi-store branch, and provided header. -func (app *BaseApp) setState(mode runTxMode, header cmtproto.Header) { +func (app *BaseApp) setState(mode execMode, header cmtproto.Header) { ms := app.cms.CacheMultiStore() baseState := &state{ ms: ms, @@ -434,19 +434,22 @@ func (app *BaseApp) setState(mode runTxMode, header cmtproto.Header) { } switch mode { - case runTxModeCheck: + case execModeCheck: baseState.ctx = baseState.ctx.WithIsCheckTx(true).WithMinGasPrices(app.minGasPrices) app.checkState = baseState - case runTxModeFinalize: - app.finalizeBlockState = baseState - - case runTxPrepareProposal: + case execModePrepareProposal: app.prepareProposalState = baseState - case runTxProcessProposal: + case execModeProcessProposal: app.processProposalState = baseState + case execModeVoteExtension: + app.voteExtensionState = baseState + + case execModeFinalize: + app.finalizeBlockState = baseState + default: panic(fmt.Sprintf("invalid runTxMode for setState: %d", mode)) } @@ -553,15 +556,15 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error { return nil } -func (app *BaseApp) getState(mode runTxMode) *state { +func (app *BaseApp) getState(mode execMode) *state { switch mode { - case runTxModeFinalize: + case execModeFinalize: return app.finalizeBlockState - case runTxPrepareProposal: + case execModePrepareProposal: return app.prepareProposalState - case runTxProcessProposal: + case execModeProcessProposal: return app.processProposalState default: @@ -578,7 +581,7 @@ func (app *BaseApp) getBlockGasMeter(ctx sdk.Context) storetypes.GasMeter { } // retrieve the context for the tx w/ txBytes and other memoized values. -func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context { +func (app *BaseApp) getContextForTx(mode execMode, txBytes []byte) sdk.Context { modeState := app.getState(mode) if modeState == nil { panic(fmt.Sprintf("state is nil for mode %v", mode)) @@ -589,11 +592,11 @@ func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx)) - if mode == runTxModeReCheck { + if mode == execModeReCheck { ctx = ctx.WithIsReCheckTx(true) } - if mode == runTxModeSimulate { + if mode == execModeSimulate { ctx, _ = ctx.CacheContext() } @@ -626,7 +629,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context // Note, gas execution info is always returned. A reference to a Result is // returned if the tx does not run out of gas and if all the messages are valid // and execute successfully. An error is returned otherwise. -func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { +func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter, so we initialize upfront. @@ -698,7 +701,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // performance benefits, but it'll be more difficult to get right. anteCtx, msCache = app.cacheTxContext(ctx, txBytes) anteCtx = anteCtx.WithEventManager(sdk.NewEventManager()) - newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) + newCtx, err := app.anteHandler(anteCtx, tx, mode == execModeSimulate) if !newCtx.IsZero() { // At this point, newCtx.MultiStore() is a store branch, or something else @@ -724,7 +727,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re anteEvents = events.ToABCIEvents() } - if mode == runTxModeCheck { + if mode == execModeCheck { err = app.mempool.Insert(ctx, tx) if err != nil { return gInfo, nil, anteEvents, priority, err @@ -756,7 +759,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Note that the state is still preserved. postCtx := runMsgCtx.WithEventManager(sdk.NewEventManager()) - newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate, err == nil) + newCtx, err := app.postHandler(postCtx, tx, mode == execModeSimulate, err == nil) if err != nil { return gInfo, nil, anteEvents, priority, err } @@ -771,7 +774,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re msCache.Write() } - if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { + if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == execModeSimulate) { // append the events in the order of occurrence result.Events = append(anteEvents, result.Events...) } @@ -785,14 +788,14 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // and DeliverTx. An error is returned if any single message fails or if a // Handler does not exist for a given message route. Otherwise, a reference to a // Result is returned. The caller must not commit state if an error is returned. -func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*sdk.Result, error) { +func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode execMode) (*sdk.Result, error) { msgLogs := make(sdk.ABCIMessageLogs, 0, len(msgs)) events := sdk.EmptyEvents() var msgResponses []*codectypes.Any // NOTE: GasWanted is determined by the AnteHandler and GasUsed by the GasMeter. for i, msg := range msgs { - if mode != runTxModeDeliver && mode != runTxModeSimulate { + if mode != runTxModeDeliver && mode != execModeSimulate { break } @@ -880,7 +883,7 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { return nil, err } - _, _, _, _, err = app.runTx(runTxPrepareProposal, bz) + _, _, _, _, err = app.runTx(execModePrepareProposal, bz) if err != nil { return nil, err } @@ -899,7 +902,7 @@ func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { return nil, err } - _, _, _, _, err = app.runTx(runTxProcessProposal, txBz) + _, _, _, _, err = app.runTx(execModeProcessProposal, txBz) if err != nil { return nil, err } diff --git a/baseapp/test_helpers.go b/baseapp/test_helpers.go index d3f24d6ef419..bc97651c629c 100644 --- a/baseapp/test_helpers.go +++ b/baseapp/test_helpers.go @@ -18,13 +18,13 @@ func (app *BaseApp) SimCheck(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, * if err != nil { return sdk.GasInfo{}, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - gasInfo, result, _, _, err := app.runTx(runTxModeCheck, bz) + gasInfo, result, _, _, err := app.runTx(execModeCheck, bz) return gasInfo, result, err } // Simulate executes a tx in simulate mode to get result and gas info. func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) { - gasInfo, result, _, _, err := app.runTx(runTxModeSimulate, txBytes) + gasInfo, result, _, _, err := app.runTx(execModeSimulate, txBytes) return gasInfo, result, err } From f894f571487461e01d10b29cfc294de184d92344 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:07:03 -0400 Subject: [PATCH 05/32] updates --- baseapp/abci.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index ca0c0abbc495..64ae8267fbdf 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -586,7 +586,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( "hash", fmt.Sprintf("%X", req.Hash), "panic", err, ) - err = fmt.Errorf("recovered panic in ExtendVote: %w", err) + err = fmt.Errorf("recovered application panic in ExtendVote: %w", err) } }() @@ -623,7 +623,7 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri "validator", fmt.Sprintf("%X", req.ValidatorAddress), "panic", err, ) - err = fmt.Errorf("recovered panic in VerifyVoteExtension: %w", err) + err = fmt.Errorf("recovered application panic in VerifyVoteExtension: %w", err) } }() From 1010bc9b57af207b9e6f124257197713fb18c281 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:08:56 -0400 Subject: [PATCH 06/32] updates --- baseapp/baseapp.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7502af9c7901..1ba63767e3a8 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -89,17 +89,21 @@ type BaseApp struct { //nolint: maligned // // - checkState: Used for CheckTx, which is set based on the previous block's // state. This state is never committed. + // // - prepareProposalState: Used for PrepareProposal, which is set based on the // previous block's state. This state is never committed. In case of multiple // consensus rounds, the state is always reset to the previous block's state. + // // - voteExtensionState: Used for ExtendVote and VerifyVoteExtension, which is // set based on the previous block's state. This state is never committed. In // case of multiple rounds, the state is always reset to the previous block's // state. - // processProposalState: Used for ProcessProposal, which is set based on the + // + // - processProposalState: Used for ProcessProposal, which is set based on the // the previous block's state. This state is never committed. In case of // multiple rounds, the state is always reset to the previous block's state. - // finalizeBlockState: Used for FinalizeBlock, which is set based on the + // + // - finalizeBlockState: Used for FinalizeBlock, which is set based on the // previous block's state. This state is committed. checkState *state prepareProposalState *state From dbee603b78ae7c4bb15bbddef3cab322293cb160 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:15:55 -0400 Subject: [PATCH 07/32] updates --- baseapp/abci.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index 64ae8267fbdf..dab1d7d51935 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -577,6 +577,12 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } + app.voteExtensionState.ctx = app.voteExtensionState.ctx. + WithConsensusParams(cp). + WithBlockGasMeter(storetypes.NewInfiniteGasMeter()). + WithBlockHeight(req.Height). + WithHeaderHash(req.Hash) + // add a deferred recover handler in case extendVote panics defer func() { if err := recover(); err != nil { From 19e3c9ce125b2ceeb29f45ed5c933b30b087eb29 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:35:15 -0400 Subject: [PATCH 08/32] updates --- baseapp/abci.go | 38 ++++++++++++++++++++++++++++++-------- baseapp/baseapp.go | 10 ++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index dab1d7d51935..2fd9c04f039a 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -452,11 +452,19 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP return nil, errors.New("PrepareProposal method not set") } - // always reset state given that PrepareProposal can timeout and be called again - emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(execModePrepareProposal, emptyHeader) + // Always reset state given that PrepareProposal can timeout and be called + // again in a subsequent round. + header := cmtproto.Header{ + ChainID: app.chainID, + Height: req.Height, + Time: req.Time, + ProposerAddress: req.ProposerAddress, + NextValidatorsHash: req.NextValidatorsHash, + } + app.setState(execModePrepareProposal, header) // CometBFT must never call PrepareProposal with a height of 0. + // // Ref: https://github.com/cometbft/cometbft/blob/059798a4f5b0c9f52aa8655fa619054a0154088c/spec/core/state.md?plain=1#L37-L38 if req.Height < 1 { return nil, errors.New("PrepareProposal called with invalid height") @@ -487,7 +495,8 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP resp, err = app.prepareProposal(app.prepareProposalState.ctx, req) if err != nil { - return nil, err + app.logger.Error("failed to prepare proposal", "height", req.Height, "error", err) + return &abci.ResponsePrepareProposal{Txs: [][]byte{}}, nil } return resp, nil @@ -519,9 +528,21 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP return nil, errors.New("ProcessProposal called with invalid height") } - // always reset state given that ProcessProposal can timeout and be called again - emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(execModeProcessProposal, emptyHeader) + // Always reset state given that ProcessProposal can timeout and be called + // again in a subsequent round. + header := cmtproto.Header{ + ChainID: app.chainID, + Height: req.Height, + Time: req.Time, + ProposerAddress: req.ProposerAddress, + NextValidatorsHash: req.NextValidatorsHash, + } + app.setState(execModeProcessProposal, header) + + // Since the application can get access to FinalizeBlock state and write to it, + // we must be sure to reset it in case ProcessProposal timeouts and is called + // again in a subsequent round. + app.setState(execModeFinalize, header) app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -549,7 +570,8 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP resp, err = app.processProposal(app.processProposalState.ctx, req) if err != nil { - return nil, err + app.logger.Error("failed to process proposal", "height", req.Height, "error", err) + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } return resp, nil diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 1ba63767e3a8..1eac77ae271d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -459,6 +459,16 @@ func (app *BaseApp) setState(mode execMode, header cmtproto.Header) { } } +// GetFinalizeBlockStateCtx returns the Context associated with the FinalizeBlock +// state. This Context can be used to write data derived from processing vote +// extensions to application state during ProcessProposal. +// +// NOTE: Do no use or write to state using this Context unless you intend for +// that state to be committed. +func (app *BaseApp) GetFinalizeBlockStateCtx() sdk.Context { + return app.finalizeBlockState.ctx +} + // GetConsensusParams returns the current consensus parameters from the BaseApp's // ParamStore. If the BaseApp has no ParamStore defined, nil is returned. func (app *BaseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams { From 45ec586b59de640ba72044394bb729e922d17a9f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:06:43 -0400 Subject: [PATCH 09/32] updates --- baseapp/abci.go | 74 ++++++++++++++++++++++++++++++++++++++++++++++ baseapp/baseapp.go | 26 +++++++++++++--- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2fd9c04f039a..67d4b8455220 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,6 +1,7 @@ package baseapp import ( + "bytes" "context" "crypto/sha256" "fmt" @@ -16,7 +17,10 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cryptoenc "github.com/cometbft/cometbft/crypto/encoding" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -1001,6 +1005,76 @@ func SplitABCIQueryPath(requestPath string) (path []string) { return path } +// ValidateVoteExtensions defines a helper function for verifying vote extension +// signatures that may be passed or manually injected into a block proposal from +// a proposer in ProcessProposal. It returns an error if any signature is invalid +// or if unexpected vote extensions and/or signatures are found. +func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64, extCommit abci.ExtendedCommitInfo) error { + cp := ctx.ConsensusParams() + extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 + + marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { + var buf bytes.Buffer + if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { + return nil, err + } + + return buf.Bytes(), nil + } + + for _, vote := range extCommit.Votes { + if !extsEnabled { + if len(vote.VoteExtension) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) + } + if len(vote.ExtensionSignature) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) + } + + continue + } + + if len(vote.ExtensionSignature) == 0 { + return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) + } + + valConsAddr := cmtcrypto.Address(vote.Validator.Address) + + validator, err := app.valStore.GetValidatorByConsAddr(ctx, valConsAddr) + if err != nil { + return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) + } + + cmtPubKeyProto, err := validator.CmtConsPublicKey() + if err != nil { + return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) + } + + cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) + if err != nil { + return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) + } + + cve := cmtproto.CanonicalVoteExtension{ + Extension: vote.VoteExtension, + Height: currentHeight - 1, // the vote extension was signed in the previous height + Round: int64(extCommit.Round), + ChainId: app.chainID, + } + + extSignBytes, err := marshalDelimitedFn(&cve) + if err != nil { + return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) + } + + if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { + return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) + } + } + + return nil +} + // FilterPeerByAddrPort filters peers by address/port. func (app *BaseApp) FilterPeerByAddrPort(info string) *abci.ResponseQuery { if app.addrPeerFilter != nil { diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 1eac77ae271d..ef08859722d0 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -14,12 +14,14 @@ import ( "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/crypto/tmhash" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/proto" "golang.org/x/exp/maps" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/mempool" @@ -28,11 +30,26 @@ import ( type ( execMode uint8 - // StoreLoader defines a customizable function to control how we load the CommitMultiStore - // from disk. This is useful for state migration, when loading a datastore written with - // an older version of the software. In particular, if a module changed the substore key name - // (or removed a substore) between two versions of the software. + // StoreLoader defines a customizable function to control how we load the + // CommitMultiStore from disk. This is useful for state migration, when + // loading a datastore written with an older version of the software. In + // particular, if a module changed the substore key name (or removed a substore) + // between two versions of the software. StoreLoader func(ms storetypes.CommitMultiStore) error + + // Validator defines the interface contract require for verifying vote extension + // signatures. Typically, this will be implemented by the x/staking module, + // which has knowledge of the CometBFT public key. + Validator interface { + CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) + } + + // ValidatorStore defines the interface contract require for verifying vote + // extension signatures. Typically, this will be implemented by the x/staking + // module, which has knowledge of the CometBFT public key. + ValidatorStore interface { + GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) + } ) const ( @@ -74,6 +91,7 @@ type BaseApp struct { //nolint: maligned extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler + valStore ValidatorStore addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. From 0d9bf0789e8ed86f0829075e0aff4807e6d1c771 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:08:08 -0400 Subject: [PATCH 10/32] updates --- baseapp/abci.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 67d4b8455220..3d222860ee22 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -588,8 +588,8 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // be returned, even if empty. // // Agreed upon vote extensions are made available to the proposer of the next -// height and are committed in subsequent height, i.e. H+2. An error is returned -// if vote extensions are not enabled or if extendVote fails or panics. +// height and are committed in the subsequent height, i.e. H+2. An error is +// returned if vote extensions are not enabled or if extendVote fails or panics. func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { // Always reset state given that ExtendVote and VerifyVoteExtension can timeout // and be called again in a subsequent round. From 36fc06da6f9eda8fae78e40cef6dec0f9e963d27 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:10:58 -0400 Subject: [PATCH 11/32] updates --- baseapp/abci.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index 3d222860ee22..f60784672f5b 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -590,6 +590,9 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // Agreed upon vote extensions are made available to the proposer of the next // height and are committed in the subsequent height, i.e. H+2. An error is // returned if vote extensions are not enabled or if extendVote fails or panics. +// +// Note, an error returned from ExtendVote will not cause the CometBFT to panic, +// so it is safe to return errors upon failure. func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { // Always reset state given that ExtendVote and VerifyVoteExtension can timeout // and be called again in a subsequent round. @@ -637,6 +640,9 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // logic in verifying a vote extension from another validator during the pre-commit // phase. The response MUST be deterministic. An error is returned if vote // extensions are not enabled or if verifyVoteExt fails or panics. +// +// Note, an error returned from VerifyVoteExtension will not cause the CometBFT +// to panic, so it is safe to return errors upon failure. func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVerifyVoteExtension) (resp *abci.ResponseVerifyVoteExtension, err error) { // If vote extensions are not enabled, as a safety precaution, we return an // error. From 9ed60b5b79c805bb0750e2b7fece1ce6cc422ff6 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 11:58:44 -0400 Subject: [PATCH 12/32] updates --- baseapp/abci.go | 22 ++++++++++++++-------- baseapp/baseapp.go | 20 +++++++++++--------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index f60784672f5b..18984466a7a5 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -49,15 +49,17 @@ func (app *BaseApp) InitChain(_ context.Context, req *abci.RequestInitChain) (*a // On a new chain, we consider the init chain block height as 0, even though // req.InitialHeight is 1 by default. initHeader := cmtproto.Header{ChainID: req.ChainId, Time: req.Time} + app.initialHeight = req.InitialHeight app.logger.Info("InitChain", "initialHeight", req.InitialHeight, "chainID", req.ChainId) - // If req.InitialHeight is > 1, then we set the initial version in the - // stores. - if req.InitialHeight > 1 { - app.initialHeight = req.InitialHeight - initHeader = cmtproto.Header{ChainID: req.ChainId, Height: req.InitialHeight, Time: req.Time} + // Set the initial height, which will be used to determine if we are proposing + // or processing the first block or not. + app.initialHeight = req.InitialHeight + // if req.InitialHeight is > 1, then we set the initial version on all stores + if req.InitialHeight > 1 { + initHeader.Height = req.InitialHeight if err := app.cms.SetInitialVersion(req.InitialHeight); err != nil { return nil, err } @@ -545,8 +547,12 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // Since the application can get access to FinalizeBlock state and write to it, // we must be sure to reset it in case ProcessProposal timeouts and is called - // again in a subsequent round. - app.setState(execModeFinalize, header) + // again in a subsequent round. However, we only want to do this after we've + // processed the first block, as we want to avoid overwriting the finalizeState + // after state changes during InitChain. + if req.Height > app.initialHeight { + app.setState(execModeFinalize, header) + } app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -1103,7 +1109,7 @@ func (app *BaseApp) FilterPeerByID(info string) *abci.ResponseQuery { // ProcessProposal. We use finalizeBlockState on the first block to be able to // access any state changes made in InitChain. func (app *BaseApp) getContextForProposal(ctx sdk.Context, height int64) sdk.Context { - if height == 1 { + if height == app.initialHeight { ctx, _ = app.finalizeBlockState.ctx.CacheContext() // clear all context data set during InitChain to avoid inconsistent behavior diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index ef08859722d0..31f227c1fceb 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -550,19 +550,21 @@ func (app *BaseApp) validateFinalizeBlockHeight(req *abci.RequestFinalizeBlock) return fmt.Errorf("invalid height: %d", req.Height) } - // expectedHeight holds the expected height to validate. + lastBlockHeight := app.LastBlockHeight() + + // expectedHeight holds the expected height to validate var expectedHeight int64 - if app.LastBlockHeight() == 0 && app.initialHeight > 1 { - // In this case, we're validating the first block of the chain (no - // previous commit). The height we're expecting is the initial height. + if lastBlockHeight == 0 && app.initialHeight > 1 { + // In this case, we're validating the first block of the chain, i.e no + // previous commit. The height we're expecting is the initial height. expectedHeight = app.initialHeight } else { // This case can mean two things: - // - either there was already a previous commit in the store, in which - // case we increment the version from there, - // - or there was no previous commit, and initial version was not set, - // in which case we start at version 1. - expectedHeight = app.LastBlockHeight() + 1 + // + // - Either there was already a previous commit in the store, in which + // case we increment the version from there. + // - Or there was no previous commit, in which case we start at version 1. + expectedHeight = lastBlockHeight + 1 } if req.Height != expectedHeight { From e9738e6ac70f537866de270b34ae7123ea5de9ac Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 12:06:24 -0400 Subject: [PATCH 13/32] updates --- baseapp/abci.go | 74 ------------- baseapp/abci_utils.go | 243 ++++++++++++++++++++++++++++++++++++++++++ baseapp/baseapp.go | 148 ------------------------- 3 files changed, 243 insertions(+), 222 deletions(-) create mode 100644 baseapp/abci_utils.go diff --git a/baseapp/abci.go b/baseapp/abci.go index 18984466a7a5..aa3da445e470 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,7 +1,6 @@ package baseapp import ( - "bytes" "context" "crypto/sha256" "fmt" @@ -17,10 +16,7 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" - cmtcrypto "github.com/cometbft/cometbft/crypto" - cryptoenc "github.com/cometbft/cometbft/crypto/encoding" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" - protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -1017,76 +1013,6 @@ func SplitABCIQueryPath(requestPath string) (path []string) { return path } -// ValidateVoteExtensions defines a helper function for verifying vote extension -// signatures that may be passed or manually injected into a block proposal from -// a proposer in ProcessProposal. It returns an error if any signature is invalid -// or if unexpected vote extensions and/or signatures are found. -func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64, extCommit abci.ExtendedCommitInfo) error { - cp := ctx.ConsensusParams() - extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 - - marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { - var buf bytes.Buffer - if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { - return nil, err - } - - return buf.Bytes(), nil - } - - for _, vote := range extCommit.Votes { - if !extsEnabled { - if len(vote.VoteExtension) > 0 { - return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) - } - if len(vote.ExtensionSignature) > 0 { - return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) - } - - continue - } - - if len(vote.ExtensionSignature) == 0 { - return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) - } - - valConsAddr := cmtcrypto.Address(vote.Validator.Address) - - validator, err := app.valStore.GetValidatorByConsAddr(ctx, valConsAddr) - if err != nil { - return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) - } - - cmtPubKeyProto, err := validator.CmtConsPublicKey() - if err != nil { - return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) - } - - cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) - if err != nil { - return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) - } - - cve := cmtproto.CanonicalVoteExtension{ - Extension: vote.VoteExtension, - Height: currentHeight - 1, // the vote extension was signed in the previous height - Round: int64(extCommit.Round), - ChainId: app.chainID, - } - - extSignBytes, err := marshalDelimitedFn(&cve) - if err != nil { - return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) - } - - if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { - return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) - } - } - - return nil -} - // FilterPeerByAddrPort filters peers by address/port. func (app *BaseApp) FilterPeerByAddrPort(info string) *abci.ResponseQuery { if app.addrPeerFilter != nil { diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go new file mode 100644 index 000000000000..28909a5e6c1b --- /dev/null +++ b/baseapp/abci_utils.go @@ -0,0 +1,243 @@ +package baseapp + +import ( + "bytes" + "fmt" + + "github.com/cockroachdb/errors" + abci "github.com/cometbft/cometbft/abci/types" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cryptoenc "github.com/cometbft/cometbft/crypto/encoding" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + + protoio "github.com/cosmos/gogoproto/io" + "github.com/cosmos/gogoproto/proto" + + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/mempool" +) + +type ( + // Validator defines the interface contract require for verifying vote extension + // signatures. Typically, this will be implemented by the x/staking module, + // which has knowledge of the CometBFT public key. + Validator interface { + CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) + } + + // ValidatorStore defines the interface contract require for verifying vote + // extension signatures. Typically, this will be implemented by the x/staking + // module, which has knowledge of the CometBFT public key. + ValidatorStore interface { + GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) + } +) + +// ValidateVoteExtensions defines a helper function for verifying vote extension +// signatures that may be passed or manually injected into a block proposal from +// a proposer in ProcessProposal. It returns an error if any signature is invalid +// or if unexpected vote extensions and/or signatures are found. +func ValidateVoteExtensions( + ctx sdk.Context, + valStore ValidatorStore, + currentHeight int64, + chainID string, + extCommit abci.ExtendedCommitInfo, +) error { + cp := ctx.ConsensusParams() + extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 + + marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { + var buf bytes.Buffer + if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { + return nil, err + } + + return buf.Bytes(), nil + } + + for _, vote := range extCommit.Votes { + if !extsEnabled { + if len(vote.VoteExtension) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) + } + if len(vote.ExtensionSignature) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) + } + + continue + } + + if len(vote.ExtensionSignature) == 0 { + return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) + } + + valConsAddr := cmtcrypto.Address(vote.Validator.Address) + + validator, err := valStore.GetValidatorByConsAddr(ctx, valConsAddr) + if err != nil { + return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) + } + + cmtPubKeyProto, err := validator.CmtConsPublicKey() + if err != nil { + return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) + } + + cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) + if err != nil { + return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) + } + + cve := cmtproto.CanonicalVoteExtension{ + Extension: vote.VoteExtension, + Height: currentHeight - 1, // the vote extension was signed in the previous height + Round: int64(extCommit.Round), + ChainId: chainID, + } + + extSignBytes, err := marshalDelimitedFn(&cve) + if err != nil { + return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) + } + + if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { + return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) + } + } + + return nil +} + +type ( + // ProposalTxVerifier defines the interface that is implemented by BaseApp, + // that any custom ABCI PrepareProposal and ProcessProposal handler can use + // to verify a transaction. + ProposalTxVerifier interface { + PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) + ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) + } + + // DefaultProposalHandler defines the default ABCI PrepareProposal and + // ProcessProposal handlers. + DefaultProposalHandler struct { + mempool mempool.Mempool + txVerifier ProposalTxVerifier + } +) + +func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) DefaultProposalHandler { + return DefaultProposalHandler{ + mempool: mp, + txVerifier: txVerifier, + } +} + +// PrepareProposalHandler returns the default implementation for processing an +// ABCI proposal. The application's mempool is enumerated and all valid +// transactions are added to the proposal. Transactions are valid if they: +// +// 1) Successfully encode to bytes. +// 2) Are valid (i.e. pass runTx, AnteHandler only). +// +// Enumeration is halted once RequestPrepareProposal.MaxBytes of transactions is +// reached or the mempool is exhausted. +// +// Note: +// +// - Step (2) is identical to the validation step performed in +// DefaultProcessProposal. It is very important that the same validation logic +// is used in both steps, and applications must ensure that this is the case in +// non-default handlers. +// +// - If no mempool is set or if the mempool is a no-op mempool, the transactions +// requested from CometBFT will simply be returned, which, by default, are in +// FIFO order. +func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { + return func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + // If the mempool is nil or a no-op mempool, we simply return the transactions + // requested from CometBFT, which, by default, should be in FIFO order. + _, isNoOp := h.mempool.(mempool.NoOpMempool) + if h.mempool == nil || isNoOp { + return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil + } + + var ( + selectedTxs [][]byte + totalTxBytes int64 + ) + + iterator := h.mempool.Select(ctx, req.Txs) + + for iterator != nil { + memTx := iterator.Tx() + + // NOTE: Since transaction verification was already executed in CheckTx, + // which calls mempool.Insert, in theory everything in the pool should be + // valid. But some mempool implementations may insert invalid txs, so we + // check again. + bz, err := h.txVerifier.PrepareProposalVerifyTx(memTx) + if err != nil { + err := h.mempool.Remove(memTx) + if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { + panic(err) + } + } else { + txSize := int64(len(bz)) + if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { + selectedTxs = append(selectedTxs, bz) + } else { + // We've reached capacity per req.MaxTxBytes so we cannot select any + // more transactions. + break + } + } + + iterator = iterator.Next() + } + + return &abci.ResponsePrepareProposal{Txs: selectedTxs}, nil + } +} + +// ProcessProposalHandler returns the default implementation for processing an +// ABCI proposal. Every transaction in the proposal must pass 2 conditions: +// +// 1. The transaction bytes must decode to a valid transaction. +// 2. The transaction must be valid (i.e. pass runTx, AnteHandler only) +// +// If any transaction fails to pass either condition, the proposal is rejected. +// Note that step (2) is identical to the validation step performed in +// DefaultPrepareProposal. It is very important that the same validation logic +// is used in both steps, and applications must ensure that this is the case in +// non-default handlers. +func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { + return func(ctx sdk.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { + for _, txBytes := range req.Txs { + _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) + if err != nil { + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil + } + } + + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil + } +} + +// NoOpPrepareProposal defines a no-op PrepareProposal handler. It will always +// return the transactions sent by the client's request. +func NoOpPrepareProposal() sdk.PrepareProposalHandler { + return func(_ sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil + } +} + +// NoOpProcessProposal defines a no-op ProcessProposal Handler. It will always +// return ACCEPT. +func NoOpProcessProposal() sdk.ProcessProposalHandler { + return func(_ sdk.Context, _ *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil + } +} diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 31f227c1fceb..0ed4c34c4fff 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -14,14 +14,12 @@ import ( "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/crypto/tmhash" - cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/proto" "golang.org/x/exp/maps" codectypes "github.com/cosmos/cosmos-sdk/codec/types" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/mempool" @@ -36,20 +34,6 @@ type ( // particular, if a module changed the substore key name (or removed a substore) // between two versions of the software. StoreLoader func(ms storetypes.CommitMultiStore) error - - // Validator defines the interface contract require for verifying vote extension - // signatures. Typically, this will be implemented by the x/staking module, - // which has knowledge of the CometBFT public key. - Validator interface { - CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) - } - - // ValidatorStore defines the interface contract require for verifying vote - // extension signatures. Typically, this will be implemented by the x/staking - // module, which has knowledge of the CometBFT public key. - ValidatorStore interface { - GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) - } ) const ( @@ -91,7 +75,6 @@ type BaseApp struct { //nolint: maligned extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler - valStore ValidatorStore addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. @@ -943,134 +926,3 @@ func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { return tx, nil } - -type ( - // ProposalTxVerifier defines the interface that is implemented by BaseApp, - // that any custom ABCI PrepareProposal and ProcessProposal handler can use - // to verify a transaction. - ProposalTxVerifier interface { - PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) - ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) - } - - // DefaultProposalHandler defines the default ABCI PrepareProposal and - // ProcessProposal handlers. - DefaultProposalHandler struct { - mempool mempool.Mempool - txVerifier ProposalTxVerifier - } -) - -func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) DefaultProposalHandler { - return DefaultProposalHandler{ - mempool: mp, - txVerifier: txVerifier, - } -} - -// PrepareProposalHandler returns the default implementation for processing an -// ABCI proposal. The application's mempool is enumerated and all valid -// transactions are added to the proposal. Transactions are valid if they: -// -// 1) Successfully encode to bytes. -// 2) Are valid (i.e. pass runTx, AnteHandler only). -// -// Enumeration is halted once RequestPrepareProposal.MaxBytes of transactions is -// reached or the mempool is exhausted. -// -// Note: -// -// - Step (2) is identical to the validation step performed in -// DefaultProcessProposal. It is very important that the same validation logic -// is used in both steps, and applications must ensure that this is the case in -// non-default handlers. -// -// - If no mempool is set or if the mempool is a no-op mempool, the transactions -// requested from CometBFT will simply be returned, which, by default, are in -// FIFO order. -func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { - return func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - // If the mempool is nil or a no-op mempool, we simply return the transactions - // requested from CometBFT, which, by default, should be in FIFO order. - _, isNoOp := h.mempool.(mempool.NoOpMempool) - if h.mempool == nil || isNoOp { - return abci.ResponsePrepareProposal{Txs: req.Txs} - } - - var ( - selectedTxs [][]byte - totalTxBytes int64 - ) - - iterator := h.mempool.Select(ctx, req.Txs) - - for iterator != nil { - memTx := iterator.Tx() - - // NOTE: Since transaction verification was already executed in CheckTx, - // which calls mempool.Insert, in theory everything in the pool should be - // valid. But some mempool implementations may insert invalid txs, so we - // check again. - bz, err := h.txVerifier.PrepareProposalVerifyTx(memTx) - if err != nil { - err := h.mempool.Remove(memTx) - if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { - panic(err) - } - } else { - txSize := int64(len(bz)) - if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { - selectedTxs = append(selectedTxs, bz) - } else { - // We've reached capacity per req.MaxTxBytes so we cannot select any - // more transactions. - break - } - } - - iterator = iterator.Next() - } - - return abci.ResponsePrepareProposal{Txs: selectedTxs} - } -} - -// ProcessProposalHandler returns the default implementation for processing an -// ABCI proposal. Every transaction in the proposal must pass 2 conditions: -// -// 1. The transaction bytes must decode to a valid transaction. -// 2. The transaction must be valid (i.e. pass runTx, AnteHandler only) -// -// If any transaction fails to pass either condition, the proposal is rejected. -// Note that step (2) is identical to the validation step performed in -// DefaultPrepareProposal. It is very important that the same validation logic -// is used in both steps, and applications must ensure that this is the case in -// non-default handlers. -func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { - return func(ctx sdk.Context, req abci.RequestProcessProposal) abci.ResponseProcessProposal { - for _, txBytes := range req.Txs { - _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) - if err != nil { - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} - } - } - - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT} - } -} - -// NoOpPrepareProposal defines a no-op PrepareProposal handler. It will always -// return the transactions sent by the client's request. -func NoOpPrepareProposal() sdk.PrepareProposalHandler { - return func(_ sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - return abci.ResponsePrepareProposal{Txs: req.Txs} - } -} - -// NoOpProcessProposal defines a no-op ProcessProposal Handler. It will always -// return ACCEPT. -func NoOpProcessProposal() sdk.ProcessProposalHandler { - return func(_ sdk.Context, _ abci.RequestProcessProposal) abci.ResponseProcessProposal { - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT} - } -} From 2dc6194e9b72b631e7be2b46a1728bc2ee003966 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 12:10:35 -0400 Subject: [PATCH 14/32] updates --- baseapp/abci.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index aa3da445e470..4080a4dd2251 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -608,6 +608,10 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } + if app.extendVote == nil { + return nil, errors.New("application ExtendVote handler not set") + } + app.voteExtensionState.ctx = app.voteExtensionState.ctx. WithConsensusParams(cp). WithBlockGasMeter(storetypes.NewInfiniteGasMeter()). @@ -653,6 +657,10 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) } + if app.verifyVoteExt == nil { + return nil, errors.New("application VerifyVoteExtension handler not set") + } + // add a deferred recover handler in case verifyVoteExt panics defer func() { if err := recover(); err != nil { From 5782ff4d32b05b020286ba669601dc4c5f317ab4 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 9 Apr 2023 10:00:41 -0400 Subject: [PATCH 15/32] updates --- baseapp/baseapp.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 48f1903844f2..d903493dc2fa 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -62,17 +62,20 @@ type BaseApp struct { txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx txEncoder sdk.TxEncoder // marshal sdk.Tx into []byte - mempool mempool.Mempool // application side mempool - anteHandler sdk.AnteHandler // ante handler for fee and auth - postHandler sdk.PostHandler // post handler, optional, e.g. for tips - initChainer sdk.InitChainer // initialize state with validators and state blob - beginBlocker sdk.LegacyBeginBlocker // logic to run before any txs - processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal - prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal - endBlocker sdk.LegacyEndBlocker // logic to run after all txs, and to determine valset changes - addrPeerFilter sdk.PeerFilter // filter peers by address and port - idPeerFilter sdk.PeerFilter // filter peers by node ID - fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. + mempool mempool.Mempool // application side mempool + anteHandler sdk.AnteHandler // ante handler for fee and auth + postHandler sdk.PostHandler // post handler, optional, e.g. for tips + initChainer sdk.InitChainer // initialize state with validators and state blob + beginBlocker sdk.LegacyBeginBlocker // (legacy ABCI) BeginBlock handler + endBlocker sdk.LegacyEndBlocker // (legacy ABCI) EndBlock handler + processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler + prepareProposal sdk.PrepareProposalHandler // ABCI PrepareProposal + extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler + verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler + + addrPeerFilter sdk.PeerFilter // filter peers by address and port + idPeerFilter sdk.PeerFilter // filter peers by node ID + fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. // manages snapshots, i.e. dumps of app state at certain intervals snapshotManager *snapshots.Manager From 6532c976f142a9511be0ba5adb1c586cc17449a2 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 9 Apr 2023 10:02:12 -0400 Subject: [PATCH 16/32] updates --- baseapp/baseapp.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index d903493dc2fa..210f7ebd9dc8 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -62,10 +62,11 @@ type BaseApp struct { txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx txEncoder sdk.TxEncoder // marshal sdk.Tx into []byte - mempool mempool.Mempool // application side mempool - anteHandler sdk.AnteHandler // ante handler for fee and auth - postHandler sdk.PostHandler // post handler, optional, e.g. for tips - initChainer sdk.InitChainer // initialize state with validators and state blob + mempool mempool.Mempool // application side mempool + anteHandler sdk.AnteHandler // ante handler for fee and auth + postHandler sdk.PostHandler // post handler, optional, e.g. for tips + + initChainer sdk.InitChainer // ABCI InitChain handler beginBlocker sdk.LegacyBeginBlocker // (legacy ABCI) BeginBlock handler endBlocker sdk.LegacyEndBlocker // (legacy ABCI) EndBlock handler processProposal sdk.ProcessProposalHandler // ABCI ProcessProposal handler From d67c9e084beec1c736a5e549278cad4090dd801d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 09:37:48 -0400 Subject: [PATCH 17/32] updates --- baseapp/baseapp.go | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 210f7ebd9dc8..e73bdf75e6f7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -86,15 +86,32 @@ type BaseApp struct { // - checkState is set on InitChain and reset on Commit // - finalizeBlockState is set on InitChain and FinalizeBlock and set to nil // on Commit. - checkState *state // for CheckTx - processProposalState *state // for ProcessProposal - prepareProposalState *state // for PrepareProposal - finalizeBlockState *state // for FinalizeBlock - - // an inter-block write-through cache provided to the context during deliverState + // + // - checkState: Used for CheckTx, which is set based on the previous block's + // state. This state is never committed. + // - prepareProposalState: Used for PrepareProposal, which is set based on the + // previous block's state. This state is never committed. In case of multiple + // consensus rounds, the state is always reset to the previous block's state. + // - voteExtensionState: Used for ExtendVote and VerifyVoteExtension, which is + // set based on the previous block's state. This state is never committed. In + // case of multiple rounds, the state is always reset to the previous block's + // state. + // processProposalState: Used for ProcessProposal, which is set based on the + // the previous block's state. This state is never committed. In case of + // multiple rounds, the state is always reset to the previous block's state. + // finalizeBlockState: Used for FinalizeBlock, which is set based on the + // previous block's state. This state is committed. + checkState *state + prepareProposalState *state + processProposalState *state + voteExtensionState *state + finalizeBlockState *state + + // An inter-block write-through cache provided to the context during the ABCI + // FinalizeBlock call. interBlockCache storetypes.MultiStorePersistentCache - // absent validators from begin block + // absent validators from FinalizeBlock voteInfos []abci.VoteInfo // paramStore is used to query for ABCI consensus parameters from an @@ -105,7 +122,7 @@ type BaseApp struct { // transaction. This is mainly used for DoS and spam prevention. minGasPrices sdk.DecCoins - // initialHeight is the initial height at which we start the baseapp + // initialHeight is the initial height at which we start the BaseApp initialHeight int64 // flag for sealing options and parameters to a BaseApp From 10a99badcf0745f13e664dad1293d38c8aef120a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:04:58 -0400 Subject: [PATCH 18/32] updates --- baseapp/abci.go | 101 ++++++++++++++++++++++++++++++++++++---- baseapp/baseapp.go | 67 +++++++++++++------------- baseapp/test_helpers.go | 4 +- 3 files changed, 128 insertions(+), 44 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 21aaae4b0b44..dba219c0ca6a 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -75,8 +75,8 @@ func (app *BaseApp) InitChain(_ context.Context, req *abci.RequestInitChain) (*a } // initialize states with a correct header - app.setState(runTxModeFinalize, initHeader) - app.setState(runTxModeCheck, initHeader) + app.setState(execModeFinalize, initHeader) + app.setState(execModeCheck, initHeader) // Store the consensus params in the BaseApp's param store. Note, this must be // done after the finalizeBlockState and context have been set as it's persisted @@ -422,14 +422,14 @@ func (app *BaseApp) legacyEndBlock(req *abci.RequestFinalizeBlock) sdk.LegacyRes // will contain relevant error information. Regardless of tx execution outcome, // the ResponseCheckTx will contain relevant gas execution context. func (app *BaseApp) CheckTx(_ context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTx, error) { - var mode runTxMode + var mode execMode switch { case req.Type == abci.CheckTxType_New: - mode = runTxModeCheck + mode = execModeCheck case req.Type == abci.CheckTxType_Recheck: - mode = runTxModeReCheck + mode = execModeReCheck default: return nil, fmt.Errorf("unknown RequestCheckTx type: %s", req.Type) @@ -469,7 +469,7 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP // always reset state given that PrepareProposal can timeout and be called again emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(runTxPrepareProposal, emptyHeader) + app.setState(execModePrepareProposal, emptyHeader) // CometBFT must never call PrepareProposal with a height of 0. // Ref: https://github.com/cometbft/cometbft/blob/059798a4f5b0c9f52aa8655fa619054a0154088c/spec/core/state.md?plain=1#L37-L38 @@ -536,7 +536,7 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // always reset state given that ProcessProposal can timeout and be called again emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(runTxProcessProposal, emptyHeader) + app.setState(execModeProcessProposal, emptyHeader) app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -570,6 +570,87 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP return resp, nil } +// ExtendVote implements the ExtendVote ABCI method and returns a ResponseExtendVote. +// It calls the application's ExtendVote handler which is responsible for performing +// application-specific business logic when sending a pre-commit for the NEXT +// block height. The extensions response may be non-deterministic but must always +// be returned, even if empty. +// +// Agreed upon vote extensions are made available to the proposer of the next +// height and are committed in subsequent height, i.e. H+2. An error is returned +// if vote extensions are not enabled or if extendVote fails or panics. +func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { + // Always reset state given that ExtendVote and VerifyVoteExtension can timeout + // and be called again in a subsequent round. + emptyHeader := cmtproto.Header{ChainID: app.chainID, Height: req.Height} + app.setState(execModeVoteExtension, emptyHeader) + + // If vote extensions are not enabled, as a safety precaution, we return an + // error. + cp := app.GetConsensusParams(app.voteExtensionState.ctx) + if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) + } + + // add a deferred recover handler in case extendVote panics + defer func() { + if err := recover(); err != nil { + app.logger.Error( + "panic recovered in ExtendVote", + "height", req.Height, + "hash", fmt.Sprintf("%X", req.Hash), + "panic", err, + ) + err = fmt.Errorf("recovered panic in ExtendVote: %w", err) + } + }() + + resp, err = app.extendVote(app.voteExtensionState.ctx, req) + if err != nil { + app.logger.Error("failed to extend vote", "height", req.Height, "error", err) + return nil, fmt.Errorf("failed to extend vote: %w", err) + } + + return resp, err +} + +// VerifyVoteExtension implements the VerifyVoteExtension ABCI method and returns +// a ResponseVerifyVoteExtension. It calls the applications' VerifyVoteExtension +// handler which is responsible for performing application-specific business +// logic in verifying a vote extension from another validator during the pre-commit +// phase. The response MUST be deterministic. An error is returned if vote +// extensions are not enabled or if verifyVoteExt fails or panics. +func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVerifyVoteExtension) (resp *abci.ResponseVerifyVoteExtension, err error) { + // If vote extensions are not enabled, as a safety precaution, we return an + // error. + cp := app.GetConsensusParams(app.voteExtensionState.ctx) + if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) + } + + // add a deferred recover handler in case verifyVoteExt panics + defer func() { + if err := recover(); err != nil { + app.logger.Error( + "panic recovered in VerifyVoteExtension", + "height", req.Height, + "hash", fmt.Sprintf("%X", req.Hash), + "validator", fmt.Sprintf("%X", req.ValidatorAddress), + "panic", err, + ) + err = fmt.Errorf("recovered panic in VerifyVoteExtension: %w", err) + } + }() + + resp, err = app.verifyVoteExt(app.voteExtensionState.ctx, req) + if err != nil { + app.logger.Error("failed to verify vote extension", "height", req.Height, "error", err) + return nil, fmt.Errorf("failed to verify vote extension: %w", err) + } + + return resp, err +} + func (app *BaseApp) legacyDeliverTx(tx []byte) *abci.ExecTxResult { gInfo := sdk.GasInfo{} resultStr := "successful" @@ -598,7 +679,7 @@ func (app *BaseApp) legacyDeliverTx(tx []byte) *abci.ExecTxResult { telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, anteEvents, err := app.runTx(runTxModeFinalize, tx) + gInfo, result, anteEvents, err := app.runTx(execModeFinalize, tx) if err != nil { resultStr = "failed" resp = sdkerrors.ResponseExecTxResultWithEvents( @@ -647,7 +728,7 @@ func (app *BaseApp) FinalizeBlock(_ context.Context, req *abci.RequestFinalizeBl // already be initialized in InitChain. Otherwise app.finalizeBlockState will be // nil, since it is reset on Commit. if app.finalizeBlockState == nil { - app.setState(runTxModeFinalize, header) + app.setState(execModeFinalize, header) } else { // In the first block, app.finalizeBlockState.ctx will already be initialized // by InitChain. Context is now updated with Header information. @@ -733,7 +814,7 @@ func (app *BaseApp) Commit(_ context.Context, _ *abci.RequestCommit) (*abci.Resp // // NOTE: This is safe because CometBFT holds a lock on the mempool for // Commit. Use the header from this latest block. - app.setState(runTxModeCheck, header) + app.setState(execModeCheck, header) app.finalizeBlockState = nil diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index e73bdf75e6f7..be7a2ce11d64 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -26,8 +26,7 @@ import ( ) type ( - // Enum mode for app.runTx - runTxMode uint8 + execMode uint8 // StoreLoader defines a customizable function to control how we load the CommitMultiStore // from disk. This is useful for state migration, when loading a datastore written with @@ -37,12 +36,13 @@ type ( ) const ( - runTxModeCheck runTxMode = iota // Check a transaction - runTxModeReCheck // Recheck a (pending) transaction after a commit - runTxModeSimulate // Simulate a transaction - runTxModeFinalize // Finalize a block proposal - runTxPrepareProposal // Prepare a block proposal - runTxProcessProposal // Process a block proposal + execModeCheck execMode = iota // Check a transaction + execModeReCheck // Recheck a (pending) transaction after a commit + execModeSimulate // Simulate a transaction + execModePrepareProposal // Prepare a block proposal + execModeProcessProposal // Process a block proposal + execModeVoteExtension // Extend or verify a pre-commit vote + execModeFinalize // Finalize a block proposal ) var _ abci.Application = (*BaseApp)(nil) @@ -375,7 +375,7 @@ func (app *BaseApp) Init() error { emptyHeader := cmtproto.Header{ChainID: app.chainID} // needed for the export command which inits from store but never calls initchain - app.setState(runTxModeCheck, emptyHeader) + app.setState(execModeCheck, emptyHeader) app.Seal() if app.cms == nil { @@ -426,7 +426,7 @@ func (app *BaseApp) IsSealed() bool { return app.sealed } // setState sets the BaseApp's state for the corresponding mode with a branched // multi-store (i.e. a CacheMultiStore) and a new Context with the same // multi-store branch, and provided header. -func (app *BaseApp) setState(mode runTxMode, header cmtproto.Header) { +func (app *BaseApp) setState(mode execMode, header cmtproto.Header) { ms := app.cms.CacheMultiStore() baseState := &state{ ms: ms, @@ -434,19 +434,22 @@ func (app *BaseApp) setState(mode runTxMode, header cmtproto.Header) { } switch mode { - case runTxModeCheck: + case execModeCheck: baseState.ctx = baseState.ctx.WithIsCheckTx(true).WithMinGasPrices(app.minGasPrices) app.checkState = baseState - case runTxModeFinalize: - app.finalizeBlockState = baseState - - case runTxPrepareProposal: + case execModePrepareProposal: app.prepareProposalState = baseState - case runTxProcessProposal: + case execModeProcessProposal: app.processProposalState = baseState + case execModeVoteExtension: + app.voteExtensionState = baseState + + case execModeFinalize: + app.finalizeBlockState = baseState + default: panic(fmt.Sprintf("invalid runTxMode for setState: %d", mode)) } @@ -559,15 +562,15 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error { return nil } -func (app *BaseApp) getState(mode runTxMode) *state { +func (app *BaseApp) getState(mode execMode) *state { switch mode { - case runTxModeFinalize: + case execModeFinalize: return app.finalizeBlockState - case runTxPrepareProposal: + case execModePrepareProposal: return app.prepareProposalState - case runTxProcessProposal: + case execModeProcessProposal: return app.processProposalState default: @@ -584,7 +587,7 @@ func (app *BaseApp) getBlockGasMeter(ctx sdk.Context) storetypes.GasMeter { } // retrieve the context for the tx w/ txBytes and other memoized values. -func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context { +func (app *BaseApp) getContextForTx(mode execMode, txBytes []byte) sdk.Context { modeState := app.getState(mode) if modeState == nil { panic(fmt.Sprintf("state is nil for mode %v", mode)) @@ -595,11 +598,11 @@ func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx)) - if mode == runTxModeReCheck { + if mode == execModeReCheck { ctx = ctx.WithIsReCheckTx(true) } - if mode == runTxModeSimulate { + if mode == execModeSimulate { ctx, _ = ctx.CacheContext() } @@ -632,7 +635,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context // Note, gas execution info is always returned. A reference to a Result is // returned if the tx does not run out of gas and if all the messages are valid // and execute successfully. An error is returned otherwise. -func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { +func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter, so we initialize upfront. @@ -704,7 +707,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // performance benefits, but it'll be more difficult to get right. anteCtx, msCache = app.cacheTxContext(ctx, txBytes) anteCtx = anteCtx.WithEventManager(sdk.NewEventManager()) - newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) + newCtx, err := app.anteHandler(anteCtx, tx, mode == execModeSimulate) if !newCtx.IsZero() { // At this point, newCtx.MultiStore() is a store branch, or something else @@ -730,7 +733,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re anteEvents = events.ToABCIEvents() } - if mode == runTxModeCheck { + if mode == execModeCheck { err = app.mempool.Insert(ctx, tx) if err != nil { return gInfo, nil, anteEvents, priority, err @@ -762,7 +765,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Note that the state is still preserved. postCtx := runMsgCtx.WithEventManager(sdk.NewEventManager()) - newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate, err == nil) + newCtx, err := app.postHandler(postCtx, tx, mode == execModeSimulate, err == nil) if err != nil { return gInfo, nil, anteEvents, priority, err } @@ -777,7 +780,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re msCache.Write() } - if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { + if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == execModeSimulate) { // append the events in the order of occurrence result.Events = append(anteEvents, result.Events...) } @@ -791,14 +794,14 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // and DeliverTx. An error is returned if any single message fails or if a // Handler does not exist for a given message route. Otherwise, a reference to a // Result is returned. The caller must not commit state if an error is returned. -func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*sdk.Result, error) { +func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode execMode) (*sdk.Result, error) { msgLogs := make(sdk.ABCIMessageLogs, 0, len(msgs)) events := sdk.EmptyEvents() var msgResponses []*codectypes.Any // NOTE: GasWanted is determined by the AnteHandler and GasUsed by the GasMeter. for i, msg := range msgs { - if mode != runTxModeDeliver && mode != runTxModeSimulate { + if mode != runTxModeDeliver && mode != execModeSimulate { break } @@ -886,7 +889,7 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { return nil, err } - _, _, _, _, err = app.runTx(runTxPrepareProposal, bz) + _, _, _, _, err = app.runTx(execModePrepareProposal, bz) if err != nil { return nil, err } @@ -905,7 +908,7 @@ func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { return nil, err } - _, _, _, _, err = app.runTx(runTxProcessProposal, txBz) + _, _, _, _, err = app.runTx(execModeProcessProposal, txBz) if err != nil { return nil, err } diff --git a/baseapp/test_helpers.go b/baseapp/test_helpers.go index d3f24d6ef419..bc97651c629c 100644 --- a/baseapp/test_helpers.go +++ b/baseapp/test_helpers.go @@ -18,13 +18,13 @@ func (app *BaseApp) SimCheck(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, * if err != nil { return sdk.GasInfo{}, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - gasInfo, result, _, _, err := app.runTx(runTxModeCheck, bz) + gasInfo, result, _, _, err := app.runTx(execModeCheck, bz) return gasInfo, result, err } // Simulate executes a tx in simulate mode to get result and gas info. func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) { - gasInfo, result, _, _, err := app.runTx(runTxModeSimulate, txBytes) + gasInfo, result, _, _, err := app.runTx(execModeSimulate, txBytes) return gasInfo, result, err } From b5aa99b32c0cba656cda59bc523e22404577f547 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:07:03 -0400 Subject: [PATCH 19/32] updates --- baseapp/abci.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index dba219c0ca6a..e9a03921549a 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -601,7 +601,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( "hash", fmt.Sprintf("%X", req.Hash), "panic", err, ) - err = fmt.Errorf("recovered panic in ExtendVote: %w", err) + err = fmt.Errorf("recovered application panic in ExtendVote: %w", err) } }() @@ -638,7 +638,7 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri "validator", fmt.Sprintf("%X", req.ValidatorAddress), "panic", err, ) - err = fmt.Errorf("recovered panic in VerifyVoteExtension: %w", err) + err = fmt.Errorf("recovered application panic in VerifyVoteExtension: %w", err) } }() From 1061ff5abc027ac4b29a3dcad09662361f12046b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:08:56 -0400 Subject: [PATCH 20/32] updates --- baseapp/baseapp.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index be7a2ce11d64..e0467310df0a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -89,17 +89,21 @@ type BaseApp struct { // // - checkState: Used for CheckTx, which is set based on the previous block's // state. This state is never committed. + // // - prepareProposalState: Used for PrepareProposal, which is set based on the // previous block's state. This state is never committed. In case of multiple // consensus rounds, the state is always reset to the previous block's state. + // // - voteExtensionState: Used for ExtendVote and VerifyVoteExtension, which is // set based on the previous block's state. This state is never committed. In // case of multiple rounds, the state is always reset to the previous block's // state. - // processProposalState: Used for ProcessProposal, which is set based on the + // + // - processProposalState: Used for ProcessProposal, which is set based on the // the previous block's state. This state is never committed. In case of // multiple rounds, the state is always reset to the previous block's state. - // finalizeBlockState: Used for FinalizeBlock, which is set based on the + // + // - finalizeBlockState: Used for FinalizeBlock, which is set based on the // previous block's state. This state is committed. checkState *state prepareProposalState *state From d12a8eacfb4e0868cf1c9185be8f5d3198d5a056 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:15:55 -0400 Subject: [PATCH 21/32] updates --- baseapp/abci.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index e9a03921549a..f7226718cbe5 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -592,6 +592,12 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } + app.voteExtensionState.ctx = app.voteExtensionState.ctx. + WithConsensusParams(cp). + WithBlockGasMeter(storetypes.NewInfiniteGasMeter()). + WithBlockHeight(req.Height). + WithHeaderHash(req.Hash) + // add a deferred recover handler in case extendVote panics defer func() { if err := recover(); err != nil { From 25e32dc778c507a9423bd52547f10a40df5dff63 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 10 Apr 2023 14:35:15 -0400 Subject: [PATCH 22/32] updates --- baseapp/abci.go | 38 ++++++++++++++++++++++++++++++-------- baseapp/baseapp.go | 10 ++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index f7226718cbe5..71699d8e692d 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -467,11 +467,19 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP return nil, errors.New("PrepareProposal method not set") } - // always reset state given that PrepareProposal can timeout and be called again - emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(execModePrepareProposal, emptyHeader) + // Always reset state given that PrepareProposal can timeout and be called + // again in a subsequent round. + header := cmtproto.Header{ + ChainID: app.chainID, + Height: req.Height, + Time: req.Time, + ProposerAddress: req.ProposerAddress, + NextValidatorsHash: req.NextValidatorsHash, + } + app.setState(execModePrepareProposal, header) // CometBFT must never call PrepareProposal with a height of 0. + // // Ref: https://github.com/cometbft/cometbft/blob/059798a4f5b0c9f52aa8655fa619054a0154088c/spec/core/state.md?plain=1#L37-L38 if req.Height < 1 { return nil, errors.New("PrepareProposal called with invalid height") @@ -502,7 +510,8 @@ func (app *BaseApp) PrepareProposal(_ context.Context, req *abci.RequestPrepareP resp, err = app.prepareProposal(app.prepareProposalState.ctx, req) if err != nil { - return nil, err + app.logger.Error("failed to prepare proposal", "height", req.Height, "error", err) + return &abci.ResponsePrepareProposal{Txs: [][]byte{}}, nil } return resp, nil @@ -534,9 +543,21 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP return nil, errors.New("ProcessProposal called with invalid height") } - // always reset state given that ProcessProposal can timeout and be called again - emptyHeader := cmtproto.Header{ChainID: app.chainID} - app.setState(execModeProcessProposal, emptyHeader) + // Always reset state given that ProcessProposal can timeout and be called + // again in a subsequent round. + header := cmtproto.Header{ + ChainID: app.chainID, + Height: req.Height, + Time: req.Time, + ProposerAddress: req.ProposerAddress, + NextValidatorsHash: req.NextValidatorsHash, + } + app.setState(execModeProcessProposal, header) + + // Since the application can get access to FinalizeBlock state and write to it, + // we must be sure to reset it in case ProcessProposal timeouts and is called + // again in a subsequent round. + app.setState(execModeFinalize, header) app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -564,7 +585,8 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP resp, err = app.processProposal(app.processProposalState.ctx, req) if err != nil { - return nil, err + app.logger.Error("failed to process proposal", "height", req.Height, "error", err) + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } return resp, nil diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index e0467310df0a..0491c7bfcffd 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -459,6 +459,16 @@ func (app *BaseApp) setState(mode execMode, header cmtproto.Header) { } } +// GetFinalizeBlockStateCtx returns the Context associated with the FinalizeBlock +// state. This Context can be used to write data derived from processing vote +// extensions to application state during ProcessProposal. +// +// NOTE: Do no use or write to state using this Context unless you intend for +// that state to be committed. +func (app *BaseApp) GetFinalizeBlockStateCtx() sdk.Context { + return app.finalizeBlockState.ctx +} + // GetConsensusParams returns the current consensus parameters from the BaseApp's // ParamStore. If the BaseApp has no ParamStore defined, nil is returned. func (app *BaseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams { From 137267de5cd69cd92ac7cc4c5a7103121fad2320 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:06:43 -0400 Subject: [PATCH 23/32] updates --- baseapp/abci.go | 74 ++++++++++++++++++++++++++++++++++++++++++++++ baseapp/baseapp.go | 26 +++++++++++++--- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 71699d8e692d..eda9929e14f0 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,6 +1,7 @@ package baseapp import ( + "bytes" "context" "crypto/sha256" "fmt" @@ -16,7 +17,10 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cryptoenc "github.com/cometbft/cometbft/crypto/encoding" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -1016,6 +1020,76 @@ func SplitABCIQueryPath(requestPath string) (path []string) { return path } +// ValidateVoteExtensions defines a helper function for verifying vote extension +// signatures that may be passed or manually injected into a block proposal from +// a proposer in ProcessProposal. It returns an error if any signature is invalid +// or if unexpected vote extensions and/or signatures are found. +func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64, extCommit abci.ExtendedCommitInfo) error { + cp := ctx.ConsensusParams() + extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 + + marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { + var buf bytes.Buffer + if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { + return nil, err + } + + return buf.Bytes(), nil + } + + for _, vote := range extCommit.Votes { + if !extsEnabled { + if len(vote.VoteExtension) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) + } + if len(vote.ExtensionSignature) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) + } + + continue + } + + if len(vote.ExtensionSignature) == 0 { + return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) + } + + valConsAddr := cmtcrypto.Address(vote.Validator.Address) + + validator, err := app.valStore.GetValidatorByConsAddr(ctx, valConsAddr) + if err != nil { + return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) + } + + cmtPubKeyProto, err := validator.CmtConsPublicKey() + if err != nil { + return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) + } + + cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) + if err != nil { + return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) + } + + cve := cmtproto.CanonicalVoteExtension{ + Extension: vote.VoteExtension, + Height: currentHeight - 1, // the vote extension was signed in the previous height + Round: int64(extCommit.Round), + ChainId: app.chainID, + } + + extSignBytes, err := marshalDelimitedFn(&cve) + if err != nil { + return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) + } + + if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { + return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) + } + } + + return nil +} + // FilterPeerByAddrPort filters peers by address/port. func (app *BaseApp) FilterPeerByAddrPort(info string) *abci.ResponseQuery { if app.addrPeerFilter != nil { diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 0491c7bfcffd..47f45e4d1fe2 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -14,12 +14,14 @@ import ( "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/crypto/tmhash" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/proto" "golang.org/x/exp/maps" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/mempool" @@ -28,11 +30,26 @@ import ( type ( execMode uint8 - // StoreLoader defines a customizable function to control how we load the CommitMultiStore - // from disk. This is useful for state migration, when loading a datastore written with - // an older version of the software. In particular, if a module changed the substore key name - // (or removed a substore) between two versions of the software. + // StoreLoader defines a customizable function to control how we load the + // CommitMultiStore from disk. This is useful for state migration, when + // loading a datastore written with an older version of the software. In + // particular, if a module changed the substore key name (or removed a substore) + // between two versions of the software. StoreLoader func(ms storetypes.CommitMultiStore) error + + // Validator defines the interface contract require for verifying vote extension + // signatures. Typically, this will be implemented by the x/staking module, + // which has knowledge of the CometBFT public key. + Validator interface { + CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) + } + + // ValidatorStore defines the interface contract require for verifying vote + // extension signatures. Typically, this will be implemented by the x/staking + // module, which has knowledge of the CometBFT public key. + ValidatorStore interface { + GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) + } ) const ( @@ -74,6 +91,7 @@ type BaseApp struct { extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler + valStore ValidatorStore addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. From dd57ca6cb861895ec1cbfb4a6b18c8ac99b5b32e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:08:08 -0400 Subject: [PATCH 24/32] updates --- baseapp/abci.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index eda9929e14f0..ec06ee9d0e85 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -603,8 +603,8 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // be returned, even if empty. // // Agreed upon vote extensions are made available to the proposer of the next -// height and are committed in subsequent height, i.e. H+2. An error is returned -// if vote extensions are not enabled or if extendVote fails or panics. +// height and are committed in the subsequent height, i.e. H+2. An error is +// returned if vote extensions are not enabled or if extendVote fails or panics. func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { // Always reset state given that ExtendVote and VerifyVoteExtension can timeout // and be called again in a subsequent round. From 2f56bbf4f624096f6e9fdc066af0c3aafe07df02 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 10:10:58 -0400 Subject: [PATCH 25/32] updates --- baseapp/abci.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index ec06ee9d0e85..7fec537f99b0 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -605,6 +605,9 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // Agreed upon vote extensions are made available to the proposer of the next // height and are committed in the subsequent height, i.e. H+2. An error is // returned if vote extensions are not enabled or if extendVote fails or panics. +// +// Note, an error returned from ExtendVote will not cause the CometBFT to panic, +// so it is safe to return errors upon failure. func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { // Always reset state given that ExtendVote and VerifyVoteExtension can timeout // and be called again in a subsequent round. @@ -652,6 +655,9 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // logic in verifying a vote extension from another validator during the pre-commit // phase. The response MUST be deterministic. An error is returned if vote // extensions are not enabled or if verifyVoteExt fails or panics. +// +// Note, an error returned from VerifyVoteExtension will not cause the CometBFT +// to panic, so it is safe to return errors upon failure. func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVerifyVoteExtension) (resp *abci.ResponseVerifyVoteExtension, err error) { // If vote extensions are not enabled, as a safety precaution, we return an // error. From 2e32ced2a08f2e391466eae7d1ce1e5e32e2df89 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 12 Apr 2023 12:20:38 +0200 Subject: [PATCH 26/32] updates --- baseapp/abci.go | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 7fec537f99b0..bfcf5a21c71b 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -49,32 +49,22 @@ func (app *BaseApp) InitChain(_ context.Context, req *abci.RequestInitChain) (*a // On a new chain, we consider the init chain block height as 0, even though // req.InitialHeight is 1 by default. initHeader := cmtproto.Header{ChainID: req.ChainId, Time: req.Time} + app.initialHeight = req.InitialHeight app.logger.Info("InitChain", "initialHeight", req.InitialHeight, "chainID", req.ChainId) - // Set the initial height, which will be used to determine if we are proposing - // or processing the first block or not. - app.initialHeight = req.InitialHeight - - // if req.InitialHeight is > 1, then we set the initial version on all stores + // If req.InitialHeight is > 1, then we set the initial version in the + // stores. if req.InitialHeight > 1 { -<<<<<<< HEAD app.initialHeight = req.InitialHeight initHeader = cmtproto.Header{ChainID: req.ChainId, Height: req.InitialHeight, Time: req.Time} - if err := app.cms.SetInitialVersion(req.InitialHeight); err != nil { - return nil, err -||||||| 401d0d72c9 - app.initialHeight = req.InitialHeight - initHeader = cmtproto.Header{ChainID: req.ChainId, Height: req.InitialHeight, Time: req.Time} - err := app.cms.SetInitialVersion(req.InitialHeight) - if err != nil { - panic(err) -======= - initHeader.Height = req.InitialHeight - if err := app.cms.SetInitialVersion(req.InitialHeight); err != nil { - panic(err) ->>>>>>> main + // if req.InitialHeight is > 1, then we set the initial version on all stores + if req.InitialHeight > 1 { + initHeader.Height = req.InitialHeight + if err := app.cms.SetInitialVersion(req.InitialHeight); err != nil { + return nil, err + } } } @@ -560,8 +550,12 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // Since the application can get access to FinalizeBlock state and write to it, // we must be sure to reset it in case ProcessProposal timeouts and is called - // again in a subsequent round. - app.setState(execModeFinalize, header) + // again in a subsequent round. However, we only want to do this after we've + // processed the first block, as we want to avoid overwriting the finalizeState + // after state changes during InitChain. + if req.Height > app.initialHeight { + app.setState(execModeFinalize, header) + } app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height). WithVoteInfos(app.voteInfos). @@ -1118,7 +1112,7 @@ func (app *BaseApp) FilterPeerByID(info string) *abci.ResponseQuery { // ProcessProposal. We use finalizeBlockState on the first block to be able to // access any state changes made in InitChain. func (app *BaseApp) getContextForProposal(ctx sdk.Context, height int64) sdk.Context { - if height == 1 { + if height == app.initialHeight { ctx, _ = app.finalizeBlockState.ctx.CacheContext() // clear all context data set during InitChain to avoid inconsistent behavior From 136403f81a93b713e2659e58b65318c4b7c2001e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 12:06:24 -0400 Subject: [PATCH 27/32] updates --- baseapp/abci.go | 74 ------------- baseapp/abci_utils.go | 243 ++++++++++++++++++++++++++++++++++++++++++ baseapp/baseapp.go | 148 ------------------------- 3 files changed, 243 insertions(+), 222 deletions(-) create mode 100644 baseapp/abci_utils.go diff --git a/baseapp/abci.go b/baseapp/abci.go index bfcf5a21c71b..cd8cf727d785 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -1,7 +1,6 @@ package baseapp import ( - "bytes" "context" "crypto/sha256" "fmt" @@ -17,10 +16,7 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" - cmtcrypto "github.com/cometbft/cometbft/crypto" - cryptoenc "github.com/cometbft/cometbft/crypto/encoding" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" - protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -1020,76 +1016,6 @@ func SplitABCIQueryPath(requestPath string) (path []string) { return path } -// ValidateVoteExtensions defines a helper function for verifying vote extension -// signatures that may be passed or manually injected into a block proposal from -// a proposer in ProcessProposal. It returns an error if any signature is invalid -// or if unexpected vote extensions and/or signatures are found. -func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64, extCommit abci.ExtendedCommitInfo) error { - cp := ctx.ConsensusParams() - extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 - - marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { - var buf bytes.Buffer - if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { - return nil, err - } - - return buf.Bytes(), nil - } - - for _, vote := range extCommit.Votes { - if !extsEnabled { - if len(vote.VoteExtension) > 0 { - return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) - } - if len(vote.ExtensionSignature) > 0 { - return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) - } - - continue - } - - if len(vote.ExtensionSignature) == 0 { - return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) - } - - valConsAddr := cmtcrypto.Address(vote.Validator.Address) - - validator, err := app.valStore.GetValidatorByConsAddr(ctx, valConsAddr) - if err != nil { - return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) - } - - cmtPubKeyProto, err := validator.CmtConsPublicKey() - if err != nil { - return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) - } - - cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) - if err != nil { - return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) - } - - cve := cmtproto.CanonicalVoteExtension{ - Extension: vote.VoteExtension, - Height: currentHeight - 1, // the vote extension was signed in the previous height - Round: int64(extCommit.Round), - ChainId: app.chainID, - } - - extSignBytes, err := marshalDelimitedFn(&cve) - if err != nil { - return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) - } - - if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { - return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) - } - } - - return nil -} - // FilterPeerByAddrPort filters peers by address/port. func (app *BaseApp) FilterPeerByAddrPort(info string) *abci.ResponseQuery { if app.addrPeerFilter != nil { diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go new file mode 100644 index 000000000000..28909a5e6c1b --- /dev/null +++ b/baseapp/abci_utils.go @@ -0,0 +1,243 @@ +package baseapp + +import ( + "bytes" + "fmt" + + "github.com/cockroachdb/errors" + abci "github.com/cometbft/cometbft/abci/types" + cmtcrypto "github.com/cometbft/cometbft/crypto" + cryptoenc "github.com/cometbft/cometbft/crypto/encoding" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + + protoio "github.com/cosmos/gogoproto/io" + "github.com/cosmos/gogoproto/proto" + + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/mempool" +) + +type ( + // Validator defines the interface contract require for verifying vote extension + // signatures. Typically, this will be implemented by the x/staking module, + // which has knowledge of the CometBFT public key. + Validator interface { + CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) + } + + // ValidatorStore defines the interface contract require for verifying vote + // extension signatures. Typically, this will be implemented by the x/staking + // module, which has knowledge of the CometBFT public key. + ValidatorStore interface { + GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) + } +) + +// ValidateVoteExtensions defines a helper function for verifying vote extension +// signatures that may be passed or manually injected into a block proposal from +// a proposer in ProcessProposal. It returns an error if any signature is invalid +// or if unexpected vote extensions and/or signatures are found. +func ValidateVoteExtensions( + ctx sdk.Context, + valStore ValidatorStore, + currentHeight int64, + chainID string, + extCommit abci.ExtendedCommitInfo, +) error { + cp := ctx.ConsensusParams() + extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 + + marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { + var buf bytes.Buffer + if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { + return nil, err + } + + return buf.Bytes(), nil + } + + for _, vote := range extCommit.Votes { + if !extsEnabled { + if len(vote.VoteExtension) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) + } + if len(vote.ExtensionSignature) > 0 { + return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight) + } + + continue + } + + if len(vote.ExtensionSignature) == 0 { + return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight) + } + + valConsAddr := cmtcrypto.Address(vote.Validator.Address) + + validator, err := valStore.GetValidatorByConsAddr(ctx, valConsAddr) + if err != nil { + return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) + } + + cmtPubKeyProto, err := validator.CmtConsPublicKey() + if err != nil { + return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err) + } + + cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtPubKeyProto) + if err != nil { + return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err) + } + + cve := cmtproto.CanonicalVoteExtension{ + Extension: vote.VoteExtension, + Height: currentHeight - 1, // the vote extension was signed in the previous height + Round: int64(extCommit.Round), + ChainId: chainID, + } + + extSignBytes, err := marshalDelimitedFn(&cve) + if err != nil { + return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err) + } + + if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) { + return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr) + } + } + + return nil +} + +type ( + // ProposalTxVerifier defines the interface that is implemented by BaseApp, + // that any custom ABCI PrepareProposal and ProcessProposal handler can use + // to verify a transaction. + ProposalTxVerifier interface { + PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) + ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) + } + + // DefaultProposalHandler defines the default ABCI PrepareProposal and + // ProcessProposal handlers. + DefaultProposalHandler struct { + mempool mempool.Mempool + txVerifier ProposalTxVerifier + } +) + +func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) DefaultProposalHandler { + return DefaultProposalHandler{ + mempool: mp, + txVerifier: txVerifier, + } +} + +// PrepareProposalHandler returns the default implementation for processing an +// ABCI proposal. The application's mempool is enumerated and all valid +// transactions are added to the proposal. Transactions are valid if they: +// +// 1) Successfully encode to bytes. +// 2) Are valid (i.e. pass runTx, AnteHandler only). +// +// Enumeration is halted once RequestPrepareProposal.MaxBytes of transactions is +// reached or the mempool is exhausted. +// +// Note: +// +// - Step (2) is identical to the validation step performed in +// DefaultProcessProposal. It is very important that the same validation logic +// is used in both steps, and applications must ensure that this is the case in +// non-default handlers. +// +// - If no mempool is set or if the mempool is a no-op mempool, the transactions +// requested from CometBFT will simply be returned, which, by default, are in +// FIFO order. +func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { + return func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + // If the mempool is nil or a no-op mempool, we simply return the transactions + // requested from CometBFT, which, by default, should be in FIFO order. + _, isNoOp := h.mempool.(mempool.NoOpMempool) + if h.mempool == nil || isNoOp { + return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil + } + + var ( + selectedTxs [][]byte + totalTxBytes int64 + ) + + iterator := h.mempool.Select(ctx, req.Txs) + + for iterator != nil { + memTx := iterator.Tx() + + // NOTE: Since transaction verification was already executed in CheckTx, + // which calls mempool.Insert, in theory everything in the pool should be + // valid. But some mempool implementations may insert invalid txs, so we + // check again. + bz, err := h.txVerifier.PrepareProposalVerifyTx(memTx) + if err != nil { + err := h.mempool.Remove(memTx) + if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { + panic(err) + } + } else { + txSize := int64(len(bz)) + if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { + selectedTxs = append(selectedTxs, bz) + } else { + // We've reached capacity per req.MaxTxBytes so we cannot select any + // more transactions. + break + } + } + + iterator = iterator.Next() + } + + return &abci.ResponsePrepareProposal{Txs: selectedTxs}, nil + } +} + +// ProcessProposalHandler returns the default implementation for processing an +// ABCI proposal. Every transaction in the proposal must pass 2 conditions: +// +// 1. The transaction bytes must decode to a valid transaction. +// 2. The transaction must be valid (i.e. pass runTx, AnteHandler only) +// +// If any transaction fails to pass either condition, the proposal is rejected. +// Note that step (2) is identical to the validation step performed in +// DefaultPrepareProposal. It is very important that the same validation logic +// is used in both steps, and applications must ensure that this is the case in +// non-default handlers. +func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { + return func(ctx sdk.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { + for _, txBytes := range req.Txs { + _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) + if err != nil { + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil + } + } + + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil + } +} + +// NoOpPrepareProposal defines a no-op PrepareProposal handler. It will always +// return the transactions sent by the client's request. +func NoOpPrepareProposal() sdk.PrepareProposalHandler { + return func(_ sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil + } +} + +// NoOpProcessProposal defines a no-op ProcessProposal Handler. It will always +// return ACCEPT. +func NoOpProcessProposal() sdk.ProcessProposalHandler { + return func(_ sdk.Context, _ *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil + } +} diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 47f45e4d1fe2..c975a3cee5c1 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -14,14 +14,12 @@ import ( "github.com/cockroachdb/errors" abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/crypto/tmhash" - cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/proto" "golang.org/x/exp/maps" codectypes "github.com/cosmos/cosmos-sdk/codec/types" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/mempool" @@ -36,20 +34,6 @@ type ( // particular, if a module changed the substore key name (or removed a substore) // between two versions of the software. StoreLoader func(ms storetypes.CommitMultiStore) error - - // Validator defines the interface contract require for verifying vote extension - // signatures. Typically, this will be implemented by the x/staking module, - // which has knowledge of the CometBFT public key. - Validator interface { - CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) - } - - // ValidatorStore defines the interface contract require for verifying vote - // extension signatures. Typically, this will be implemented by the x/staking - // module, which has knowledge of the CometBFT public key. - ValidatorStore interface { - GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) - } ) const ( @@ -91,7 +75,6 @@ type BaseApp struct { extendVote sdk.ExtendVoteHandler // ABCI ExtendVote handler verifyVoteExt sdk.VerifyVoteExtensionHandler // ABCI VerifyVoteExtension handler - valStore ValidatorStore addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. @@ -947,134 +930,3 @@ func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { return tx, nil } - -type ( - // ProposalTxVerifier defines the interface that is implemented by BaseApp, - // that any custom ABCI PrepareProposal and ProcessProposal handler can use - // to verify a transaction. - ProposalTxVerifier interface { - PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) - ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) - } - - // DefaultProposalHandler defines the default ABCI PrepareProposal and - // ProcessProposal handlers. - DefaultProposalHandler struct { - mempool mempool.Mempool - txVerifier ProposalTxVerifier - } -) - -func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier) DefaultProposalHandler { - return DefaultProposalHandler{ - mempool: mp, - txVerifier: txVerifier, - } -} - -// PrepareProposalHandler returns the default implementation for processing an -// ABCI proposal. The application's mempool is enumerated and all valid -// transactions are added to the proposal. Transactions are valid if they: -// -// 1) Successfully encode to bytes. -// 2) Are valid (i.e. pass runTx, AnteHandler only). -// -// Enumeration is halted once RequestPrepareProposal.MaxBytes of transactions is -// reached or the mempool is exhausted. -// -// Note: -// -// - Step (2) is identical to the validation step performed in -// DefaultProcessProposal. It is very important that the same validation logic -// is used in both steps, and applications must ensure that this is the case in -// non-default handlers. -// -// - If no mempool is set or if the mempool is a no-op mempool, the transactions -// requested from CometBFT will simply be returned, which, by default, are in -// FIFO order. -func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHandler { - return func(ctx sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - // If the mempool is nil or a no-op mempool, we simply return the transactions - // requested from CometBFT, which, by default, should be in FIFO order. - _, isNoOp := h.mempool.(mempool.NoOpMempool) - if h.mempool == nil || isNoOp { - return abci.ResponsePrepareProposal{Txs: req.Txs} - } - - var ( - selectedTxs [][]byte - totalTxBytes int64 - ) - - iterator := h.mempool.Select(ctx, req.Txs) - - for iterator != nil { - memTx := iterator.Tx() - - // NOTE: Since transaction verification was already executed in CheckTx, - // which calls mempool.Insert, in theory everything in the pool should be - // valid. But some mempool implementations may insert invalid txs, so we - // check again. - bz, err := h.txVerifier.PrepareProposalVerifyTx(memTx) - if err != nil { - err := h.mempool.Remove(memTx) - if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { - panic(err) - } - } else { - txSize := int64(len(bz)) - if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { - selectedTxs = append(selectedTxs, bz) - } else { - // We've reached capacity per req.MaxTxBytes so we cannot select any - // more transactions. - break - } - } - - iterator = iterator.Next() - } - - return abci.ResponsePrepareProposal{Txs: selectedTxs} - } -} - -// ProcessProposalHandler returns the default implementation for processing an -// ABCI proposal. Every transaction in the proposal must pass 2 conditions: -// -// 1. The transaction bytes must decode to a valid transaction. -// 2. The transaction must be valid (i.e. pass runTx, AnteHandler only) -// -// If any transaction fails to pass either condition, the proposal is rejected. -// Note that step (2) is identical to the validation step performed in -// DefaultPrepareProposal. It is very important that the same validation logic -// is used in both steps, and applications must ensure that this is the case in -// non-default handlers. -func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHandler { - return func(ctx sdk.Context, req abci.RequestProcessProposal) abci.ResponseProcessProposal { - for _, txBytes := range req.Txs { - _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) - if err != nil { - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} - } - } - - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT} - } -} - -// NoOpPrepareProposal defines a no-op PrepareProposal handler. It will always -// return the transactions sent by the client's request. -func NoOpPrepareProposal() sdk.PrepareProposalHandler { - return func(_ sdk.Context, req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - return abci.ResponsePrepareProposal{Txs: req.Txs} - } -} - -// NoOpProcessProposal defines a no-op ProcessProposal Handler. It will always -// return ACCEPT. -func NoOpProcessProposal() sdk.ProcessProposalHandler { - return func(_ sdk.Context, _ abci.RequestProcessProposal) abci.ResponseProcessProposal { - return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT} - } -} From 75873b6e210ce6da02f918a49191dd915da2a6fb Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 11 Apr 2023 12:10:35 -0400 Subject: [PATCH 28/32] updates --- baseapp/abci.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/baseapp/abci.go b/baseapp/abci.go index cd8cf727d785..dc209949ffea 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -611,6 +611,10 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } + if app.extendVote == nil { + return nil, errors.New("application ExtendVote handler not set") + } + app.voteExtensionState.ctx = app.voteExtensionState.ctx. WithConsensusParams(cp). WithBlockGasMeter(storetypes.NewInfiniteGasMeter()). @@ -656,6 +660,10 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) } + if app.verifyVoteExt == nil { + return nil, errors.New("application VerifyVoteExtension handler not set") + } + // add a deferred recover handler in case verifyVoteExt panics defer func() { if err := recover(); err != nil { From ace86487b3589838e7f19cb5a3e14de9d48180fe Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 12 Apr 2023 15:24:03 -0400 Subject: [PATCH 29/32] updates --- baseapp/abci.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index bf5e6043a4e4..49464d6f5478 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -58,6 +58,7 @@ func (app *BaseApp) InitChain(_ context.Context, req *abci.RequestInitChain) (*a initHeader.Height = req.InitialHeight if err := app.cms.SetInitialVersion(req.InitialHeight); err != nil { return nil, err + } } // initialize states with a correct header @@ -619,14 +620,14 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // add a deferred recover handler in case extendVote panics defer func() { - if err := recover(); err != nil { + if r := recover(); r != nil { app.logger.Error( "panic recovered in ExtendVote", "height", req.Height, "hash", fmt.Sprintf("%X", req.Hash), "panic", err, ) - err = fmt.Errorf("recovered application panic in ExtendVote: %w", err) + err = fmt.Errorf("recovered application panic in ExtendVote: %v", r) } }() @@ -662,15 +663,15 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri // add a deferred recover handler in case verifyVoteExt panics defer func() { - if err := recover(); err != nil { + if r := recover(); r != nil { app.logger.Error( "panic recovered in VerifyVoteExtension", "height", req.Height, "hash", fmt.Sprintf("%X", req.Hash), "validator", fmt.Sprintf("%X", req.ValidatorAddress), - "panic", err, + "panic", r, ) - err = fmt.Errorf("recovered application panic in VerifyVoteExtension: %w", err) + err = fmt.Errorf("recovered application panic in VerifyVoteExtension: %v", r) } }() From 34001803c304f4b2332d67bf05b685c8a1422c39 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 14 Apr 2023 21:00:04 -1000 Subject: [PATCH 30/32] updates --- baseapp/abci.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 49464d6f5478..ea9867aba797 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -592,9 +592,6 @@ func (app *BaseApp) ProcessProposal(_ context.Context, req *abci.RequestProcessP // Agreed upon vote extensions are made available to the proposer of the next // height and are committed in the subsequent height, i.e. H+2. An error is // returned if vote extensions are not enabled or if extendVote fails or panics. -// -// Note, an error returned from ExtendVote will not cause the CometBFT to panic, -// so it is safe to return errors upon failure. func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) (resp *abci.ResponseExtendVote, err error) { // Always reset state given that ExtendVote and VerifyVoteExtension can timeout // and be called again in a subsequent round. @@ -634,7 +631,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( resp, err = app.extendVote(app.voteExtensionState.ctx, req) if err != nil { app.logger.Error("failed to extend vote", "height", req.Height, "error", err) - return nil, fmt.Errorf("failed to extend vote: %w", err) + return &abci.ResponseExtendVote{VoteExtension: []byte{}}, nil } return resp, err @@ -646,9 +643,6 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // logic in verifying a vote extension from another validator during the pre-commit // phase. The response MUST be deterministic. An error is returned if vote // extensions are not enabled or if verifyVoteExt fails or panics. -// -// Note, an error returned from VerifyVoteExtension will not cause the CometBFT -// to panic, so it is safe to return errors upon failure. func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVerifyVoteExtension) (resp *abci.ResponseVerifyVoteExtension, err error) { // If vote extensions are not enabled, as a safety precaution, we return an // error. @@ -678,7 +672,7 @@ func (app *BaseApp) VerifyVoteExtension(_ context.Context, req *abci.RequestVeri resp, err = app.verifyVoteExt(app.voteExtensionState.ctx, req) if err != nil { app.logger.Error("failed to verify vote extension", "height", req.Height, "error", err) - return nil, fmt.Errorf("failed to verify vote extension: %w", err) + return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil } return resp, err From 925ee71a7b88c4acaa3fcdf436b2d8b0cad78643 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sun, 16 Apr 2023 08:05:05 -1000 Subject: [PATCH 31/32] updates --- baseapp/abci_utils.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 28909a5e6c1b..048bf3fc3dac 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -80,6 +80,9 @@ func ValidateVoteExtensions( if err != nil { return fmt.Errorf("failed to get validator %X: %w", valConsAddr, err) } + if validator == nil { + return fmt.Errorf("validator %X not found", valConsAddr) + } cmtPubKeyProto, err := validator.CmtConsPublicKey() if err != nil { From 6a3104a84c62368b25dca86982cde22b3ba0f89e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 17 Apr 2023 09:38:30 -1000 Subject: [PATCH 32/32] updates --- baseapp/baseapp.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index c975a3cee5c1..0a66e0245663 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -464,8 +464,10 @@ func (app *BaseApp) setState(mode execMode, header cmtproto.Header) { // state. This Context can be used to write data derived from processing vote // extensions to application state during ProcessProposal. // -// NOTE: Do no use or write to state using this Context unless you intend for +// NOTE: +// - Do NOT use or write to state using this Context unless you intend for // that state to be committed. +// - Do NOT use or write to state using this Context on the first block. func (app *BaseApp) GetFinalizeBlockStateCtx() sdk.Context { return app.finalizeBlockState.ctx }