eth_estimateGas CIP-64 and CIP-66 compatibility
#91
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
from
8ce472c to
38f8f25
Compare
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
from
38f8f25 to
d8eb5a6
Compare
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
6 times, most recently
from
d33f580 to
8a7081a
Compare
ezdac
changed the title
eth_estimateGas CIP-64 and CIP-66 compatibility
ezdac
commented
May 2, 2024
There was a problem hiding this comment.
The diff here looks a bit confusing - basically I just moved the CeloAPI from the celoapi/backend.go to the celoapi/api.go file, and then added the new CeloBackend implementation in the celoapi/backend.go file.
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
from
8a7081a to
2833a73
Compare
This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token.
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
from
2833a73 to
545e43e
Compare
karlb
reviewed
May 14, 2024
karlb
reviewed
May 14, 2024
Closed
ezdac
force-pushed
the
ezdac/fix-estimate-gas
branch
from
dc37292 to
947fc1c
Compare
karlb
approved these changes
May 14, 2024
karlb
pushed a commit
that referenced
this pull request
May 15, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
May 15, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
May 15, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
palango
pushed a commit
that referenced
this pull request
May 31, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
carterqw2
pushed a commit
that referenced
this pull request
Jun 11, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Jul 10, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Jul 10, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Jul 12, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Aug 20, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Aug 26, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Aug 30, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
pushed a commit
that referenced
this pull request
Oct 11, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
added a commit
that referenced
this pull request
Dec 13, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
added a commit
that referenced
this pull request
Dec 16, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
karlb
added a commit
that referenced
this pull request
Dec 17, 2024
* rpc: include feeCurrency in transaction-args This commit fixes wrong gas calculation in `eth_estimateGas` calls, when the additional `feeCurrency` parameter is used. The TransactionArgs struct is used in transaction related endpoints like `eth_sendTransaction`, `eth_signTransaction`, `eth_estimateGas` and many more. CIP-64 and CIP-66 transaction types make use of an additional transaction parameter `feeCurrency` and some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshaling and the value was never passed to the EVM. Now the TransactionArgs struct includes an optional FeeCurrency field for correct unmarshaling, and the field is passed along downstream when constructing EVM messages out of the struct. This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token. * Rename celoapi file * Add Backend wrapper for Celo functionality * Make transaction-args CIP-64/66 compatible * Make eth_estimateGas CIP64 and CIP66 compatible * Move error message inside function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the issue?
This fixes wrong gas calculation in
eth_estimateGascalls, when the additionalfeeCurrencyparameter is used.The TransactionArgs struct is used in transaction related endpoints like
eth_sendTransaction,eth_signTransaction,eth_estimateGasand many more.CIP-64 and CIP-66 transaction types make use of an additional transaction parameter
feeCurrencyand some client libraries are already sending this in the RPC request body, however the remote procedures omitted this during unmarshalling and the value was never passed to the EVM.Additionally, the
TransactionArgsto EVMMessageconversion was not compatible with fee-currency conversion yet (see #109) and currency-conversion of internal checks against e.g. thebase-feewere missing. Similarly, the binary-search-like gas-estimation algorithm did not consider the fee currency conversion during determining upper-bounds of the search-space.What this PR implements
ethapi.Backendwrapperethapi.CeloBackendis implemented that extends the original with some Celo-specific methods. The initial implementation (celoapi.Backend) of this wrapped backend has a LRU cache for exchange rates. It is now passed to some RPC services instead of the unwrapped backend.FeeCurrencyfield for correct unmarshalling, and the field is passed along downstream when constructing EVM messages out of the struct.This e.g. allows gas estimation to consider the different intrinsic gas for transactions paid in non-native token.
MaxFeeInFeeCurrencyand this field is thus required in order to be converted internally to a CIP-66 transactionThe binary search algorithm inDoEstimatenow simulates CIP-64/66 transactions once without any gas-payment deduction, in order to determine the available fee-currency balance for capping the search-space. This is analogous to the EIP1559/legacy transactions, but since the transactions fee-currency "value" isn't encoded directly as a transaction field (Value), the full transaction has to be simulated to observe the fee currencies' contract state after applying the transaction.