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

Problem: user can specify a excessive tx gas limit to occupy block space without paying the price #989

Closed
yihuang opened this issue Mar 14, 2022 · 1 comment · Fixed by #991

Comments

@yihuang
Copy link
Contributor

yihuang commented Mar 14, 2022

System info: ethermint main

Steps to reproduce:

After a recent gas meter fix, tendermint will include tx to block according to the sum of tx gas limits, but we'll refund the unused gas for EVM transaction, which means one can occupy block space with low cost by specifying an excessive tx gas limit.

Expected behavior: shouldn't be vulnerable to dos attack

Actual behavior: [What actually happened]

Additional info: [Include gist of relevant config, logs, etc.]

@yihuang
Copy link
Contributor Author

yihuang commented Mar 15, 2022

Compare solutions

Non-breaking

Change the gasWanted returned in check tx.

  1. Execute the tx in check tx mode, and return the gasUsed.
    • Performance degradation
  2. 0 this is what we have before the gas meter fix, can mitigate the issue by limiting the mempool size. (or using a smaller block size limit, need an upgrade).
  3. Return some other constant value to limit the number of transaction in block to: block gas limit / constant gasWanted.

Breaking

Change the gas refund rules.

  1. Don't refund unused gas at all, same as cosmos-sdk
    • hurt UX.
  2. Refund a percentage of it, Refunding unused but allocated gas cosmos/cosmos-sdk#2150

Long term solution

PrepareProposal in abci++
We can run the tx in the PrepareProposal phase, and truncate the preliminary transaction list according to the gas used.

yihuang added a commit to yihuang/ethermint that referenced this issue Mar 16, 2022
fedekunze added a commit that referenced this issue Mar 16, 2022
yihuang added a commit to yihuang/ethermint that referenced this issue Mar 22, 2022
yihuang added a commit to yihuang/ethermint that referenced this issue Mar 24, 2022
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.

1 participant