Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[Light client] NotEnoughBaseGas for RPC without specifying any Gas. #9031

Closed
Tbaut opened this issue Jul 3, 2018 · 5 comments · Fixed by #9824
Closed

[Light client] NotEnoughBaseGas for RPC without specifying any Gas. #9031

Tbaut opened this issue Jul 3, 2018 · 5 comments · Fixed by #9824
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@Tbaut
Copy link
Contributor

Tbaut commented Jul 3, 2018

I'm running:

  • Which Parity version?: full node 1.11.5, now light node v2.1.3
  • Which operating system?: Linux
  • How installed?: binaries
  • Are you fully synchronized?: yes
  • Which network are you connected to?: kovan and mainnet
  • Did you try to restart the node?: yes

Edit:
I could reproduce with Parity Ethereum --light v2.1.3 this call on Kovan and mainner:

curl --data '{"method":"eth_estimateGas","params":[{"value":"0x0","data":"0x","from":"0x00dfc93112abd2578503b667b95491b101281f2b"}],"id":"c31e5d5f401bea24cf8b40340f532cee","jsonrpc":"2.0"}' -H "Content-Type: application/json" -X POST localhost:8545
{"jsonrpc":"2.0","error":{"code":-32015,"message":"Transaction execution error.","data":"NotEnoughBaseGas { required: 53000, got: 50000 }"},"id":"c31e5d5f401bea24cf8b40340f532cee"}

The node with rpc logging:

2018-10-22 20:27:28   TRACE rpc  Request: {"method":"eth_estimateGas","params":[{"value":"0x0","data":"0x","from":"0x00dfc93112abd2578503b667b95491b101281f2b"}],"id":"c31e5d5f401bea24cf8b40340f532cee","jsonrpc":"2.0"}.
2018-10-22 20:27:28   DEBUG rpc  [Some(Str("c31e5d5f401bea24cf8b40340f532cee"))] Took 231ms
2018-10-22 20:27:28   DEBUG rpc  Response: {"jsonrpc":"2.0","error":{"code":-32015,"message":"Transaction execution error.","data":"NotEnoughBaseGas { required: 53000, got: 50000 }"},"id":"c31e5d5f401bea24cf8b40340f532cee"}.

I get the error NotEnoughBaseGas when calling:

$ curl --data '{"method":"eth_estimateGas","params":[{"from":"0x004702bdc..028600"}],"id":1,"jsonrpc":"2.0"}' -H "Content-Type: application/json" -X POST localhost:8545
{"jsonrpc":"2.0","error":{"code":-32015,"message":"Transaction execution error.","data":"NotEnoughBaseGas { required: 0xcf08, got: 0xc350 }"},"id":1}

a workaround is to specify the gas, but the default one should be higher.

@Tbaut Tbaut added P5-sometimesoon 🌲 Issue is worth doing soon. F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. labels Jul 3, 2018
@Tbaut Tbaut added this to the 1.12 milestone Jul 3, 2018
@5chdn 5chdn modified the milestones: 2.0, 2.1 Jul 17, 2018
@Tbaut
Copy link
Contributor Author

Tbaut commented Sep 4, 2018

couldn't reproduce on Kovan and Mainnet on v2.0.1

@Tbaut Tbaut closed this as completed Sep 4, 2018
@Tbaut
Copy link
Contributor Author

Tbaut commented Oct 22, 2018

I could reproduce with Parity Ethereum --light v2.1.3 this call on Kovan and mainnet:

curl --data '{"method":"eth_estimateGas","params":[{"value":"0x0","data":"0x","from":"0x00dfc93112abd2578503b667b95491b101281f2b"}],"id":"c31e5d5f401bea24cf8b40340f532cee","jsonrpc":"2.0"}' -H "Content-Type: application/json" -X POST localhost:8545
{"jsonrpc":"2.0","error":{"code":-32015,"message":"Transaction execution error.","data":"NotEnoughBaseGas { required: 53000, got: 50000 }"},"id":"c31e5d5f401bea24cf8b40340f532cee"}

The node with rpc logging:

2018-10-22 20:27:28   TRACE rpc  Request: {"method":"eth_estimateGas","params":[{"value":"0x0","data":"0x","from":"0x00dfc93112abd2578503b667b95491b101281f2b"}],"id":"c31e5d5f401bea24cf8b40340f532cee","jsonrpc":"2.0"}.
2018-10-22 20:27:28   DEBUG rpc  [Some(Str("c31e5d5f401bea24cf8b40340f532cee"))] Took 231ms
2018-10-22 20:27:28   DEBUG rpc  Response: {"jsonrpc":"2.0","error":{"code":-32015,"message":"Transaction execution error.","data":"NotEnoughBaseGas { required: 53000, got: 50000 }"},"id":"c31e5d5f401bea24cf8b40340f532cee"}.

@Tbaut Tbaut reopened this Oct 22, 2018
@Tbaut Tbaut changed the title NotEnoughBaseGas for RPC without specifying any Gas. [Light client] NotEnoughBaseGas for RPC without specifying any Gas. Oct 22, 2018
@Tbaut Tbaut removed this from the 2.1 Whatever milestone Oct 24, 2018
@niklasad1
Copy link
Collaborator

niklasad1 commented Oct 24, 2018

Yeah, basically we have it hardcoded 50_000 as start gas price if it is not specified, https://github.com/paritytech/parity-ethereum/blob/master/rpc/src/v1/helpers/light_fetch.rs#L197

But, I don't really understand the reason behind it to be honest according to my calculations it should be as follows:

  • G_transaction - 21 000
  • G_create - 32 000
  • G_call - 700

-> The start gas price should at least 53 700

/cc @rphmeier can you confirm if this seems reasonable to you or if I'm wrong?! Thanks

@cheme
Copy link
Contributor

cheme commented Oct 25, 2018

@niklasad1 I also think it would be a good idea to put the initial gas price a bit up if there is like average operation cost being just a bit higher in most common use case scenario (I am not sure of this value). But there is probably another thing.

This code is related to #6155 , from my understanding before #9591 , the gas calculation/estimation for a transaction was running through multiple iterative call to target the right amount (recursive call of the fn and retry with gas * 2 on every failure). I fear that by making the on demand Execute call virtual, this part of the code broke (since it is virtual we no longer have outofgas errors).

I am not sure of what should be done, there should be a reason explaining why we went to such costly gas estimation strategy at the time. But I do not really imagine why we couldn't directly use the gas value returned by the virtual execution (maybe with an additional fixed margin)
Maybe the virtual execution was implemented after this code fn, and then the fix would be to simply use the returned gas cost from the execution response (no more looping, no more initial cost just run it virtual and use the cosumed gas).
And of course it would close issue 6155 since we do not have to consider outofgas error anymor.

Maybe the issue with this approach is that it does not conform to LES execute (maybe this call should fail when there is not enough gas and we should revert to not running virtual for it). It would mean that this not so good looking gas calculation is here because of the specs. In this case putting the initial value a bit higher would probably be a good idea. But I would rather keep execution virtual and use the gas returned value.

@niklasad1
Copy link
Collaborator

@cheme

Thanks, for clarifying those are excellent points and I think you are right. I want to keep these executions virtual. Also, AFAIK these executions doesn't consume any gas so it should probably be safe to bump up the default gas price, but I'm not really sure if it is relevant in this case.

What about the handling ExecutionError::NotEnoughBaseGas in the same way i.e., start and double until it succeeds or reaches the BlockGasLimit just be on_the_safe_side and start from 53 700 and then regard all other things as errors?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
4 participants