fix: debug trace bridge call matching

[arnaubennassar]

🔄 Changes Summary

• Backports 5e75ba1a (fix: debug trace bridge call matching (#1605)) to release/0.10.
• Filters debug trace matching to actual CALL frames in bridgesync and claimsync, avoiding calldata extraction from DELEGATECALL, STATICCALL, CALLCODE, or missing-type frames.
• Adds call tracer frame type constants and includes the type field in decoded trace calls.
• Makes claim calldata extraction reject ambiguous distinct matching bridge calls while accepting duplicate identical matches.
• Extends bridge and claim sync tests for call-frame filtering, ambiguous calldata, and mocked debug traces.

⚠️ Breaking Changes

• 🛠️ Config: None
• 🔌 API/CLI: None
• 🗑️ Deprecated Features: None

📋 Config Updates

• 🧾 Diff/Config snippet: None

✅ Testing

• 🤖 Automatic: go test ./bridgesync ./claimsync
• 🖱️ Manual: Not run

🐞 Issues

• None

🔗 Related PRs

• Backport of fix: debug trace bridge call matching #1605

📝 Notes

• Cherry-pick applied cleanly on release/0.10.

[claude]

Claude finished @arnaubennassar's task in 1m 48s —— View job

---

Code Review Complete

Tasks

• Read all changed files
• Analyze core logic changes
• Review test coverage
• Check for bugs, security issues, and performance concerns
• Provide comprehensive feedback

---

🚀 What's New

This PR backports critical debug trace filtering logic to release/0.10 that ensures only actual CALL frames are processed when extracting bridge and claim transaction data. The fix prevents misinterpreting DELEGATECALL, STATICCALL, CALLCODE, or untyped frames as bridge/claim operations, which could lead to incorrect data extraction or security vulnerabilities.

Key changes:

• Adds call tracer frame type constants (CALL, DELEGATECALL, STATICCALL, etc.)
• Includes type field in Call struct for both bridgesync and claimsync
• Filters findCall to only match frames with Type == CallTypeCall
• Enhances claimsync to reject ambiguous calldata (multiple distinct matching bridge calls)
• Comprehensive test coverage for all edge cases

🐛 Bug Fixes

Security & Correctness Fix:

• bridgesync/downloader.go:575 & claimsync/downloader.go:468: Now correctly filters to only CALL type frames, preventing extraction from delegate calls that could have different security contexts
• claimsync/downloader.go:532-571: Prevents ambiguous claim calldata extraction by requiring exactly one matching bridge call per claim event

📋 Config Updates

None

⚠️ Breaking Changes

None - This is a backward-compatible bug fix that makes the behavior more correct

---

Review Feedback

🟢 Strengths

1. Excellent Security Fix - The filtering by call type is critical. DELEGATECALL frames execute in the caller's context, so extracting calldata from them could lead to security issues or incorrect attribution.

2. Comprehensive Test Coverage - The PR includes extensive test additions:

   • TestFindCallSkipsNonCallFrameTypes in both bridgesync and claimsync
   • Tests for ambiguous calldata scenarios
   • Updated all existing tests to include the Type field

3. Clear Constants - Well-documented call type constants make the code self-documenting.

4. Ambiguity Detection - The claimsync implementation correctly rejects scenarios where multiple distinct bridge calls match the same claim event, which could indicate an attack or data integrity issue.

🟡 Suggestions (Minor)

1. Missing Type Field Handling (bridgesync/downloader.go:575, claimsync/downloader.go:468)

   • Issue: The code checks currentCall.Type == CallTypeCall, but what if the Type field is empty (zero value)?
   • Observation: In tests, there are cases with {To: bridgeAddr, From: fromAddr} (no Type field), which would have empty string as Type
   • Impact: Empty Type would be treated as non-CALL and filtered out, which might be the intended behavior, but it's implicit
   • Recommendation: Consider explicit handling or add a test comment documenting this behavior

   // Current code:
   if currentCall.To == targetAddr && currentCall.Type == CallTypeCall {
   
   // Consider being explicit about missing type handling:
   if currentCall.To == targetAddr {
       if currentCall.Type != CallTypeCall && currentCall.Type != "" {
           // Log that we're skipping non-CALL frame
           continue
       }
       if currentCall.Type == "" {
           logger.Warnf("encountered call frame without type field, skipping for safety")
           continue
       }
       // ... proceed with CALL frame
   }

2. Error Message Precision (claimsync/downloader.go:566)

   • The error message says "found %d matching bridge calls" but it actually means "found %d distinct matching bridge calls"
   • Consider: "found %d distinct matching bridge calls (expected exactly 1)"

3. Performance Consideration (claimsync/downloader.go:575-582)

   • The hasClaimCandidate function uses reflect.DeepEqual which can be expensive
   • For most cases, there will only be 1-2 candidates, so impact is minimal
   • Consider: If performance becomes an issue, you could implement a custom equality check for Claim struct that's more efficient

🔴 Potential Issues

None found. The implementation is solid.

Code Quality Observations

1. Consistent Implementation: The same fix is applied consistently across both bridgesync and claimsync packages
2. Test Quality: Tests cover edge cases including:
   • Direct calls vs. indirect calls
   • Reverted vs. successful calls
   • Multiple claim candidates (same vs. different)
   • Mixed call frame types
3. Backward Compatibility: Existing mocked tests are updated to include Type: CallTypeCall, maintaining test validity

Security Analysis

✅ No security issues identified

The changes actually improve security by:

• Preventing DELEGATECALL frames from being processed (these execute in caller's context)
• Preventing STATICCALL frames from being mistaken for state-changing operations
• Detecting and rejecting ambiguous calldata scenarios

Performance Analysis

✅ Negligible performance impact

• The type check is a simple string comparison: O(1)
• The ambiguity detection adds minimal overhead (typically 1-2 candidates)
• The reflect.DeepEqual is only called when multiple candidates exist (rare case)

---

Summary

This is a high-quality, well-tested backport that fixes a critical issue with call frame type filtering. The implementation correctly prevents non-CALL frames from being processed, which is essential for security and correctness.

Recommendation: Approve ✅

The code is production-ready. The minor suggestions above are optional improvements, not blockers.

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.5% Coverage on New Code
0.6% Duplication on New Code

See analysis details on SonarQube Cloud