Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove DEFAULT_GAS once gas estimation is available #39

Closed
nhynes opened this issue Sep 6, 2022 · 4 comments
Closed

Remove DEFAULT_GAS once gas estimation is available #39

nhynes opened this issue Sep 6, 2022 · 4 comments
Assignees
Labels
go Pull requests that update Go code p:1 Priority: high

Comments

@nhynes
Copy link
Contributor

nhynes commented Sep 6, 2022

Description

We would like to use eth_estimateGas when possible and return the non-hardcoded amount.

@CedarMist
Copy link
Member

Currently the JS and Go compat APIs return a hard-coded amount when asked to estimate gas.

See:

estimateGas: () => DEFAULT_GAS,

// EstimateGas implements ContractTransactor.

However, the implementation of eth_estimateGas in oasis-web3-gateway seems to have a full implementation.

See:
https://github.com/oasisprotocol/oasis-web3-gateway/blob/2fa2ad9f5170260324db51daca4638d346e43c91/rpc/eth/api.go#L483

Need to double check gas estimation works as expected, then re-enable it.

@kostko
Copy link
Member

kostko commented Jun 19, 2023

Yes, this is now supported, although for gas estimation will currently always zeroize the sender in order to avoid it being used to bypass access control and potentially exposing secret data through a side channel. The problem is that estimate gas queries are not signed, so you cannot authenticate the caller.

For this reason, when execution fails during gas estimation, the estimate will currently be artifically inflated to account for the fact that access control checks are usually in the front (as otherwise the estimate would be an underestimate most of the time). This is not ideal and needs some more thought.

@aefhm aefhm self-assigned this Jul 6, 2023
@aefhm aefhm added p:1 Priority: high go Pull requests that update Go code js labels Jul 6, 2023
@aefhm
Copy link
Contributor

aefhm commented Sep 5, 2023

For this reason, when execution fails during gas estimation, the estimate will currently be artifically inflated to account for the fact that access control checks are usually in the front (as otherwise the estimate would be an underestimate most of the time). This is not ideal and needs some more thought.

I vote this is an okay present day solution.

@CedarMist
Copy link
Member

If you're not using signed queries, and passing explicit authentication to the function (e.g. an EIP-712 signature, username/password, WebAuthN attestation) this will not be a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code p:1 Priority: high
Projects
None yet
Development

No branches or pull requests

4 participants