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 nil pointer when block does not exist #31104

Closed
wants to merge 1 commit into from

Conversation

ysk49
Copy link

@ysk49 ysk49 commented Jan 31, 2025

This PR fixes the following thing:

When block does not exist, hash = block.Hash() will cause a panic.

internal/ethapi/api.go

// GetRawHeader retrieves the RLP encoding for a single header.
func (api *DebugAPI) GetRawHeader(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) {
	var hash common.Hash
	if h, ok := blockNrOrHash.Hash(); ok {
		hash = h
	} else {
		block, err := api.b.BlockByNumberOrHash(ctx, blockNrOrHash)
		if err != nil {
			return nil, err
		} 
                hash = block.Hash()
	}
	...
}

@jwasinger
Copy link
Contributor

If the block body doesn't exist, an error will be returned: https://github.com/ethereum/go-ethereum/blob/master/eth/api_backend.go#L178

@s1na
Copy link
Contributor

s1na commented Jan 31, 2025

I guess you are using ethclient for this and don't handle when a block is nil. But in most of the JSON-RPC API if an item does not exist the server returns nil. It is the same e.g. in eth_getBlockByNumber.

@MariusVanDerWijden
Copy link
Member

While I think that this PR is on the wrong track, the issue you're reporting is correct. I would fix it in the following way:

diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go
index d9cec560ea..bdc1fea381 100644
--- a/internal/ethapi/api.go
+++ b/internal/ethapi/api.go
@@ -1711,6 +1711,9 @@ func (api *DebugAPI) GetRawReceipts(ctx context.Context, blockNrOrHash rpc.Block
                if err != nil {
                        return nil, err
                }
+               if block == nil {
+                       return nil, fmt.Errorf("block #%v not found", blockNrOrHash)
+               }
                hash = block.Hash()
        }
        receipts, err := api.b.GetReceipts(ctx, hash)

Please fix all four occurrences in internal/ethapi/api.go

@@ -526,7 +526,12 @@ func (b testBackend) BlockByHash(ctx context.Context, hash common.Hash) (*types.
}
func (b testBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Block, error) {
if blockNr, ok := blockNrOrHash.Number(); ok {
return b.BlockByNumber(ctx, blockNr)
block, err := b.BlockByNumber(ctx, blockNr)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should fix the b.BlockByNumber, by returning the error if the required block is not existent

Copy link
Author

Choose a reason for hiding this comment

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

@rjl493456442
Thank you for reviewing.
I was thinking the same thing, so I'll fix it to what you said.

@ysk49
Copy link
Author

ysk49 commented Feb 3, 2025

The following GetBlockByNumber function is used in several places, but only b.BlockByNumber did not implement error handling when block is nil.

https://github.com/ethereum/go-ethereum/blob/master/eth/api_backend.go#L146

So I think we also should fix the b.BlockByNumber, by returning the error if the required block is not existent.
And I'll remake the branch and PR to do it.

func (bc *core.BlockChain) GetBlockByNumber(number uint64) *types.Block

@ysk49 ysk49 closed this Feb 3, 2025
@ysk49 ysk49 deleted the fix_block_nil_pointer branch February 3, 2025 05:25
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.

6 participants