fix nil pointer when block does not exist

[ysk49]

This PR fixes "nil pointer error occurs when block is nil".

The following GetBlockByNumber function is used in several places, but only in func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Block, error) at eth/api_backend.go 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.

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

quote: #31104 (comment)

[ysk49]

block, err := api.backend.BlockByNumber(ctx, number), returning err if block is nil.
Therefore, this handling is unnecessary and has been removed.
https://github.com/ethereum/go-ethereum/pull/31116/files#diff-bd288cd51c1874c7e65ce0d35047c0a98366479c8924606c9efc577f6a98f75cR113

[ysk49]

api.b.BlockByNumber(ctx, rpc.BlockNumber(number)) returns an error if block is nil,
so if block == nil {...} handling becomes unnecessary.

[s1na]

As expressed in the last PR, I disagree with this change as of now. The spec requires us to return nil for not found objects: https://ethereum.github.io/execution-apis/api-documentation/

So if there is a panic, please check the block == nil and return nil, nil in the API.

[MariusVanDerWijden]

Seems like @rjl493456442 @s1na and I have different opinions on how to do this change, but we all agree that it should be fixed. We should discuss on triage

[ysk49]

@s1na
Thank you for your comment.
I also understand and agree with your opinion that nil should be returned if the block does not exist, in accordance with the JSON-RPC API specification.

Even if you adopt my modification, I think it will return nil if the block does not exist.
As you mention, looking in GetBlockReceipts() in internal/ethapi/api.go, I see the following statement, which returns nil if the block does not exist. https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L596

If if block == nil is needed in other places where api.b.BlockByNumberOrHash(ctx, blockNrOrHash) is used, I'll modify it.

if block == nil || err != nil {
	// When the block doesn't exist, the RPC method should return JSON null
	// as per specification.
	return nil, nil
}

[s1na]

As you mention, looking in GetBlockReceipts() in internal/ethapi/api.go, I see the following statement, which returns nil if the block does not exist.

Right but as the PR stands some methods like GetBlockByNumber will return an error.

[MariusVanDerWijden]

Please define this as an error, so it can be caught and handled

[lightclient]

Not sure if I agree with returning error. I think my preference would be to fix any place where we don't check for nil and continue returning nil when the block isn't found.

[Moameri55]

Very good thanks