Skip to content

Dedupe anchors returning the same quote#284

Open
githoboman wants to merge 6 commits into
ezedike-evan:mainfrom
githoboman:dedupe-anchors-returning-the-same-quote-id
Open

Dedupe anchors returning the same quote#284
githoboman wants to merge 6 commits into
ezedike-evan:mainfrom
githoboman:dedupe-anchors-returning-the-same-quote-id

Conversation

@githoboman
Copy link
Copy Markdown
Contributor

Summary
The ticket named lib/stellar/rates-engine.ts, but that file didn't exist — the actual rate aggregation lives in computeRateComparison (sep24.ts:359), and AnchorRate had no quote-id field to dedupe on. So I created the module the ticket assumed, plus the field needed to carry SEP-38 quote ids.

Changes (3 files):
Closes #175

types/index.ts — added optional quoteId?: string to AnchorRate, populated from SEP-38 firm quotes. Optional, so no existing AnchorRate construction breaks.

lib/stellar/rates-engine.ts (new) — dedupeByQuoteId(rates):

Map-based dedupe keyed by quoteId.
On collision, earliest-received wins (smallest updatedAt); updatedAt ties keep the incumbent, so input order breaks them.
Rates without a quoteId (SEP-24 fees, unavailable placeholders) can't collide and always pass through.
Original order preserved; a collapsed group is emitted at its first appearance.
tests/rates-dedupe.spec.ts (new) — 8 tests with synthetic collisions: two-anchor collapse, order-independence of earliest-received, distinct ids preserved, three-way collision, quoteId-less passthrough, order preservation, tie-breaking, empty input.

Verification:

New test: 8/8 passing.
Related suites (rate-ranking, compute-total, types): 22/22 passing.
ESLint clean on both new files.
One note: the pre-existing tsc --noEmit errors in sep10-state, sep24-asset-format, and sep24-withdraw test files are unrelated to this change (they're about Networks properties, argument counts, and Sep1TomlData shape) and were already failing before I touched anything.

I haven't wired dedupeByQuoteId into computeRateComparison yet — the ticket scoped this to the dedupe primitive + test. If you want it applied in the live aggregation path (dedupe before picking bestRateId), say the word and I'll hook it in.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 31, 2026

@githoboman is attempting to deploy a commit to the ezedikeevan's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented May 31, 2026

@githoboman 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! 🚀

Learn more about application limits

Copy link
Copy Markdown
Owner

@ezedike-evan ezedike-evan left a comment

Choose a reason for hiding this comment

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

Thanks @githoboman — the dedupe primitive (dedupeByQuoteId in lib/stellar/rates-engine.ts) and its 8 tests in tests/rates-dedupe.spec.ts) are exactly right for #175: map-keyed by quoteId, earliest-received wins on collision, quoteId-less rates pass through, order preserved. The logic and tests are solid.

I cannot merge this PR as-is because it bundles changes that belong to different tickets.

What's in scope for #175 (2–3 files):

  • lib/stellar/rates-engine.ts — the dedupe function
  • tests/rates-dedupe.spec.ts — the collision tests
  • types/index.ts (quoteId?: string on AnchorRate) — the field the dedupe keys on (acceptable here)

What's bundled in that doesn't belong:

  1. app/globals.css — mobile bottom-sheet swipe-to-dismiss styles. Not related to rate deduplication.
  2. components/offramp/ExecuteDrawer.tsx — swipe-to-dismiss touch handlers + scroll-lock useEffect. Unrelated to rates.
  3. tests/sep38-auth.spec.ts — SEP-38 JWT auth/re-auth test suite. Not scoped to #175.

The PR description itself notes the contributor hasn't wired dedupeByQuoteId into computeRateComparison yet — that's fine for this ticket, the primitive + test is the stated scope.

