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

Check which beacons of a beacon set need to be updated #67

Closed
Siegrift opened this issue Nov 2, 2023 · 3 comments · Fixed by #95
Closed

Check which beacons of a beacon set need to be updated #67

Siegrift opened this issue Nov 2, 2023 · 3 comments · Fixed by #95
Assignees

Comments

@Siegrift
Copy link
Collaborator

Siegrift commented Nov 2, 2023

See the slack thread and a bit older thread.

The old Airseeker implementation always attempts to update all beacons and uses tryMulticall to avoid issues with some updates reverting. This could be prevented by checking which beacons can be updated and updating only those.

Specifically, we could:

  1. Get all on chain values of the data feed beacons
  2. Filter beacons for which we have more up to date signed data (these could be updated)
  3. Check whether updating beacons above would trigger the beacon set update (reuse on-chain value for non-updated beacons)
  4. Do not use dummy data multicall estimation (there is no reason for multi call to revert apart from RPC provider failure).
  5. Maybe not use the fixed 2_000_000 gas limit in case multicall estimation fails?
  6. Use multicall instead of tryMulticall to do dAPI updates
@Siegrift Siegrift changed the title Check which beacons of a data feed need to be updated Consider checking which beacons of a data feed need to be updated Nov 2, 2023
@bbenligiray
Copy link
Member

Anyone can use the signed API to update an individual Beacon so not using tryMulticall() is a bad idea. The issue with omitting individual Beacon updates (while using tryMulticall()) is similar, it provides more freedom to third parties to execute specific combinations of updates. A redundant Beacon update is still useful in disabling all signed data coming before it. Arguably this is still optional as it's not bulletproof.

@bbenligiray
Copy link
Member

I misunderstood, this is about Beacon updates that are guaranteed to revert (barring a block reorg). This is safe to do but we should still use tryMulticall().

@Siegrift Siegrift changed the title Consider checking which beacons of a data feed need to be updated Check which beacons of a data feed need to be updated Nov 7, 2023
@Siegrift
Copy link
Collaborator Author

Siegrift commented Nov 7, 2023

Agreed on call to do first check which beacon updates will revert and omit those. The implementation should even omit the beacon set update (as well as beacon updates) if it detects that the beacon set doesn't need to be updated.

We should still continue using tryMulticall because people can attack us by taking the signed data from mempool and doing a beacon update with higher gas price making their transaction be included before ours and making our multicall revert.

The spec should also be updated.

@aquarat aquarat self-assigned this Nov 7, 2023
@aquarat aquarat changed the title Check which beacons of a data feed need to be updated Check which beacons of a beacon set need to be updated Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants