Skip to content

Commit

Permalink
feat: rollback GetBlockWithTxs of cosmos service.proto for compatib…
Browse files Browse the repository at this point in the history
…ility with cosmos-sdk APIs (#1085)

* feat: rollback `GetBlockWithTxs` of cosmos service.proto for compatibility with cosmos-sdk APIs

* fix: not to throw error when no txs in block

* chore: update changelog

* chore: add warning explanation of `GetBlockWithTxs` in `cosmos/tx/v1beta1/service.proto`

* chore: add more unittest

* chore: change unittest function name.
  • Loading branch information
zemyblue committed Aug 23, 2023
1 parent 4e315d6 commit 3185422
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Features
* (x/auth) [\#1011](https://github.com/Finschia/finschia-sdk/pull/1011) add the api for querying next account number
* (server/grpc) [\#1017](https://github.com/Finschia/finschia-sdk/pull/1017) support custom r/w gRPC options (backport cosmos/cosmos-sdk#11889)
* (x/auth) [\#1085](https://github.com/Finschia/finschia-sdk/pull/1085) rollback GetBlockWithTxs of cosmos service.proto for compatibility with cosmos-sdk APIs
* (proto) [\#1087](https://github.com/Finschia/finschia-sdk/pull/1087) add tendermint query apis for compatibility with cosmos-sdk

### Improvements
Expand Down
2 changes: 1 addition & 1 deletion client/docs/statik/statik.go

Large diffs are not rendered by default.

14 changes: 13 additions & 1 deletion client/docs/swagger-ui/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21210,7 +21210,19 @@ paths:
'/cosmos/tx/v1beta1/txs/block/{height}':
get:
summary: GetBlockWithTxs fetches a block with decoded txs.
description: 'Since: cosmos-sdk 0.45.2'
description: >-
Since: cosmos-sdk 0.45.2

WARNING: In `GetBlockWithTxs` for compatibility with cosmos-sdk API, the
result converted from Ostracon block type

to tendermint block type without `entropy` is returned.

Therefore, verification fails with the tendermint block validation
method.

For original information, please check `GetBlockWithTxs` in
`lbm/tx/v1beta1/service.proto`.
operationId: GetBlockWithTxs
responses:
'200':
Expand Down
2 changes: 1 addition & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -8553,7 +8553,7 @@ Service defines a gRPC service for interacting with transactions.
| `GetTxsEvent` | [GetTxsEventRequest](#cosmos.tx.v1beta1.GetTxsEventRequest) | [GetTxsEventResponse](#cosmos.tx.v1beta1.GetTxsEventResponse) | GetTxsEvent fetches txs by event. | GET|/cosmos/tx/v1beta1/txs|
| `GetBlockWithTxs` | [GetBlockWithTxsRequest](#cosmos.tx.v1beta1.GetBlockWithTxsRequest) | [GetBlockWithTxsResponse](#cosmos.tx.v1beta1.GetBlockWithTxsResponse) | GetBlockWithTxs fetches a block with decoded txs.

Since: cosmos-sdk 0.45.2 | GET|/cosmos/tx/v1beta1/txs/block/{height}|
Since: cosmos-sdk 0.45.2 WARNING: In `GetBlockWithTxs` for compatibility with cosmos-sdk API, the result converted from Ostracon block type to tendermint block type without `entropy` is returned. Therefore, verification fails with the tendermint block validation method. For original information, please check `GetBlockWithTxs` in `lbm/tx/v1beta1/service.proto`. | GET|/cosmos/tx/v1beta1/txs/block/{height}|

<!-- end services -->

Expand Down
4 changes: 4 additions & 0 deletions proto/cosmos/tx/v1beta1/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ service Service {
// GetBlockWithTxs fetches a block with decoded txs.
//
// Since: cosmos-sdk 0.45.2
// WARNING: In `GetBlockWithTxs` for compatibility with cosmos-sdk API, the result converted from Ostracon block type
// to tendermint block type without `entropy` is returned.
// Therefore, verification fails with the tendermint block validation method.
// For original information, please check `GetBlockWithTxs` in `lbm/tx/v1beta1/service.proto`.
rpc GetBlockWithTxs(GetBlockWithTxsRequest) returns (GetBlockWithTxsResponse) {
option (google.api.http).get = "/cosmos/tx/v1beta1/txs/block/{height}";
}
Expand Down
8 changes: 8 additions & 0 deletions types/tx/service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

86 changes: 85 additions & 1 deletion x/auth/tx/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import (
gogogrpc "github.com/gogo/protobuf/grpc"
"github.com/golang/protobuf/proto" //nolint: staticcheck
"github.com/grpc-ecosystem/grpc-gateway/runtime"
tmtypes "github.com/tendermint/tendermint/proto/tendermint/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/Finschia/finschia-sdk/client"
"github.com/Finschia/finschia-sdk/client/grpc/tmservice"
codectypes "github.com/Finschia/finschia-sdk/codec/types"
sdk "github.com/Finschia/finschia-sdk/types"
sdkerrors "github.com/Finschia/finschia-sdk/types/errors"
pagination "github.com/Finschia/finschia-sdk/types/query"
txtypes "github.com/Finschia/finschia-sdk/types/tx"
)
Expand Down Expand Up @@ -161,9 +164,90 @@ func (s txServer) GetTx(ctx context.Context, req *txtypes.GetTxRequest) (*txtype
}, nil
}

// protoTxProvider is a type which can provide a proto transaction. It is a
// workaround to get access to the wrapper TxBuilder's method GetProtoTx().
// ref: https://github.com/cosmos/cosmos-sdk/issues/10347
type protoTxProvider interface {
GetProtoTx() *txtypes.Tx
}

// GetBlockWithTxs returns a block with decoded txs.
func (s txServer) GetBlockWithTxs(ctx context.Context, req *txtypes.GetBlockWithTxsRequest) (*txtypes.GetBlockWithTxsResponse, error) {
return nil, status.Error(codes.Unimplemented, "service not supported")
if req == nil {
return nil, status.Error(codes.InvalidArgument, "request cannot be nil")
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
currentHeight := sdkCtx.BlockHeight()

if req.Height < 1 || req.Height > currentHeight {
return nil, sdkerrors.ErrInvalidHeight.Wrapf("requested height %d but height must not be less than 1 "+
"or greater than the current height %d", req.Height, currentHeight)
}

blockID, block, err := tmservice.GetProtoBlock(ctx, s.clientCtx, &req.Height)
if err != nil {
return nil, err
}

var offset, limit uint64
if req.Pagination != nil {
offset = req.Pagination.Offset
limit = req.Pagination.Limit
} else {
offset = 0
limit = pagination.DefaultLimit
}

blockTxs := block.Data.Txs
blockTxsLn := uint64(len(blockTxs))
txs := make([]*txtypes.Tx, 0, limit)
if offset >= blockTxsLn && blockTxsLn != 0 {
return nil, sdkerrors.ErrInvalidRequest.Wrapf("out of range: cannot paginate %d txs with offset %d and limit %d", blockTxsLn, offset, limit)
}
decodeTxAt := func(i uint64) error {
tx := blockTxs[i]
txb, err := s.clientCtx.TxConfig.TxDecoder()(tx)
if err != nil {
return err
}
p, ok := txb.(protoTxProvider)
if !ok {
return sdkerrors.ErrTxDecode.Wrapf("could not cast %T to %T", txb, txtypes.Tx{})
}
txs = append(txs, p.GetProtoTx())
return nil
}
if req.Pagination != nil && req.Pagination.Reverse {
for i, count := offset, uint64(0); i > 0 && count != limit; i, count = i-1, count+1 {
if err = decodeTxAt(i); err != nil {
return nil, err
}
}
} else {
for i, count := offset, uint64(0); i < blockTxsLn && count != limit; i, count = i+1, count+1 {
if err = decodeTxAt(i); err != nil {
return nil, err
}
}
}

// convert ostracon's block struct to tendermint's block struct
tmBlock := tmtypes.Block{
Header: block.Header,
Data: block.Data,
Evidence: block.Evidence,
LastCommit: block.LastCommit,
}

return &txtypes.GetBlockWithTxsResponse{
Txs: txs,
BlockId: &blockID,
Block: &tmBlock,
Pagination: &pagination.PageResponse{
Total: blockTxsLn,
},
}, nil
}

func (s txServer) BroadcastTx(ctx context.Context, req *txtypes.BroadcastTxRequest) (*txtypes.BroadcastTxResponse, error) {
Expand Down
34 changes: 23 additions & 11 deletions x/auth/tx/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/Finschia/finschia-sdk/types/tx/signing"
authclient "github.com/Finschia/finschia-sdk/x/auth/client"
authtest "github.com/Finschia/finschia-sdk/x/auth/client/testutil"
tx2 "github.com/Finschia/finschia-sdk/x/auth/tx"
bankcli "github.com/Finschia/finschia-sdk/x/bank/client/testutil"
banktypes "github.com/Finschia/finschia-sdk/x/bank/types"
)
Expand Down Expand Up @@ -603,14 +604,16 @@ func (s IntegrationTestSuite) TestGetBlockWithTxs_GRPC() {
req *tx.GetBlockWithTxsRequest
expErr bool
expErrMsg string
expTxsLen int
}{
{"nil request", nil, true, "request cannot be nil"},
{"empty request", &tx.GetBlockWithTxsRequest{}, true, "service not supported"},
{"bad height", &tx.GetBlockWithTxsRequest{Height: 99999999}, true, "service not supported"},
{"bad pagination", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 1000, Limit: 100}}, true, "service not supported"},
{"good request", &tx.GetBlockWithTxsRequest{Height: s.txHeight}, true, "service not supported"},
{"with pagination request", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 0, Limit: 1}}, true, "service not supported"},
{"page all request", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 0, Limit: 100}}, true, "service not supported"},
{"nil request", nil, true, "request cannot be nil", 0},
{"empty request", &tx.GetBlockWithTxsRequest{}, true, "height must not be less than 1 or greater than the current height", 0},
{"bad height", &tx.GetBlockWithTxsRequest{Height: 99999999}, true, "height must not be less than 1 or greater than the current height", 0},
{"bad pagination", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 1000, Limit: 100}}, true, "out of range", 0},
{"good request", &tx.GetBlockWithTxsRequest{Height: s.txHeight}, false, "", 1},
{"with pagination request", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 0, Limit: 1}}, false, "", 1},
{"page all request", &tx.GetBlockWithTxsRequest{Height: s.txHeight, Pagination: &query.PageRequest{Offset: 0, Limit: 100}}, false, "", 1},
{"block with 0 tx", &tx.GetBlockWithTxsRequest{Height: s.txHeight - 1, Pagination: &query.PageRequest{Offset: 0, Limit: 100}}, false, "", 0},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
Expand All @@ -621,7 +624,9 @@ func (s IntegrationTestSuite) TestGetBlockWithTxs_GRPC() {
s.Require().Contains(err.Error(), tc.expErrMsg)
} else {
s.Require().NoError(err)
s.Require().Equal("foobar", grpcRes.Txs[0].Body.Memo)
if tc.expTxsLen > 0 {
s.Require().Equal("foobar", grpcRes.Txs[0].Body.Memo)
}
s.Require().Equal(grpcRes.Block.Header.Height, tc.req.Height)
if tc.req.Pagination != nil {
s.Require().LessOrEqual(len(grpcRes.Txs), int(tc.req.Pagination.Limit))
Expand All @@ -631,6 +636,13 @@ func (s IntegrationTestSuite) TestGetBlockWithTxs_GRPC() {
}
}

func (s IntegrationTestSuite) TestGetBlockWithTxs() {
srv := tx2.NewTxServer(client.Context{}, nil, nil)

_, err := srv.GetBlockWithTxs(context.Background(), nil)
s.Require().Contains(err.Error(), "request cannot be nil")
}

func (s IntegrationTestSuite) TestGetBlockWithTxs_GRPCGateway() {
val := s.network.Validators[0]
testCases := []struct {
Expand All @@ -642,17 +654,17 @@ func (s IntegrationTestSuite) TestGetBlockWithTxs_GRPCGateway() {
{
"empty params",
fmt.Sprintf("%s/cosmos/tx/v1beta1/txs/block/0", val.APIAddress),
true, "service not supported",
true, "height must not be less than 1 or greater than the current height",
},
{
"bad height",
fmt.Sprintf("%s/cosmos/tx/v1beta1/txs/block/%d", val.APIAddress, 9999999),
true, "service not supported",
true, "height must not be less than 1 or greater than the current height",
},
{
"good request",
fmt.Sprintf("%s/cosmos/tx/v1beta1/txs/block/%d", val.APIAddress, s.txHeight),
true, "service not supported",
false, "",
},
}
for _, tc := range testCases {
Expand Down

0 comments on commit 3185422

Please sign in to comment.