Skip to content

feat: decode contract event logs using verified ABIs#80

Open
pthmas wants to merge 5 commits into
mainfrom
mewing-bill
Open

feat: decode contract event logs using verified ABIs#80
pthmas wants to merge 5 commits into
mainfrom
mewing-bill

Conversation

@pthmas

@pthmas pthmas commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a background worker that decodes EVM contract event logs using verified contract ABIs (including proxy contracts). Decoded results are persisted and surfaced through the API.

Changes

  • contract_abi.rs: Resolves ABIs for contracts — direct or via EIP-1967/EIP-1822 proxy storage slots (calls eth_getStorageAt to find implementation address)
  • event_log_decode.rs: Core decoding logic — matches topic0 to ABI events, decodes indexed/non-indexed params, serializes results; defines decode statuses (pending, decoded, no_abi, no_matching_event, decode_failed)
  • indexer/event_log_decode_worker.rs: Background worker — claims jobs from event_log_decode_jobs queue (SKIP LOCKED), processes logs in batches of 500 per address, supports full rescans
  • Migration 20260528000001: Adds decode_status, decoded_at, decode_attempted_at, decode_source columns to event_logs; creates event_log_decode_jobs queue table; adds partial index on pending logs
  • api/handlers/logs.rs: Simplified — strips proxy forwarding logic, returns decoded fields in log responses
  • api/handlers/proxy.rs: Cleaned up — removes log-related proxy passthrough now handled natively
  • api/handlers/contracts.rs: Queues a decode job when a contract ABI is uploaded
  • indexer/indexer.rs + main.rs: Spawns EventLogDecodeWorker as a tokio task alongside existing workers
  • frontend/src/types/index.ts: Adds EventLog type fields for decode status and decoded params
  • tests/integration/logs.rs: Integration tests for log API responses with decoded data

Summary by CodeRabbit

Release Notes

  • New Features

    • Event logs now display decode status, decoded event metadata (name, signature, parameters), and decode timestamps in API responses.
    • Automatic background event log decoding worker processes and tracks decode attempts.
    • Enhanced ABI resolution for proxy and implementation contracts.
  • Improvements

    • Transaction input data formatted as hex strings in API responses.
    • Enriched event log endpoints with comprehensive decode tracking and status information.

@pthmas pthmas self-assigned this May 28, 2026
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an end-to-end event log decoding pipeline: a DB migration extends event_logs with decode state fields and creates event_log_decode_jobs; a new contract_abi module handles proxy detection and ABI resolution; a new event_log_decode module drives ABI-based decoding, API response mapping, and job enqueueing; a new background worker claims and processes decode jobs; API log handlers are unified to return EventLogApiResponse; and frontend types are updated to match.

Changes

Event Log Decoding Pipeline

Layer / File(s) Summary
DB schema: decode state columns, jobs table
backend/migrations/20260528000001_event_log_decode_state.sql
Adds decode_status, decoded_at, decode_attempted_at, decode_source to event_logs with backfill and check constraints; creates event_log_decode_jobs table and supporting indexes.
Common types: hex serialization and EventLog decode fields
backend/crates/atlas-common/src/types.rs
Transaction.input_data gains serialize_bytes_as_hex; EventLog gains four decode state/metadata fields.
contract_abi module: proxy detection and ABI loading
backend/crates/atlas-server/src/contract_abi.rs, backend/crates/atlas-server/src/lib.rs
New module with EIP-1967/1822 slot-read proxy detection, DB-cached resolve_proxy, merge_abis, load_contract_abi, load_combined_abi, and proxies_using_implementation.
event_log_decode module: decoder, API response, job enqueueing
backend/crates/atlas-server/src/event_log_decode.rs, backend/crates/atlas-server/src/lib.rs
Defines decode-status constants, EventLogApiResponse, EventLogDecoder (ABI-indexed by topic0 selector), apply_decode_attempt reconciliation, and enqueue_decode_jobs/enqueue_jobs_for_verified_contract_tx helpers.
EventLogDecodeWorker: background job claim-and-process loop
backend/crates/atlas-server/src/indexer/event_log_decode_worker.rs, backend/crates/atlas-server/src/indexer/mod.rs
Worker claims jobs atomically with FOR UPDATE SKIP LOCKED, decodes event_logs in batches (full-rescan or incremental with cursor), updates decoded fields, deletes completed jobs, and increments retry on failure.
Indexer: enqueue decode jobs after batch write
backend/crates/atlas-server/src/indexer/indexer.rs, backend/crates/atlas-server/src/indexer/gap_fill_worker.rs, backend/crates/atlas-server/src/main.rs
write_batch/write_batch_and_clear_failed_block gain a pool parameter; after each committed batch enqueue_decode_jobs is called; EventLogDecodeWorker spawned at server startup.
verify_contract: transactional insert and decode job enqueueing
backend/crates/atlas-server/src/api/handlers/contracts.rs
contract_abis insert runs inside an explicit transaction; already-verified detected via rows_affected(); enqueue_jobs_for_verified_contract_tx called atomically; bytecode normalization zeroes immutable refs before CBOR strip.
proxy.rs: delegate to contract_abi module
backend/crates/atlas-server/src/api/handlers/proxy.rs
get_proxy_info and get_combined_abi replace ~212 lines of inline SQL and local ABI-merge helpers with calls to resolve_proxy, load_contract_abi, load_combined_abi, and proxies_using_implementation.
logs.rs: unify endpoints to EventLogApiResponse
backend/crates/atlas-server/src/api/handlers/logs.rs
All three log endpoints return EventLogApiResponse; SQL projections expanded for decode columns; EnrichedEventLog removed; enrich_decoded_logs_with_known_signatures added for topic0 fallback.
Frontend: EventLog type update and abiDecode guard
frontend/src/types/index.ts, frontend/src/utils/abiDecode.ts
EventLog interface gains decode discriminated union and decoded event fields; DecodedEventLog becomes a type alias; decodeInputData adds typeof string guard.
Integration tests: logs, schema, transactions, addresses
backend/crates/atlas-server/tests/integration/logs.rs, backend/crates/atlas-server/tests/integration/...
New log tests seed decoded and pending rows, assert endpoint shapes and signature fallback; schema tests extended for new table/indexes; transaction/address tests assert hex input_data.

Sequence Diagram(s)

sequenceDiagram
    rect rgba(100, 149, 237, 0.5)
        Note over Indexer,event_log_decode_jobs: Indexing path
    end
    participant Indexer
    participant write_batch_internal
    participant event_log_decode_jobs
    participant EventLogDecodeWorker
    participant build_decoder_for_address
    participant load_combined_abi
    participant event_logs DB

    Indexer->>write_batch_internal: pool, copy_client, batch
    write_batch_internal->>write_batch_internal: commit DB transaction
    write_batch_internal->>event_log_decode_jobs: enqueue_decode_jobs(addresses, full_rescan=false)

    loop background worker
        EventLogDecodeWorker->>event_log_decode_jobs: CLAIM_JOBS_SQL FOR UPDATE SKIP LOCKED
        event_log_decode_jobs-->>EventLogDecodeWorker: claimed job (address, full_rescan)
        EventLogDecodeWorker->>build_decoder_for_address: address
        build_decoder_for_address->>load_combined_abi: address (proxy-aware ABI)
        load_combined_abi-->>build_decoder_for_address: ResolvedContractAbi
        build_decoder_for_address-->>EventLogDecodeWorker: EventLogDecoder
        EventLogDecodeWorker->>event_logs DB: fetch batch (incremental or full rescan)
        event_logs DB-->>EventLogDecodeWorker: Vec<EventLog>
        EventLogDecodeWorker->>EventLogDecodeWorker: decode_log + apply_decode_attempt per log
        EventLogDecodeWorker->>event_logs DB: UPDATE decode fields in transaction
        EventLogDecodeWorker->>event_log_decode_jobs: DELETE job on success
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • evstack/atlas#54: Directly modifies the verify_contract handler flow introduced in that PR, adding transactional ABI insert and decode job enqueueing to the same contract verification code path.

Suggested reviewers

  • tac0turtle

🐇 A hop through the logs, decoded with care,
The ABI awakens, events in the air!
Proxy slots read, CBOR stripped clean,
Jobs claimed and committed — best pipeline I've seen.
0x hex strings now shine in the light,
This rabbit declares: the decode is right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 and specifically describes the main feature being added: decoding contract event logs using verified ABIs. It is concise and directly aligned with the PR's primary objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 mewing-bill

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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.

pthmas added 4 commits June 4, 2026 10:51
- input_data now serializes as 0x-prefixed hex string instead of
  raw bytes; frontend null guard updated to match string type
- normalize_bytecode_for_comparison must run before strip_metadata
  so immutable offsets apply against full unstripped bytecode
Keep EventLogApiResponse mapping from HEAD; fix query.pagination.page
→ query.page bug introduced during merge.
@pthmas pthmas marked this pull request as ready for review June 17, 2026 08:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
backend/crates/atlas-server/src/contract_abi.rs (1)

20-230: 🏗️ Heavy lift

Add same-file unit coverage for the new ABI/proxy helpers.

This module adds slot parsing, cache refresh, ABI merging, and ABI loading behavior but has no #[cfg(test)] mod tests. At minimum, cover merge_abis and zero/error/non-zero slot responses.

As per coding guidelines, "backend/crates/**/*.rs: Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file; run with cargo test --workspace."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/contract_abi.rs` around lines 20 - 230, Add a
`#[cfg(test)] mod tests` block at the end of the contract_abi.rs file to cover
the new ABI and proxy detection logic. Create unit tests for the `merge_abis`
function that verify behavior when both ABIs are present, when only one is
present, and when neither is present. Additionally, create tests for the
`read_address_slot` function that mock RPC responses for non-zero addresses
(success case), zero addresses (should return None), and error responses from
the RPC endpoint to ensure the function correctly parses storage slots and
handles edge cases. These tests should not require actual database or RPC calls;
use mock data or mocking libraries as appropriate.

Source: Coding guidelines

backend/crates/atlas-server/src/api/handlers/proxy.rs (1)

100-104: ⚡ Quick win

Reuse the direct ABI in the non-proxy branch.

Line 102 calls load_combined_abi after this handler already resolved proxy == None and loaded proxy_abi; that helper performs those lookups again, including RPC proxy-slot checks for non-proxies. Reuse the loaded ABI to avoid duplicate external work.

♻️ Proposed refactor
     } else {
         // Not a proxy, just return the contract's ABI
-        let combined = load_combined_abi(&state.pool, &state.rpc_url, &address)
-            .await?
-            .map(|resolved| resolved.abi);
+        let combined = proxy_abi.as_ref().map(|abi| abi.abi.clone());
         Ok(Json(CombinedAbiResponse {
             is_proxy: false,
             proxy_address: address,
             implementation_address: None,
             proxy_type: None,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/api/handlers/proxy.rs` around lines 100 -
104, In the non-proxy branch of the handler (the else block starting around line
100), the code redundantly calls load_combined_abi which re-performs proxy slot
checks and lookups that were already completed in determining proxy == None.
Instead of calling load_combined_abi again in the else branch, directly reuse
the ABI that was already resolved or loaded during the prior proxy detection
logic to avoid duplicate external work and RPC calls.
backend/crates/atlas-server/src/api/handlers/logs.rs (1)

190-230: ⚡ Quick win

Add same-file coverage for signature fallback enrichment.

This new logic is currently outside the unit tests below. Please add a small #[cfg(test)] case for the pure behavior here—preserving existing decoded names and filling only unresolved topic0 matches. As per coding guidelines, “Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/api/handlers/logs.rs` around lines 190 - 230,
Add unit test coverage for the enrich_decoded_logs_with_known_signatures
function by creating a #[cfg(test)] mod tests block in the same file. The test
should verify two key behaviors: that existing event_name values are preserved
when already populated, and that unresolved topic0 values get filled with the
correct name and event_signature from the signature map. Mock the AppState and
EventLog inputs to isolate the function logic from database dependencies.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/crates/atlas-common/src/types.rs`:
- Around line 50-55: The serialize_bytes_as_hex function lacks unit test
coverage. Add a #[cfg(test)] mod tests block at the end of the same file
containing unit tests for the serialize_bytes_as_hex function. Create at least
two test cases: one testing serialization of empty bytes (which should produce
the output "0x") and another testing serialization of a known byte vector (which
should produce "0x" followed by the hex-encoded representation of those bytes).
Use serde_json or an appropriate serializer in the test to verify the function
produces the expected hexadecimal string output.

In `@backend/crates/atlas-server/src/contract_abi.rs`:
- Around line 26-30: The eth_getStorageAt RPC call in the contract_abi function
currently uses a hardcoded "latest" block tag when resolving proxy
implementations, which causes issues with decoding older logs after proxy
upgrades. Replace the hardcoded "latest" string with the actual log block number
or implement proxy resolution that uses the specific block where the log
occurred. This ensures the correct implementation ABI is selected for each log
based on its block timestamp rather than always using the current
implementation.
- Around line 38-49: The code currently uses unwrap_or("0x") on the response's
"result" field, which silently defaults to "0x" when the result is missing.
However, a JSON-RPC error response contains an "error" field instead of a
"result" field, so this approach masks actual errors and misclassifies failed
requests. Instead of using unwrap_or("0x") directly on the result, first check
if the resp contains an "error" field using resp.get("error"), and if it does,
extract and propagate that error as an AtlasError. Only return the default "0x"
value when both the "result" and "error" fields are absent from the response.

In `@backend/crates/atlas-server/src/event_log_decode.rs`:
- Around line 319-323: The ON CONFLICT DO UPDATE SET clauses in the upsert
statements (appearing around lines 319-323 and 358-362) are clearing the
error_message field but not resetting the retry_count to 0 when a decode job is
re-enqueued. This leaves stale retry counts that no longer reflect the fresh
attempt. Add `retry_count = 0` to both SET clauses in addition to the existing
`error_message = NULL` to ensure both fields are properly reset on re-enqueue,
maintaining semantic correctness and accurate observability metrics.

In `@backend/crates/atlas-server/src/indexer/indexer.rs`:
- Around line 966-969: Refactor the `enqueue_decode_jobs` function to accept a
`&mut Transaction` parameter (similar to the
`enqueue_jobs_for_verified_contract_tx` pattern in contracts.rs). Then move the
call to `enqueue_decode_jobs` to execute before `pg_tx.commit()` at line 966,
passing the mutable transaction reference to it. This ensures the decode job
enqueuing happens atomically within the transaction, so if it fails, the
watermark update in the transaction rolls back. Apply the same fix to the
`enqueue_decode_jobs` call in `write_batch_and_clear_failed_block` (around line
750) so it also enqueues jobs before the transaction commits rather than after.

---

Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/logs.rs`:
- Around line 190-230: Add unit test coverage for the
enrich_decoded_logs_with_known_signatures function by creating a #[cfg(test)]
mod tests block in the same file. The test should verify two key behaviors: that
existing event_name values are preserved when already populated, and that
unresolved topic0 values get filled with the correct name and event_signature
from the signature map. Mock the AppState and EventLog inputs to isolate the
function logic from database dependencies.

In `@backend/crates/atlas-server/src/api/handlers/proxy.rs`:
- Around line 100-104: In the non-proxy branch of the handler (the else block
starting around line 100), the code redundantly calls load_combined_abi which
re-performs proxy slot checks and lookups that were already completed in
determining proxy == None. Instead of calling load_combined_abi again in the
else branch, directly reuse the ABI that was already resolved or loaded during
the prior proxy detection logic to avoid duplicate external work and RPC calls.

In `@backend/crates/atlas-server/src/contract_abi.rs`:
- Around line 20-230: Add a `#[cfg(test)] mod tests` block at the end of the
contract_abi.rs file to cover the new ABI and proxy detection logic. Create unit
tests for the `merge_abis` function that verify behavior when both ABIs are
present, when only one is present, and when neither is present. Additionally,
create tests for the `read_address_slot` function that mock RPC responses for
non-zero addresses (success case), zero addresses (should return None), and
error responses from the RPC endpoint to ensure the function correctly parses
storage slots and handles edge cases. These tests should not require actual
database or RPC calls; use mock data or mocking libraries as appropriate.
🪄 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: e0cb6df0-07d7-4e0e-a86a-2bdaca190238

📥 Commits

Reviewing files that changed from the base of the PR and between 8f89538 and 036b3ab.

📒 Files selected for processing (20)
  • backend/crates/atlas-common/src/types.rs
  • backend/crates/atlas-server/src/api/handlers/contracts.rs
  • backend/crates/atlas-server/src/api/handlers/logs.rs
  • backend/crates/atlas-server/src/api/handlers/proxy.rs
  • backend/crates/atlas-server/src/contract_abi.rs
  • backend/crates/atlas-server/src/event_log_decode.rs
  • backend/crates/atlas-server/src/indexer/event_log_decode_worker.rs
  • backend/crates/atlas-server/src/indexer/gap_fill_worker.rs
  • backend/crates/atlas-server/src/indexer/indexer.rs
  • backend/crates/atlas-server/src/indexer/mod.rs
  • backend/crates/atlas-server/src/lib.rs
  • backend/crates/atlas-server/src/main.rs
  • backend/crates/atlas-server/tests/integration/addresses.rs
  • backend/crates/atlas-server/tests/integration/logs.rs
  • backend/crates/atlas-server/tests/integration/main.rs
  • backend/crates/atlas-server/tests/integration/schema.rs
  • backend/crates/atlas-server/tests/integration/transactions.rs
  • backend/migrations/20260528000001_event_log_decode_state.sql
  • frontend/src/types/index.ts
  • frontend/src/utils/abiDecode.ts

Comment on lines +50 to +55
fn serialize_bytes_as_hex<S>(bytes: &[u8], serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("0x{}", hex::encode(bytes)))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add in-file unit tests for serialize_bytes_as_hex.

This introduces new serialization logic but the file has no #[cfg(test)] mod tests coverage. Please add unit tests here for at least empty bytes ("0x") and a known byte vector ("0x...") to lock the contract.

Proposed test block
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde::Serialize;
+
+    #[derive(Serialize)]
+    struct Wrap<'a> {
+        #[serde(serialize_with = "serialize_bytes_as_hex")]
+        bytes: &'a [u8],
+    }
+
+    #[test]
+    fn serialize_bytes_as_hex_empty() {
+        let v = Wrap { bytes: &[] };
+        let s = serde_json::to_string(&v).unwrap();
+        assert_eq!(s, r#"{"bytes":"0x"}"#);
+    }
+
+    #[test]
+    fn serialize_bytes_as_hex_non_empty() {
+        let v = Wrap { bytes: &[0xde, 0xad, 0xbe, 0xef] };
+        let s = serde_json::to_string(&v).unwrap();
+        assert_eq!(s, r#"{"bytes":"0xdeadbeef"}"#);
+    }
+}

As per coding guidelines, "backend/**/*.rs: Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file" and "backend/crates/**/*.rs: Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file; run with cargo test --workspace".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn serialize_bytes_as_hex<S>(bytes: &[u8], serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("0x{}", hex::encode(bytes)))
}
fn serialize_bytes_as_hex<S>(bytes: &[u8], serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("0x{}", hex::encode(bytes)))
}
#[cfg(test)]
mod tests {
use super::*;
use serde::Serialize;
#[derive(Serialize)]
struct Wrap<'a> {
#[serde(serialize_with = "serialize_bytes_as_hex")]
bytes: &'a [u8],
}
#[test]
fn serialize_bytes_as_hex_empty() {
let v = Wrap { bytes: &[] };
let s = serde_json::to_string(&v).unwrap();
assert_eq!(s, r#"{"bytes":"0x"}"#);
}
#[test]
fn serialize_bytes_as_hex_non_empty() {
let v = Wrap { bytes: &[0xde, 0xad, 0xbe, 0xef] };
let s = serde_json::to_string(&v).unwrap();
assert_eq!(s, r#"{"bytes":"0xdeadbeef"}"#);
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-common/src/types.rs` around lines 50 - 55, The
serialize_bytes_as_hex function lacks unit test coverage. Add a #[cfg(test)] mod
tests block at the end of the same file containing unit tests for the
serialize_bytes_as_hex function. Create at least two test cases: one testing
serialization of empty bytes (which should produce the output "0x") and another
testing serialization of a known byte vector (which should produce "0x" followed
by the hex-encoded representation of those bytes). Use serde_json or an
appropriate serializer in the test to verify the function produces the expected
hexadecimal string output.

Source: Coding guidelines

Comment on lines +26 to +30
let body = serde_json::json!({
"jsonrpc": "2.0",
"method": "eth_getStorageAt",
"params": [address, slot, "latest"],
"id": 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Resolve proxy implementation at the log block, not latest.

Using "latest" means full rescans and older pending logs can be decoded with the current implementation ABI after a proxy upgrade. Pass a block tag through proxy resolution or persist implementation history and select the ABI for each log’s block.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/contract_abi.rs` around lines 26 - 30, The
eth_getStorageAt RPC call in the contract_abi function currently uses a
hardcoded "latest" block tag when resolving proxy implementations, which causes
issues with decoding older logs after proxy upgrades. Replace the hardcoded
"latest" string with the actual log block number or implement proxy resolution
that uses the specific block where the log occurred. This ensures the correct
implementation ABI is selected for each log based on its block timestamp rather
than always using the current implementation.

Comment on lines +38 to +49
let resp: serde_json::Value = client
.post(rpc_url)
.json(&body)
.send()
.await
.map_err(|e| AtlasError::Rpc(format!("eth_getStorageAt failed: {e}")))?
.json()
.await
.map_err(|e| AtlasError::Rpc(format!("failed to parse eth_getStorageAt response: {e}")))?;

let raw = resp.get("result").and_then(|r| r.as_str()).unwrap_or("0x");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate JSON-RPC errors instead of treating them as empty slots.

A JSON-RPC error response has no result, so unwrap_or("0x") returns Ok(None) and can misclassify proxies as direct contracts.

🐛 Proposed fix
-    let resp: serde_json::Value = client
+    let response = client
         .post(rpc_url)
         .json(&body)
         .send()
         .await
-        .map_err(|e| AtlasError::Rpc(format!("eth_getStorageAt failed: {e}")))?
+        .map_err(|e| AtlasError::Rpc(format!("eth_getStorageAt failed: {e}")))?
+        .error_for_status()
+        .map_err(|e| AtlasError::Rpc(format!("eth_getStorageAt HTTP error: {e}")))?;
+
+    let resp: serde_json::Value = response
         .json()
         .await
         .map_err(|e| AtlasError::Rpc(format!("failed to parse eth_getStorageAt response: {e}")))?;
 
-    let raw = resp.get("result").and_then(|r| r.as_str()).unwrap_or("0x");
+    if let Some(error) = resp.get("error") {
+        return Err(AtlasError::Rpc(format!(
+            "eth_getStorageAt returned error: {error}"
+        )));
+    }
+
+    let raw = resp
+        .get("result")
+        .and_then(|r| r.as_str())
+        .ok_or_else(|| {
+            AtlasError::Rpc("eth_getStorageAt response missing string result".to_string())
+        })?;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/contract_abi.rs` around lines 38 - 49, The
code currently uses unwrap_or("0x") on the response's "result" field, which
silently defaults to "0x" when the result is missing. However, a JSON-RPC error
response contains an "error" field instead of a "result" field, so this approach
masks actual errors and misclassifies failed requests. Instead of using
unwrap_or("0x") directly on the result, first check if the resp contains an
"error" field using resp.get("error"), and if it does, extract and propagate
that error as an AtlasError. Only return the default "0x" value when both the
"result" and "error" fields are absent from the response.

Comment on lines +319 to +323
ON CONFLICT (address) DO UPDATE SET
full_rescan = event_log_decode_jobs.full_rescan OR EXCLUDED.full_rescan,
requested_at = EXCLUDED.requested_at,
updated_at = EXCLUDED.updated_at,
error_message = NULL",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect decode-job schema and worker claim logic.
# Expectation: If retry/lock metadata gates job selection, the upserts in event_log_decode.rs should reset it on conflict.

rg -n -C4 'event_log_decode_jobs|retry_count|max_retries|locked_at|claimed|SKIP LOCKED|error_message' --iglob '*.rs' --iglob '*.sql'

Repository: evstack/atlas

Length of output: 50369


🏁 Script executed:

# Check if event_log_decode_worker has any max retry limit or filtering
rg -n 'max_retries|retry_count|RETRY' backend/crates/atlas-server/src/indexer/event_log_decode_worker.rs

Repository: evstack/atlas

Length of output: 139


Consider resetting retry_count to 0 when re-enqueueing a decode job.

On re-enqueue (lines 319–323, 358–362), these upserts clear error_message = NULL but leave retry_count unchanged. When an ABI upload or full-rescan request is issued for a previously failed address, the stale retry_count should be reset to 0 to reflect a fresh attempt. While the worker uses pessimistic locking (FOR UPDATE SKIP LOCKED) rather than retry-count filtering, resetting it maintains semantic correctness and prevents confusing observability metrics.

Add retry_count = 0 to both upsert SET clauses alongside error_message = NULL.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/event_log_decode.rs` around lines 319 - 323,
The ON CONFLICT DO UPDATE SET clauses in the upsert statements (appearing around
lines 319-323 and 358-362) are clearing the error_message field but not
resetting the retry_count to 0 when a decode job is re-enqueued. This leaves
stale retry counts that no longer reflect the fresh attempt. Add `retry_count =
0` to both SET clauses in addition to the existing `error_message = NULL` to
ensure both fields are properly reset on re-enqueue, maintaining semantic
correctness and accurate observability metrics.

Comment on lines 966 to +969
pg_tx.commit().await?;

enqueue_decode_jobs(pool, &event_log_addresses, false).await?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Critical: enqueue_decode_jobs must be called inside the transaction before commit.

The transaction commits at line 966, updating the last_indexed_block watermark (lines 941–950). Then enqueue_decode_jobs is called at line 968 after the commit. If enqueue_decode_jobs fails:

  1. The error propagates via ? and crashes the indexer.
  2. run_with_retry restarts the indexer, which calls get_start_block() and reads the already-updated watermark.
  3. The indexer resumes from the next block, permanently skipping the blocks whose decode jobs were not enqueued.
  4. No automatic rescan mechanism exists to recover the lost jobs.

This breaks the decode pipeline's correctness guarantee. The blocks are indexed but never decoded.

Correct pattern (as seen in contracts.rs lines 260–290):

let mut tx = pool.begin().await?;
// ... inserts ...
enqueue_jobs_tx(&mut tx, ...).await?;  // inside transaction
tx.commit().await?;

If the enqueue fails, the transaction rolls back atomically, leaving the watermark unchanged so the indexer retries the same blocks.

Recommended fix: Refactor enqueue_decode_jobs to accept &mut Transaction (similar to enqueue_jobs_for_verified_contract_tx) and call it before pg_tx.commit() at line 966. Alternatively, move the watermark update after the enqueue call so failures trigger a full batch retry.

The same issue affects write_batch_and_clear_failed_block (line 750), which deletes the failed_blocks row inside the transaction (lines 953–964), then calls enqueue outside. If enqueue fails, the row is deleted but no job is created, and the gap-fill worker cannot retry.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/crates/atlas-server/src/indexer/indexer.rs` around lines 966 - 969,
Refactor the `enqueue_decode_jobs` function to accept a `&mut Transaction`
parameter (similar to the `enqueue_jobs_for_verified_contract_tx` pattern in
contracts.rs). Then move the call to `enqueue_decode_jobs` to execute before
`pg_tx.commit()` at line 966, passing the mutable transaction reference to it.
This ensures the decode job enqueuing happens atomically within the transaction,
so if it fails, the watermark update in the transaction rolls back. Apply the
same fix to the `enqueue_decode_jobs` call in
`write_batch_and_clear_failed_block` (around line 750) so it also enqueues jobs
before the transaction commits rather than after.

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