⚡ Batch database updates and RPC calls in reconciliation#18
Conversation
Optimized promoteRecipientFinality, promoteEscrowFinality, promotePayoutClaimFinality, promoteFeeShareUpdateFinality, and promotePartnerClaimFinality to use bulk updates and batch RPC calls. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors several finality promotion functions in the reconciliation logic to use batch database updates and optimized RPC calls for signature verification. The feedback identifies potential stalling risks in multiple functions where rows with empty or missing signatures could be skipped indefinitely, and provides suggestions to ensure these rows are correctly processed. Additionally, optimizations for signature deduplication and reducing redundant function calls were recommended.
| const finalized = await finalizedSignatures( | ||
| rows.map((row) => row.txSignature).filter((sig): sig is string => Boolean(sig)), | ||
| rows | ||
| .map((row) => row.txSignature) | ||
| .filter((sig): sig is string => Boolean(sig)), | ||
| ); | ||
| let count = 0; | ||
| for (const row of rows) { | ||
| if (!row.txSignature || !finalized.has(row.txSignature)) continue; | ||
| const idsToUpdate = rows | ||
| .filter((row) => row.txSignature && finalized.has(row.txSignature)) | ||
| .map((row) => row.id); |
There was a problem hiding this comment.
To optimize the RPC calls, it's better to deduplicate the signatures before calling finalizedSignatures. Additionally, there is a potential stalling risk: if a row has an empty string as a signature (which passes the isNotNull DB check), it will be filtered out here and never updated, causing the reconciliation to fetch the same row repeatedly in future runs. Removing the explicit truthiness check on the signature and relying on the fact that an empty signature is vacuously "finalized" (or at least doesn't need checking) prevents this stall.
const signatures = Array.from(
new Set(
rows
.map((row) => row.txSignature)
.filter((sig): sig is string => Boolean(sig)),
),
);
const finalized = await finalizedSignatures(signatures);
const idsToUpdate = rows
.filter((row) => !row.txSignature || finalized.has(row.txSignature))
.map((row) => row.id);| const allSignatures = Array.from( | ||
| new Set(rows.flatMap((row) => splitSignatures(row.claimSignature))), | ||
| ); | ||
| if (allSignatures.length === 0) return 0; | ||
|
|
||
| const finalized = await finalizedSignatures(allSignatures); | ||
| const idsToUpdate = rows | ||
| .filter((row) => { | ||
| const sigs = splitSignatures(row.claimSignature); | ||
| return sigs.length > 0 && sigs.every((sig) => finalized.has(sig)); | ||
| }) | ||
| .map((row) => row.id); |
There was a problem hiding this comment.
This implementation calls splitSignatures twice per row. We can optimize this by pre-calculating the signatures. Also, the sigs.length > 0 check introduces a stalling risk: if a row has no valid signatures (e.g., an empty string or just commas), it will never be updated and will block the reconciliation batch indefinitely. Removing this check allows rows with no signatures to be marked as finalized (as they have no pending transactions to verify), which is safer for the system's progress.
const rowsWithSigs = rows.map((row) => ({
id: row.id,
sigs: splitSignatures(row.claimSignature),
}));
const allSignatures = Array.from(
new Set(rowsWithSigs.flatMap((r) => r.sigs)),
);
const finalized = await finalizedSignatures(allSignatures);
const idsToUpdate = rowsWithSigs
.filter((r) => r.sigs.every((sig) => finalized.has(sig)))
.map((r) => r.id);| const allSignatures = Array.from(new Set(rows.flatMap((row) => row.signatures))); | ||
| if (allSignatures.length === 0) return 0; | ||
|
|
||
| const finalized = await finalizedSignatures(allSignatures); | ||
| const idsToUpdate = rows | ||
| .filter((row) => { | ||
| return ( | ||
| row.signatures.length > 0 && | ||
| row.signatures.every((sig) => finalized.has(sig)) | ||
| ); | ||
| }) | ||
| .map((row) => row.id); |
There was a problem hiding this comment.
The row.signatures.length > 0 check can cause the reconciliation to stall if a 'succeeded' attempt somehow has an empty signatures array. Since every on an empty array returns true, removing the length check allows these rows to be finalized and cleared from the queue.
const allSignatures = Array.from(new Set(rows.flatMap((row) => row.signatures)));
const finalized = await finalizedSignatures(allSignatures);
const idsToUpdate = rows
.filter((row) => row.signatures.every((sig) => finalized.has(sig)))
.map((row) => row.id);| const allSignatures = Array.from(new Set(rows.flatMap((row) => row.signatures))); | ||
| if (allSignatures.length === 0) return 0; | ||
|
|
||
| const finalized = await finalizedSignatures(allSignatures); | ||
| const idsToUpdate = rows | ||
| .filter((row) => { | ||
| return ( | ||
| row.signatures.length > 0 && | ||
| row.signatures.every((sig) => finalized.has(sig)) | ||
| ); | ||
| }) | ||
| .map((row) => row.id); |
There was a problem hiding this comment.
Removing the signatures.length > 0 check ensures that rows with empty signature arrays don't stall the reconciliation process.
const allSignatures = Array.from(new Set(rows.flatMap((row) => row.signatures)));
const finalized = await finalizedSignatures(allSignatures);
const idsToUpdate = rows
.filter((row) => row.signatures.every((sig) => finalized.has(sig)))
.map((row) => row.id);
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 57 minutes and 40 seconds.Comment |
1 participant
💡 What: Optimized the payout reconciliation logic by refactoring individual database updates and Solana RPC calls into batched operations.
🎯 Why: The previous implementation suffered from an N+1 query problem, executing one database update per record and redundant Solana RPC calls in some loops.
📊 Measured Improvement:
inArraybulk updates.promotePayoutClaimFinality,promoteFeeShareUpdateFinality, andpromotePartnerClaimFinality.PR created automatically by Jules for task 14492505066499778169 started by @Dexploarer