feat: ABCI 1.0 baseapp integration #13453
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,12 +10,13 @@ import ( | |
| "syscall" | ||
| "time" | ||
|
|
||
| abci "github.com/tendermint/tendermint/abci/types" | ||
| tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
| "google.golang.org/grpc/codes" | ||
| grpcstatus "google.golang.org/grpc/status" | ||
|
|
||
| "github.com/cosmos/gogoproto/proto" | ||
|
|
||
| "github.com/cosmos/cosmos-sdk/codec" | ||
| snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" | ||
| "github.com/cosmos/cosmos-sdk/telemetry" | ||
|
|
@@ -249,8 +250,19 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc | |
| // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md | ||
| // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md | ||
| func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { | ||
| memTxs, selectErr := app.mempool.Select(req.Txs, req.MaxTxBytes) | ||
| if selectErr != nil { | ||
| panic(selectErr) | ||
| } | ||
| var txs [][]byte | ||
| for _, memTx := range memTxs { | ||
alexanderbez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| bz, encErr := app.txEncoder(memTx) | ||
| if encErr != nil { | ||
| panic(encErr) | ||
| } | ||
| txs = append(txs, bz) | ||
| } | ||
| return abci.ResponsePrepareProposal{Txs: txs} | ||
alexanderbez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // ProcessProposal implements the ProcessProposal ABCI method and returns a | ||
|
|
@@ -266,7 +278,19 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) abci.Respon | |
| // Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md | ||
| // Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md | ||
| func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal { | ||
| ctx := app.checkState.ctx | ||
|
|
||
| for _, txBytes := range req.Txs { | ||
| anteCtx, _ := app.cacheTxContext(ctx, txBytes) | ||
| tx, err := app.txDecoder(txBytes) | ||
| if err != nil { | ||
| return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} | ||
|
There was a problem hiding this comment. This reject looks right to me, as a TX that fails to decode is a sign that the encoder was buggy, independently of the actual App state There was a problem hiding this comment. Yeah, I agree. If we cannot decode a tx, that indicates some malicious behavior by the proposer in There was a problem hiding this comment. Failure to decode a tx, or any other error encountered when parsing/validating incoming data, doesn't automatically indicate There was a problem hiding this comment. @alexanderbez For the moment, Tendermint will prevent that (malformed) proposal from being decided by prevoting To your second question, you can and should return reject in this case |
||
| } | ||
| ctx, err = app.anteHandler(anteCtx, tx, false) | ||
|
There was a problem hiding this comment. Why are you not running this on There was a problem hiding this comment. @JeancarloBarrios this doesn't look quite correct. In the ADR, we allude to the app having full control over @sergio-mena for the record, we call There was a problem hiding this comment. Yes I have updated this i still need to push but I updated after our conversation yesterday There was a problem hiding this comment. @alexanderbez be careful here folks. Let me describe an example and you tell me if that can happen.
Can this happen? |
||
| if err != nil { | ||
| return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} | ||
| } | ||
| } | ||
| return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT} | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package baseapp | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "sort" | ||
| "strings" | ||
|
|
@@ -20,6 +21,7 @@ import ( | |
| storetypes "github.com/cosmos/cosmos-sdk/store/types" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| "github.com/cosmos/cosmos-sdk/types/mempool" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -55,8 +57,9 @@ type BaseApp struct { //nolint: maligned | |
| msgServiceRouter *MsgServiceRouter // router for redirecting Msg service messages | ||
| interfaceRegistry codectypes.InterfaceRegistry | ||
| 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.AnteHandler // post handler, optional, e.g. for tips | ||
| initChainer sdk.InitChainer // initialize state with validators and state blob | ||
|
|
@@ -160,6 +163,11 @@ func NewBaseApp( | |
| option(app) | ||
| } | ||
|
|
||
| // if execution of options has left certain required fields nil, let's set them to default values | ||
| if app.mempool == nil { | ||
| app.mempool = mempool.DefaultPriorityMempool() | ||
| } | ||
|
|
||
| if app.interBlockCache != nil { | ||
| app.cms.SetInterBlockCache(app.interBlockCache) | ||
| } | ||
|
|
@@ -653,9 +661,22 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re | |
| anteEvents = events.ToABCIEvents() | ||
| } | ||
|
|
||
| if mode == runTxModeCheck { | ||
| err = app.mempool.Insert(ctx, tx.(mempool.Tx)) | ||
| if err != nil { | ||
| return gInfo, nil, anteEvents, priority, err | ||
| } | ||
|
There was a problem hiding this comment. What's the difference between this and lines 678-683 below ? There was a problem hiding this comment. Yeah, this doesn't look right @JeancarloBarrios. We should only be inserting into the app-side mempool on CheckTx once, which we already do below. There was a problem hiding this comment. Agree; sorry I have not fully updated this I will push a bit later today the implementation on likes 678-683has been removed. |
||
| } else if mode == runTxModeDeliver { | ||
|
There was a problem hiding this comment. for my own understanding, why is it okay to not handle other types of There was a problem hiding this comment. We remove during |
||
| err = app.mempool.Remove(tx.(mempool.Tx)) | ||
|
There was a problem hiding this comment. What happens if another node gossips a tx after it is delivered (and removed from There was a problem hiding this comment. @sergio-mena we're not relying or assuming anything about Tendermint here. IIRC, Tendermint reaps the txs from its mempool prior to making a block proposal. Thus, the txs should not be in its mempool and thus gossiped. There was a problem hiding this comment. Two things:
|
||
| if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { | ||
| return gInfo, nil, anteEvents, priority, | ||
| fmt.Errorf("failed to remove tx from mempool: %w", err) | ||
| } | ||
| } | ||
alexanderbez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // TODO remove nil check when implemented | ||
| if mode == runTxModeCheck && app.mempool != nil { | ||
| err = app.mempool.Insert(ctx, tx.(mempool.Tx)) | ||
| if err != nil { | ||
| return gInfo, nil, anteEvents, priority, err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.