How to fix:

  1. Strip app/globals.css, components/offramp/ExecuteDrawer.tsx, and tests/sep38-auth.spec.ts out of this branch.
  2. Keep lib/stellar/rates-engine.ts, tests/rates-dedupe.spec.ts, and the types/index.ts change.
  3. Push the cleaned branch — this will be ready to merge immediately.

(The swipe-to-dismiss and sep38-auth work should live in their own focused PRs.)

@githoboman
Copy link
Copy Markdown
Contributor Author

Thanks @githoboman — the dedupe primitive (dedupeByQuoteId in lib/stellar/rates-engine.ts) and its 8 tests in tests/rates-dedupe.spec.ts) are exactly right for #175: map-keyed by quoteId, earliest-received wins on collision, quoteId-less rates pass through, order preserved. The logic and tests are solid.

I cannot merge this PR as-is because it bundles changes that belong to different tickets.

What's in scope for #175 (2–3 files):

  • lib/stellar/rates-engine.ts — the dedupe function
  • tests/rates-dedupe.spec.ts — the collision tests
  • types/index.ts (quoteId?: string on AnchorRate) — the field the dedupe keys on (acceptable here)

What's bundled in that doesn't belong:

  1. app/globals.css — mobile bottom-sheet swipe-to-dismiss styles. Not related to rate deduplication.
  2. components/offramp/ExecuteDrawer.tsx — swipe-to-dismiss touch handlers + scroll-lock useEffect. Unrelated to rates.
  3. tests/sep38-auth.spec.ts — SEP-38 JWT auth/re-auth test suite. Not scoped to [#084] dedupe anchors returning the same quote id #175.

The PR description itself notes the contributor hasn't wired dedupeByQuoteId into computeRateComparison yet — that's fine for this ticket, the primitive + test is the stated scope.

How to fix:

  1. Strip app/globals.css, components/offramp/ExecuteDrawer.tsx, and tests/sep38-auth.spec.ts out of this branch.
  2. Keep lib/stellar/rates-engine.ts, tests/rates-dedupe.spec.ts, and the types/index.ts change.
  3. Push the cleaned branch — this will be ready to merge immediately.

(The swipe-to-dismiss and sep38-auth work should live in their own focused PRs.)

Result
dedupe-anchors-returning-the-same-quote-id now contains exactly 3 files vs main:

lib/stellar/rates-engine.ts — the dedupe primitive
tests/rates-dedupe.spec.ts — the 8 collision tests (still 8/8 passing)
types/index.ts — quoteId?: string on AnchorRate
Stripped out (no longer on this branch): app/globals.css, components/offramp/ExecuteDrawer.tsx, lib/stellar/sep38.ts, tests/sep38-auth.spec.ts.

How: Each concern was already its own commit, so this was clean — I reset the branch to main and cherry-picked only the dedupe commit (58b9e12). No content changed; the in-scope commit applied verbatim (new SHA 4072a60 from the rebase onto main).

Nothing lost: the full original branch is preserved at backup/dedupe-anchors-full (all 3 commits), so the SEP-38 auth work (b2657a2) and the ExecuteDrawer swipe-to-dismiss work (0a3ecca) are intact and ready to be lifted into their own focused PRs.

@ezedike-evan
Copy link
Copy Markdown
Owner

Still bundling three unrelated changesets. The dedupe primitive (rates-engine.ts + tests/rates-dedupe.spec.ts + types/index.ts quoteId field) is the ticket scope — but this PR also adds the full swipe-to-dismiss bottom sheet (app/globals.css, drag-offset state, scroll-lock effect, and all touch handlers in ExecuteDrawer.tsx) and the 243-line sep38-auth.spec.ts that tests a completely separate SEP-38 auth feature. Please strip ExecuteDrawer.tsx, app/globals.css, and tests/sep38-auth.spec.ts from this branch so it contains only the dedupe module, its test, and the quoteId type addition.

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.

[#084] dedupe anchors returning the same quote id

2 participants