eth_coinbase impl

[sambukowski]

Pending block still not implemented so using Latest. - closes #1871

[gakonst]

I think we may wanna move this to the Consensus Engine that will eventually also control the builder? #1845

[codecov-commenter]

Codecov Report

Merging #1896 (a1726da) into main (f6a420f) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1896      +/-   ##
==========================================
- Coverage   73.12%   73.07%   -0.06%     
==========================================
  Files         418      418              
  Lines       51187    51189       +2     
==========================================
- Hits        37433    37408      -25     
- Misses      13754    13781      +27     

Flag	Coverage Δ
integration-tests	19.61% <100.00%> (+<0.01%)	⬆️
unit-tests	67.30% <0.00%> (-0.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
crates/rpc/rpc/src/eth/api/server.rs	98.46% <100.00%> (+<0.01%)	⬆️

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[sambukowski]

@gakonst would it also make sense to put the impls for eth_gasPrice and eth_maxPriorityFeePerGas into the Consensus Engine as well?

[mattsse]

eth_coinbase is only relevant in block building and is a per node setting so we can't use the latest state.

I'm not sure how this would be implemented,

ref previous attempt to implement this:
#1597 (comment)

this handler is usually also disabled, eg:

cast rpc eth_coinbase --rpc-url "https://eth-mainnet.alchemyapi.io/v2/Lc7oIGYeL_QvInzI0Wiu_pOZZDEKBrdf"

[mattsse]

@gakonst would it also make sense to put the impls for eth_gasPrice and eth_maxPriorityFeePerGas into the Consensus Engine as well?

we still need impls for these endpoints.

we need some kind of oracle that does some estimation re gas price, worth checking out geth's impl of this, because we should mirror how this is calculated.

[gakonst]

we need some kind of oracle that does some estimation re gas price, worth checking out geth's impl of this, because we should mirror how this is calculated.

Geth's gas price oracle logic is here: https://github.com/ethereum/go-ethereum/blob/b3f43c89b3b884d5d0b8e1a239847f54a291a19b/eth/gasprice/gasprice.go#L144-L223

[gakonst]

@Rjected @mattsse re: building you may wanna take this over?

[mattsse]

I'm closing this because coinbase is now a CL setting and sent to the EL via the suggestedFeeRecipient field on a per payload basis which is used as coinbase.

geth supports setting a coinbase address that is returned for this endpoint but no longer used.