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

no RPC client is defined in offline mode REST API port 1317 #13695

Closed
ducphamle2 opened this issue Oct 30, 2022 · 7 comments
Closed

no RPC client is defined in offline mode REST API port 1317 #13695

ducphamle2 opened this issue Oct 30, 2022 · 7 comments
Assignees

Comments

@ducphamle2
Copy link

ducphamle2 commented Oct 30, 2022

Summary of Bug

I pumped my local chain's go.mod from version 0.45.9 to 0.45.10 and started a completely new chain locally. The chain ran fine, but when I tried to call any REST APIs, it always gave me the error: {"error":"no RPC client is defined in offline mode"}

image

When I use version 0.45.9, I can call the APIs just fine. It seems that the update to 0.45.10 has added some configurations that are required to enable REST APIs that I do not know about.

When I followed the Ignite tutorial and pumped the version to 0.45.10, strangely the scaffold project was still able to call REST APIs. Hence, I don't know what my local chain is missing here, since calling REST APIs with version 0.45.9 still works.

Version

Cosmos SDK 0.45.10
Tendermint 0.34.22

Steps to Reproduce

  1. Clone the repository: git clone https://github.com/oraichain/orai.git && git checkout upgrade-to-sdk-04510
  2. Call: docker-compose up -d orai
  3. Enter the container and run: ./scripts/setup_genesis.sh
  4. oraid start
  5. curl http://localhost:1317/node_info

you can checkout to the branch upgrade-to-sdk-0459 and rebuild the binary inside the container to see the difference

To rebuild the binary, call: make build LEDGER_ENABLED=false BUILD_TAGS=muslc GOMOD_FLAGS= VERSION=0.41.0

@alexanderbez
Copy link
Contributor

@julienrbrt did we make any changes to the Context recently? I don't even know why starting a node would read or use Offline at all...?

@tubackkhoa
Copy link

tubackkhoa commented Oct 31, 2022

Fixed by modifying server/start.go clientCtx := clientCtx.WithClient(local.New(tmNode)) => clientCtx = clientCtx.WithClient(local.New(tmNode))

Better re-assign Client by adding initClientCtx, err := client.ReadPersistentCommandFlags(initClientCtx, cmd.Flags()) in rootCmd init.

@julienrbrt
Copy link
Member

So, I have tried to reproduce, but it seems like it is a bug on your chain?
-> oraichain/orai@upgrade-to-sdk-0459...upgrade-to-sdk-04510

simapp works fine in v0.45.10. The changes @tubackkhoa made in your root.go seems to have resolved your issue. I think we can close this. Feel free to re-open if you have any question.

@williamchong
Copy link
Contributor

This is breaking our LCD too after upgrading from 0.45.9 to 0.45.11 likecoin/likecoin-chain@67ccf9d
It seems the mitigation in rootCmd mentioned above somehow doesn't work for us. It would be best a minor SDK version doesn't break functionality, or at least add a warning in change log.

@nnkken
Copy link
Contributor

nnkken commented Nov 30, 2022

Below is the diff of server/start.go from v0.45.9 to v0.45.10:

$ git diff v0.45.9..v0.45.10 server/start.go
diff --git a/server/start.go b/server/start.go
index ec536ba20..53f266a49 100644
--- a/server/start.go
+++ b/server/start.go
@@ -299,10 +299,14 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
        // service if API or gRPC is enabled, and avoid doing so in the general
        // case, because it spawns a new local tendermint RPC client.
        if (config.API.Enable || config.GRPC.Enable) && tmNode != nil {
-               clientCtx = clientCtx.WithClient(local.New(tmNode))
+               clientCtx := clientCtx.WithClient(local.New(tmNode))

                app.RegisterTxService(clientCtx)
                app.RegisterTendermintService(clientCtx)
+
+               if a, ok := app.(types.ApplicationQueryService); ok {
+                       a.RegisterNodeService(clientCtx)
+               }
        }

        var apiSrv *api.Server

I don't see why having this change:

-               clientCtx = clientCtx.WithClient(local.New(tmNode))
+               clientCtx := clientCtx.WithClient(local.New(tmNode))

This causes the code afterwards does not receive the tmNode as client in clientCtx:

	if config.API.Enable {
		genDoc, err := genDocProvider()
		if err != nil {
			return err
		}

		clientCtx := clientCtx.WithHomeDir(home).WithChainID(genDoc.ChainID)
// ...

(There is also and if block for GRPC afterwards, so I think GRPC is also affected?)

I think from both application logic point of view and backward compatibility point of view, the old behavior should be kept.

Also, the fix should not be client.ReadPersistentCommandFlags, but clientconfig.ReadFromClientConfig, since ReadFromClientConfig setups the context with WithClient.

I tried adding clientconfig.ReadFromClientConfig in rootCmd, but then it resulted in a panic. I'm not sure if it's SDK code problem, chain code problem or configuration problem, need to investigate deeper.

BTW this is the first time I know that there is a config/client.toml. As a developer, I would say that the SDK code is already quite complicated, it's not possible for me to trace every changes. So I hope development of the SDK to focus more on backward compatibility (especially for minor version upgrades).

@nnkken
Copy link
Contributor

nnkken commented Nov 30, 2022

OK my problem with ReadFromClientConfig is that I didn't have WithViper set in initClientCtx, so it is nil and caused nil pointer panic.

@julienrbrt
Copy link
Member

I don't see why having this change:

-               clientCtx = clientCtx.WithClient(local.New(tmNode))
+               clientCtx := clientCtx.WithClient(local.New(tmNode))

This causes the code afterwards does not receive the tmNode as client in clientCtx:

To be honest, I am not certain why we don't have that in 0.45, 0.47 and main. For some reason, v0.46 does not have this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants