Conversation
|
We're building your pull request over on Zeet. |
| client: thirdwebClient, | ||
| }); | ||
|
|
||
| currentBalance = await balanceOf({ |
There was a problem hiding this comment.
wait what is contract here? this is to get a erc20 balance, that's not what you want here
There was a problem hiding this comment.
Yes you can subscribe to either erc20 balance or native balance changes
- Add support for creating new webhooks with optional labels - Allow using existing webhooks by ID - Validate webhook event type and revocation status - Improve error handling for webhook creation and selection
- Replace manual balance fetching with `getWalletBalance` method - Support both native token and ERC20 token balance retrieval - Remove redundant contract and RPC client initialization code
…ions - Update Prisma schema, migration, and database indexes - Modify TypeScript interfaces and schemas across services and routes - Ensure consistent naming for token-related address fields - Update worker and database interaction methods
| description: "Webhook URL to create a new webhook", | ||
| examples: ["https://example.com/webhook"], | ||
| }), | ||
| webhookLabel: Type.Optional( |
There was a problem hiding this comment.
Nit: can make this non-optional
| threshold: Type.Optional( | ||
| Type.Object({ | ||
| min: Type.Optional(Type.String({ | ||
| description: "Minimum balance threshold", |
There was a problem hiding this comment.
If we want the dev to provide wei, we should explicitly make that clear and remind them the token's decimals matter. E.g. 0.1 ETH and 0.1 USDC will have different values.
I'm debating if tokens ("0.1") is more intuitive and less error prone (to get decimals wrong). Also precision doesn't matter because it's just a threshold anyway.
src/prisma/schema.prisma
Outdated
| cursorDelaySeconds Int @default(2) @map("cursorDelaySeconds") | ||
| contractSubscriptionsRetryDelaySeconds String @default("10") @map("contractSubscriptionsRetryDelaySeconds") | ||
|
|
||
| balanceSubscriptionsCronSchedule String? @map("balanceSubscriptionsCronSchedule") |
There was a problem hiding this comment.
Expose this in some config update route?
| /** | ||
| * Array of conditions to monitor for this wallet | ||
| */ | ||
| conditions: Array<({ |
There was a problem hiding this comment.
Make a reusable type for this?
| validateConditions, | ||
| } from "../../shared/schemas/wallet-subscription-conditions"; | ||
|
|
||
| interface WalletSubscriptionWithWebhook { |
There was a problem hiding this comment.
We shouldn't need to define a different interface right? Shouldn't we be passing the type
import type { WalletSubscription } from "@prisma/client" around?
Or if we intend to pass the type joined with Webhook, then something like:
type WalletSubscriptionWithWebhook =
WalletSubscriptions & {
webhook: Webhooks
}
| import type { WalletCondition } from "../../shared/schemas/wallet-subscription-conditions"; | ||
| import type { Webhooks } from "@prisma/client"; | ||
|
|
||
| interface WalletSubscriptionWithWebhook { |
There was a problem hiding this comment.
Same comment, we shouldn't need to define a new interface
| chain, | ||
| }); | ||
|
|
||
| if (currentValue && subscription.webhookId && subscription.webhook) { |
There was a problem hiding this comment.
Shouldn't webhook always be set already?
We should assert(subscription.webhook) above then and not do any work if there's no valid webhook to even receive the request (e.g. deleted webhook).
| } | ||
| } catch (error) { | ||
| // Log error but continue processing other subscriptions | ||
| const message = error instanceof Error ? error.message : String(error); |
There was a problem hiding this comment.
Use prettifyError(error)
| const subscriptions = await getAllWalletSubscriptions({ | ||
| page: 1, | ||
| limit: 1000, // Process 1000 subscriptions at a time | ||
| }); |
There was a problem hiding this comment.
Don't put this limit here because Engine allows creating > 1k subscriptions, and any more are silently ignored.
Instead just enforce this limit on the "create wallet subscription" endpoint: If > 1k non-deleted subscriptions, don't allow creating any more.
| default: { | ||
| // For TypeScript exhaustiveness check | ||
| const _exhaustiveCheck: never = condition; | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
You shouldn't need this if you type condition.type properly. Typescript knows theres no unhandled cases.
I see WalletCondition already should have type: 'token_balance_lt' | 'token_balance_gt' because it infers from a Zod schema that enforces this. Does typescript error if you remove this default case?
| condition: WalletCondition; | ||
| walletAddress: string; | ||
| chain: Chain; | ||
| }): Promise<string | undefined> { |
There was a problem hiding this comment.
Nit: null seems more correct than undefined here, as an explicit "I checked and there is no value to return".
3 participants
PR-Codex overview
This PR introduces a
WalletSubscriptionsfeature, allowing users to create, update, retrieve, and delete wallet subscriptions based on specified conditions. It also integrates a worker to process these subscriptions and trigger webhooks when conditions are met.Detailed summary
WalletSubscriptionsServicefor managing wallet subscriptions.add,update,get-all, anddelete.WalletSubscriptionWorker) to process subscriptions and trigger webhooks.wallet_subscriptions.WALLET_SUBSCRIPTION.