Skip to content

Fix: delete limo removed opps#568

Merged
anihamde merged 3 commits intomainfrom
fix/delete-limo-removed-opps
Jun 4, 2025
Merged

Fix: delete limo removed opps#568
anihamde merged 3 commits intomainfrom
fix/delete-limo-removed-opps

Conversation

@anihamde
Copy link
Contributor

@anihamde anihamde commented Jun 4, 2025

This PR fixes a bug whereby limo opportunities that hadn't yet been removed were liable to be deleted from the db.

@vercel
Copy link

vercel bot commented Jun 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
swap-staging ⬜️ Ignored (Inspect) Visit Preview Jun 4, 2025 4:05pm

danimhr
danimhr previously approved these changes Jun 4, 2025
let n_opportunities_deleted = sqlx::query!(
"WITH rows_to_delete AS (
SELECT id FROM opportunity WHERE chain_id = $1 AND creation_time < $2 LIMIT $3
SELECT id FROM opportunity WHERE chain_id = $1 AND creation_time < $2 AND removal_time IS NOT NULL LIMIT $3
Copy link
Contributor

Choose a reason for hiding this comment

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

this query is not fast so we should not merge it as we have no index on removal_time (do we?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Imagine there are opportunities in-memory, then the server gets restarted, now we have some opps in db without removal time which we can safely remove.

@danimhr danimhr dismissed their stale review June 4, 2025 14:00

Misclicked

@anihamde anihamde merged commit a4e3163 into main Jun 4, 2025
3 checks passed
@anihamde anihamde deleted the fix/delete-limo-removed-opps branch June 4, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants