fix: handle custom RPC log formats in SafeFactory deployment (#650) #1279
Conversation
…obal#650) This fixes an issue where SafeFactory.deploySafe() would incorrectly throw 'SafeProxy was not deployed correctly' on custom chains due to variations in how different RPC providers format event logs. - Added robust event log parsing with multiple fallback mechanisms - Added comprehensive test suite for different log formats - Handles both v1.0.0 and v1.3.0+ Safe versions - Supports logs with missing/null topics - Handles case sensitivity in addresses Fixes safe-global#650
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if (args && args.length) return args[0] as string | ||
| } | ||
| } catch (e) { | ||
| console.log('Fast path failed:', e) |
There was a problem hiding this comment.
Bug: Debug Output Polluting Production Console
A console.log statement was left in the production code that logs error details when the fast path for event parsing fails. This debugging code pollutes console output in production environments and should be removed or replaced with proper error handling.
|
🤖 AUTOMATED NOTICE — Action Required This PR fixes #650 (SafeFactory failing to detect ProxyCreation on some custom RPCs). Relevant tests covering this change passed locally (protocol-kit tests: 48 passing). However, the CLA Assistant check is blocking merge because one or more committers have not signed the Contributor License Agreement (CLA). What to do (quick):
Detected committers (please verify): Germán Martínez, Dani Somoza, Yago Pérez Vázquez, Daniel (dasanra), Richard Meissner, dependabot (automated). See the CLA Assistant details for the full list. If you want an automated author-rewrite: reply with and supply the canonical author name/email to use, and confirm is OK. I will then perform a safe rewrite and post an update. Generated: Automated-style reminder to speed review & unblock merge. Confidence: high. Thank you! |
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if (isAddress(proxyCreationAddress)) { | ||
| // Return extracted address if it matches the expected address format | ||
| return proxyCreationAddress | ||
| } |
There was a problem hiding this comment.
Bug: Fallback returns wrong address for Safe v1.4.1+
For Safe v1.4.1+, the ProxyCreation event has the proxy address as an indexed parameter, which means it's stored in topics[1], not in the data field. The data field only contains the singleton address. When the fast path fails for v1.4.1+ deployments, the manual extraction fallback at log.data.slice(26, 66) extracts from the wrong field, returning the singleton address instead of the deployed proxy address.
1 participant
This fixes an issue where SafeFactory.deploySafe() would incorrectly throw 'SafeProxy was not deployed correctly' on custom chains due to variations in how different RPC providers format event logs.
Fixes #650
What it solves
Resolves #
How this PR fixes it
Note
Strengthens SafeProxy address extraction with topic-based fast path and robust fallbacks; adds e2e tests for multiple versions and malformed logs.
packages/protocol-kit/src/contracts/utils.ts):getSafeAddressFromDeploymentTx:ProxyCreation.log.data(bytes 12–32) with validation, then ABI decode per-log.logs; final explicit error if no address found.packages/protocol-kit/tests/e2e/getSafeAddressFromDeploymentTx.test.ts):Written by Cursor Bugbot for commit c87953c. This will update automatically on new commits. Configure here.