Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: Remove grpc-web/rosetta and improve server performance #1418

Merged
merged 10 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,24 +232,6 @@ jobs:
name: "${{ github.sha }}-${{ matrix.part }}-race-output"
path: ./${{ matrix.part }}-race-output.txt

# TODO finschia: enable this test
# test-rosetta:
# runs-on: ubuntu-latest
# timeout-minutes: 10
# steps:
# - uses: actions/checkout@v2
# - uses: technote-space/get-diff-action@v4
# id: git_diff
# with:
# PATTERNS: |
# **/**.go
# go.mod
# go.sum
# - name: test rosetta
# run: |
# make test-rosetta
# if: env.GIT_DIFF

# TODO ebony: enable this test
# liveness-test:
# runs-on: ubuntu-latest
Expand Down
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Removed

### Breaking Changes
* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24
* (consensus) [\#1178](https://github.com/Finschia/finschia-sdk/pull/1178) change the consensus from Ostracon to Tendermint v0.34.24
* (server): [\#1418](https://github.com/Finschia/finschia-sdk/pull/1418) Remove grpc-web/rosetta and modify grpc-gateway
tkxkd0159 marked this conversation as resolved.
Show resolved Hide resolved

### State Machine Breaking

Expand All @@ -118,4 +119,4 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (docs) [\#1059](https://github.com/Finschia/finschia-sdk/pull/1059) create ERRORS.md for x/module
* (docs) [\#1083](https://github.com/Finschia/finschia-sdk/pull/1083) Add detailed explanation about default events
* (x/token,collection) [#1201](https://github.com/Finschia/finschia-sdk/pull/1201) Deprecate legacy features on x/token,collection
* (build) [\#1393](https://github.com/Finschia/finschia-sdk/pull/1393) add current directory as suffix for docker container
* (build) [\#1393](https://github.com/Finschia/finschia-sdk/pull/1393) add current directory as suffix for docker container
13 changes: 0 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -517,19 +517,6 @@ localnet-debug: localnet-stop localnet-build-dlv localnet-build-nodes
.PHONY: localnet-start localnet-stop localnet-debug localnet-build-env \
localnet-build-dlv localnet-build-nodes

###############################################################################
### rosetta ###
###############################################################################

rosetta-data:
-docker container rm data_dir_build
docker build -t rosetta-ci:latest -f contrib/rosetta/node/Dockerfile .
docker run --name data_dir_build -t rosetta-ci:latest sh /rosetta/data.sh
docker cp data_dir_build:/tmp/data.tar.gz "$(CURDIR)/contrib/rosetta/node/data.tar.gz"
docker container rm data_dir_build

.PHONY: rosetta-data

###############################################################################
### tools ###
###############################################################################
Expand Down
9 changes: 9 additions & 0 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"github.com/pkg/errors"
"github.com/spf13/viper"
rpcclient "github.com/tendermint/tendermint/rpc/client"
"google.golang.org/grpc"
"gopkg.in/yaml.v2"

"github.com/Finschia/finschia-sdk/codec"
Expand All @@ -23,6 +24,7 @@
type Context struct {
FromAddress sdk.AccAddress
Client rpcclient.Client
GRPCClient *grpc.ClientConn
ChainID string
// Deprecated: Codec codec will be changed to Codec: codec.Codec
JSONCodec codec.JSONCodec
Expand Down Expand Up @@ -145,6 +147,13 @@
return ctx
}

// WithGRPCClient returns a copy of the context with an updated GRPC client
// instance.
func (ctx Context) WithGRPCClient(grpcClient *grpc.ClientConn) Context {
ctx.GRPCClient = grpcClient
return ctx

Check warning on line 154 in client/context.go

View check run for this annotation

Codecov / codecov/patch

client/context.go#L152-L154

Added lines #L152 - L154 were not covered by tests
}

// WithUseLedger returns a copy of the context with an updated UseLedger flag.
func (ctx Context) WithUseLedger(useLedger bool) Context {
ctx.UseLedger = useLedger
Expand Down
10 changes: 8 additions & 2 deletions client/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply interface{}, opts ...grpc.CallOption) (err error) {
// Two things can happen here:
// 1. either we're broadcasting a Tx, in which call we call Tendermint's broadcast endpoint directly,
// 2. or we are querying for state, in which case we call ABCI's Query.
// 2-1. or we are querying for state, in which case we call grpc if grpc client set.
// 2-2. or we are querying for state, in which case we call ABCI's Query if grpc client not set.

// In both cases, we don't allow empty request args (it will panic unexpectedly).
if reflect.ValueOf(req).IsNil() {
Expand All @@ -55,7 +56,12 @@
return err
}

// Case 2. Querying state.
if ctx.GRPCClient != nil {
// Case 2-1. Invoke grpc.
return ctx.GRPCClient.Invoke(grpcCtx, method, req, reply, opts...)

Check warning on line 61 in client/grpc_query.go

View check run for this annotation

Codecov / codecov/patch

client/grpc_query.go#L61

Added line #L61 was not covered by tests
}

// Case 2-2. Querying state via abci query.
reqBz, err := ctx.gRPCCodec().Marshal(req)
if err != nil {
return err
Expand Down
29 changes: 0 additions & 29 deletions contrib/rosetta/README.md

This file was deleted.

12 changes: 0 additions & 12 deletions contrib/rosetta/configuration/bootstrap.json

This file was deleted.

59 changes: 0 additions & 59 deletions contrib/rosetta/configuration/data.sh

This file was deleted.

25 changes: 0 additions & 25 deletions contrib/rosetta/configuration/faucet.py

This file was deleted.

51 changes: 0 additions & 51 deletions contrib/rosetta/configuration/rosetta.json

This file was deleted.

17 changes: 0 additions & 17 deletions contrib/rosetta/configuration/run_tests.sh

This file was deleted.

5 changes: 0 additions & 5 deletions contrib/rosetta/configuration/send_funds.sh

This file was deleted.

Loading
Loading