Replace mutex with sync.Map for ABI caching

[AmineAfia]

TL;DR

Replace mutex-protected map with sync.Map for ABI caching to improve concurrency.

What changed?

• Replaced the manual mutex-protected map implementation with Go's built-in sync.Map for ABI caching
• Modified GetABIForContractWithCache to use sync.Map methods (Load, Store) instead of manual locking
• Removed mutex parameters from function signatures in DecodeLogs and DecodeTransactions
• Simplified the concurrent processing of logs and transactions by eliminating explicit mutex handling

How to test?

1. Run concurrent processing of logs and transactions to verify that ABI caching works correctly
2. Verify that decoded logs and transactions contain the expected data
3. Check for any race conditions or deadlocks that might have been introduced

Why make this change?

The previous implementation used a manual mutex to protect a map during concurrent access, which can lead to contention and reduced performance. sync.Map is specifically designed for concurrent use cases where items are frequently read but infrequently updated, which matches the ABI caching pattern. This change improves code safety and performance by using a more appropriate concurrency primitive.

Summary by CodeRabbit

• Refactor
  • Improved concurrency handling for ABI caching, resulting in more reliable and efficient processing of logs and transactions.
  • Simplified internal logic for decoding logs and transactions, reducing potential synchronization issues.

[coderabbitai]

Walkthrough

Thread-safe caching for contract ABIs was refactored across the codebase by replacing manual map and mutex usage with Go's sync.Map. This change affects functions handling ABI retrieval and decoding of logs and transactions, simplifying concurrency control and updating related function signatures and internal logic accordingly.

Changes

File(s)	Change Summary
internal/common/abi.go	Refactored ABI cache to use sync.Map for concurrency; updated GetABIForContractWithCache signature.
internal/common/log.go	Updated log decoding to use sync.Map for ABI cache; removed mutex usage in decoding logic; improved panic recovery logging.
internal/common/transaction.go	Updated transaction decoding to use sync.Map for ABI cache; removed mutex usage in decoding logic.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant DecodeFunc
    participant ABI_Cache(sync.Map)
    participant ABI_Loader

    Caller->>DecodeFunc: Start decoding (log/transaction)
    DecodeFunc->>ABI_Cache: Load ABI for contract
    alt ABI found in cache
        ABI_Cache-->>DecodeFunc: Return cached ABI
    else ABI not found
        DecodeFunc->>ABI_Loader: Fetch ABI from source
        alt Fetch success
            ABI_Loader-->>DecodeFunc: Return ABI
            DecodeFunc->>ABI_Cache: Store ABI in cache
        else Fetch error
            ABI_Loader-->>DecodeFunc: Return nil
            DecodeFunc->>ABI_Cache: Store nil in cache
        end
    end
    DecodeFunc-->>Caller: Return decoded result

Loading

Possibly related PRs

• catch panic from decodeLogFunc #215: Related refactor of DecodeLogs using sync.Map for ABI caching and enhanced panic recovery logging.
• improve abi parsing #213: Related concurrency improvement by adding mutex in GetABIForContract for thread-safe ABI JSON decoding.

Suggested reviewers

• AmineAfia
• nischitpra

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 666bea0 and 5196a3f.

📒 Files selected for processing (3)

• internal/common/abi.go (1 hunks)
• internal/common/log.go (2 hunks)
• internal/common/transaction.go (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/common/transaction.go
• internal/common/abi.go

🧰 Additional context used

🧬 Code Graph Analysis (1)

internal/common/log.go (1)

internal/common/abi.go (1)

• GetABIForContractWithCache (37-51)

🔇 Additional comments (6)

internal/common/log.go (6)

90-90: LGTM: Proper sync.Map initialization for concurrent ABI caching.

The change from a plain map with manual mutex to sync.Map is appropriate for this use case where multiple goroutines will be accessing the ABI cache concurrently.

---

92-94: LGTM: Function signature simplified correctly.

The removal of the mutex parameter from decodeLogFunc aligns with the new sync.Map approach, and the call to GetABIForContractWithCache now properly passes the sync.Map pointer instead of a mutex.

---

113-113: LGTM: Goroutine signature maintains correct parameter passing.

The goroutine function signature correctly captures the loop variables by value, preventing race conditions that could occur if variables were captured by reference.

---

118-120: Excellent improvement: Enhanced panic recovery with contextual logging.

The addition of txHash and logIndex to the panic recovery logging provides valuable debugging context that will help identify which specific log caused the panic.

---

124-124: LGTM: Proper fallback handling in panic recovery.

The assignment of a default DecodedLog with the original log data ensures that even if decoding fails with a panic, the result slice maintains the correct structure and the original log information is preserved.

---

128-130: LGTM: Function call and goroutine closure correctly updated.

The call to decodeLogFunc and the goroutine parameter passing are consistent with the simplified function signature that no longer requires mutex handling.

✨ Finishing Touches

• 📝 Generate Docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[AmineAfia]

• Replace mutex with sync.Map for ABI caching #218 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.