feat (internal/ethapi): implement eth_getBlockReceipts

[kamikazechaser]

Description

Addition of a new ETH RPC method eth_getBlockReceipts.

ethereum/execution-apis#438

Standardized and implemented in several eth clients e.g. geth, nethermind, nimbus, besu e.t.c.

Other changes

None.

Tested

internal/ethapi/api.go is not covered by any tests.

To be tested on a devnet/testnet with existing chaindata.

Related issues

Resolves #2187

Backwards compatibility

Backwards compatible.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 55.76%. Comparing base (f0adb14) to head (d77ad8e).
Report is 26 commits behind head on master.

❗ Current head d77ad8e differs from pull request most recent head 33d4baa. Consider uploading reports for the commit 33d4baa to get more accurate results

Files	Patch %	Lines
internal/ethapi/api.go	0.00%	15 Missing ⚠️
ethclient/ethclient.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2284      +/-   ##
==========================================
+ Coverage   55.06%   55.76%   +0.70%     
==========================================
  Files         684      684              
  Lines      114596    92000   -22596     
==========================================
- Hits        63097    51308   -11789     
+ Misses      47617    36795   -10822     
- Partials     3882     3897      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[palango]

Looks good so far, please add the missing docstring.

[palango]

For future reference, upstream PR: ethereum/go-ethereum#27702

[palango]

@kamikazechaser I had another look and I think we need to add the effective gas price to the receipts as it is done in GetTransactionReceipt, otherwise the receipts will differ.

celo-blockchain/internal/ethapi/api.go

Lines 1693 to 1710 in d77ad8e

	if !s.b.ChainConfig().IsLondon(bigblock) {
	fields["effectiveGasPrice"] = hexutil.Uint64(tx.GasPrice().Uint64())
	} else {
	// var gasPrice *big.Int = new(big.Int)
	if tx.Type() == types.DynamicFeeTxType || tx.Type() == types.CeloDynamicFeeTxType || tx.Type() == types.CeloDynamicFeeTxV2Type {
	header, err := s.b.HeaderByHash(ctx, blockHash)
	if err != nil {
	return nil, err
	}
	gasPriceMinimum, err := s.b.GasPriceMinimumForHeader(ctx, tx.FeeCurrency(), header)
	if err == nil {
	fields["effectiveGasPrice"] = hexutil.Uint64(new(big.Int).Add(gasPriceMinimum, tx.EffectiveGasTipValue(gasPriceMinimum)).Uint64())
	}
	// if err != nil, it's due to a state prune. In this case no effectiveGasPrice will be returned.
	} else {
	fields["effectiveGasPrice"] = hexutil.Uint64(tx.GasPrice().Uint64())
	}
	}

[kamikazechaser]

@palango I have refactored the generateReceiptResponse helper to also accept a context and a backend interface. Tested on devnet.

I have also noticed there is an GetBlockReceipt method that is not a standardized API (added in #1628) that calls into generateReceiptResponse. because the tx passed by this method is nil, no effectiveGasPrice is assigned to that response. I would personally propose to deprecate that method and strictly follow the standardized spec. Anyways let me know what should be done for this case.

[palango]

@palango I have refactored the generateReceiptResponse helper to also accept a context and a backend interface. Tested on devnet.

Thanks, the changes look good. Needs a merge from master, where a new transaction type was added in the meantime, but the conflict is easy to resolve.

I have also noticed there is an GetBlockReceipt method that is not a standardized API (added in #1628) that calls into generateReceiptResponse. because the tx passed by this method is nil, no effectiveGasPrice is assigned to that response. I would personally propose to deprecate that method and strictly follow the standardized spec. Anyways let me know what should be done for this case.

Yes, this is the RPC API to get receipts for celo system transactions. Not having the field added for them is fine.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 55.76%. Comparing base (f0adb14) to head (d77ad8e).
Report is 26 commits behind head on master.

❗ Current head d77ad8e differs from pull request most recent head 9eeeb3d. Consider uploading reports for the commit 9eeeb3d to get more accurate results

Files	Patch %	Lines
internal/ethapi/api.go	0.00%	15 Missing ⚠️
ethclient/ethclient.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2284      +/-   ##
==========================================
+ Coverage   55.06%   55.76%   +0.70%     
==========================================
  Files         684      684              
  Lines      114596    92000   -22596     
==========================================
- Hits        63097    51308   -11789     
+ Misses      47617    36795   -10822     
- Partials     3882     3897      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kamikazechaser]

Not having the field added for them is fine.

Sounds good. I have also added the DenominatedFeeCurrency changes.

[github-actions]

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit 6351612

coverage: 51.1% of statements across all listed packages

coverage:  63.4% of statements in consensus/istanbul
coverage:  43.3% of statements in consensus/istanbul/announce
coverage:  56.0% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  65.6% of statements in consensus/istanbul/core
coverage:  50.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.2% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

[github-actions]