api3dao / airseeker

A service powering the dAPIs
MIT License
2 stars 5 forks source link

Async beaconset beacons update #343

Closed acenolaza closed 1 month ago

acenolaza commented 2 months ago

Closes https://github.com/api3dao/airseeker-v2/issues/329

What does this change

This PR tries to enable updating individual beacons of a beacon set data feed that does not require an update.

  1. Most relevant changes are in getUpdatableFeeds(). This feature can be enabled by setting a value in config file. This value represents the deviation of individual beacons. A new boolean is returned in each UpdatableDataFeed object indicating if Api3ServerV1.updateBeaconSetWithBeacons() calldata needs to be added to multicall.
  2. For this type of updates I've made an "executive decision" of not setting anything since this is really an edge case and I don't see the value of retrying with higher gas price, etc. Airseeker will print a warning tho when this happens.
  3. Used that flag from 1 in createUpdateFeedCalldatas() to skip updating the beacon set
bdrhn9 commented 2 months ago

Can you wait me before merge? I'm planning to review tomorrow.

acenolaza commented 1 month ago

Rather than extending UpdatableDataFeed with shouldUpdateBeaconSet, we may define isAsyncBeaconUpdate.

I don't have a strong opinion about which one is better. @Siegrift thoughts?

Siegrift commented 1 month ago

I don't have a strong opinion about which one is better. @Siegrift thoughts?

I don't particularly like the shouldUpdateBeaconSet but I would need to see how the code would look like with isAsyncBeaconUpdate. The problem I have with that field is that you also need to populate it for single beacon feeds, for which the field makes no sense (and you have to have an additional check in the if statement before changing the pending tx info).

acenolaza commented 1 month ago

Thank you both for the valuable feedback 🙏🏻 I'm going to merge this now to be able to have this functionality ready. We can continue the discussion to refactor the shouldUpdateBeaconSet flag on a different issue