Add API client & function to calculate max gas price #4

andreogle · 2020-06-16T12:24:37Z

In the interests of starting small and keeping PRs easy to review, here's a small change that will get gas prices from 2 APIs and calculate the maximum "fastest" gas price of these.

Added

Added axios for making HTTP requests. Axios is extremely configurable, well maintained and has a promise-like API.
Added a Go style helper for working with promises. This removes the ugliness of having to try/catch everything. You get a tuple that has 2 elements - an error if one was thrown and the result if the function succeeded.
Added a getMaxGasPrice function that makes multiple requests and then gets the maximum gas price from all of the successful responses. Failed responses are discarded.

Currently this will get gas prices from:
a. Etherchain
b. Eth Gas Station
c. (disabled) Honeycomb Gas Price Feed

I see the Honeycomb Gas Price Feed on Ropsten has not been updated in around 2 months so I've left that out for now, but the parts are there to enable it again.

TODO

I'll add tests if you're happy with what I've got so far

packages/node/src/core/index.ts

packages/node/src/core/ethereum/gas-prices.ts

packages/node/src/core/ethereum/contracts/gas-price-feed.ts

packages/node/src/core/ethereum/gas-prices.ts

packages/node/src/core/ethereum/contracts/gas-price-feed.ts

bbenligiray · 2020-06-16T13:18:44Z

Looks great so far. Do you think we would be calling the gas price APIs through the same interface we call the APIs (i.e. external adapter) in the future? Or would these calls go directly to axios as they do now?

andreogle · 2020-06-16T13:32:42Z

I'm not sure. Would that be a single endpoint that the serverless function calls instead of 4-5 APIs directly? Would that be an endpoint we hosted?

Calling these APIs directly is nice because one or more could be down and we would still get an answer. If there was a single endpoint, that could be better down and we would have problems. Calling them directly also seems better for transparency.

bbenligiray · 2020-06-16T13:34:33Z

No I mean if CoinAPI was hosting this node, the node would be calling the CoinAPI API along with these gas price APIs. I'm asking if we would have two classes of calls (the API call and gas price calls) or would they all be treated the same as much as possible?

andreogle · 2020-06-16T14:00:43Z

I'm not really sure yet. I would need to see what the other class of call would look like. I don't really see a problem with having different ways of calling APIs at the moment. They will all share a common interface (in core/clients/http.ts) that would standardize at that level.

I updated the way it calls each API to remove a lot of the duplication. You simply need to declare each API at the top with a URL and onSuccess function that normalizes the response. I like this much more

andreogle · 2020-06-16T14:11:13Z

These are also only the gas prices for mainnet. We'll need to think about what to do for testnets like ropsten

…ce response

… feed

…tract

bbenligiray · 2020-06-16T16:12:59Z

I'm not really sure yet. I would need to see what the other class of call would look like. I don't really see a problem with having different ways of calling APIs at the moment. They will all share a common interface (in core/clients/http.ts) that would standardize at that level.

Okay. I agree that it's early, I was mostly thinking out loud.

I updated the way it calls each API to remove a lot of the duplication. You simply need to declare each API at the top with a URL and onSuccess function that normalizes the response. I like this much more

Yeah that's pretty elegant.

These are also only the gas prices for mainnet. We'll need to think about what to do for testnets like ropsten

I like Chainlink's approach of not giving a fuck and using mainnet gas prices because

There are no Ropsten gas price APIs
Mainnet gas price tends to be higher than Ropsten and wasting gas is not an issue
Even if Ropsten gas price goes higher than mainnet gas price, not fulfilling requests on Ropsten is not an issue
It's good testing grounds for keeping up with mainnet gas prices

If it comes to that, we can always use our on-chain feed to override gas prices, both on mainnet and Ropsten.

bbenligiray · 2020-06-16T16:24:46Z

It also needs a fallback gas price for when none of the sources return anything useful. I think that should be 40 gwei nowadays.

There's also a max gas price of 1000 gwei in the Chainlink version. It used to be 50 gwei (?), but then the gas price actually exceeded that and that wrecked all nodes. 1000 gwei is better than nothing because a gas price API can return a very high price and cause the node wallet to be drained instantly. On the other hand, we don't want to set something that we may want to change later. In that sense, 1000 gwei doesn't sound that bad.

Again, the Chainlink version adds 1 gwei to the gas price but that's pretty useless at the current prices. The extrapolation I added is an adaptive solution, but it requires the gas price history, so we can't use it here. So we can forget about it for now.

In the future, I'm hoping that we're going to be able to send these kind of configuration variables to the node over the chain. You make a transaction on ChainAPI, that emits a configuration event (variables are encoded in CBOR), the node reads the most recent configuration event at the start of each loop to set these.

andreogle · 2020-06-17T09:31:04Z

Thanks. With the current setup, it will get mainnet gas prices from the 4 APIs and the data feed from ropsten and then take the maximum across the successful responses. I'll just leave it at that until it becomes a problem.

I quite like the idea of the node being able to configure itself based on onchain events. We can definitely work towards that eventually.

Max gas price
I definitely think we need a maximum gas price. I'll go with 1000 gwei for now.
Fallback gas price
Something would have to go catastrophically wrong for us to get 0 usable responses back. In any case, it doesn't hurt to have a fallback price. I'll go with 40 gwei for now.
API configuration
I also think we need a way to configure which APIs to use. Say anyblock adds back authentication at some point in the future. The logs would get spammed every minute with an error until we manage to get an update out that would remove it. It would be better to have this configurable somehow although I'm not sure what would be the ideal way to do this.

bbenligiray · 2020-06-17T09:58:48Z

I'm okay with all of those.

I think gas price API configuration is quite doable, though it's probably not needed for now. We can configure that through events too, but updating that by redeploying the node (just as you would update integration specs) sounds more sensible to me.

andreogle · 2020-06-17T10:04:56Z

Alright cool. I'll just make a note about API configuration so we don't forget

andreogle · 2020-06-17T14:15:34Z

I think this is ready to go but waiting for #5 to be merged first

package.json

packages/node/src/core/ethereum/gas-prices.ts

packages/node/src/core/ethereum/index.ts

bbenligiray · 2020-06-20T11:08:51Z

packages/node/src/core/ethereum/index.ts

+  return process.env.ETHEREUM_NETWORK || 'ropsten';
+}
+
+export function getProvider() {


Why not use this pattern here?
https://github.com/clc-group/oracle-monitor/blob/master/src/eth.ts#L5-L9

I want to avoid having any singleton global variables in the node. It looks like they have weird behaviour:

https://medium.com/tensult/aws-lambda-function-issues-with-global-variables-eb5785d4b876

I've been thinking about how best to manage state. I've been thinking of possibly using a simple state object that would store things we don't want to keep initializing repeatedly or passing around individually.

Something like this that would get created on each function call. Each property then gets set once and the state gets passed around to functions lower down:

const state = { provider: null, gasPrice: null, };

Sounds good

I think they are also using that weird behavior as a shitty way to persist state between function invocations
https://engineering.flosports.tv/persistent-data-in-lambda-it-works-ca0c1b25879e

1. Change all gas prices to be denominated in Wei instead of Gwei 2. Add a global state object intended to reduce calls to the Ethereum node and re-use objects 3. Add utils for Wei/Gwei conversions

packages/node/src/core/ethereum/contracts/types.ts

packages/node/src/core/ethereum/gas-prices.ts

bbenligiray · 2020-06-21T12:25:27Z

packages/node/src/core/state.ts

+  // Do this upfront to reduce potential extra calls later on
+  const [err, network] = await go(provider.getNetwork());
+  if (err) {
+    // TODO: not sure how exactly to handle errors here


I was thinking that we could use two providers simultaneously (both to detect requests and make transactions to fulfill them), but Airnode depends on the Ethereum node's version of truth and if we try to use two Ethereum nodes with different versions of truth, we may end up with weird problems. So in a single sleep-wake cycle, the node should decide to use a provider (e.g. the one with the higher blockNumber) and stick with it. Then, it can retry provider calls until the serverless function times out (in 60 seconds total), as we can't really skip any of the provider calls.

But yes, I think all provider calls need a wrapper that handles what provider to use

Yeah I think that sounds like a good idea. I was wondering about how we could control the function time out, but I see serverless provides that for us. Yeah I'll need to implement the ability to retry provider calls in a separate PR

andreogle added 2 commits June 16, 2020 14:07

Add API client & function to calculate max gas price

53e148c

Upper case ABI in Contract type

8a2c7a1

andreogle requested a review from bbenligiray June 16, 2020 12:24

andreogle self-assigned this Jun 16, 2020

andreogle commented Jun 16, 2020

View reviewed changes

bbenligiray reviewed Jun 16, 2020

View reviewed changes

packages/node/src/core/ethereum/gas-prices.ts Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 16, 2020

View reviewed changes

packages/node/src/core/ethereum/gas-prices.ts Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 16, 2020

View reviewed changes

packages/node/src/core/ethereum/contracts/gas-price-feed.ts Outdated Show resolved Hide resolved

andreogle added 2 commits June 16, 2020 15:36

Use fast gas price intead of fastest. Add POA.network and Anyblock feeds

81c595f

Remove duplication and use declarative way of calling gas price APIs

e0975e8

andreogle added 3 commits June 16, 2020 16:21

Add lodash and handle when case when failing to normalize the gas pri…

0223bfb

…ce response

Fix Ropsten gas feed address. Add back code to call the contract data…

97e154e

… feed

Handle errors when attemping to get gas price from gas price feed con…

02a9d8f

…tract

Add maximum gas price and fallback gas price

27ea9f5

andreogle added 6 commits June 17, 2020 12:07

Add TODO about dynamically configuring gas price APIs

5b49365

Merge branch 'master' into poc

107cf31

Add Jest and basic HTTP test

1bc11eb

Test gas-prices module and GasPriceFeed contract

034874b

Add Github actions for linting and tests

8f695fa

Fix npm run test script

07db3ff

andreogle added the don't merge label Jun 17, 2020

andreogle added 5 commits June 18, 2020 13:11

Reduce timeout to 5 seconds

25d9020

Test promise-utils

35d386f

Refactor ethereum related functions

151a98b

Attempt to get gas price from Eth node (Addresses #6)

3825004

Fix typo in test description

d4e9d0b

andreogle removed the don't merge label Jun 19, 2020

andreogle and others added 7 commits June 19, 2020 17:42

Merge branch 'master' into 'poc'

5b994d5

Try use relative path

58c63f5

Move Jest back to each individual package

44b98c1

Merge branch 'poc' of github.com:clc-group/airnode into poc

bd7182a

Use existing build script

3198f65

Add script to watch node tests

1e00166

Move build to package test script

8217d12

bbenligiray reviewed Jun 20, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 20, 2020

View reviewed changes

packages/node/src/core/ethereum/gas-prices.ts Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 20, 2020

View reviewed changes

packages/node/src/core/ethereum/index.ts Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 20, 2020

View reviewed changes

bbenligiray approved these changes Jun 20, 2020

View reviewed changes

andreogle added 2 commits June 20, 2020 15:11

Add stream flag to test script

e5129bd

Refactor to gas-prices and add state

b898da4

1. Change all gas prices to be denominated in Wei instead of Gwei 2. Add a global state object intended to reduce calls to the Ethereum node and re-use objects 3. Add utils for Wei/Gwei conversions

andreogle requested a review from bbenligiray June 21, 2020 11:39

bbenligiray reviewed Jun 21, 2020

View reviewed changes

packages/node/src/core/ethereum/contracts/types.ts Outdated Show resolved Hide resolved

bbenligiray reviewed Jun 21, 2020

View reviewed changes

packages/node/src/core/ethereum/gas-prices.ts Show resolved Hide resolved

bbenligiray reviewed Jun 21, 2020

View reviewed changes

Fix typo

b88677b

andreogle merged commit 3a16e17 into master Jun 22, 2020

andreogle deleted the poc branch June 22, 2020 09:13

andreogle mentioned this pull request Jun 22, 2020

Use the gas price reported by the Ethereum node #6

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API client & function to calculate max gas price #4

Add API client & function to calculate max gas price #4

andreogle commented Jun 16, 2020 •

edited

Loading

bbenligiray commented Jun 16, 2020

andreogle commented Jun 16, 2020

bbenligiray commented Jun 16, 2020 •

edited

Loading

andreogle commented Jun 16, 2020

andreogle commented Jun 16, 2020

bbenligiray commented Jun 16, 2020

bbenligiray commented Jun 16, 2020 •

edited

Loading

andreogle commented Jun 17, 2020

bbenligiray commented Jun 17, 2020

andreogle commented Jun 17, 2020

andreogle commented Jun 17, 2020

bbenligiray Jun 20, 2020

andreogle Jun 20, 2020 •

edited

Loading

bbenligiray Jun 20, 2020

bbenligiray Jun 20, 2020

bbenligiray Jun 21, 2020

bbenligiray Jun 21, 2020

andreogle Jun 21, 2020

Add API client & function to calculate max gas price #4

Add API client & function to calculate max gas price #4

Conversation

andreogle commented Jun 16, 2020 • edited Loading

Added

TODO

bbenligiray commented Jun 16, 2020

andreogle commented Jun 16, 2020

bbenligiray commented Jun 16, 2020 • edited Loading

andreogle commented Jun 16, 2020

andreogle commented Jun 16, 2020

bbenligiray commented Jun 16, 2020

bbenligiray commented Jun 16, 2020 • edited Loading

andreogle commented Jun 17, 2020

bbenligiray commented Jun 17, 2020

andreogle commented Jun 17, 2020

andreogle commented Jun 17, 2020

bbenligiray Jun 20, 2020

Choose a reason for hiding this comment

andreogle Jun 20, 2020 • edited Loading

Choose a reason for hiding this comment

bbenligiray Jun 20, 2020

Choose a reason for hiding this comment

bbenligiray Jun 20, 2020

Choose a reason for hiding this comment

bbenligiray Jun 21, 2020

Choose a reason for hiding this comment

bbenligiray Jun 21, 2020

Choose a reason for hiding this comment

andreogle Jun 21, 2020

Choose a reason for hiding this comment

andreogle commented Jun 16, 2020 •

edited

Loading

bbenligiray commented Jun 16, 2020 •

edited

Loading

bbenligiray commented Jun 16, 2020 •

edited

Loading

andreogle Jun 20, 2020 •

edited

Loading