Skip to content
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

gasPrice for Foreign chain transactions should be requested through gasprice.poa.network #16

Closed
rstormsf opened this issue Feb 13, 2018 · 13 comments
Assignees
Labels

Comments

@rstormsf
Copy link
Contributor

rstormsf commented Feb 13, 2018

As an example, ICO wizard makes an API call to https://gasprice.poa.network/, get the fastest or lowest(@igorbarinov )
IF service is not available, use hardcoded value.
I'd recommend using standard value from https://gasprice.poa.network/

Should be done on Rust side

https://gasprice.poa.network/

@akolotov please approve

@akolotov
Copy link
Contributor

:) it was in my todo of issues to create for the projects.

Since personally I don't like the idea of yet another service to be requested, I thought about an oracle which will put gas price into the bridge contract. My rationales are:

  • we need two different prices: one for POA and another for Foundation;
  • we are tying with POA in the core of the bridge without strict necessity.

My suggestion are

  1. create public fields gasPrice in both contracts HomeBridge and ForeignBridge;
  2. initialize gasPrice as part of deployment of contracts.

This will be Phase I

Later in Phase II we will:

  1. provide updateGasPrice method in both contracts to set new gasPrice with some consensus;
  2. develop a tool or the bridge subsystem which will call updateGasPrice periodically.

@rstormsf if you agree let's change the label to solidity and modify the title to reflect the idea of the changing.

@akolotov
Copy link
Contributor

@rstormsf @igorbarinov did you discuss it? should we have a call together to discuss it?

@igorbarinov
Copy link
Member

We have gasprice.poa.network on support and monitoring. It's as good as a supported onchain oracle. I'd like to stick with it with an option to hardcode gasprice if that host is not available. I suggest having 21Gwei as a hardcoded parameter.

@igorbarinov
Copy link
Member

@phahulin could you create a public dashboard in site24x7 for gasprice oracle
for host health and API health

@banteg thank you for the code https://github.com/banteg/gasprice

@igorbarinov
Copy link
Member

Nice, could we have outage number? duration?
disk space also will be good to know

@igorbarinov
Copy link
Member

Pashi, nice dashboard! But gasprice oracle is out of sync 🙈😬

  • please add logging for gasprice and parity service
  • add corresponding checks on log files

@akolotov
Copy link
Contributor

OK. Here is my suggestion:

  1. we introduce public fields gasPrice in both contracts HomeBridge and ForeignBridge;
  2. gasPrice will be initialized as part of deployment of contracts:
    • it equals 1 gwei in HomeBridge
    • it equals 21 gwei in ForeignBridge
  3. In order to perform deposit_relay or withdraw_confirm the bridge will request http://gasprice.poa.network and if it is not available gasPrice of ForeignBridge will be requested.
  4. In order to perform withdraw_relay, gasPrice of HomeBridge will be requested.

@akolotov akolotov changed the title (Feature) Estimate Gas Price on rust side in order to make a tx to ForeignBridge gasPrice for Foreign chain transactions should be requested through gasprice.poa.network Feb 20, 2018
@akolotov akolotov added the to do label Feb 20, 2018
@priom priom self-assigned this Mar 12, 2018
@priom priom added in progress and removed to do labels Mar 19, 2018
@priom priom added to do and removed in progress labels Apr 5, 2018
@rstormsf
Copy link
Contributor Author

rstormsf commented May 1, 2018

Bridge should read from config.toml

gas_price_oracle_url = "https://gasprice.poa.network/"
gas_price_speed_type = "fast"

and receive this value in gwei, convert to hex value in wei for signing tx on foreign.
this value could be cached only for short period of time ( 5-10min)

@akolotov
Copy link
Contributor

akolotov commented May 5, 2018

Here is the recent requirements based on discussions:

  1. We do provide options in [home] and [foreign] of configuration file to allow configure a JSON source of gas price:
gas_price_oracle_url = "https://gasprice.poa.network/"
gas_price_speed_type = "fast"
gas_price_update_timeout = 600
  1. We expect that the gas price oracle will return information in the following form:
    {"block_number":5559089,"block_time":14.824,"instant":50.0,"standard":4.0,"slow":2.0,"fast":5.0,"health":true}
  2. We add the parameter default_gas_price in [home]and[foreign]`
  3. We remove gas_price from [transactions] for deposit_relay, withdraw_confirm and withdraw_relay.

The logic is as follows:

  1. Try to get gas price from the oracle for the corresponding network. It needs to be done on regular basis with usage of gas_price_update_timeout.
  2. If the oracle is not accessible or returns incorrect information, the default gas price (default_gas_price) is used for the corresponding network.

@DrPeterVanNostrand
Copy link
Contributor

DrPeterVanNostrand commented May 9, 2018

In the JSON returned from GET https://gasprice.poa.network:

{
  "block_number": 5584026,
  "block_time": 14.307,
  "instant": 41,
  "standard": 5,
  "slow": 4,
  "fast": 6,
  "health": true
}

does anyone know what does the "health" key-value pair means? Does health: true indicate that the gas prices values returned from the API are trustworthy, while health: false indicates otherwise? If that is the case, then we should use the default gas price in the cases of:

  1. health: false
  2. request duration to gas_price_oracle_url has exceeded gas_price_update_timeout?

@banteg
Copy link

banteg commented May 9, 2018

health: false means the node is not fully synced and the numbers are off.

@DrPeterVanNostrand
Copy link
Contributor

Ok, cool. Thanks!

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

No branches or pull requests

7 participants