Skip to content
This repository has been archived by the owner on Jun 3, 2022. It is now read-only.

Out of gas im improvements #1

Open
3 tasks
frozeman opened this issue Aug 8, 2016 · 14 comments
Open
3 tasks

Out of gas im improvements #1

frozeman opened this issue Aug 8, 2016 · 14 comments
Labels

Comments

@frozeman
Copy link

frozeman commented Aug 8, 2016

This issue is a reminder and discussion thread for the following improvements:

  • eth_call should return an RPC error, if the call throws in the EVM (Another option would be to return null instead of 0x, as of right now)
  • eth_estimateGas should return an RPC error if the all the gas was used
  • eth_getTransactionReceipt should contain a outOfGas property, so that devs can see if a tx threw.

Error code EIP ethereum/EIPs#136

@frozeman
Copy link
Author

We should implement this ASAP @bas-vk @chriseth @debris

@debris
Copy link

debris commented Oct 25, 2016

cc @tomusdrw @jacogr

@axic
Copy link
Member

axic commented Oct 25, 2016

Agree with eth_call and eth_estimateGas.

@kumavis what do you think, which makes more sense, null or RPC error?

@frozeman
Copy link
Author

frozeman commented Oct 25, 2016

Linking the custom error codes we would need to implement for that ethereum/EIPs#136 (Discussion still ongoing)

@kumavis
Copy link
Member

kumavis commented Oct 25, 2016

eth_getTransactionReceipt should contain an outOfGas property, so that devs can see if a tx threw.

A TransactionReceipt is an ethereum obj that is part of the consensus. I don't think we should decorate that with extra information. Though, on the other hand, when we lookup a tx, it includes the block reference, which is not part of the normal tx object.

{ eth_call, eth_estimateGas } should return an RPC error

agree

@frozeman
Copy link
Author

frozeman commented Nov 9, 2016

The receipt returned over RPC is not necessarily the same as the one send over the p2p network.
As its mainly used by dapps to deal with the network, we should add things which are basic to understand what happened in this network.

Though i agree we shouldn't decorate it with a lot of unrelated info.

@frozeman
Copy link
Author

@bas-vk @obscuren is the outOfGas property already in the receipt?

@kumavis
Copy link
Member

kumavis commented Jan 27, 2017

@frozeman
canonical txReceipt is:

  • post-tx stateRoot
  • gasUsed
  • tx log bloom bitvector
  • logs

https://github.com/ethereumjs/ethereumjs-vm/blob/master/lib/runBlock.js#L117-L122

@frozeman
Copy link
Author

Sure but the RPC endpoint is not necessarily the cananonical receipt but instead a tool for Dapp devs to understand their tx execution and properly understand the e execution without much hassle.

@kumavis
Copy link
Member

kumavis commented Jan 28, 2017

@bas-vk @obscuren is the outOfGas property already in the receipt?

@frozeman oh did you mean the eth_getTransactionReceipt RPC result?
Top-level OOG is not included in the RPC result.

response from parity it looks like this:

├─ jsonrpc: 2.0
├─ result
│  ├─ blockHash: 0x37bdc90cc344d9e0f1abc1e7630ce097f8a751cecbfac4d375780fe365045370
│  ├─ blockNumber: 0x2ed304
│  ├─ contractAddress
│  ├─ cumulativeGasUsed: 0x159f5
│  ├─ gasUsed: 0xc807
│  ├─ logs
│  │  └─ 0
│  │     ├─ address: 0x2aa6222e3f4304d25ad73906832912ef0b3d3dda
│  │     ├─ blockHash: 0x37bdc90cc344d9e0f1abc1e7630ce097f8a751cecbfac4d375780fe365045370
│  │     ├─ blockNumber: 0x2ed304
│  │     ├─ data: 0x00000000000000000000000070ad465e0bab6504002ad58c744ed89c7da3852400000000000000000000000000000000000000000000000000b1a2bc2ec5000000000000000000000000000004cb1ade2bd72acd758d4f7abacbc633fc53285700000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000
│  │     ├─ logIndex: 0x0
│  │     ├─ topics
│  │     │  └─ 0: 0x92ca3a80853e6663fa31fa10b99225f18d4902939b4c53a9caae9043f6efd004
│  │     ├─ transactionHash: 0x5c25b138dda6b02b67c31b61256f609934c74b6c2e62e01defc58325c2625e61
│  │     ├─ transactionIndex: 0x1
│  │     └─ type: mined
│  ├─ logsBloom: 0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000020000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000040000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000
│  ├─ root: 0x005a674291dabc7ab4c060a77fc57d5835e8ca0a7a74429333faf98f8aebb100
│  ├─ transactionHash: 0x5c25b138dda6b02b67c31b61256f609934c74b6c2e62e01defc58325c2625e61
│  └─ transactionIndex: 0x1
└─ id: 120

@frozeman
Copy link
Author

this is the interface repo. So yes it is about the rpc :)
Thanks for the parity response.
The proposal issue here is a request to add it.

@Arkadiy @debris can you guys add this?

@debris
Copy link

debris commented Jan 30, 2017

@frozeman I haven't been recently working on rpc... I think it's more sensible to ask @jacogr or @tomusdrw ;)

@kumavis
Copy link
Member

kumavis commented Jan 31, 2017

For better OOG detection when using eth_call and eth_estimateGas
see #8

@bas-vk
Copy link
Member

bas-vk commented Feb 2, 2017

eth_estimateGas should return an RPC error if the all the gas was used

To clarify, users can optionally specify gas as an argument. This method should return an error when the user has specified gas and the gas amount isn't sufficient? If the user has not specified the gas amount geth uses the pending block gas limit as a limit. Should this method also return an OOG error if it runs out of gas in that case?

eth_getTransactionReceipt should contain a outOfGas property, so that devs can see if a tx threw.

There are more reasons a transaction can throw. Out of gas is one of them. In the simulateTransaction proposal a didError indication. Maybe that is what you are looking for? Problem is that it doesn't give a context what went wrong and users must do a trace.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants