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

fix(rpc): handle case when node is not persisting abci responses #1863

Merged
merged 8 commits into from
Oct 13, 2023

Conversation

GAtom22
Copy link
Contributor

@GAtom22 GAtom22 commented Oct 13, 2023

Description

When setting the configuration parameter discard_abci_responses = true, all JSON-RPC methods that rely on TendermintBlockResult (e.g.: eth_gasPrice, eth_getBlockByNumber, eth_getBlockByHash, eth_getTransactionByHash, eth_getTransactionReceipt, eth_getTransactionLogs, eth_feeHistory) will not return the expected result (usually null) and in some cases cause a panic.

For example, in eth_gasPrice the error logs are:

4:48PM INF Served eth_gasPrice conn=10.100.1.9:40160 duration=594.644458 err="method handler crashed" module=geth reqid=2231
4:48PM ERR RPC method eth_gasPrice crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 109462359 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/service.go:200 +0x85
panic({0x2c07660?, 0x59fa270?})
	/usr/local/go/src/runtime/panic.go:914 +0x21f
github.com/evmos/evmos/v14/rpc/backend.(*Backend).GasPrice(0x1?)
	/go/src/github.com/evmos/evmos/rpc/backend/call_tx.go:391 +0x2e
github.com/evmos/evmos/v14/rpc/namespaces/ethereum/eth.(*PublicAPI).GasPrice(0xc1a7ac8120)
	/go/src/github.com/evmos/evmos/rpc/namespaces/ethereum/eth/api.go:304 +0x4d
reflect.Value.call({0xc0128ec7d0?, 0xc01e1108c8?, 0x7efc27a56370?}, {0x308f76e, 0x4}, {0xc4ea675410, 0x1, 0x0?})
	/usr/local/go/src/reflect/value.go:596 +0xce7
reflect.Value.Call({0xc0128ec7d0?, 0xc01e1108c8?, 0x0?}, {0xc4ea675410?, 0xc06c69acb0?, 0x53e005?})
	/usr/local/go/src/reflect/value.go:380 +0xb9
github.com/ethereum/go-ethereum/rpc.(*callback).call(0xc014c7da40, {0x3fbc150?, 0xc4d9003b80}, {0xc4aeabec80, 0xc}, {0x5d82240, 0x0, 0x4f54cf?})
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/service.go:206 +0x37c
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0xc4aeabec90?, {0x3fbc150?, 0xc4d9003b80?}, 0xc4da7ad030, 0x0?, {0x5d82240?, 0x2dd3700?, 0xc06c69ae08?})
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:388 +0x3c
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0xc4daf22240, 0xc4ea6753b0, 0xc4da7ad030)
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:336 +0x22f
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0xc4daf22240, 0x30?, 0xc4da7ad030)
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:297 +0xbd
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0xc4ea6753b0)
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:138 +0x2f
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1()
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:225 +0xbe
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc in goroutine 109099080
	/go/pkg/mod/github.com/evmos/go-ethereum@v1.10.26-evmos-rc2/rpc/handler.go:221 +0x79
 module=geth
4:48PM INF Served eth_gasPrice conn=10.100.1.8:47380 duration=600.599497 err="method handler crashed" module=geth reqid=2232
4:48PM INF Served eth_getLogs conn=10.100.1.8:43508 duration=497.484693 err="failed to fetch header by number (latest): block result not found for height 16461313" module=geth reqid=2

This PR adds the corresponding changes to handle this gracefully and avoid the panic.
As a result, the user will get the following response:

{"jsonrpc":"2.0","id":73,"error":{"code":-32000,"message":"block result not found for height 22. node is not persisting abci responses"}}

Closes #XXX

rpc/backend/call_tx.go Fixed Show fixed Hide fixed
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #1863 (0b75e81) into main (576e2c2) will decrease coverage by 0.02%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1863      +/-   ##
==========================================
- Coverage   70.53%   70.52%   -0.02%     
==========================================
  Files         315      315              
  Lines       23404    23412       +8     
==========================================
+ Hits        16509    16511       +2     
- Misses       6079     6084       +5     
- Partials      816      817       +1     
Files Coverage Δ
rpc/backend/backend.go 76.47% <ø> (ø)
rpc/backend/blocks.go 84.92% <100.00%> (ø)
rpc/backend/chain_info.go 81.28% <100.00%> (-0.11%) ⬇️
rpc/backend/call_tx.go 59.41% <57.14%> (-0.33%) ⬇️
rpc/backend/utils.go 43.09% <40.00%> (-0.41%) ⬇️

rpc/backend/call_tx.go Fixed Show fixed Hide fixed
@github-actions github-actions bot added the tests label Oct 13, 2023
@GAtom22 GAtom22 marked this pull request as ready for review October 13, 2023 21:35
@GAtom22 GAtom22 requested a review from a team as a code owner October 13, 2023 21:35
@GAtom22 GAtom22 requested review from facs95 and 0xstepit and removed request for a team October 13, 2023 21:35
@fedekunze fedekunze merged commit d06c4c8 into main Oct 13, 2023
36 of 38 checks passed
@fedekunze fedekunze deleted the GAtom22/handle-crash branch October 13, 2023 21:57
@Vvaradinov
Copy link
Contributor

Vvaradinov commented Oct 16, 2023

https://github.com/Mergifyio backport release/v15.0.x

@mergify
Copy link
Contributor

mergify bot commented Oct 16, 2023

backport release/v15.0.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 16, 2023
(cherry picked from commit d06c4c8)

# Conflicts:
#	CHANGELOG.md
GAtom22 added a commit that referenced this pull request Oct 16, 2023
…kport #1863) (#1868)

* fix(rpc): handle case when node is not persisting abci responses (#1863)

(cherry picked from commit d06c4c8)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

* (test) fix port to avoid collision

---------

Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: tom <tomasguerraalda@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants