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

Add additional chain parameters required to configure an Airnode #5

Open
bbenligiray opened this issue Apr 13, 2023 · 8 comments
Open

Comments

@bbenligiray
Copy link
Member

The Airnode config file has many parameters that we set differently between chains, such as ones relating to number of blocks or gas parameters. These are typically based on some chain characteristics (what is the average block time, does the provider return a block base fee, etc.) The chain objects should include both these characteristics and the (recommended) config parameters derived from these.

@bbenligiray
Copy link
Member Author

@RiVal-cz asked about if this package will export chain parameters that can help with Market payments. This issue covers that in that once it exports the block time of the chains, the Market can use a function of that as the minimum confirmations required.

@andreogle
Copy link
Member

I'm happy to address this but need some more info on the fields we want to add and what the expected values should be for each existing chain

@bbenligiray
Copy link
Member Author

The only way I see this issue being addressed is I ask everyone to find out all the personally-kept chain taxonomies spread across multiple environments because they won't come and tell us. Also, these properties should be auto-populated/updated because entering them manually is error-prone (especially important because we're requested to integrate a lot of chains so we want juniors to be able to this correctly) and they change over time.
For example, block time can be estimated by using the provider URL to find the block number from a month ago, and then dividing the difference to find the block time. We can have a Github Action that does this periodically to adapt to drifts in the value.

@andreogle
Copy link
Member

ChainAPI also needs the following values if they exist:

  1. The block history limit
  2. The fulfillment gas limit
  3. The withdrawal remainder in wei

@bbenligiray
Copy link
Member Author

  1. I think you should derive the block history limit from blockTimeMs at your end. For example, Aurora blockTimeMs is 1135, the Airnode coordinator runs every minute, if you want to give the Airnode 10 chances to fulfill a request, you need the request to remain in scope for 10 minutes. 10 * 60 / 1.135 = 529 blocks. The 10 minutes bit is opinionated, which is why it's not appropriate for us to have the number 529 here. One thing to make sure here is for the public provider to support a getLogs call with a 529 window.
  2. Ideally start omitting the fulfillment gas limit in the Airnode config (though it's a v0.12 feature).
  3. Withdrawal remainder seems to strictly be an Optimism thing and there is no correct value for it, so I'm not sure what to do about it. Another point is I'm not sure if @Ashar2shahid is testing the RRP integrations strictly, for example, including withdrawals. I know that we're on at least one Optimism fork (I don't remember which chain it was) so this should also be relevant there.

@Ashar2shahid
Copy link
Contributor

Ashar2shahid commented Sep 1, 2023

  1. @bbenligiray andre asked me the same question and my response was for at least 2 invocations. 10 seems a lot especially considering some l2s have blocktimes of 200ms. Maybe 5 would be more appropriate
  2. Agreed, I omitted it completely
  3. I referred to Chain Idiosyncrasies when creating configs but haven't personally tested withdrawals on each chain only fulfills. will make a note of this for future deployments

@bbenligiray
Copy link
Member Author

bbenligiray commented Sep 1, 2023

Fewer number of invocations is only safe to do with a good gas price strategy. Provider recommend will have fulfillments dropped during congestion.

Apparently the Optimism fork is Mantle, so we don't expect withdrawals to work with no withdrawal remainder defined there for example.

@Ashar2shahid
Copy link
Contributor

I see, makes sense.

Base is also built on the OP stack so it might also be an issue there

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

No branches or pull requests

3 participants