Skip to content

fill in all the missing fields of status rpc#599

Merged
tzdybal merged 1 commit intoevstack:mainfrom
gupadhyaya:handle_client_status
Nov 23, 2022
Merged

fill in all the missing fields of status rpc#599
tzdybal merged 1 commit intoevstack:mainfrom
gupadhyaya:handle_client_status

Conversation

@gupadhyaya
Copy link
Copy Markdown
Contributor

Fill in all the missing fields of Status() rpc for compatibility with Tendermint RPC.
Fixes #241

@gupadhyaya gupadhyaya requested a review from tzdybal as a code owner November 11, 2022 03:58
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 11, 2022

Codecov Report

Merging #599 (b4e9a28) into main (b96837b) will decrease coverage by 0.17%.
The diff coverage is 11.76%.

@@            Coverage Diff             @@
##             main     rollkit/rollkit#599      +/-   ##
==========================================
- Coverage   55.79%   55.61%   -0.18%     
==========================================
  Files          50       50              
  Lines        9542     9576      +34     
==========================================
+ Hits         5324     5326       +2     
- Misses       3425     3455      +30     
- Partials      793      795       +2     
Impacted Files Coverage Δ
p2p/client.go 62.32% <0.00%> (-0.59%) ⬇️
rpc/client/client.go 43.92% <0.00%> (-1.54%) ⬇️
config/config.go 80.64% <62.50%> (-6.32%) ⬇️
conv/config.go 50.00% <100.00%> (+2.94%) ⬆️
block/manager.go 65.20% <0.00%> (-1.04%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@gupadhyaya
Copy link
Copy Markdown
Contributor Author

gupadhyaya commented Nov 16, 2022

sample output from the manual testing, queried using http://127.0.0.1:26657/status, also used https://rpc-mamaki.pops.one/status as reference:

{
  "jsonrpc": "2.0",
  "result": {
    "node_info": {
      "protocol_version": {
        "p2p": "8",
        "block": "11",
        "app": "0"
      },
      "id": "12D3KooWQuDYj9sUzzfwAhtvuKZ2xh5aTAoAQNjEMpd7LkwZBop3",
      "listen_addr": "/ip4/0.0.0.0/tcp/26656",
      "network": "gm",
      "version": "0.4.0",
      "channels": "",
      "moniker": "Ganeshas-MacBook-Pro-2.local",
      "other": {
        "tx_index": "on",
        "rpc_address": "tcp://127.0.0.1:26657"
      }
    },
    "sync_info": {
      "latest_block_hash": "0000000000000000000000000000000000000000000000000000000000000000",
      "latest_app_hash": "0000000000000000000000000000000000000000000000000000000000000000",
      "latest_block_height": "34",
      "latest_block_time": "1970-01-01T00:00:01.668591409Z",
      "earliest_block_hash": "0000000000000000000000000000000000000000000000000000000000000000",
      "earliest_app_hash": "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855",
      "earliest_block_height": "1",
      "earliest_block_time": "1970-01-01T00:00:01.668586886Z",
      "catching_up": true
    },
    "validator_info": {
      "address": "09B70D95B518CDDE51AD97E4F94DD12EEA673138",
      "pub_key": {
        "type": "tendermint/PubKeyEd25519",
        "value": "RLIM+4TzmjVX8X7W2RGhJgoVNvzQJGQfBd0fabIuIlg="
      },
      "voting_power": "100"
    }
  },
  "id": -1
}

@gupadhyaya gupadhyaya changed the title [wip] fill in all the missing fields of status rpc fill in all the missing fields of status rpc Nov 16, 2022
@tzdybal tzdybal requested review from a team, Manav-Aggarwal, S1nus and nashqueue and removed request for a team and Manav-Aggarwal November 16, 2022 12:45
Comment thread config/defaults.go
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Looks good!
The only small issue is the "version" information (as commented).

Comment thread rpc/client/client.go
Comment thread rpc/client/client.go Outdated
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Oh, and you should add a simple test to rpc/client/client_test.go, asserting that we're returning reasonable values.

@gupadhyaya
Copy link
Copy Markdown
Contributor Author

Oh, and you should add a simple test to rpc/client/client_test.go, asserting that we're returning reasonable values.

@tzdybal added tests.

remove additional moniker flag from rollmint and use the base config moniker flag

adding basic unit tests

fixing lint issue
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

I really love how detailed is the test 👍

@tzdybal tzdybal requested review from Manav-Aggarwal and removed request for S1nus November 22, 2022 06:57
Comment thread rpc/client/client.go
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Created follow up issue for conversation.

Comment thread rpc/client/client.go
Comment thread rpc/client/client.go
@tzdybal tzdybal merged commit 3a518be into evstack:main Nov 23, 2022
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

Successfully merging this pull request may close these issues.

Finish Status method implementation

5 participants