Skip to content

Commit

Permalink
fix(stf,server/v2/cometbft): fix default events + improve codec handl…
Browse files Browse the repository at this point in the history
…ing (backport #22837) (#22848)

Co-authored-by: Julien Robert <[email protected]>
  • Loading branch information
mergify[bot] and julienrbrt authored Dec 12, 2024
1 parent aa32fa0 commit af4ecd9
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 60 deletions.
23 changes: 9 additions & 14 deletions server/v2/cometbft/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"google.golang.org/protobuf/reflect/protoregistry"

"cosmossdk.io/collections"
addresscodec "cosmossdk.io/core/address"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/comet"
corecontext "cosmossdk.io/core/context"
Expand All @@ -35,8 +34,6 @@ import (
"cosmossdk.io/server/v2/streaming"
"cosmossdk.io/store/v2/snapshots"
consensustypes "cosmossdk.io/x/consensus/types"

"github.com/cosmos/cosmos-sdk/codec"
)

const (
Expand All @@ -52,13 +49,12 @@ type consensus[T transaction.Tx] struct {
logger log.Logger
appName, version string
app appmanager.AppManager[T]
appCodec codec.Codec
txCodec transaction.Codec[T]
store types.Store
listener *appdata.Listener
snapshotManager *snapshots.Manager
streamingManager streaming.Manager
mempool mempool.Mempool[T]
appCodecs AppCodecs[T]

cfg Config
chainID string
Expand All @@ -84,16 +80,15 @@ type consensus[T transaction.Tx] struct {
addrPeerFilter types.PeerFilter // filter peers by address and port
idPeerFilter types.PeerFilter // filter peers by node ID

queryHandlersMap map[string]appmodulev2.Handler
getProtoRegistry func() (*protoregistry.Files, error)
consensusAddressCodec addresscodec.Codec
cfgMap server.ConfigMap
queryHandlersMap map[string]appmodulev2.Handler
getProtoRegistry func() (*protoregistry.Files, error)
cfgMap server.ConfigMap
}

// CheckTx implements types.Application.
// It is called by cometbft to verify transaction validity
func (c *consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxRequest) (*abciproto.CheckTxResponse, error) {
decodedTx, err := c.txCodec.Decode(req.Tx)
decodedTx, err := c.appCodecs.TxCodec.Decode(req.Tx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -325,7 +320,7 @@ func (c *consensus[T]) InitChain(ctx context.Context, req *abciproto.InitChainRe
ctx,
br,
req.AppStateBytes,
c.txCodec)
c.appCodecs.TxCodec)
if err != nil {
return nil, fmt.Errorf("genesis state init failure: %w", err)
}
Expand Down Expand Up @@ -392,7 +387,7 @@ func (c *consensus[T]) PrepareProposal(
LastCommit: toCoreExtendedCommitInfo(req.LocalLastCommit),
})

txs, err := c.prepareProposalHandler(ciCtx, c.app, c.txCodec, req)
txs, err := c.prepareProposalHandler(ciCtx, c.app, c.appCodecs.TxCodec, req)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -438,7 +433,7 @@ func (c *consensus[T]) ProcessProposal(
LastCommit: toCoreCommitInfo(req.ProposedLastCommit),
})

err := c.processProposalHandler(ciCtx, c.app, c.txCodec, req)
err := c.processProposalHandler(ciCtx, c.app, c.appCodecs.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{
Expand Down Expand Up @@ -567,7 +562,7 @@ func (c *consensus[T]) internalFinalizeBlock(
// TODO(tip): can we expect some txs to not decode? if so, what we do in this case? this does not seem to be the case,
// considering that prepare and process always decode txs, assuming they're the ones providing txs we should never
// have a tx that fails decoding.
decodedTxs, err := decodeTxs(c.logger, req.Txs, c.txCodec)
decodedTxs, err := decodeTxs(c.logger, req.Txs, c.appCodecs.TxCodec)
if err != nil {
return nil, nil, nil, err
}
Expand Down
16 changes: 9 additions & 7 deletions server/v2/cometbft/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,13 +887,15 @@ func setUpConsensus(t *testing.T, gasLimit uint64, mempool mempool.Mempool[mock.
}

return &consensus[mock.Tx]{
logger: log.NewNopLogger(),
appName: "testing-app",
app: am,
mempool: mempool,
store: mockStore,
cfg: Config{AppTomlConfig: DefaultAppTomlConfig()},
txCodec: mock.TxCodec{},
logger: log.NewNopLogger(),
appName: "testing-app",
app: am,
mempool: mempool,
store: mockStore,
cfg: Config{AppTomlConfig: DefaultAppTomlConfig()},
appCodecs: AppCodecs[mock.Tx]{
TxCodec: mock.TxCodec{},
},
chainID: "test",
getProtoRegistry: sync.OnceValues(proto.MergedRegistry),
queryHandlersMap: queryHandler,
Expand Down
2 changes: 1 addition & 1 deletion server/v2/cometbft/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ require (
cosmossdk.io/schema v0.4.0 //main
cosmossdk.io/server/v2 v2.0.0-20241211154953-a38a6a2c8bc8 // main
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880 // main
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa // main
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877 // main
cosmossdk.io/store/v2 v2.0.0-20241209145349-34f407d6367a // main
cosmossdk.io/x/consensus v0.0.0-00010101000000-000000000000
github.com/cometbft/cometbft v1.0.0-rc2.0.20241127125717-4ce33b646ac9
Expand Down
4 changes: 2 additions & 2 deletions server/v2/cometbft/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ cosmossdk.io/server/v2 v2.0.0-20241211154953-a38a6a2c8bc8 h1:Z1tRewzCemRc4iwKPFG
cosmossdk.io/server/v2 v2.0.0-20241211154953-a38a6a2c8bc8/go.mod h1:RAectNg/rAaq0AHOuxbxY2YVTYTVBJCTVg5wHpCIZhE=
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880 h1:0mtB8fSvDjD835WwWF4rGk9qy5TjVjk2jsW14L37v0E=
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880/go.mod h1:elhlrldWtm+9U4PxE0G3wjz83yQwVVGVAOncXJPY1Xc=
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa h1:2V9nqgL50nw45HcQw1UBRQ/y0QBzrgfLIStPSxFnMtY=
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa/go.mod h1:4e9SzLyeGptQ3tSR6nKCNwCu7Ye4uUS2WIJih29dG2c=
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877 h1:9NTR/GHLFkKOZEBqLs3fMaVKUfsEfuboj+JUcq0PmWI=
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877/go.mod h1:XDY0jKCQwtK0NACzQ6BKepv+2GWbHuYsjLQhxl+itpY=
cosmossdk.io/store v1.0.0-rc.0.0.20241204123127-eb3bf8b0469d h1:KQM4Q6kjwlM4HuDZRV8/ZDXX3whjfStndYNTsRrbboQ=
cosmossdk.io/store v1.0.0-rc.0.0.20241204123127-eb3bf8b0469d/go.mod h1:oZBBY4BrkYnghr6MFL0MP5mGqpkPedHcWkXwXddd6tU=
cosmossdk.io/store/v2 v2.0.0-20241209145349-34f407d6367a h1:v300rqUJV1QuNhCcMlhzwgOEWATIknxv8qqwxNfDv0E=
Expand Down
41 changes: 20 additions & 21 deletions server/v2/cometbft/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/std"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/query"
Expand Down Expand Up @@ -117,12 +116,13 @@ func (t txServer[T]) GetBlockWithTxs(ctx context.Context, req *txtypes.GetBlockW
}
decodeTxAt := func(i uint64) error {
tx := blockTxs[i]
txb, err := t.clientCtx.TxConfig.TxDecoder()(tx)
fmt.Println("TxDecoder", txb, err)
txb, err := t.txCodec.Decode(tx)
if err != nil {
return err
}
p, err := txb.(interface{ AsTx() (*txtypes.Tx, error) }).AsTx()

// txServer works only with sdk.Tx
p, err := any(txb).(interface{ AsTx() (*txtypes.Tx, error) }).AsTx()
if err != nil {
return err
}
Expand Down Expand Up @@ -256,13 +256,19 @@ func (t txServer[T]) Simulate(ctx context.Context, req *txtypes.SimulateRequest)
msgResponses = append(msgResponses, anyMsg)
}

event, err := intoABCIEvents(txResult.Events, map[string]struct{}{}, false)
if err != nil {
return nil, status.Errorf(codes.Unknown, "failed to convert events: %v", err)
}

return &txtypes.SimulateResponse{
GasInfo: &sdk.GasInfo{
GasUsed: txResult.GasUsed,
GasWanted: txResult.GasWanted,
},
Result: &sdk.Result{
MsgResponses: msgResponses,
Events: event,
},
}, nil
}
Expand All @@ -273,15 +279,17 @@ func (t txServer[T]) TxDecode(ctx context.Context, req *txtypes.TxDecodeRequest)
return nil, status.Error(codes.InvalidArgument, "invalid empty tx bytes")
}

txb, err := t.clientCtx.TxConfig.TxDecoder()(req.TxBytes)
txb, err := t.txCodec.Decode(req.TxBytes)
if err != nil {
return nil, err
}

tx, err := txb.(interface{ AsTx() (*txtypes.Tx, error) }).AsTx() // TODO: maybe we can break the Tx interface to add this also
// txServer works only with sdk.Tx
tx, err := any(txb).(interface{ AsTx() (*txtypes.Tx, error) }).AsTx()
if err != nil {
return nil, err
}

return &txtypes.TxDecodeResponse{
Tx: tx,
}, nil
Expand Down Expand Up @@ -350,7 +358,7 @@ func (t txServer[T]) TxEncodeAmino(_ context.Context, req *txtypes.TxEncodeAmino
var stdTx legacytx.StdTx
err := t.clientCtx.LegacyAmino.UnmarshalJSON([]byte(req.AminoJson), &stdTx)
if err != nil {
return nil, err
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid request %s", err))
}

encodedBytes, err := t.clientCtx.LegacyAmino.Marshal(stdTx)
Expand Down Expand Up @@ -466,7 +474,7 @@ func (c *consensus[T]) maybeHandleExternalServices(ctx context.Context, req *abc
if strings.HasPrefix(req.Path, "/cosmos.base.tendermint.v1beta1.Service") {
rpcClient, _ := rpchttp.New(c.cfg.ConfigTomlConfig.RPC.ListenAddress)

cometQServer := cmtservice.NewQueryServer(rpcClient, c.Query, c.consensusAddressCodec)
cometQServer := cmtservice.NewQueryServer(rpcClient, c.Query, c.appCodecs.ConsensusAddressCodec)
paths := strings.Split(req.Path, "/")
if len(paths) <= 2 {
return nil, fmt.Errorf("invalid request path: %s", req.Path)
Expand Down Expand Up @@ -516,27 +524,18 @@ func (c *consensus[T]) maybeHandleExternalServices(ctx context.Context, req *abc

// Handle tx service
if strings.HasPrefix(req.Path, "/cosmos.tx.v1beta1.Service") {
// init simple client context
amino := codec.NewLegacyAmino()
std.RegisterLegacyAminoCodec(amino)
txConfig := authtx.NewTxConfig(
c.appCodec,
c.appCodec.InterfaceRegistry().SigningContext().AddressCodec(),
c.appCodec.InterfaceRegistry().SigningContext().ValidatorAddressCodec(),
authtx.DefaultSignModes,
)
rpcClient, _ := client.NewClientFromNode(c.cfg.AppTomlConfig.Address)

// init simple client context
clientCtx := client.Context{}.
WithLegacyAmino(amino).
WithCodec(c.appCodec).
WithTxConfig(txConfig).
WithLegacyAmino(c.appCodecs.LegacyAmino.(*codec.LegacyAmino)).
WithCodec(c.appCodecs.AppCodec).
WithNodeURI(c.cfg.AppTomlConfig.Address).
WithClient(rpcClient)

txService := txServer[T]{
clientCtx: clientCtx,
txCodec: c.txCodec,
txCodec: c.appCodecs.TxCodec,
app: c.app,
consensus: c,
}
Expand Down
2 changes: 1 addition & 1 deletion server/v2/cometbft/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *consensus[T]) handleQueryApp(ctx context.Context, path []string, req *a

switch path[1] {
case "simulate":
tx, err := c.txCodec.Decode(req.Data)
tx, err := c.appCodecs.TxCodec.Decode(req.Data)
if err != nil {
return nil, errorsmod.Wrap(err, "failed to decode tx")
}
Expand Down
23 changes: 16 additions & 7 deletions server/v2/cometbft/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

addresscodec "cosmossdk.io/core/address"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/registry"
"cosmossdk.io/core/server"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
Expand Down Expand Up @@ -66,14 +67,24 @@ type CometBFTServer[T transaction.Tx] struct {
store types.Store
}

// AppCodecs contains all codecs that the CometBFT server requires
// provided by the application. They are extracted in struct to not be API
// breaking once amino is completely deprecated or new codecs should be added.
type AppCodecs[T transaction.Tx] struct {
TxCodec transaction.Codec[T]

// The following codecs are only required for the gRPC services
AppCodec codec.Codec
LegacyAmino registry.AminoRegistrar
ConsensusAddressCodec addresscodec.Codec
}

func New[T transaction.Tx](
logger log.Logger,
appName string,
store types.Store,
app appmanager.AppManager[T],
appCodec codec.Codec,
txCodec transaction.Codec[T],
consensusAddressCodec addresscodec.Codec,
appCodecs AppCodecs[T],
queryHandlers map[string]appmodulev2.Handler,
decoderResolver decoding.DecoderResolver,
serverOptions ServerOptions[T],
Expand All @@ -84,7 +95,7 @@ func New[T transaction.Tx](
serverOptions: serverOptions,
cfgOptions: cfgOptions,
app: app,
txCodec: txCodec,
txCodec: appCodecs.TxCodec,
store: store,
}
srv.logger = logger.With(log.ModuleKey, srv.Name())
Expand Down Expand Up @@ -172,8 +183,7 @@ func New[T transaction.Tx](
cfg: srv.config,
store: store,
logger: logger,
txCodec: txCodec,
appCodec: appCodec,
appCodecs: appCodecs,
listener: listener,
snapshotManager: snapshotManager,
streamingManager: srv.serverOptions.StreamingManager,
Expand All @@ -192,7 +202,6 @@ func New[T transaction.Tx](
addrPeerFilter: srv.serverOptions.AddrPeerFilter,
idPeerFilter: srv.serverOptions.IdPeerFilter,
cfgMap: cfg,
consensusAddressCodec: consensusAddressCodec,
}

c.optimisticExec = oe.NewOptimisticExecution(
Expand Down
2 changes: 1 addition & 1 deletion simapp/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ require (
cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5 // indirect
cosmossdk.io/schema v0.4.0 // indirect
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880 // indirect; main
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa // indirect; main
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877 // indirect; main
cosmossdk.io/store v1.1.1-0.20240909133312-50288938d1b6 // indirect; main
cosmossdk.io/x/tx v1.0.0-alpha.2 // indirect; main
filippo.io/edwards25519 v1.1.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions simapp/v2/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ cosmossdk.io/server/v2 v2.0.0-20241211154953-a38a6a2c8bc8 h1:Z1tRewzCemRc4iwKPFG
cosmossdk.io/server/v2 v2.0.0-20241211154953-a38a6a2c8bc8/go.mod h1:RAectNg/rAaq0AHOuxbxY2YVTYTVBJCTVg5wHpCIZhE=
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880 h1:0mtB8fSvDjD835WwWF4rGk9qy5TjVjk2jsW14L37v0E=
cosmossdk.io/server/v2/appmanager v0.0.0-20241203212527-7d117425d880/go.mod h1:elhlrldWtm+9U4PxE0G3wjz83yQwVVGVAOncXJPY1Xc=
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa h1:2V9nqgL50nw45HcQw1UBRQ/y0QBzrgfLIStPSxFnMtY=
cosmossdk.io/server/v2/stf v0.0.0-20241204101618-7fa2356c07aa/go.mod h1:4e9SzLyeGptQ3tSR6nKCNwCu7Ye4uUS2WIJih29dG2c=
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877 h1:9NTR/GHLFkKOZEBqLs3fMaVKUfsEfuboj+JUcq0PmWI=
cosmossdk.io/server/v2/stf v0.0.0-20241212104257-e6948eeda877/go.mod h1:XDY0jKCQwtK0NACzQ6BKepv+2GWbHuYsjLQhxl+itpY=
cosmossdk.io/store v1.0.0-rc.0.0.20241204123127-eb3bf8b0469d h1:KQM4Q6kjwlM4HuDZRV8/ZDXX3whjfStndYNTsRrbboQ=
cosmossdk.io/store v1.0.0-rc.0.0.20241204123127-eb3bf8b0469d/go.mod h1:oZBBY4BrkYnghr6MFL0MP5mGqpkPedHcWkXwXddd6tU=
cosmossdk.io/store/v2 v2.0.0-20241209145349-34f407d6367a h1:v300rqUJV1QuNhCcMlhzwgOEWATIknxv8qqwxNfDv0E=
Expand Down
9 changes: 6 additions & 3 deletions simapp/v2/simdv2/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,12 @@ func InitRootCmd[T transaction.Tx](
simApp.Name(),
simApp.Store(),
simApp.App.AppManager,
simApp.AppCodec(),
&client.DefaultTxDecoder[T]{TxConfig: deps.TxConfig},
deps.ClientContext.ConsensusAddressCodec,
cometbft.AppCodecs[T]{
AppCodec: simApp.AppCodec(),
TxCodec: &client.DefaultTxDecoder[T]{TxConfig: deps.TxConfig},
LegacyAmino: deps.ClientContext.LegacyAmino,
ConsensusAddressCodec: deps.ClientContext.ConsensusAddressCodec,
},
simApp.App.QueryHandlers(),
simApp.App.SchemaDecoderResolver(),
initCometOptions[T](),
Expand Down
2 changes: 1 addition & 1 deletion tests/systemtests/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func TestSimulateTx_GRPC(t *testing.T) {
require.NoError(t, err)
// Check the result and gas used are correct.
//
// The 12 events are:
// The 10 events are:
// - Sending Fee to the pool: coin_spent, coin_received and transfer
// - tx.* events: tx.fee, tx.acc_seq, tx.signature
// - Sending Amount to recipient: coin_spent, coin_received and transfer
Expand Down

0 comments on commit af4ecd9

Please sign in to comment.