-
Notifications
You must be signed in to change notification settings - Fork 294
fix: handle custom RPC log formats in SafeFactory deployment (#650) #1279
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -444,27 +444,64 @@ export function getSafeAddressFromDeploymentTx( | |
| txReceipt: FormattedTransactionReceipt, | ||
| safeVersion: SafeVersion | ||
| ): string { | ||
| const eventHash = toEventHash(getProxyCreationEvent(safeVersion)) | ||
| const proxyCreationEvent = txReceipt?.logs.find((event) => event.topics[0] === eventHash) | ||
|
|
||
| if (!proxyCreationEvent) { | ||
| if (!txReceipt?.logs?.length) { | ||
| throw new Error('SafeProxy was not deployed correctly') | ||
| } | ||
|
|
||
| const { data, topics } = proxyCreationEvent | ||
| // Try to find the proxy creation event by topic first (fast path) | ||
| try { | ||
| const eventHash = toEventHash(getProxyCreationEvent(safeVersion)) | ||
| const proxyCreationEvent = txReceipt?.logs.find((event) => event.topics?.[0] === eventHash) | ||
|
|
||
| const { args } = decodeEventLog({ | ||
| abi: parseAbi([getProxyCreationEvent(safeVersion)]), | ||
| eventName: 'ProxyCreation', | ||
| data, | ||
| topics | ||
| }) | ||
| if (proxyCreationEvent) { | ||
| const { data, topics } = proxyCreationEvent | ||
|
|
||
| if (!args || !args.length) { | ||
| throw new Error('SafeProxy was not deployed correctly') | ||
| const { args } = decodeEventLog({ | ||
| abi: parseAbi([getProxyCreationEvent(safeVersion)]), | ||
| eventName: 'ProxyCreation', | ||
| data, | ||
| topics | ||
| }) | ||
|
|
||
| if (args && args.length) return args[0] as string | ||
| } | ||
| } catch (e) { | ||
| console.log('Fast path failed:', e) | ||
| } | ||
|
|
||
| // Fallback: some RPCs or chains may return logs in a slightly different format | ||
| // Try each log with manual address extraction first, then ABI decoding | ||
| for (const log of txReceipt.logs) { | ||
| // Skip obviously invalid logs | ||
| if (!log.data || log.data === '0x') continue | ||
|
|
||
| // Try to find the address in the data field, regardless of version | ||
| if (log.data !== '0x') { | ||
| // Slice out bytes 12-32 (20 bytes) which should contain the address | ||
| // This matches both v1.3.0+ with padding and v1.0.0 without padding | ||
| const proxyCreationAddress = '0x' + log.data.slice(26, 66).toLowerCase() | ||
| if (isAddress(proxyCreationAddress)) { | ||
| // Return extracted address if it matches the expected address format | ||
| return proxyCreationAddress | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Fallback returns wrong address for Safe v1.4.1+For Safe v1.4.1+, the |
||
| } | ||
|
|
||
| // Try ABI decoding as last resort | ||
| try { | ||
| const { args } = decodeEventLog({ | ||
| abi: parseAbi([getProxyCreationEvent(safeVersion)]), | ||
| eventName: 'ProxyCreation', | ||
| data: log.data, | ||
| topics: log.topics || [] | ||
| }) | ||
|
|
||
| if (args && args.length) return args[0] as string | ||
| } catch (e) { | ||
| // ignore and continue trying other logs | ||
| } | ||
| } | ||
|
|
||
| return args[0] as string | ||
| throw new Error('SafeProxy was not deployed correctly') | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import { describe } from 'mocha'; | ||
| import chai from 'chai'; | ||
| import { getSafeAddressFromDeploymentTx } from '@safe-global/protocol-kit/contracts/utils'; | ||
| import type { FormattedTransactionReceipt, Log } from 'viem'; | ||
|
|
||
| const { expect } = chai; | ||
|
|
||
| describe('getSafeAddressFromDeploymentTx', () => { | ||
| const mockSafeAddress = '0x123456789A123456789A123456789A123456789A'; | ||
|
|
||
| const createMockLog = ( | ||
| topics: `0x${string}`[] | null = [], | ||
| data: `0x${string}` = '0x' | ||
| ): Log<bigint, number, false> => ({ | ||
| address: '0x1234567890123456789012345678901234567890', | ||
| topics: topics as [`0x${string}`], | ||
| data, | ||
| blockHash: '0x1234567890123456789012345678901234567890123456789012345678901234', | ||
| blockNumber: 1n, | ||
| transactionHash: '0x1234567890123456789012345678901234567890123456789012345678901234', | ||
| transactionIndex: 0, | ||
| logIndex: 0, | ||
| removed: false | ||
| }); | ||
|
|
||
| const createMockReceipt = (logs: Log<bigint, number, false>[]): FormattedTransactionReceipt => ({ | ||
| blockHash: '0x1234567890123456789012345678901234567890123456789012345678901234', | ||
| blockNumber: 1n, | ||
| contractAddress: null, | ||
| cumulativeGasUsed: 100000n, | ||
| effectiveGasPrice: 1000000000n, | ||
| from: '0x0000000000000000000000000000000000000001', | ||
| gasUsed: 50000n, | ||
| logs, | ||
| logsBloom: '0x00', | ||
| status: 'success', | ||
| to: '0x0000000000000000000000000000000000000002', | ||
| transactionHash: '0x1234567890123456789012345678901234567890123456789012345678901234', | ||
| transactionIndex: 0, | ||
| type: 'eip1559' | ||
| }); | ||
|
|
||
| describe('with Safe v1.3.0', () => { | ||
| const eventTopic = '0xa38789425dbeee0239e16ff2d2567e31720127fbc6430758c1a4efc6aef29f80' as const; | ||
| const proxyData = `0x000000000000000000000000${mockSafeAddress.slice(2)}0000000000000000000000000000000000000000000000000000000000000000` as `0x${string}`; | ||
|
|
||
| it('should extract address from standard ProxyCreation event format', () => { | ||
| const mockLog = createMockLog([eventTopic], proxyData); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| const extractedAddress = getSafeAddressFromDeploymentTx(receipt, '1.3.0'); | ||
| expect(extractedAddress.toLowerCase()).to.equal(mockSafeAddress.toLowerCase()); | ||
| }); | ||
|
|
||
| it('should handle logs without proper topics but valid data', () => { | ||
| const mockLog = createMockLog([], proxyData); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| const extractedAddress = getSafeAddressFromDeploymentTx(receipt, '1.3.0'); | ||
| expect(extractedAddress.toLowerCase()).to.equal(mockSafeAddress.toLowerCase()); | ||
| }); | ||
|
|
||
| it('should handle custom RPC logs with null topics', () => { | ||
| const mockLog = createMockLog(null, proxyData); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| const extractedAddress = getSafeAddressFromDeploymentTx(receipt, '1.3.0'); | ||
| expect(extractedAddress.toLowerCase()).to.equal(mockSafeAddress.toLowerCase()); | ||
| }); | ||
| }); | ||
|
|
||
| describe('with Safe v1.0.0', () => { | ||
| const eventTopic = '0x4f51faf6c4561ff95f067657e43439f0f856d97c04d9ec9070a6199ad418e235' as const; | ||
| const proxyData = `0x000000000000000000000000${mockSafeAddress.slice(2)}` as `0x${string}`; | ||
|
|
||
| it('should extract address from legacy ProxyCreation event format', () => { | ||
| const mockLog = createMockLog([eventTopic], proxyData); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| const extractedAddress = getSafeAddressFromDeploymentTx(receipt, '1.0.0'); | ||
| expect(extractedAddress.toLowerCase()).to.equal(mockSafeAddress.toLowerCase()); | ||
| }); | ||
|
|
||
| it('should handle legacy logs without proper topics', () => { | ||
| const mockLog = createMockLog([], proxyData); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| const extractedAddress = getSafeAddressFromDeploymentTx(receipt, '1.0.0'); | ||
| expect(extractedAddress.toLowerCase()).to.equal(mockSafeAddress.toLowerCase()); | ||
| }); | ||
| }); | ||
|
|
||
| describe('error cases', () => { | ||
| it('should throw if no valid ProxyCreation event is found', () => { | ||
| const mockLog = createMockLog([], '0x'); | ||
| const receipt = createMockReceipt([mockLog]); | ||
|
|
||
| expect(() => getSafeAddressFromDeploymentTx(receipt, '1.3.0')) | ||
| .to.throw('SafeProxy was not deployed correctly'); | ||
| }); | ||
|
|
||
| it('should throw if receipt has no logs', () => { | ||
| const receipt = createMockReceipt([]); | ||
| expect(() => getSafeAddressFromDeploymentTx(receipt, '1.3.0')) | ||
| .to.throw('SafeProxy was not deployed correctly'); | ||
| }); | ||
|
|
||
| it('should throw if logs array is null', () => { | ||
| const receipt = { | ||
| ...createMockReceipt([]), | ||
| logs: null | ||
| } as unknown as FormattedTransactionReceipt; | ||
|
|
||
| expect(() => getSafeAddressFromDeploymentTx(receipt, '1.3.0')) | ||
| .to.throw('SafeProxy was not deployed correctly'); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Debug Output Polluting Production Console
A
console.logstatement 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.