Conversation
📝 WalkthroughWalkthroughImplemented ERC-20 supply history tracking by accumulating supply deltas during transaction indexing and conditionally computing indexed total supply in API responses when supply history is marked complete. Removes on-chain Changes
Sequence Diagram(s)sequenceDiagram
participant Indexer
participant BlockBatch
participant Database
participant APIHandler
participant State
Indexer->>BlockBatch: Detect Transfer (mint/burn)
BlockBatch->>BlockBatch: apply_supply_delta(contract, delta)
Indexer->>BlockBatch: Detect Transfer (transfer between addresses)
BlockBatch->>BlockBatch: Record balance_delta per address
Indexer->>Database: write_batch() with supply_map
Database->>Database: UPDATE erc20_contracts.total_supply from unnest(deltas)
note over Indexer: Reaches chain head
Indexer->>State: Mark ERC20_SUPPLY_HISTORY_COMPLETE = "true"
State->>Database: UPDATE indexer_state
sequenceDiagram
participant Client
participant APIHandler
participant State
participant Database as DB: erc20_balances
Client->>APIHandler: GET /api/tokens/{address}
APIHandler->>State: has_complete_erc20_supply_history()?
State->>Database: SELECT value FROM indexer_state WHERE key=?
Database-->>State: "true"
State-->>APIHandler: true
APIHandler->>Database: SELECT SUM(balance) FROM erc20_balances WHERE balance > 0
Database-->>APIHandler: indexed_supply
APIHandler-->>Client: {total_supply: indexed_supply}
note over APIHandler: If supply_history incomplete
APIHandler-->>Client: {total_supply: stored_value}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 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
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/mod.rs (1)
30-41: Extract the completion check into a pure helper and cover it in this file’s tests.The new
indexer_statebranch is only exercised through the DB path here. Pulling theSome("true")check into a tiny pure function would let the existing#[cfg(test)]module cover missing-row /"false"/ malformed-value cases without a database fixture.As per coding guidelines, "Add unit tests for new logic in a
#[cfg(test)] mod testsblock in the same file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/mod.rs` around lines 30 - 41, Extract the "Some(\"true\")" check into a tiny pure helper (e.g., fn parse_erc20_supply_history_complete(value: Option<&str>) -> bool) and have has_complete_erc20_supply_history call that helper; then add a #[cfg(test)] mod tests in the same file that unit-tests the helper for None (missing row), Some("false"), Some("true"), and malformed values to cover all cases without needing the DB fixture. Ensure the helper name and has_complete_erc20_supply_history are referenced in tests so the DB path remains unchanged but logic is fully exercised by unit tests.
🤖 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/tokens.rs`:
- Around line 101-102: The current logic lets indexed supply leak when history
is incomplete: when has_complete_erc20_supply_history(&state.pool) is false you
must not surface erc20_contracts.total_supply (or any value derived from
get_indexed_total_supply) to callers of get_token or get_token_holders; change
the false-branch so that get_token clears/suppresses contract.total_supply (set
to None) and ensure get_token_holders does not fall back to the stored
erc20_contracts.total_supply value—either query a trusted on-chain source
instead or return no total_supply until has_complete_erc20_supply_history
returns true; apply the same change to the other occurrence around lines 137-145
where total_supply is currently used.
In `@backend/crates/atlas-server/src/indexer/indexer.rs`:
- Around line 122-127: The logic that sets ERC20 supply-history completion (via
get_erc20_supply_history_status and set_erc20_supply_history_complete) must only
mark complete when there are no outstanding gaps or permanently failed blocks;
update the code so it checks the failed_blocks set (or parked failed count) and
any deferred/failed retry state before setting the flag, and ensure any code
paths that move a block into failed_blocks immediately call
set_erc20_supply_history_complete(false) to reset the flag; apply this change to
the same pattern found around the other similar blocks (the other
get_/set_erc20_supply_history_status usages) so the flag represents a gap-free
condition and is cleared when a block is deferred to failed_blocks.
---
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 30-41: Extract the "Some(\"true\")" check into a tiny pure helper
(e.g., fn parse_erc20_supply_history_complete(value: Option<&str>) -> bool) and
have has_complete_erc20_supply_history call that helper; then add a #[cfg(test)]
mod tests in the same file that unit-tests the helper for None (missing row),
Some("false"), Some("true"), and malformed values to cover all cases without
needing the DB fixture. Ensure the helper name and
has_complete_erc20_supply_history are referenced in tests so the DB path remains
unchanged but logic is fully exercised by unit tests.
🪄 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: 59fbd274-7d3d-4133-ac21-ffcb5ad929fd
📒 Files selected for processing (9)
backend/crates/atlas-server/src/api/handlers/addresses.rsbackend/crates/atlas-server/src/api/handlers/mod.rsbackend/crates/atlas-server/src/api/handlers/tokens.rsbackend/crates/atlas-server/src/indexer/indexer.rsbackend/crates/atlas-server/src/lib.rsbackend/crates/atlas-server/src/main.rsbackend/crates/atlas-server/src/state_keys.rsbackend/crates/atlas-server/tests/integration/addresses.rsbackend/crates/atlas-server/tests/integration/tokens.rs
✅ Files skipped from review due to trivial changes (3)
- backend/crates/atlas-server/src/lib.rs
- backend/crates/atlas-server/src/main.rs
- backend/crates/atlas-server/src/api/handlers/addresses.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/crates/atlas-server/tests/integration/tokens.rs
- backend/crates/atlas-server/tests/integration/addresses.rs
| if has_complete_erc20_supply_history(&state.pool).await? { | ||
| contract.total_supply = Some(get_indexed_total_supply(&state.pool, &address).await?); |
There was a problem hiding this comment.
The incomplete-history path still exposes indexed supply.
backend/crates/atlas-server/src/indexer/indexer.rs now advances erc20_contracts.total_supply from indexed mint/burn deltas. When has_complete_erc20_supply_history() is false, get_token leaves that field untouched and get_token_holders falls back to it, so a fresh sync can still return a partial supply before the completeness flag flips. The false branch needs a separate trusted source, or it should suppress total_supply until history is complete.
Also applies to: 137-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/crates/atlas-server/src/api/handlers/tokens.rs` around lines 101 -
102, The current logic lets indexed supply leak when history is incomplete: when
has_complete_erc20_supply_history(&state.pool) is false you must not surface
erc20_contracts.total_supply (or any value derived from
get_indexed_total_supply) to callers of get_token or get_token_holders; change
the false-branch so that get_token clears/suppresses contract.total_supply (set
to None) and ensure get_token_holders does not fall back to the stored
erc20_contracts.total_supply value—either query a trusted on-chain source
instead or return no total_supply until has_complete_erc20_supply_history
returns true; apply the same change to the other occurrence around lines 137-145
where total_supply is currently used.
| let erc20_supply_history_status = self.get_erc20_supply_history_status().await?; | ||
| let mut erc20_supply_backfill_pending = matches!(erc20_supply_history_status, Some(false)) | ||
| || (erc20_supply_history_status.is_none() && start_block == 0); | ||
| if erc20_supply_history_status.is_none() && start_block == 0 { | ||
| self.set_erc20_supply_history_complete(false).await?; | ||
| } |
There was a problem hiding this comment.
Don’t mark supply history complete while gaps can still exist.
This state machine never checks for outstanding failed blocks. If a block exhausts retries and is parked in failed_blocks, current_block > head can still flip ERC20_SUPPLY_HISTORY_COMPLETE_KEY to true, and once the key is true later gaps will not turn it back off. Since the API uses this flag to trust indexed supply, that can expose incorrect totals after any permanent hole in the indexed history. Tie the flag to a gap-free condition, and reset it when a block is deferred into failed_blocks.
Also applies to: 207-210, 407-410
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/crates/atlas-server/src/indexer/indexer.rs` around lines 122 - 127,
The logic that sets ERC20 supply-history completion (via
get_erc20_supply_history_status and set_erc20_supply_history_complete) must only
mark complete when there are no outstanding gaps or permanently failed blocks;
update the code so it checks the failed_blocks set (or parked failed count) and
any deferred/failed retry state before setting the flag, and ensure any code
paths that move a block into failed_blocks immediately call
set_erc20_supply_history_complete(false) to reset the flag; apply this change to
the same pattern found around the other similar blocks (the other
get_/set_erc20_supply_history_status usages) so the flag represents a gap-free
condition and is cleared when a block is deferred to failed_blocks.
1 participant
Summary
Reindexing is enough to apply this during development, so this PR does not include a backfill migration.
Summary by CodeRabbit