Adjust BaseFee burning mechanism

[AnnaShaleva]

Please note that the target branch is policy-basefee (#166).

I've reviewed all the modules that become dependent from the node state after #166. It turns out that these modules are created with the full backend and does not work with the light backend anyway, thus it's not such critical to explicitly use the node state there. The only concern is about Oracle service: it didn't have any state (or DB) dependency before these BaseFee changes and it has this dependency now. And this service has an ability to process batch of blocks, which makes it extremely resource-consuming to retrieve state for every potential block where BaseFee change might have happen.

@txhsl, @chenquanyu, @nicolegys ready for review. We'll need tests for t8n tool and, which is more important, for Oracle service.

[AnnaShaleva]

Ready for review, the PR comment is updated, please, take a look.

[AnnaShaleva]

@nicolegys, could you please test this branch? Especially the Oracle service with his fee history call.

[chenquanyu]

@nicolegys, could you please test this branch? Especially the Oracle service with his fee history call.

@songb2 This is our new tester.

[AnnaShaleva]

I just realized that unit-tests are failing, mostly due to the suitable mocks absence. I'll fix the UTs, but the code itself is ready for testing, @songb2.

[songb2]

I've tested the following apis by changing baseFee and minGasTipCap in policy and also made some transactions, the reponses changes as expected:

1. eth_maxPriorityFeePerGas
2. eth_gasPrice
3. eth_feeHistory

[AnnaShaleva]

@txhsl, @chenquanyu, please check out 6 latest commits, I haven't touched the previously reviewed changes because they are the target of an audit. Tests are fixed, but the implementation is also adjusted, I found a couple of bugs.

@songb2, could you please check that the updated branch works OK with Metamask and it's possible for Metamask to generate and properly submit transaction to the network if BaseFee and minGasTipCap values are changed in Policy. Also, we need a test for t8ntool: create an Env configuration that includes NeoXBurn fork and some currentBaseFee value set in the Env config. Try to execute some transaction with this Env t8ntool configuration and check whether the BaseFee limit is properly checked against the pre-configured value.

Otherwise ready to be merged.

[songb2]

@AnnaShaleva @chenquanyu @txhsl
Still in testing, but I tested metamask and also some eth apis and found the following problems, please help confirm if they are issues:

1. go to Edit gas fee page, Low, Market, and Aggressive options don't show numbers
   
2. By default basefee and minGasTipCap are all 1 gwei, so the eth_maxPriorityFeePerGas is also 1 gwei, and eth_gasPrice is 2 gwei.
   When I change minGasTipCap to 0.5 gwei, but eth_maxPriorityFeePerGas is still 1 gwei (I think it should change to 0.5 gwei), and eth_gasPrice is still 2 gwei (I think it should be 1.5 gwei)

condition	baseFee	minGasTipGap	eth_maxPriorityFeePerGas	eth_gasPrice
default value in policy	1	1	1	2
decrease minGasTipGap to 0.5	1	0.5	1	2

And the issue can also refected on metamask ui, the Priority Fee should be 0.5 gwei, Max base fee should be 1.5 gwei.

[txhsl]

1. go to Edit gas fee page, Low, Market, and Aggressive options don't show numbers

I believe this is because our network ID is not supported by their gas API. Metamask doesn't fetch these data from RPC, it sends request to https://gas.api.cx.metamask.io/networks/{networkid}/suggestedGasFees.

[AnnaShaleva]

I believe this is because our network ID is not supported by their gas API.

I initially was sure that it was something wrong on our side, but if it depends on network ID, then there's nothing we can do about it.

When I change minGasTipCap to 0.5 gwei, but eth_maxPriorityFeePerGas is still 1 gwei (I think it should change to 0.5 gwei), and eth_gasPrice is still 2 gwei (I think it should be 1.5 gwei)

About this behaviour: to calculate eth_maxPriorityFeePerGas we use maximum from minGasTipCap and GAS price:

go-ethereum/eth/api_backend.go

Lines 366 to 372 in 47d2712

	func (b *EthAPIBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
	suggestTipCap, minGasTipCap, err := b.gpo.SuggestTipCap(ctx)
	if err != nil {
	return nil, err
	}
	return cmath.BigMax(suggestTipCap, minGasTipCap), nil
	}

So I believe that if GAS price is still 1 gwei, then eth_maxPriorityFeePerGas returns the proper value.

And regarding eth_gasPrice: to calculate it we use suggested TipCap added to the latest block's BaseFee:

go-ethereum/internal/ethapi/api.go

Lines 77 to 85 in 47d2712

	func (s *EthereumAPI) GasPrice(ctx context.Context) (*hexutil.Big, error) {
	tipcap, err := s.b.SuggestGasTipCap(ctx)
	if err != nil {
	return nil, err
	}
	if head := s.b.CurrentHeader(); head.BaseFee != nil {
	tipcap.Add(tipcap, head.BaseFee)
	}
	return (*hexutil.Big)(tipcap), err

where suggested TipCap is a maximum from minGasTipCap and GAS price. I believe block's BaseFee is 1, and GAS price is still 1, so the described behaviour works according to the code. The question is: should we calculate suggested TipCap as a maximum from minGasTipCap and GAS price? @chenquanyu, @txhsl, what do you think?

[songb2]

@AnnaShaleva @chenquanyu @txhsl
For the tn8tool, I try to set neoXBurnTime to cmd/evm/testdata/29/env.json, but it doesn't take effect, seems that it wasn't read by config.go. I print some logs in IsNeoXBurn, it shows that the field is nil, as as result, it return false,.
func isTimestampForked(s *uint64, head uint64) bool {
if s == nil {
return false
}
return *s <= head
}
I use the command to test ../../build/bin/evm t8n --input.alloc=./testdata/29/alloc.json --input.txs=./testdata/29/txs.json --input.env=./testdata/29/env.json --state.fork=Shanghai

[AnnaShaleva]

but it doesn't take effect, seems that it wasn't read by config.go

Will take a look at it, probably more modifications are needed to make t8n tool work with NeoXBurn.

[AnnaShaleva]

but it doesn't take effect, seems that it wasn't read by config.go

@songb2, turns out that fork should be specified via state.fork CLI flag, not via t8ntool env config. Please, check the last commit, I've extended the list of valid hardforks recognizable by t8ntool, so you have to specify --state.fork=NeoXBurn, it works like Shanghai+burn-related changes. Please, check this issue one more time and see how it works:

go-ethereum/tests/state_test_util.go

Line 141 in ba2663c

func GetChainConfig(forkString string) (baseConfig *params.ChainConfig, eips []int, err error) {

[chenquanyu]

The question is: should we calculate suggested TipCap as a maximum from minGasTipCap and GAS price? @chenquanyu, @txhsl, what do you think?

It looks like GAS price comes from the 60% quantile of the historical block gasTip list. If we use the maximum value of minGasTipCap and GAS price, then if we reduce minGasTipCap in Policy, suggestTipCap will always remain the same. So I suggest we just use minGasTipCap as suggestTipCap.

[AnnaShaleva]

So I suggest we just use minGasTipCap as suggestTipCap.

@chenquanyu, @txhsl, implemented, check out the last commit please.

@songb2 we need to test this version with Metamask.

Additional changes implemented.

[AnnaShaleva]

I'm merging this PR into policy-basefee so that it's possible to update the base branch. However, final test with Metamask is still needed, we're not going to release without it. Thus, @songb2, please, write the test results to #166. Any further discussions are moved to the original PR.