search

ethereum-optimism / op-geth

GNU Lesser General Public License v3.0
254 stars 652 forks source link

support estimate gas for blob tx #299

Closed zhiqiangxu closed 2 months ago

zhiqiangxu commented 2 months ago

Changes for TransactionArgs are pulled from this pr.

Currently when batcher estimates gas, it doesn't pass parameters for blobs.

I'm going to create a pr for that after this one is merged since it refers to the CallMsg type from op-geth.

ajsutton commented 2 months ago

Is there a reason we need to port this individually instead of just waiting for the next upstream release to be merged in (there's work to get us up to date happening now)?

zhiqiangxu commented 2 months ago

Is there a reason we need to port this individually instead of just waiting for the next upstream release to be merged in (there's work to get us up to date happening now)?

Currently the batcher is already sending blob tx to L1, but when it estimates the gas, it's not estimating the exact CallMsg, it's not a problem when the batch inbox is an EOA, but it may become a problem when the batch inbox is a contract(i.e, for customized deployment of OP Stack).

The exact CallMsg is like this.

ajsutton commented 2 months ago

The batch inbox must not be a contract (it should be an arbitrary address with no code and no key so its completely unowned). So it sounds like this isn't an issue and we can just pull the fix in with the other upstream changes.

zhiqiangxu commented 2 months ago

The batch inbox must not be a contract (it should be an arbitrary address with no code and no key so its completely unowned). So it sounds like this isn't an issue and we can just pull the fix in with the other upstream changes.

When integrating OP Stack with EthStorage(a long term DA solution), the batch inbox is a contract, here are more details.

tynes commented 2 months ago

@zhiqiangxu Appreciate the changes but there are no tests around them. This is adding extra work for us, based on what @ajsutton is saying these exact changes are in the upstream geth release and we will get to them as part of pulling in the latest geth changes. We do this on a regular basis

zhiqiangxu commented 2 months ago

Never mind, we'll host the changes downstream then:)