Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 54 seconds. ⌛ 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 (9)
📝 WalkthroughWalkthroughThis PR changes ERC-20 total supply calculation from stored metadata to dynamically computed values from indexed token balances. It removes totalSupply() metadata fetching, updates API handlers to prefer indexed aggregations when transfers exist, tracks supply deltas during indexing, and includes database migration and integration tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIHandler
participant Database as DB (erc20_contracts)
participant IndexedDB as DB (erc20_balances)
rect rgba(255, 200, 100, 0.5)
Note over APIHandler,IndexedDB: New Flow: Total Supply Calculation
Client->>APIHandler: GET /api/addresses/{erc20_addr}
APIHandler->>Database: Query contract & check for transfers
Database-->>APIHandler: Contract row + transfer_count
alt transfer_count > 0 OR total_supply IS NULL
APIHandler->>IndexedDB: SELECT SUM(balance) WHERE balance > 0
IndexedDB-->>APIHandler: Computed total_supply
else
APIHandler->>Database: Use stored total_supply
Database-->>APIHandler: Stored value
end
APIHandler-->>Client: Response with total_supply
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/crates/atlas-server/src/indexer/metadata.rs (1)
284-315:⚠️ Potential issue | 🟠 MajorKeep a bootstrap supply for tokens first seen mid-history.
erc20_balancesandsupply_mapare only built from transfers that Atlas indexes fromconfig.start_blockonward. For a token that already had circulating supply before the indexer first sees it, removing the one-timetotalSupply()read leaves no baseline to fall back to, so later mint/burn deltas can never reconstruct the real supply. Please keep a bootstrap snapshot here, or persist a separate baseline column that the indexed deltas can build on.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/indexer/metadata.rs` around lines 284 - 315, fetch_erc20_contract_metadata no longer captures a token's pre-index history supply; call IERC20Metadata::totalSupply().call().await (or equivalent totalSupply() on the token contract) when fetching metadata and persist that value as a bootstrap baseline so later indexed transfer deltas can reconstruct real supply. Update the DB write in fetch_erc20_contract_metadata to bind and store the returned total_supply into a new column (e.g., bootstrap_supply or initial_supply) on the erc20_contracts row (and optionally a bootstrap_block column) instead of relying solely on deltas, and ensure the function still sets metadata_fetched = true after saving the bootstrap value.backend/crates/atlas-server/src/api/handlers/tokens.rs (1)
100-145:⚠️ Potential issue | 🟠 MajorDon't treat “some indexed rows exist” as “supply is complete.”
transfer_count > 0andtotal.0 > 0only mean we've indexed some activity, not thaterc20_balancescontains the full preexisting holder set. On a deployment that starts mid-chain, these branches can replace a valid stored supply with the sum of post-start deltas and skew holder percentages. Please gate the balance-derived override on an explicit backfill/completeness signal instead ofCOUNT(*) > 0.
🧹 Nitpick comments (1)
backend/crates/atlas-server/tests/integration/tokens.rs (1)
222-248: Please cover the changed/holderssupply path too.This PR also changes
get_token_holders, but the current suite still exercises that endpoint with matching stored and indexed supply, so a regression there would pass unnoticed. Adding a staleerc20_contracts.total_supplycase for/api/tokens/:address/holderswould pin the new percentage behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/tests/integration/tokens.rs` around lines 222 - 248, Add a parallel test to cover the /api/tokens/:address/holders path: copy the pattern from get_token_detail_prefers_indexed_supply_over_stale_stored_value (seed_token_data, mutate erc20_contracts.total_supply to a stale value via sqlx::query), then call the holders endpoint (Request::builder().uri(format!("/api/tokens/{}/holders", TOKEN_A))) and assert the response.status is OK and that the response's total_supply field and any percentage fields for holders are computed using the indexed supply (e.g., still "1000000") rather than the stale stored value; reference the existing test name and the get_token_holders endpoint to locate where to add this new assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/atlas-server/src/api/handlers/addresses.rs`:
- Around line 251-260: The erc20 total_supply override currently triggers based
on has_erc20_transfers and will return partial sums when the indexer started
late; change the logic in the erc20_contract handling so you only call
get_indexed_erc20_total_supply and override erc20.total_supply when the explicit
completeness/backfill signal used by the token handler is true (i.e., the same
"index complete" check the token handler uses), and do not rely solely on
has_erc20_transfers; keep the branch returning Some(erc20) otherwise and leave
total_supply as None when the completeness flag is not set.
In `@backend/migrations/20240109000001_recompute_erc20_supply.sql`:
- Around line 4-15: The current UPDATE can overwrite correct total_supply with
incomplete sums; change the WHERE clause logic so you only overwrite when the
computed b.total_supply is nonzero or when the existing c.total_supply is NULL
(i.e., preserve positive existing totals). In practice, modify the UPDATE on
erc20_contracts to set total_supply from the subquery only if b.total_supply > 0
OR c.total_supply IS NULL, referencing erc20_contracts.total_supply and the
computed b.total_supply from the erc20_balances aggregation to avoid zeroing out
known-good snapshots.
---
Outside diff comments:
In `@backend/crates/atlas-server/src/indexer/metadata.rs`:
- Around line 284-315: fetch_erc20_contract_metadata no longer captures a
token's pre-index history supply; call
IERC20Metadata::totalSupply().call().await (or equivalent totalSupply() on the
token contract) when fetching metadata and persist that value as a bootstrap
baseline so later indexed transfer deltas can reconstruct real supply. Update
the DB write in fetch_erc20_contract_metadata to bind and store the returned
total_supply into a new column (e.g., bootstrap_supply or initial_supply) on the
erc20_contracts row (and optionally a bootstrap_block column) instead of relying
solely on deltas, and ensure the function still sets metadata_fetched = true
after saving the bootstrap value.
---
Nitpick comments:
In `@backend/crates/atlas-server/tests/integration/tokens.rs`:
- Around line 222-248: Add a parallel test to cover the
/api/tokens/:address/holders path: copy the pattern from
get_token_detail_prefers_indexed_supply_over_stale_stored_value
(seed_token_data, mutate erc20_contracts.total_supply to a stale value via
sqlx::query), then call the holders endpoint
(Request::builder().uri(format!("/api/tokens/{}/holders", TOKEN_A))) and assert
the response.status is OK and that the response's total_supply field and any
percentage fields for holders are computed using the indexed supply (e.g., still
"1000000") rather than the stale stored value; reference the existing test name
and the get_token_holders endpoint to locate where to add this new assertion.
🪄 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: 1074d95b-09b1-4216-a69b-51e3b41f3034
📒 Files selected for processing (8)
backend/crates/atlas-server/src/api/handlers/addresses.rsbackend/crates/atlas-server/src/api/handlers/tokens.rsbackend/crates/atlas-server/src/indexer/batch.rsbackend/crates/atlas-server/src/indexer/indexer.rsbackend/crates/atlas-server/src/indexer/metadata.rsbackend/crates/atlas-server/tests/integration/addresses.rsbackend/crates/atlas-server/tests/integration/tokens.rsbackend/migrations/20240109000001_recompute_erc20_supply.sql
| UPDATE erc20_contracts AS c | ||
| SET total_supply = COALESCE(b.total_supply, 0) | ||
| FROM ( | ||
| SELECT | ||
| erc20_contracts.address, | ||
| COALESCE(SUM(balance), 0) AS total_supply | ||
| FROM erc20_contracts | ||
| LEFT JOIN erc20_balances ON erc20_balances.contract_address = erc20_contracts.address | ||
| AND erc20_balances.balance > 0 | ||
| GROUP BY erc20_contracts.address | ||
| ) AS b | ||
| WHERE c.address = b.address; |
There was a problem hiding this comment.
This backfill can overwrite good supply with partial balance state.
erc20_balances is only authoritative after a full historical backfill. If indexing begins after a token already exists, this update can replace a previously correct snapshot with 0 or just the post-start delta sum, and the metadata fetcher no longer repopulates that baseline. Please preserve the existing value until the contract's indexed history is known-complete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/migrations/20240109000001_recompute_erc20_supply.sql` around lines 4
- 15, The current UPDATE can overwrite correct total_supply with incomplete
sums; change the WHERE clause logic so you only overwrite when the computed
b.total_supply is nonzero or when the existing c.total_supply is NULL (i.e.,
preserve positive existing totals). In practice, modify the UPDATE on
erc20_contracts to set total_supply from the subquery only if b.total_supply > 0
OR c.total_supply IS NULL, referencing erc20_contracts.total_supply and the
computed b.total_supply from the erc20_balances aggregation to avoid zeroing out
known-good snapshots.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Reindexing is enough to apply this during development, so this PR does not include a backfill migration.