-
Notifications
You must be signed in to change notification settings - Fork 388
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
Create eth_call.md #55
Conversation
DockBoss
commented
Aug 12, 2021
- Geth
- OpenEthereum
- Nethermind
- Besu
- Erigon
- [ ] Geth - [ ] OpenEthereum - [ ] Nethermind - [ ] Besu - [ ] Erigon
cc @MicahZoltu @timbeiko @marcindsobczak @gnidan @rekmarks @rakitadragan @vorot93 This is a PR for adding edgecases for the proposed method. In particular, an edgecase is defined as any behavior outside of the OpenRPC specification defined in this repository. This needs approval from all 5 clients before it can be merged. This is one method in a slew soon to come. |
src/description/eth_call.md
Outdated
|
||
* **MUST** accept a `from` account that does not exist on-chain and if `from` is not defined it **MUST** be set to `0x0000000000000000000000000000000000000000` | ||
|
||
* If the `to` is null or not defined on-chain it **MUST** return an empty hex string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is this true? Can contract creations not set returndata
?
src/description/eth_call.md
Outdated
|
||
* If the `to` is null or not defined on-chain it **MUST** return an empty hex string | ||
|
||
* If `max_fee_per_gas` or `max_priority_fee_per_gas` is set the other must be set manually, otherwise they both **MUST** be set to the `gasPrice` or 0 when `gasPrice` is null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the first part of this can be captured by the OpenRPC specification, and thus we shouldn't repeat it here. If it isn't yet captured by the OpenRPC specification then we should fix that (I thought I already did, but maybe not).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is incorrect to say that these should be "set to the gasPrice
". If the user is trying to simulate a legacy transaction, then these should not be set and only gasPrice
should be set.
src/description/eth_call.md
Outdated
@@ -0,0 +1,15 @@ | |||
* **MUST NOT** affect on-chain state | |||
|
|||
* **MUST** accept a `from` account that does not exist on-chain and if `from` is not defined it **MUST** be set to `0x0000000000000000000000000000000000000000` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of the "set" terminology here. Consider instead "MUST be interpreted as ..." instead? The receiver isn't "setting" anything, they are reading/interpreting incoming data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to read this sentence several times, it first looked to me like eth-call must accept only from-addresses that do not exist.
src/description/eth_call.md
Outdated
|
||
* If `max_fee_per_gas` or `max_priority_fee_per_gas` is set the other must be set manually, otherwise they both **MUST** be set to the `gasPrice` or 0 when `gasPrice` is null | ||
|
||
* **MUST** accept `gas` that is greater than or equal to the minimum price for the current transaction call and less than `2^64 - 1` or `0xffffffffffffffff` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be captured by the specification, so shouldn't be included as an edge case. Also, it is undecided whether this is capped at 2^64-1
or 2^53
(there are two open competing EIPs). Also, "price" is not the right word to use here. I think you mean "cost", but gas cost prior to execution is unknown so that cannot be validated by the client.
src/description/eth_call.md
Outdated
|
||
* **MUST** accept `gas` that is greater than or equal to the minimum price for the current transaction call and less than `2^64 - 1` or `0xffffffffffffffff` | ||
|
||
* **MUST** consider gas to equal 0 if the `gas` parameter is equal to `null` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want this. Instead I think what we want is to specify that either the behavior is undefined, or that the call is executed with XXX gas, or that the call is executed with infinite gas. I don't know what each client does, but I suspect they all do something different here (some I think even may use block gas limit).
Saying that the call should be executed with 0 gas just means "the call should fail with out of gas immediately" which is unlikely the desired behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I do not think this is the behaviour. I think the the clients use some specific number for the gas limit in such cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I found for geth it sets the gas to 80000000, I do not know for the other clients.
src/description/eth_call.md
Outdated
|
||
* If any fee or `value` is a non-zero value your account balance **MUST** be checked to ensure account has enough funds | ||
|
||
* **MUST** accept an optional input parameter `data` to interact with contract methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be expressed in the specification, so no need for an edge case.
Co-authored-by: Micah Zoltu <micah@zoltu.net>
@OhArthits do you represent a client? |
Probably just spam. |
It is a spam account. Please report to GitHub. |
src/description/eth_call.md
Outdated
|
||
* **MUST** consider gas to equal 0 if the `gas` parameter is equal to `null` | ||
|
||
* If any non-zero fee or `value` is provided the `from` account balance **MUST** be checked to ensure account has enough funds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if not enough balance on sender account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call will not go through. It will give an insufficient funds error.
@@ -0,0 +1,11 @@ | |||
* **MUST NOT** affect on-chain state | |||
|
|||
* **MUST** allow a `from` parameter that does not exist on-chain and if `from` is not defined it **MUST** be interpreted as `0x0000000000000000000000000000000000000000` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the from account is not an EOA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playing around with different from addresses I found that as long as it a 20 byte hex string the behavior is the same.