Skip to content
This repository was archived by the owner on Jul 9, 2024. It is now read-only.

Filter empty sponsors #400

Merged
merged 4 commits into from
May 24, 2023
Merged

Filter empty sponsors #400

merged 4 commits into from
May 24, 2023

Conversation

bdrhn9
Copy link
Contributor

@bdrhn9 bdrhn9 commented May 21, 2023

Closes #309

I implemented this feature differently than what was suggested in the related issue. The reason behind this choice is primarily to optimize memory usage and expedite the overall process. The suggested implementation would require to keep all non-funded sponsors in memory, necessitating an iteration over them to filter out the unfunded sponsors using the isFunded attribute in each update cycle. My implementation, in contrast, is quite straightforward. It identifies funded sponsors at the beginning of the program, updates the config.triggers.dataFeedUpdates state to include only those funded sponsors, and allows Airseeker to perform its tasks as usual. Tho, if suggested approach makes more sense to you, please let me know. I'll quickly switch over.

Additionally, to leverage Promise.any, I updated version of ECMAScript from 2020 to 2021.

@bdrhn9 bdrhn9 self-assigned this May 21, 2023
@bdrhn9 bdrhn9 marked this pull request as ready for review May 21, 2023 22:46
@bdrhn9 bdrhn9 requested review from aquarat and acenolaza May 21, 2023 22:47
@acenolaza
Copy link
Contributor

acenolaza commented May 23, 2023

So if I understood this correctly:

  1. Burak suggested checking the balance of the sponsor wallets and store them in memory when iterating over the triggers from config.
  2. Bedirhan suggested checking the balance of all sponsor wallets before iterating over the triggers and filter out all triggers for which the sponsor wallet has a balance of zero.

I think both approaches are valid and at the end of the day, both require the same amount of RPC calls (which is what concerns me the most). I guess option #2 might be slightly easier to merge with currently being tested multicall branch. Also option #2 might make it easier to batch all rpc calls if that's ever supported by ethers.js (or by calling a contract that checks the balances of multiple EOA).

@bbenligiray
Copy link
Member

2 also has the advantage of having been tested for a while now

option api3dao/airseeker#2 might make it easier to batch all rpc calls if that's ever supported by ethers.js (or by calling a contract that checks the balances of multiple EOA)

Good point, Api3ServerV1 inherits ExtendedSelfMulticall to enable that. I created api3dao/airseeker#406

@bdrhn9
Copy link
Contributor Author

bdrhn9 commented May 24, 2023

So we are going with the solution 2? If so, this PR is surely ready to review. Thank you both for your comments :)

return { sponsorAddress, chainId, isEmpty };
};

export const filterEmptySponsors = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea but shouldn't we merge this function into initializeSponsorWallets since we are already iterating over the sponsor list from triggers? This way we will also avoid deriving the private key for wallets that might not be going to be used.

Copy link
Contributor Author

@bdrhn9 bdrhn9 May 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it'd be great but we need to use these private keys while deriving sponsor wallet addresses here. So initializeSponsorWallets process is needed to be done before filtering. But we can also prune state.sponsorWalletsPrivateKey here.

Copy link
Contributor

@acenolaza acenolaza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the minor change suggested here I think this LGTM 🔥

@bdrhn9 bdrhn9 merged commit 98aced0 into main May 24, 2023
@bdrhn9 bdrhn9 deleted the filter-empty-sponsors branch May 24, 2023 20:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check sponsor wallet balances initially and omit the ones that are not funded
3 participants