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

Address balance listener worker #819

Closed
wants to merge 8 commits into from
Closed

Conversation

nischitpra
Copy link
Contributor

@nischitpra nischitpra commented Dec 16, 2024

PR-Codex overview

This PR introduces a new config column in the webhooks and configuration tables, enhances the webhook creation process to include configuration details, and implements an addressBalanceListener that triggers webhooks based on address balance thresholds.

Detailed summary

  • Added config column to webhooks and configuration tables.
  • Updated get-configuration.ts to initialize addressBalanceListenerCronSchedule.
  • Modified toWebhookSchema to include config.
  • Enhanced createWebhookRoute to handle config.
  • Implemented addressBalanceListener to track address balance changes.
  • Developed trackAddressBalance to enqueue webhooks based on balance thresholds.
  • Created e2e tests for the address balance listener functionality.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@nischitpra nischitpra enabled auto-merge (squash) December 17, 2024 00:24
@@ -236,6 +236,10 @@ export const getConfiguration = async (): Promise<ParsedConfig> => {
config = await updateConfiguration({
indexerListenerCronSchedule: "*/5 * * * * *",
});
} else if (!config.addressBalanceListenerCronSchedule) {
config = await updateConfiguration({
addressBalanceListenerCronSchedule: "0 */5 * * * *",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is 5 minutes a good poll interval?

@d4mr d4mr self-requested a review December 25, 2024 23:28
Copy link
Member

@d4mr d4mr left a comment

Choose a reason for hiding this comment

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

In the current implementation, in order to create a balance listener, a user creates a webhook with a config (newly added field), where a user can specify options like address, chainId and threshold).

The webhooks table is mostly intended to store URLs that engine should call, and business logic/config pertaining to when and why should live elsewhere.

We utilise webhooks today for contract subscription, where the flow is to first create a webhook, and then setting up a subscription (this is where the business logic/config resides, like which contract to listen to, which events to index etc).

A similar pattern could be used here. We create a new balance-subscriptions table and endpoint. Logic/config related to the subscription should reside here (we can even expand later beyond this original scope, adding things like tracking erc20 balances). To make it general, we could have the threshold subscription be an optional configuration, default behaviour being we report any balance changes. IMO config should not be inside the webhook table.

@nischitpra
Copy link
Contributor Author

The main idea of having config in webhook table it to prevent creating multiple tables. Each webhook already has a type which can be used to perform specific tasks and should they require additional configurability, the config column helps specify it. It is an optional jsonb so basicaly the content can be anything so works for future webhooks as well.

I had a discussing with @arcoraven before doing it this way as I wasn't sure if I should create a new table just for the config or just add it to the existing table.

Copy link

github-actions bot commented Jan 8, 2025

This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this PR will be closed in 3 days.

@github-actions github-actions bot closed this Jan 11, 2025
auto-merge was automatically disabled January 11, 2025 00:57

Pull request was closed

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

Successfully merging this pull request may close these issues.

2 participants