Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Inconsistent baseFee check logic in code #755

Closed
yihuang opened this issue Nov 16, 2021 · 2 comments · Fixed by #855
Closed

Inconsistent baseFee check logic in code #755

yihuang opened this issue Nov 16, 2021 · 2 comments · Fixed by #855

Comments

@yihuang
Copy link
Contributor

yihuang commented Nov 16, 2021

System info: ethermint main

Following the discussion here: #671 (comment), @fedekunze

Steps to reproduce:

Currently, different places have different logic to check base fee:

Better to encapsulate a common logic to do this.

Expected behavior: consistent way to check base fee.

Actual behavior: inconsistency

Additional info:

@yihuang
Copy link
Contributor Author

yihuang commented Dec 23, 2021

if not london_hardfork {
    # reject DynamicFeeTx
    # no `baseFeePerGas` field in block response
    # baseFee = nil
} else {
    # allow DynamicFeeTx
    # add `baseFeePerGas` field in block response
    if feemarketParams.NoBaseFee or height < feemarketParams.EnableHeight {
        # baseFee = 0
    } else {
        # init baseFee to initBaseFee and adjust in later blocks
    }
}

ref: https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1211

@thomas-nguy
Copy link
Contributor

It looks reasonable to me.

For the case NoBaseFee or height < feemarketParams.EnableHeigh
The cost of the transaction will be only the gas-cap which will be the local value per validator (min-gas-price). In that case we are also consistent with previous behavior when london hard fork is not activated

yihuang added a commit to yihuang/ethermint that referenced this issue Dec 27, 2021
Closes: evmos#755

```
if not london_hardfork {
    # reject DynamicFeeTx
    # no `baseFeePerGas` field in block response
    # baseFee = nil
} else {
    # allow DynamicFeeTx
    # add `baseFeePerGas` field in block response
    if feemarketParams.NoBaseFee or height < feemarketParams.EnableHeight {
        # baseFee = 0
    } else {
        # init baseFee to initBaseFee and adjust in later blocks
    }
}
```

Update x/evm/keeper/keeper.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

add unit tests

Update app/ante/utils_test.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

changelog
fedekunze pushed a commit that referenced this issue Dec 28, 2021
Closes: #755

```
if not london_hardfork {
    # reject DynamicFeeTx
    # no `baseFeePerGas` field in block response
    # baseFee = nil
} else {
    # allow DynamicFeeTx
    # add `baseFeePerGas` field in block response
    if feemarketParams.NoBaseFee or height < feemarketParams.EnableHeight {
        # baseFee = 0
    } else {
        # init baseFee to initBaseFee and adjust in later blocks
    }
}
```

Update x/evm/keeper/keeper.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

add unit tests

Update app/ante/utils_test.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

changelog
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants