Open
Conversation
Add 53 unit tests covering atlas-common (error codes, pagination) and atlas-indexer (batch operations, block collection, metadata resolution). Add GitHub Actions workflow to build backend and frontend Docker images.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a GitHub Actions Docker build workflow, refactors CI into separate formatting/clippy/test jobs, updates the backend Dockerfile with cargo-chef multi-stage builds, and adds extensive unit tests across atlas-common and atlas-indexer. No production logic or public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PR as "PR / Push"
participant GH as "GitHub Actions"
participant BX as "Docker Buildx"
participant GHA as "GHA Cache/Registry"
participant IMG as "Built Image"
PR->>GH: trigger workflow (push to main or pull_request)
GH->>BX: setup buildx & platforms (linux/amd64)
GH->>BX: build backend/indexer (target) with cache-from GHA
BX->>GHA: fetch cache layers
BX->>IMG: produce backend cache-only output
GH->>BX: build backend/api (target) with cache-from/to GHA
BX->>GHA: update cache
GH->>BX: build frontend context with cache-from/to GHA
BX->>GHA: update cache
BX->>IMG: produce frontend cache-only output
GH->>PR: report job statuses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🤖 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-common/src/error.rs`:
- Around line 58-107: The new unit tests in error.rs (functions
not_found_returns_404, invalid_input_returns_400, unauthorized_returns_401,
internal_error_returns_500, rpc_error_returns_502, metadata_fetch_returns_502,
config_error_returns_500, verification_error_returns_400,
bytecode_mismatch_returns_400, compilation_error_returns_422) have rustfmt/style
violations causing CI failure; run cargo fmt (or rustfmt) on
backend/crates/atlas-common/src/error.rs to reformat the file and commit the
changes so the tests adhere to project formatting standards and the workflow can
pass.
In `@backend/crates/atlas-indexer/src/indexer.rs`:
- Around line 942-1053: Tests in indexer.rs are failing rustfmt; run rustfmt
(cargo fmt) to reformat the file and commit the changes so the test functions
(e.g., collect_empty_block_populates_block_fields,
collect_multiple_blocks_accumulate_in_order,
collect_erc20_transfer_populates_transfer_and_balance_arrays,
collect_erc20_mint_skips_zero_address_balance_delta) match the project's
formatting rules; if rustfmt flags any specific lines, apply those
whitespace/indentation fixes and re-run cargo fmt to ensure CI passes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/docker.ymlbackend/crates/atlas-common/src/error.rsbackend/crates/atlas-common/src/types.rsbackend/crates/atlas-indexer/src/batch.rsbackend/crates/atlas-indexer/src/indexer.rsbackend/crates/atlas-indexer/src/metadata.rs
pthmas
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/crates/atlas-indexer/src/indexer.rs (1)
981-988: Optional: deduplicate transfer-topic/log fixture literals for maintainability.The same transfer topic and log scaffold are repeated across tests. Reusing
TRANSFER_TOPICand a tiny log-builder helper would reduce drift risk.♻️ Suggested refactor
- "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + TRANSFER_TOPIC,+ fn make_transfer_log( + address: &str, + topics: Vec<&str>, + data: &str, + ) -> serde_json::Value { + serde_json::json!([{ + "address": address, + "topics": topics, + "data": data, + "blockNumber": "0x1", + "transactionHash": "0x0000000000000000000000000000000000000000000000000000000000000001", + "transactionIndex": "0x0", + "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000001", + "logIndex": "0x0", + "removed": false + }]) + }Also applies to: 1031-1038, 1065-1072, 1099-1106, 1144-1151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-indexer/src/indexer.rs` around lines 981 - 988, Create a shared constant TRANSFER_TOPIC and a small helper function (e.g., build_transfer_log or make_log) to produce the repeated log JSON fixture instead of duplicating the topic and scaffold in multiple places; update each test site in indexer.rs that currently repeats the same topic/log (the blocks around the duplicated fixtures at the ranges noted) to call the helper and reference TRANSFER_TOPIC so the topic string and the JSON structure are maintained in one place for easier updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/crates/atlas-indexer/src/indexer.rs`:
- Around line 981-988: Create a shared constant TRANSFER_TOPIC and a small
helper function (e.g., build_transfer_log or make_log) to produce the repeated
log JSON fixture instead of duplicating the topic and scaffold in multiple
places; update each test site in indexer.rs that currently repeats the same
topic/log (the blocks around the duplicated fixtures at the ranges noted) to
call the helper and reference TRANSFER_TOPIC so the topic string and the JSON
structure are maintained in one place for easier updates.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/crates/atlas-common/src/error.rsbackend/crates/atlas-common/src/types.rsbackend/crates/atlas-indexer/src/indexer.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/crates/atlas-common/src/types.rs
- backend/crates/atlas-common/src/error.rs
Reduces CI wall time by running format check, clippy, and tests concurrently instead of sequentially.
Split indexer and api into separate parallel jobs. Add GHA cache to the indexer build which was previously rebuilding from scratch every run.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/docker.yml (2)
3-7: Consider path filters to skip Docker CI for unrelated PR changes.This can cut CI time/cost when only docs or non-Docker paths change.
⚙️ Optional trigger optimization
on: pull_request: + paths: + - backend/** + - frontend/** + - .github/workflows/docker.yml push: branches: - main + paths: + - backend/** + - frontend/** + - .github/workflows/docker.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker.yml around lines 3 - 7, Add path-based filtering to the GitHub Actions trigger to skip running the Docker CI when unrelated files change: update the workflow's on.pull_request and/or on.push triggers (the existing keys 'on', 'pull_request', 'push', 'branches') to include 'paths' and/or 'paths-ignore' entries that limit runs to Docker-related files (e.g., Dockerfile, docker/**, build scripts) and exclude docs/tests paths; ensure you add matching path patterns under the same trigger blocks so CI only runs for relevant PRs/commits.
30-31: Split cache scope for parallel backend jobs to avoid overwrites.Lines 30–31 and 50–51 both write to
scope=backendconcurrently. Docker'stype=ghacache backend will overwrite prior exports when multiple jobs write to the same scope—only the last writer's cache persists. Split into distinct scopes per job (e.g.,backend-indexer,backend-api) to eliminate contention and improve cache hit stability.♻️ Recommended cache-scope split
# build-indexer - cache-from: type=gha,scope=backend - cache-to: type=gha,scope=backend,mode=max + cache-from: type=gha,scope=backend-indexer + cache-to: type=gha,scope=backend-indexer,mode=max # build-api - cache-from: type=gha,scope=backend - cache-to: type=gha,scope=backend,mode=max + cache-from: type=gha,scope=backend-api + cache-to: type=gha,scope=backend-api,mode=maxAlso applies to: 50–51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker.yml around lines 30 - 31, The workflow currently uses the same GitHub Actions cache scope "backend" in the cache-from/cache-to entries, which causes parallel backend jobs to overwrite each other's cache; update the cache scope names in the cache-from and cache-to entries (the two lines that read "cache-from: type=gha,scope=backend" and "cache-to: type=gha,scope=backend,mode=max") to unique, job-specific scopes (for example "backend-indexer" and "backend-api") for each backend job so each job writes to its own scope and avoids contention; ensure you change both the cache-from and cache-to entries for each job to the same unique scope name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docker.yml:
- Line 19: Replace all reusable action tags (e.g., the shown uses:
actions/checkout@v4 and the other action uses in build-indexer, build-api, and
build-frontend jobs) with the corresponding immutable full commit SHAs for each
action and add a trailing comment referencing the original tag (for example: //
pinned to actions/checkout@v4). Update every "uses: ..." entry across the three
jobs to the full SHA form so tags cannot be retargeted, and after changes enable
Dependabot for the github-actions ecosystem to keep those SHAs up to date.
---
Nitpick comments:
In @.github/workflows/docker.yml:
- Around line 3-7: Add path-based filtering to the GitHub Actions trigger to
skip running the Docker CI when unrelated files change: update the workflow's
on.pull_request and/or on.push triggers (the existing keys 'on', 'pull_request',
'push', 'branches') to include 'paths' and/or 'paths-ignore' entries that limit
runs to Docker-related files (e.g., Dockerfile, docker/**, build scripts) and
exclude docs/tests paths; ensure you add matching path patterns under the same
trigger blocks so CI only runs for relevant PRs/commits.
- Around line 30-31: The workflow currently uses the same GitHub Actions cache
scope "backend" in the cache-from/cache-to entries, which causes parallel
backend jobs to overwrite each other's cache; update the cache scope names in
the cache-from and cache-to entries (the two lines that read "cache-from:
type=gha,scope=backend" and "cache-to: type=gha,scope=backend,mode=max") to
unique, job-specific scopes (for example "backend-indexer" and "backend-api")
for each backend job so each job writes to its own scope and avoids contention;
ensure you change both the cache-from and cache-to entries for each job to the
same unique scope name.
Splits the Rust build into dependency cooking (cached) and source compilation, so code-only changes skip the expensive dependency build.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/Dockerfile (1)
31-37:⚠️ Potential issue | 🟠 MajorRun both runtime images as a non-root user.
Line 31 and Line 40 define final runtime stages but never set
USER, so containers run as root by default. This weakens container isolation.🔐 Proposed hardening diff
FROM alpine:3.21 AS indexer -RUN apk add --no-cache ca-certificates +RUN addgroup -S atlas && adduser -S -G atlas atlas \ + && apk add --no-cache ca-certificates -COPY --from=builder /app/target/release/atlas-indexer /usr/local/bin/ +COPY --from=builder --chown=atlas:atlas /app/target/release/atlas-indexer /usr/local/bin/ +USER atlas CMD ["atlas-indexer"] @@ FROM alpine:3.21 AS api -RUN apk add --no-cache ca-certificates +RUN addgroup -S atlas && adduser -S -G atlas atlas \ + && apk add --no-cache ca-certificates -COPY --from=builder /app/target/release/atlas-api /usr/local/bin/ +COPY --from=builder --chown=atlas:atlas /app/target/release/atlas-api /usr/local/bin/ +USER atlas EXPOSE 3000 CMD ["atlas-api"]Also applies to: 40-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/Dockerfile` around lines 31 - 37, The runtime stages (e.g., the indexer stage that contains COPY --from=builder /app/target/release/atlas-indexer and CMD ["atlas-indexer"], and the other final runtime stage around lines 40-47) run as root; create a non-root user and group (Alpine: use addgroup/adduser) in each final stage, change ownership of the copied binary to that user (chown the atlas-indexer file) and switch to that user with USER before the CMD; ensure you perform these steps in both final runtime stages so the container processes run unprivileged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/Dockerfile`:
- Around line 31-37: The runtime stages (e.g., the indexer stage that contains
COPY --from=builder /app/target/release/atlas-indexer and CMD ["atlas-indexer"],
and the other final runtime stage around lines 40-47) run as root; create a
non-root user and group (Alpine: use addgroup/adduser) in each final stage,
change ownership of the copied binary to that user (chown the atlas-indexer
file) and switch to that user with USER before the CMD; ensure you perform these
steps in both final runtime stages so the container processes run unprivileged.
cargo-chef v0.1.77 requires rustc 1.86+ which is incompatible with the rust:1.85-alpine base image. Switch to the official lukemathwalker/cargo-chef Docker image which bundles a compatible cargo-chef binary.
The version downgrades for Rust 1.85 compatibility (home, wit-bindgen, wasip2) must happen before preparing the recipe, otherwise cargo-chef cook resolves incompatible versions from the lockfile.
cargo-chef adds complexity with Rust 1.85 version pinning and doesn't improve build times on cold cache. GHA layer caching handles caching across runs already.
Pins actions/checkout, docker/setup-buildx-action, and docker/build-push-action to immutable commit SHAs to prevent supply-chain attacks via tag retargeting. Adds dependabot.yml to auto-update those SHAs weekly.
tac0turtle
reviewed
Mar 4, 2026
Comment on lines
+410
to
+424
| fn limit_above_max_clamps_to_100() { | ||
| let p = Pagination { | ||
| page: 1, | ||
| limit: 150, | ||
| }; | ||
| assert_eq!(p.limit(), 100); | ||
| } | ||
|
|
||
| #[test] | ||
| fn limit_at_max_is_unchanged() { | ||
| let p = Pagination { | ||
| page: 1, | ||
| limit: 100, | ||
| }; | ||
| assert_eq!(p.limit(), 100); |
Contributor
There was a problem hiding this comment.
these sort of tests dont really test anything. we should remove superfluous tests like this
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add 53 unit tests across the indexer and common crates, covering error handling, pagination, batch operations, block collection (ERC-20/721 detection), and URI resolution. Add a new Docker CI workflow to build backend and frontend images on pull requests and pushes to main.
Closes #10
Changes
Summary by CodeRabbit
Chores
Tests