updated project files - #517
Conversation
changes made
|
@Toyinoje is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Toyinoje Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough
ChangesPledge Reconcile Idempotency
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/src/services/campaignStore.ts (1)
847-882: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHandle only the transaction-hash conflict
INSERT OR IGNOREalso hides NOT NULL violations onpledges(the FK still fails normally), so thechanges === 0path can misclassify bad pledge data asDB_CONFLICT. UseON CONFLICT(transaction_hash) DO NOTHINGor catch the unique-constraint error instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/services/campaignStore.ts` around lines 847 - 882, The pledge insert logic in campaignStore’s transaction currently uses INSERT OR IGNORE, which can mask non-transaction-hash validation issues and incorrectly fall through to DB_CONFLICT. Update the insertedNewPledge flow to handle only the transaction_hash uniqueness case in the db.transaction block, either by switching the INSERT in that pledge insertion path to ON CONFLICT(transaction_hash) DO NOTHING or by catching the unique-constraint error explicitly, and keep the existing getPledgeByTransactionHash / duplicatePledge campaignId conflict handling for true transaction-hash collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/services/db.ts`:
- Around line 163-185: The cleanup and campaign recalculation in the migration
logic are running as separate exec calls, so a failure can leave the database
partially migrated. Update the migration path in initDb()/migrate(db) so the
DELETE FROM pledges, UPDATE campaigns, and the unique-index creation all execute
inside a single transaction, using the existing database object and the
migration flow in db.ts to keep the whole change atomic.
- Around line 174-185: The migration in db.ts only deduplicates pledges and
updates campaigns, but leaves duplicate campaign_events rows with the same
blockchain_metadata.txHash intact. Update the migration to also clean duplicates
in campaign_events using the relevant txHash field, or explicitly document in
the migration and history flow that duplicate pledge history is intentionally
retained. Focus on the existing migration block around the campaigns update and
the campaign_events history path that feeds the endpoint.
- Around line 163-172: The deduplication query in the pledges cleanup is
preserving the wrong row because it always uses MIN(id), which can keep a
refunded legacy record instead of the active pledge. Update the deletion logic
in the database cleanup flow that runs the DELETE FROM pledges statement so the
survivor for each transaction_hash group is chosen from a non-refunded row when
any exist, and only falls back to a refunded row if all rows for that hash are
refunded. Make the change in the db service around the pledge deduplication
query so rebuilt pledged totals keep valid funds.
---
Nitpick comments:
In `@backend/src/services/campaignStore.ts`:
- Around line 847-882: The pledge insert logic in campaignStore’s transaction
currently uses INSERT OR IGNORE, which can mask non-transaction-hash validation
issues and incorrectly fall through to DB_CONFLICT. Update the insertedNewPledge
flow to handle only the transaction_hash uniqueness case in the db.transaction
block, either by switching the INSERT in that pledge insertion path to ON
CONFLICT(transaction_hash) DO NOTHING or by catching the unique-constraint error
explicitly, and keep the existing getPledgeByTransactionHash / duplicatePledge
campaignId conflict handling for true transaction-hash collisions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89207b55-8e3d-4afc-9070-51ac013706a2
📒 Files selected for processing (6)
backend/src/index.tsbackend/src/services/__tests__/mutation.test.tsbackend/src/services/campaignStore.test.tsbackend/src/services/campaignStore.tsbackend/src/services/db.tsbackend/tests/integration.test.ts
| database.exec(` | ||
| DELETE FROM pledges | ||
| WHERE id NOT IN ( | ||
| SELECT MIN(id) | ||
| FROM pledges | ||
| WHERE transaction_hash IS NOT NULL | ||
| GROUP BY transaction_hash | ||
| ) | ||
| AND transaction_hash IS NOT NULL; | ||
| `); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the migration and nearby transaction usage.
sed -n '130,220p' backend/src/services/db.ts
echo
echo '--- related transaction_hash usage ---'
rg -n "transaction_hash|txHash|pledged_amount|refund" backend/src -g '!**/node_modules/**'Repository: ritik4ever/stellar-goal-vault
Length of output: 22122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- db.ts around migration wrapper ---'
sed -n '1,140p' backend/src/services/db.ts
echo
sed -n '140,230p' backend/src/services/db.ts
echo
echo '--- campaignStore transaction_hash insert/update paths ---'
sed -n '700,780p' backend/src/services/campaignStore.ts
echo
sed -n '820,920p' backend/src/services/campaignStore.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 11861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "getPledgeByTransactionHash|transactionHash already belongs|INSERT OR IGNORE INTO pledges|transaction_hash =" backend/src/services/campaignStore.ts
echo
sed -n '180,240p' backend/src/services/campaignStore.ts
echo
sed -n '780,860p' backend/src/services/campaignStore.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 5440
Keep the active pledge when deduplicating transaction_hash.
MIN(id) can preserve a refunded legacy row and delete the active pledge, which makes the pledged total rebuild drop valid funds. Pick a non-refunded row as the survivor when a hash group mixes refund states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/services/db.ts` around lines 163 - 172, The deduplication query
in the pledges cleanup is preserving the wrong row because it always uses
MIN(id), which can keep a refunded legacy record instead of the active pledge.
Update the deletion logic in the database cleanup flow that runs the DELETE FROM
pledges statement so the survivor for each transaction_hash group is chosen from
a non-refunded row when any exist, and only falls back to a refunded row if all
rows for that hash are refunded. Make the change in the db service around the
pledge deduplication query so rebuilt pledged totals keep valid funds.
| database.exec(` | ||
| DELETE FROM pledges | ||
| WHERE id NOT IN ( | ||
| SELECT MIN(id) | ||
| FROM pledges | ||
| WHERE transaction_hash IS NOT NULL | ||
| GROUP BY transaction_hash | ||
| ) | ||
| AND transaction_hash IS NOT NULL; | ||
| `); | ||
|
|
||
| database.exec(` | ||
| UPDATE campaigns | ||
| SET pledged_amount = COALESCE( | ||
| ( | ||
| SELECT SUM(amount) | ||
| FROM pledges | ||
| WHERE pledges.campaign_id = campaigns.id | ||
| AND refunded_at IS NULL | ||
| ), | ||
| 0 | ||
| ); | ||
| `); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect current migration transaction patterns without executing repository code.
rg -n -C4 'function migrate|database\.transaction|BEGIN|COMMIT|DELETE FROM pledges|CREATE UNIQUE INDEX IF NOT EXISTS idx_pledges_transaction_hash' backend/src/services/db.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 902
🏁 Script executed:
#!/bin/bash
# Inspect the migrate() function and its call sites for any surrounding transaction.
sed -n '1,260p' backend/src/services/db.ts
printf '\n--- call sites ---\n'
rg -n -C3 '\bmigrate\(' backend/srcRepository: ritik4ever/stellar-goal-vault
Length of output: 7242
Wrap the cleanup, recalculation, and unique-index creation in one migration transaction. initDb() calls migrate(db) directly, so these database.exec(...) steps run separately; a failure between them can leave the database half-migrated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/services/db.ts` around lines 163 - 185, The cleanup and campaign
recalculation in the migration logic are running as separate exec calls, so a
failure can leave the database partially migrated. Update the migration path in
initDb()/migrate(db) so the DELETE FROM pledges, UPDATE campaigns, and the
unique-index creation all execute inside a single transaction, using the
existing database object and the migration flow in db.ts to keep the whole
change atomic.
| database.exec(` | ||
| UPDATE campaigns | ||
| SET pledged_amount = COALESCE( | ||
| ( | ||
| SELECT SUM(amount) | ||
| FROM pledges | ||
| WHERE pledges.campaign_id = campaigns.id | ||
| AND refunded_at IS NULL | ||
| ), | ||
| 0 | ||
| ); | ||
| `); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find event consumers/projections that may still observe duplicate pledged events.
rg -n -C4 "campaign_events|blockchain_metadata|txHash|event_type.*pledged|recordEvent" backend/src backend/testsRepository: ritik4ever/stellar-goal-vault
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== eventHistory outline ==\n'
ast-grep outline backend/src/services/eventHistory.ts --view expanded || true
printf '\n== db outline ==\n'
ast-grep outline backend/src/services/db.ts --view expanded || true
printf '\n== eventIndexer outline ==\n'
ast-grep outline backend/src/services/eventIndexer.ts --view expanded || true
printf '\n== campaign history consumers ==\n'
rg -n -C3 "getCampaignHistory|getEventByTxHash|getEventsBySource|getEventsByLedger|campaign_events" backend/src backend/tests | head -n 250Repository: ritik4ever/stellar-goal-vault
Length of output: 20337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== backend/src/services/db.ts (160-220) ==\n'
sed -n '160,220p' backend/src/services/db.ts | cat -n
printf '\n== backend/src/services/eventIndexer.ts (110-145, 250-330) ==\n'
sed -n '110,145p' backend/src/services/eventIndexer.ts | cat -n
printf '\n---\n'
sed -n '250,330p' backend/src/services/eventIndexer.ts | cat -n
printf '\n== backend/src/services/eventHistory.ts (60-170) ==\n'
sed -n '60,170p' backend/src/services/eventHistory.ts | cat -n
printf '\n== campaignStore pledge/reconcile paths ==\n'
rg -n -C3 "recordEvent\\(|reconcileOnChainPledge|addPledge\\(|existing: true|getCampaignHistory\\(campaign.id\\).*pledged" backend/src/services/campaignStore.ts backend/src/services/campaignStore.test.ts backend/src/services/eventIndexer.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 21210
Deduplicate campaign_events too, or document that duplicate pledge history is retained
This migration only cleans pledges; any existing campaign_events rows with the same blockchain_metadata.txHash stay in place. Since the history endpoint reads campaign_events directly, duplicate pledge entries will still surface in user-facing history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/services/db.ts` around lines 174 - 185, The migration in db.ts
only deduplicates pledges and updates campaigns, but leaves duplicate
campaign_events rows with the same blockchain_metadata.txHash intact. Update the
migration to also clean duplicates in campaign_events using the relevant txHash
field, or explicitly document in the migration and history flow that duplicate
pledge history is intentionally retained. Focus on the existing migration block
around the campaigns update and the campaign_events history path that feeds the
endpoint.
2 participants
changes made
closed #238
Pull Request
What Changed
A clear and concise description of what this PR changes and why.
Related Issues
Testing Done
Describe the tests you ran and how to reproduce them.
Security Review
If this PR touches API endpoints, authentication, database queries, or contract code, check the applicable items from SECURITY_CHECKLIST.md. Paste relevant items below:
Checklist
npm test/cargo test)Screenshots (if applicable)
Add screenshots or recordings for visual changes.
Summary by CodeRabbit
Bug Fixes
200when the pledge already exists, and201only for newly processed pledges.Tests