Skip to content

Fix panics on non-existing block RPC requests #254

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
Merged
merged 1 commit into from
Mar 3, 2022
Merged

Fix panics on non-existing block RPC requests #254

merged 1 commit into from
Mar 3, 2022

Conversation

thaarok
Copy link
Contributor

@thaarok thaarok commented Feb 21, 2022

Fix conditions in ethapi to fix logged panics like this:

ERROR[02-21|13:22:13.963] RPC method eth_getTransactionByHash crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 211568 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
 	/home/opera/go/pkg/mod/github.com/!fantom-foundation/[email protected]/rpc/service.go:200 +0x89
 panic({0x13d2760, 0x21cbe90})
 	/usr/local/go/src/runtime/panic.go:1038 +0x215
 github.com/Fantom-foundation/go-opera/ethapi.(*PublicTransactionPoolAPI).GetTransactionByHash(0xc00e050540, {0x18465f8, 0xc0155001c0}, {0x6c, 0x71, 0x94, 0x68, 0xfc, 0xf5, 0xd1, ...})
	/home/opera/go/src/pebble-go-opera/ethapi/api.go:1553 +0xcd

@thaarok thaarok requested a review from andrecronje as a code owner February 21, 2022 15:34
uprendis
uprendis reviewed Feb 21, 2022
View reviewed changes
ethapi/api.go
@@ -998,7 +998,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
// Recap the highest gas limit with account's available balance.
if feeCap.BitLen() != 0 {
state, _, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
if err != nil {
if state == nil || err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

StateAndHeaderByNumberOrHash returns an error if not found, but it's okay to double check

hadv
hadv reviewed Feb 22, 2022
View reviewed changes
@thaarok
Copy link
Contributor Author

thaarok commented Mar 2, 2022 •
edited
Loading

@uprendis @cyberbono3 @hadv Hello, I checked Contribution Guidelines - since this PR has review from more than two developers and I have no permission to merge it, can I ask you, reviewers, to merge this?

You may merge the Pull Request in once you have the sign-off of two other developers, or if you do not have permission to do that, you may request the second reviewer to merge it for you.

@uprendis uprendis merged commit ed551c4 into Fantom-foundation:develop Mar 3, 2022
@thaarok thaarok deleted the fix-getTransactionByHash-nil branch March 3, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hadv hadv hadv approved these changes

@cyberbono3 cyberbono3 cyberbono3 approved these changes

@uprendis uprendis uprendis approved these changes

@andrecronje andrecronje Awaiting requested review from andrecronje andrecronje is a code owner

@rus-alex rus-alex Awaiting requested review from rus-alex

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thaarok @hadv @cyberbono3 @uprendis