fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

yun-yeo · 2021-10-12T01:33:48Z

Description

When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return

Error: rosetta: (502) bad gateway

This is due to huge genesis load rejection from Tendermint.

In current implementation, rosetta server requests genesis to get initial height

c.tmRPC.Genesis(ctx)

but, this will be failed with below message when the genesis is huge

{
  "jsonrpc": "2.0",
  "id": -1,
  "error": {
    "code": -32603,
    "message": "Internal error",
    "data": "genesis response is large, please use the genesis_chunked API instead"
  }
}

To fix this, we can use following lines

status, err := c.tmRPC. GenesisChunked(ctx)

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

codecov · 2021-10-12T01:50:44Z

Codecov Report

Merging #10340 (ef42cff) into master (26bcb1e) will decrease coverage by 0.01%.
The diff coverage is 36.17%.

@@            Coverage Diff             @@
##           master   #10340      +/-   ##
==========================================
- Coverage   64.27%   64.26%   -0.02%     
==========================================
  Files         572      570       -2     
  Lines       54230    54222       -8     
==========================================
- Hits        34859    34846      -13     
- Misses      17389    17391       +2     
- Partials     1982     1985       +3

Impacted Files	Coverage Δ
x/auth/client/cli/tx_multisign.go	`0.00% <0.00%> (ø)`
x/auth/client/cli/validate_sigs.go	`0.00% <0.00%> (ø)`
x/auth/tx/mode_handler.go	`78.94% <ø> (-2.01%)`	⬇️
x/auth/tx/direct_aux.go	`88.88% <25.00%> (-11.12%)`	⬇️
server/rosetta/client_online.go	`1.70% <28.57%> (+1.70%)`	⬆️
client/tx/tx.go	`41.23% <100.00%> (+0.30%)`	⬆️
server/rosetta/converter.go	`56.55% <100.00%> (+0.17%)`	⬆️
x/auth/middleware/sigverify.go	`66.66% <100.00%> (+0.16%)`	⬆️
x/auth/tx/legacy_amino_json.go	`88.00% <100.00%> (ø)`
x/bank/keeper/keeper.go	`72.10% <100.00%> (+0.38%)`	⬆️
... and 1 more

server/rosetta/client_online.go

CHANGELOG.md

alexanderbez · 2021-10-12T14:23:36Z

server/rosetta/client_online.go

+		return 0, err
+	}
+
+	re, err := regexp.Compile("\"initial_height\":\"(\\d+)\"")


this works, but can we not just decode into a map? I guess this would be faster...

I tried, but the chunked data is not complete JSON string, so hard to decode it to map.

Really? The chunked response is a JSON object. How is it not complete? Do you have an example response?

Ah I mean, the chunked genesis is not complete JSON.

The response itself is JSON.

Here is genesis_chunk response example

…nesis() due to huge genesis load rejection from server

It is possible to submit a TX with a fees object containing a Coin with a nil amount. This results in a rather cryptic redacted panic response when the basic validation checks fee Coins for negative amounts. This PR adds an additional check for nil to provide a friendlier error message. --- *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification (note: No issue exists) - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) (note: First PR against the SDK so please comment with what needs to be done) - [x] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [x] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) Update CHANGELOG.md

## Description Closes: cosmos#10292  --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)

tac0turtle

LGTM.

amaury1093 · 2021-10-13T11:00:47Z

Nice! Could we fix the failing test?

…ad tmRPC.Genesis() (#10340)  ## Description When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 6789862) # Conflicts: # CHANGELOG.md

…ad tmRPC.Genesis() (#10340)  ## Description When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 6789862) # Conflicts: # CHANGELOG.md # server/rosetta/client_online.go

…ad tmRPC.Genesis() (backport #10340) (#10399) * fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (#10340)  ## Description When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 6789862) # Conflicts: # CHANGELOG.md * fix conflicts Co-authored-by: yys <sw.yunsuk@gmail.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com>

…ad tmRPC.Genesis() (backport cosmos#10340) (cosmos#10399) * fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (cosmos#10340)  ## Description When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 6789862) # Conflicts: # CHANGELOG.md * fix conflicts Co-authored-by: yys <sw.yunsuk@gmail.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com>

…ad tmRPC.Genesis() (backport cosmos#10340) (cosmos#10399) * fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (cosmos#10340)  When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 6789862) * fix conflicts Co-authored-by: yys <sw.yunsuk@gmail.com> Co-authored-by: marbar3778 <marbar3778@yahoo.com>

yun-yeo requested review from aaronc and alexanderbez as code owners October 12, 2021 01:33

tac0turtle reviewed Oct 12, 2021

View reviewed changes

server/rosetta/client_online.go Outdated Show resolved Hide resolved

robert-zaremba added backport/0.42.x (Stargate) labels Oct 12, 2021

yun-yeo changed the title ~~[Rosetta] fix getHeight function to use tmRPC.Status() instead tmRPC.Genesis()~~ [Rosetta] fix getHeight function to use tmRPC.GenesisChunk() instead tmRPC.Genesis() Oct 12, 2021

yun-yeo changed the title ~~[Rosetta] fix getHeight function to use tmRPC.GenesisChunk() instead tmRPC.Genesis()~~ [Rosetta] fix getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() Oct 12, 2021

yun-yeo requested a review from tac0turtle October 12, 2021 12:15

yun-yeo changed the title ~~[Rosetta] fix getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis()~~ fix - rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() Oct 12, 2021

amaury1093 self-assigned this Oct 12, 2021

alexanderbez approved these changes Oct 12, 2021

View reviewed changes

yun-yeo and others added 3 commits October 13, 2021 00:25

fix getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Ge…

e3e42cd

…nesis() due to huge genesis load rejection from server

yun-yeo force-pushed the bugfix/rosetta-genensis-block-loading branch from 67a6431 to 8ba4aaf Compare October 12, 2021 15:26

github-actions bot added the C:x/staking label Oct 12, 2021

clean changelog

12833f2

tac0turtle approved these changes Oct 13, 2021

View reviewed changes

tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 13, 2021

tac0turtle changed the title ~~fix - rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis()~~ fix: rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() Oct 13, 2021

yun-yeo added 2 commits October 19, 2021 15:14

fix test

e9bb140

Merge branch 'master' into bugfix/rosetta-genensis-block-loading

ef42cff

github-actions bot removed the C:x/staking label Oct 19, 2021

mergify bot merged commit 6789862 into cosmos:master Oct 19, 2021

mergify bot mentioned this pull request Oct 19, 2021

fix: rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (backport #10340) #10399

Merged

mergify bot mentioned this pull request Oct 19, 2021

fix: rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (backport #10340) #10400

Closed

fedekunze mentioned this pull request Nov 14, 2021

all: cherry pick olympus mons tharsis/cosmos-sdk#1

Closed

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

yun-yeo commented Oct 12, 2021 •

edited

codecov bot commented Oct 12, 2021 •

edited

alexanderbez Oct 12, 2021

yun-yeo Oct 12, 2021

alexanderbez Oct 13, 2021 •

edited

yun-yeo Oct 19, 2021 •

edited

tac0turtle left a comment

amaury1093 commented Oct 13, 2021

fix: rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

fix: rosetta getHeight function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

Conversation

yun-yeo commented Oct 12, 2021 • edited

Description

Author Checklist

Reviewers Checklist

codecov bot commented Oct 12, 2021 • edited

Codecov Report

alexanderbez Oct 12, 2021

Choose a reason for hiding this comment

yun-yeo Oct 12, 2021

Choose a reason for hiding this comment

alexanderbez Oct 13, 2021 • edited

Choose a reason for hiding this comment

yun-yeo Oct 19, 2021 • edited

Choose a reason for hiding this comment

tac0turtle left a comment

Choose a reason for hiding this comment

amaury1093 commented Oct 13, 2021

fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() #10340

yun-yeo commented Oct 12, 2021 •

edited

codecov bot commented Oct 12, 2021 •

edited

alexanderbez Oct 13, 2021 •

edited

yun-yeo Oct 19, 2021 •

edited