oasisprotocol / sapphire-paratime

Oasis Sapphire - the confidential EVM-compatible ParaTime for the Oasis Network
https://oasisprotocol.org/sapphire
Apache License 2.0
37 stars 27 forks source link

Remove DEFAULT_GAS once gas estimation is available #39

Closed nhynes closed 9 months ago

nhynes commented 2 years ago

Description

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

CedarMist commented 1 year ago

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

See: https://github.com/oasisprotocol/sapphire-paratime/blob/2ee5b3868f56134e9b6ec2cdb2a87cd0104de0ce/clients/js/src/compat.ts#L171 https://github.com/oasisprotocol/sapphire-paratime/blob/2ee5b3868f56134e9b6ec2cdb2a87cd0104de0ce/clients/go/compat.go#L231

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.