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.

Sign up for GitHub

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

Jump to bottom

fix: empty REST chain-id #1146

Closed
wants to merge 3 commits into from
Closed

fix: empty REST chain-id #1146

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b971da9
fix: empty REST chain-id
mrostamii Mar 11, 2024
619457e
Merge branch 'develop' into master
mrostamii Apr 12, 2024
9b2be3f
update: chaid-id flag to chain
mrostamii Apr 12, 2024
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
3 changes: 1 addition & 2 deletions server/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/context"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/rpc"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/go-kit/log"
Expand Down Expand Up @@ -76,7 +75,7 @@ func StartRestServer(mainCtx ctx.Context, cdc *codec.Codec, registerRoutesFn fun
logger.Info(
fmt.Sprintf(
"Starting application REST service (chain-id: %q)...",
viper.GetString(flags.FlagChainID),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this value comes from our fork of cosmos-sdk.
I'd probably change the flag value here and raise a PR against maticnetwork/cosmos-sdk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That actually came to my mind too, but didn't want to touch that repo. Btw, let's close this PR once that sdk is modified since that already fixes the issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mrostamii, would you be raising the PR on our cosmos fork ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcello33 @Raneet10 Maybe better to change FlagChainID to FlagChain in both repositories? Tested locally and it was fine, but not sure if it's an accepted strategy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds good, please proceed @mrostamii

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mrostamii any update on this? Also, please change target branch to develop. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
CC @marcello33 @Raneet10

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrostamii once the cosmos PR is merged, the version in heimdall should be updated with a new release and this field flags.FlagChainID (now changed to string "chain") needs to be updated to flags.FlagChain

viper.GetString("chain"),
),
)

Expand Down
Loading