Skip to content

perf(daemon): drop redundant DISTINCT in getAddressWalletInfo#401

Open
andreabadesso wants to merge 1 commit intomasterfrom
perf/daemon-drop-distinct-getaddresswalletinfo
Open

perf(daemon): drop redundant DISTINCT in getAddressWalletInfo#401
andreabadesso wants to merge 1 commit intomasterfrom
perf/daemon-drop-distinct-getaddresswalletinfo

Conversation

@andreabadesso
Copy link
Copy Markdown
Collaborator

@andreabadesso andreabadesso commented Apr 17, 2026

Motivation

Tier 2 fix #7 from #395. address.address is a unique key in the address table, so the DISTINCT clause on SELECT DISTINCT a.address, a.wallet_id, ... is logically redundant — the join can't produce duplicate rows. MySQL honors it anyway by adding a sort/dedup step on top of the join result.

Small per-event savings (~0.5–2ms depending on address count), but this query runs in the sync hot path so it compounds at mainnet scale.

Branches directly off master — independent of the benchmarking stack (#396#400).

Acceptance Criteria

  • DISTINCT removed from the SELECT in getAddressWalletInfo
  • Comment explains why, so a future reader doesn't re-add it defensively
  • Existing db tests pass unchanged (no behavior change)

Validation

  • yarn tsc --noEmit — clean
  • yarn test --testPathPattern='db' — 79/79 pass

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged
  • Make sure either the unit tests and/or the QA tests are capable of testing the new features (existing db tests cover this function)
  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. (no new deps)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Updated wallet information database query handling.

Tier 2 fix #7 from #395. `address.address` is a unique key, so the
DISTINCT clause is redundant for this SELECT — it only forces MySQL to
add a sort/dedup step on top of the join result.

Small per-event savings (~0.5-2ms depending on address count), and the
query runs once per event in the sync hot path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

The getAddressWalletInfo function in the database module has been updated to remove the DISTINCT keyword from its SQL query. The query now returns SELECT a.address, ... instead of SELECT DISTINCT a.address, ..., while maintaining all joins, filters, and result mapping logic.

Changes

Cohort / File(s) Summary
SQL Query Modification
packages/daemon/src/db/index.ts
Removed DISTINCT keyword from the address wallet info SELECT query. All other query structure (joins, filtering, mapping) remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A keyword gone, the query flows free,
No DISTINCT rows need now to decree,
The wallet joins bright, addresses clear,
Reorganization whispers draw near! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The linked issue #7 (feat: handle reorg on the daemon) appears unrelated to this performance optimization of removing DISTINCT from a SQL query. Clarify whether this PR is actually related to issue #7, or if it should be linked to a different performance/optimization issue instead.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing a redundant DISTINCT keyword from a SQL query in getAddressWalletInfo for performance optimization.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to removing DISTINCT from one SQL query in getAddressWalletInfo, which aligns with the stated objective of a performance optimization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/daemon-drop-distinct-getaddresswalletinfo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/daemon/src/db/index.ts (1)

941-973: Optional: consider aligning wallet-service for consistency.

The equivalent getAddressWalletInfo in packages/wallet-service/src/db/index.ts (lines 227–252) still uses SELECT DISTINCT with the same join shape. If the reasoning here holds (and it does, given identical schema), the same cleanup could be applied there for consistency — or, conversely, a short note in the commit/PR body explaining why only the daemon path was changed would help future readers. Not blocking for this PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/daemon/src/db/index.ts` around lines 941 - 973, The daemon's
getAddressWalletInfo removes the redundant SELECT DISTINCT; mirror this change
in the wallet-service implementation by updating the getAddressWalletInfo
function in packages/wallet-service/src/db/index.ts to remove DISTINCT from the
same JOIN query (or add a brief commit/PR note explaining why only the daemon
was changed), ensuring the SQL, variable names (e.g., addressWalletMap,
AddressesWalletsRow) and result mapping remain consistent with the daemon
version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/daemon/src/db/index.ts`:
- Around line 941-973: The daemon's getAddressWalletInfo removes the redundant
SELECT DISTINCT; mirror this change in the wallet-service implementation by
updating the getAddressWalletInfo function in
packages/wallet-service/src/db/index.ts to remove DISTINCT from the same JOIN
query (or add a brief commit/PR note explaining why only the daemon was
changed), ensuring the SQL, variable names (e.g., addressWalletMap,
AddressesWalletsRow) and result mapping remain consistent with the daemon
version.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fd486825-94c0-436e-84f6-fe0ebf7489e4

📥 Commits

Reviewing files that changed from the base of the PR and between 179abcd and 6286f6e.

📒 Files selected for processing (1)
  • packages/daemon/src/db/index.ts

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.

1 participant