Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve CallTracer type detection for transaction tracing #2010

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

gabriel-aranha-cw
Copy link
Contributor

@gabriel-aranha-cw gabriel-aranha-cw commented Feb 12, 2025

User description

Enhance the default trace generation for CallTracer by dynamically determining the transaction type (CALL or CREATE) based on the presence of a 'to' address


PR Type

Enhancement, Bug fix


Description

  • Improve CallTracer type detection for transaction tracing

  • Dynamically determine transaction type (CALL or CREATE)

  • Base type on presence of 'to' address

  • Enhance default trace generation for CallTracer


Changes walkthrough 📝

Relevant files
Enhancement
evm.rs
Implement dynamic transaction type detection in CallTracer

src/eth/executor/evm.rs

  • Refactored default_trace function for CallTracer
  • Added logic to determine transaction type (CALL/CREATE)
  • Improved structure of CallFrame creation
  • Removed unnecessary .into() conversion
  • +14/-6   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Enhance the default trace generation for CallTracer by dynamically determining the transaction type (CALL or CREATE) based on the presence of a 'to' address
    Copy link

    github-actions bot commented Feb 12, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 098a812)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The new code doesn't handle potential errors when accessing tx.to(). Consider adding error handling or using expect() instead of unwrapping.

    let typ = match tx.to() {
        Some(_) => "CALL",
        None => "CREATE",
    }

    Copy link

    PR Code Suggestions ✨

    @gabriel-aranha-cw gabriel-aranha-cw marked this pull request as ready for review February 12, 2025 14:05
    Copy link

    Persistent review updated to latest commit 098a812

    Copy link

    PR Code Suggestions ✨

    @gabriel-aranha-cw gabriel-aranha-cw enabled auto-merge (squash) February 12, 2025 14:44
    @gabriel-aranha-cw gabriel-aranha-cw merged commit e2e1043 into main Feb 12, 2025
    39 checks passed
    @gabriel-aranha-cw gabriel-aranha-cw deleted the fix-add-case-contract-creation branch February 12, 2025 18:16
    @gabriel-aranha-cw
    Copy link
    Contributor Author

    Final benchmark:
    Run ID: bench-4135894373

    Git Info:

    Configuration:

    • Target Account Strategy: Default

    RPS Stats: Max: 1955.00, Min: 1191.00, Avg: 1778.59, StdDev: 72.59
    TPS Stats: Max: 1932.00, Min: 1634.00, Avg: 1726.93, StdDev: 82.32

    Plot: View Plot

    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.

    2 participants