From faf5335e2503ad9978d59d434ffde4f8bb44b193 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:28:27 +0700 Subject: [PATCH 1/7] check result err --- server/v2/appmanager/appmanager.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/v2/appmanager/appmanager.go b/server/v2/appmanager/appmanager.go index cef0fc57cce7..09c11a9e6fb9 100644 --- a/server/v2/appmanager/appmanager.go +++ b/server/v2/appmanager/appmanager.go @@ -128,7 +128,8 @@ func (a AppManager[T]) ValidateTx(ctx context.Context, tx T) (appmanager.TxResul if err != nil { return appmanager.TxResult{}, err } - return a.stf.ValidateTx(ctx, latestState, a.config.ValidateTxGasLimit, tx), nil + res := a.stf.ValidateTx(ctx, latestState, a.config.ValidateTxGasLimit, tx) + return res, res.Error } // Simulate runs validation and execution flow of a Tx. From f6b5f43efb3c6fe371b21dc7aa4f19974e71cf0b Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:29:12 +0700 Subject: [PATCH 2/7] check handler & right slice --- server/v2/cometbft/abci.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 54f1c153cc96..b2bdf35fbd95 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -315,8 +315,12 @@ func (c *Consensus[T]) PrepareProposal( return nil, errors.New("PrepareProposal called with invalid height") } - decodedTxs := make([]T, len(req.Txs)) - for i, tx := range req.Txs { + if c.prepareProposalHandler == nil { + return nil, errors.New("no prepare proposal function was set") + } + + var decodedTxs []T + for _, tx := range req.Txs { decTx, err := c.txCodec.Decode(tx) if err != nil { // TODO: vote extension meta data as a custom type to avoid possibly accepting invalid txs @@ -325,7 +329,7 @@ func (c *Consensus[T]) PrepareProposal( continue } - decodedTxs[i] = decTx + decodedTxs = append(decodedTxs, decTx) } ciCtx := contextWithCometInfo(ctx, comet.Info{ @@ -356,7 +360,15 @@ func (c *Consensus[T]) ProcessProposal( ctx context.Context, req *abciproto.ProcessProposalRequest, ) (*abciproto.ProcessProposalResponse, error) { - decodedTxs := make([]T, len(req.Txs)) + if req.Height < 1 { + return nil, errors.New("ProcessProposal called with invalid height") + } + + if c.processProposalHandler == nil { + return nil, errors.New("no process proposal function was set") + } + + var decodedTxs []T for _, tx := range req.Txs { decTx, err := c.txCodec.Decode(tx) if err != nil { From 532edc8e43c0860657af6ffd0946ef93bd05e00d Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:29:32 +0700 Subject: [PATCH 3/7] fix type --- server/v2/cometbft/handlers/defaults.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/v2/cometbft/handlers/defaults.go b/server/v2/cometbft/handlers/defaults.go index c16064974b1f..79d95441b155 100644 --- a/server/v2/cometbft/handlers/defaults.go +++ b/server/v2/cometbft/handlers/defaults.go @@ -8,11 +8,11 @@ import ( abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" "github.com/cosmos/gogoproto/proto" - consensusv1 "cosmossdk.io/api/cosmos/consensus/v1" appmanager "cosmossdk.io/core/app" "cosmossdk.io/core/store" "cosmossdk.io/core/transaction" "cosmossdk.io/server/v2/cometbft/mempool" + consensustypes "cosmossdk.io/x/consensus/types" ) type AppManager[T transaction.Tx] interface { @@ -41,14 +41,14 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { var maxBlockGas uint64 - res, err := app.Query(ctx, 0, &consensusv1.QueryParamsRequest{}) + res, err := app.Query(ctx, 0, &consensustypes.QueryParamsRequest{}) if err != nil { return nil, err } - paramsResp, ok := res.(*consensusv1.QueryParamsResponse) + paramsResp, ok := res.(*consensustypes.QueryParamsResponse) if !ok { - return nil, fmt.Errorf("unexpected consensus params response type; expected: %T, got: %T", &consensusv1.QueryParamsResponse{}, res) + return nil, fmt.Errorf("unexpected consensus params response type; expected: %T, got: %T", &consensustypes.QueryParamsResponse{}, res) } if b := paramsResp.GetParams().Block; b != nil { @@ -110,19 +110,19 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { return nil } - _, ok := req.(*abci.PrepareProposalRequest) + _, ok := req.(*abci.ProcessProposalRequest) if !ok { return fmt.Errorf("invalid request type: %T", req) } - res, err := app.Query(ctx, 0, &consensusv1.QueryParamsRequest{}) + res, err := app.Query(ctx, 0, &consensustypes.QueryParamsRequest{}) if err != nil { return err } - paramsResp, ok := res.(*consensusv1.QueryParamsResponse) + paramsResp, ok := res.(*consensustypes.QueryParamsResponse) if !ok { - return fmt.Errorf("unexpected consensus params response type; expected: %T, got: %T", &consensusv1.QueryParamsResponse{}, res) + return fmt.Errorf("unexpected consensus params response type; expected: %T, got: %T", &consensustypes.QueryParamsResponse{}, res) } var maxBlockGas uint64 From 96171dfb3f1caed5f00780f2a98e99351fbd35a3 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Tue, 13 Aug 2024 14:39:08 +0700 Subject: [PATCH 4/7] move tx decode to handler --- server/v2/cometbft/abci.go | 29 ++-------------- server/v2/cometbft/handlers/defaults.go | 44 +++++++++++++++---------- 2 files changed, 28 insertions(+), 45 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index b2bdf35fbd95..7cbaed85496e 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -319,19 +319,6 @@ func (c *Consensus[T]) PrepareProposal( return nil, errors.New("no prepare proposal function was set") } - var decodedTxs []T - for _, tx := range req.Txs { - decTx, err := c.txCodec.Decode(tx) - if err != nil { - // TODO: vote extension meta data as a custom type to avoid possibly accepting invalid txs - // continue even if tx decoding fails - c.logger.Error("failed to decode tx", "err", err) - continue - } - - decodedTxs = append(decodedTxs, decTx) - } - ciCtx := contextWithCometInfo(ctx, comet.Info{ Evidence: toCoreEvidence(req.Misbehavior), ValidatorsHash: req.NextValidatorsHash, @@ -339,7 +326,7 @@ func (c *Consensus[T]) PrepareProposal( LastCommit: toCoreExtendedCommitInfo(req.LocalLastCommit), }) - txs, err := c.prepareProposalHandler(ciCtx, c.app, decodedTxs, req) + txs, err := c.prepareProposalHandler(ciCtx, c.app, c.txCodec, req) if err != nil { return nil, err } @@ -368,18 +355,6 @@ func (c *Consensus[T]) ProcessProposal( return nil, errors.New("no process proposal function was set") } - var decodedTxs []T - for _, tx := range req.Txs { - decTx, err := c.txCodec.Decode(tx) - if err != nil { - // TODO: vote extension meta data as a custom type to avoid possibly accepting invalid txs - // continue even if tx decoding fails - c.logger.Error("failed to decode tx", "err", err) - continue - } - decodedTxs = append(decodedTxs, decTx) - } - ciCtx := contextWithCometInfo(ctx, comet.Info{ Evidence: toCoreEvidence(req.Misbehavior), ValidatorsHash: req.NextValidatorsHash, @@ -387,7 +362,7 @@ func (c *Consensus[T]) ProcessProposal( LastCommit: toCoreCommitInfo(req.ProposedLastCommit), }) - err := c.processProposalHandler(ciCtx, c.app, decodedTxs, req) + err := c.processProposalHandler(ciCtx, c.app, c.txCodec, req) if err != nil { c.logger.Error("failed to process proposal", "height", req.Height, "time", req.Time, "hash", fmt.Sprintf("%X", req.Hash), "err", err) return &abciproto.ProcessProposalResponse{ diff --git a/server/v2/cometbft/handlers/defaults.go b/server/v2/cometbft/handlers/defaults.go index 79d95441b155..6e1ec19aa38a 100644 --- a/server/v2/cometbft/handlers/defaults.go +++ b/server/v2/cometbft/handlers/defaults.go @@ -6,7 +6,6 @@ import ( "fmt" abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" - "github.com/cosmos/gogoproto/proto" appmanager "cosmossdk.io/core/app" "cosmossdk.io/core/store" @@ -33,12 +32,7 @@ func NewDefaultProposalHandler[T transaction.Tx](mp mempool.Mempool[T]) *Default } func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { - return func(ctx context.Context, app AppManager[T], txs []T, req proto.Message) ([]T, error) { - abciReq, ok := req.(*abci.PrepareProposalRequest) - if !ok { - return nil, fmt.Errorf("expected abci.PrepareProposalRequest, invalid request type: %T,", req) - } - + return func(ctx context.Context, app AppManager[T], codec transaction.Codec[T], req *abci.PrepareProposalRequest) ([]T, error) { var maxBlockGas uint64 res, err := app.Query(ctx, 0, &consensustypes.QueryParamsRequest{}) @@ -55,6 +49,8 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { maxBlockGas = uint64(b.MaxGas) } + txs := decodeTxs(codec, req.Txs) + defer h.txSelector.Clear() // If the mempool is nil or NoOp we simply return the transactions @@ -64,7 +60,7 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { _, isNoOp := h.mempool.(mempool.NoOpMempool[T]) if h.mempool == nil || isNoOp { for _, tx := range txs { - stop := h.txSelector.SelectTxForProposal(ctx, uint64(abciReq.MaxTxBytes), maxBlockGas, tx) + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx) if stop { break } @@ -88,7 +84,7 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { return nil, err } } else { - stop := h.txSelector.SelectTxForProposal(ctx, uint64(abciReq.MaxTxBytes), maxBlockGas, memTx) + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, memTx) if stop { break } @@ -102,7 +98,7 @@ func (h *DefaultProposalHandler[T]) PrepareHandler() PrepareHandler[T] { } func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { - return func(ctx context.Context, app AppManager[T], txs []T, req proto.Message) error { + return func(ctx context.Context, app AppManager[T], codec transaction.Codec[T], req *abci.ProcessProposalRequest) error { // If the mempool is nil we simply return ACCEPT, // because PrepareProposal may have included txs that could fail verification. _, isNoOp := h.mempool.(mempool.NoOpMempool[T]) @@ -110,11 +106,6 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { return nil } - _, ok := req.(*abci.ProcessProposalRequest) - if !ok { - return fmt.Errorf("invalid request type: %T", req) - } - res, err := app.Query(ctx, 0, &consensustypes.QueryParamsRequest{}) if err != nil { return err @@ -130,6 +121,8 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { maxBlockGas = uint64(b.MaxGas) } + txs := decodeTxs(codec, req.Txs) + var totalTxGas uint64 for _, tx := range txs { _, err := app.ValidateTx(ctx, tx) @@ -153,18 +146,33 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { } } +func decodeTxs[T transaction.Tx](codec transaction.Codec[T], txsBz [][]byte) []T { + var txs []T + for _, tx := range txsBz { + decTx, err := codec.Decode(tx) + if err != nil { + // TODO: vote extension meta data as a custom type to avoid possibly accepting invalid txs + // continue even if tx decoding fails + continue + } + + txs = append(txs, decTx) + } + return txs +} + // NoOpPrepareProposal defines a no-op PrepareProposal handler. It will always // return the transactions sent by the client's request. func NoOpPrepareProposal[T transaction.Tx]() PrepareHandler[T] { - return func(ctx context.Context, app AppManager[T], txs []T, req proto.Message) ([]T, error) { - return txs, nil + return func(ctx context.Context, app AppManager[T], codec transaction.Codec[T], req *abci.PrepareProposalRequest) ([]T, error) { + return decodeTxs(codec, req.Txs), nil } } // NoOpProcessProposal defines a no-op ProcessProposal Handler. It will always // return ACCEPT. func NoOpProcessProposal[T transaction.Tx]() ProcessHandler[T] { - return func(context.Context, AppManager[T], []T, proto.Message) error { + return func(context.Context, AppManager[T], transaction.Codec[T], *abci.ProcessProposalRequest) error { return nil } } From c73480994f22b6025be814ec2188ba993026cc59 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Tue, 13 Aug 2024 14:39:25 +0700 Subject: [PATCH 5/7] fix interface types --- server/v2/cometbft/handlers/handlers.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/v2/cometbft/handlers/handlers.go b/server/v2/cometbft/handlers/handlers.go index 0354a4a497af..9008490f6a44 100644 --- a/server/v2/cometbft/handlers/handlers.go +++ b/server/v2/cometbft/handlers/handlers.go @@ -4,7 +4,6 @@ import ( "context" abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" - "github.com/cosmos/gogoproto/proto" "cosmossdk.io/core/store" "cosmossdk.io/core/transaction" @@ -13,11 +12,11 @@ import ( type ( // PrepareHandler passes in the list of Txs that are being proposed. The app can then do stateful operations // over the list of proposed transactions. It can return a modified list of txs to include in the proposal. - PrepareHandler[T transaction.Tx] func(context.Context, AppManager[T], []T, proto.Message) ([]T, error) + PrepareHandler[T transaction.Tx] func(context.Context, AppManager[T], transaction.Codec[T], *abci.PrepareProposalRequest) ([]T, error) // ProcessHandler is a function that takes a list of transactions and returns a boolean and an error. // If the verification of a transaction fails, the boolean is false and the error is non-nil. - ProcessHandler[T transaction.Tx] func(context.Context, AppManager[T], []T, proto.Message) error + ProcessHandler[T transaction.Tx] func(context.Context, AppManager[T], transaction.Codec[T], *abci.ProcessProposalRequest) error // VerifyVoteExtensionhandler is a function type that handles the verification of a vote extension request. // It takes a context, a store reader map, and a request to verify a vote extension. From 5841d3bc48aea44a658842f8deec21c26ea79a23 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 14 Aug 2024 15:46:51 +0700 Subject: [PATCH 6/7] update comment --- server/v2/cometbft/handlers/defaults.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/v2/cometbft/handlers/defaults.go b/server/v2/cometbft/handlers/defaults.go index 6e1ec19aa38a..68743ce553a6 100644 --- a/server/v2/cometbft/handlers/defaults.go +++ b/server/v2/cometbft/handlers/defaults.go @@ -146,6 +146,9 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { } } +// decodeTxs decodes the txs bytes into a decoded txs +// If there a fail decoding tx, remove from the list +// TODO: should return an err here? func decodeTxs[T transaction.Tx](codec transaction.Codec[T], txsBz [][]byte) []T { var txs []T for _, tx := range txsBz { From 6856241fe68012bfe3bcb026c6b2985285e33020 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 4 Sep 2024 14:00:57 +0700 Subject: [PATCH 7/7] update proposal handler --- server/v2/cometbft/handlers/defaults.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server/v2/cometbft/handlers/defaults.go b/server/v2/cometbft/handlers/defaults.go index 68743ce553a6..3fd46da6fe94 100644 --- a/server/v2/cometbft/handlers/defaults.go +++ b/server/v2/cometbft/handlers/defaults.go @@ -121,7 +121,16 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { maxBlockGas = uint64(b.MaxGas) } - txs := decodeTxs(codec, req.Txs) + // Decode request txs bytes + // If there an tx decoded fail, return err + var txs []T + for _, tx := range req.Txs { + decTx, err := codec.Decode(tx) + if err != nil { + return fmt.Errorf("failed to decode tx: %w", err) + } + txs = append(txs, decTx) + } var totalTxGas uint64 for _, tx := range txs { @@ -148,14 +157,12 @@ func (h *DefaultProposalHandler[T]) ProcessHandler() ProcessHandler[T] { // decodeTxs decodes the txs bytes into a decoded txs // If there a fail decoding tx, remove from the list -// TODO: should return an err here? +// Used for prepare proposal func decodeTxs[T transaction.Tx](codec transaction.Codec[T], txsBz [][]byte) []T { var txs []T for _, tx := range txsBz { decTx, err := codec.Decode(tx) if err != nil { - // TODO: vote extension meta data as a custom type to avoid possibly accepting invalid txs - // continue even if tx decoding fails continue }