feat: replace Transfer event with Proxy event in TransferProxy#1394
feat: replace Transfer event with Proxy event in TransferProxy#1394mrice32 wants to merge 2 commits into
Conversation
This will help link depositIds to CCTP and other non Across txfers.
mrice32
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91d3c27196
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bytes32 depositor | ||
| bytes32 recipient, |
There was a problem hiding this comment.
deposit parameter list
The deposit signature is now syntactically invalid because bytes32 depositor is immediately followed by bytes32 recipient without a comma. This prevents Solidity from parsing the contract, so this commit cannot be compiled or deployed in any environment.
Useful? React with 👍 / 👎.
| uint32, // exclusivityParameter | ||
| bytes calldata message | ||
| ) external { | ||
| emit Proxy(depositor, depositNonce, inputToken, inputAmount); |
There was a problem hiding this comment.
depositNonce before emitting Proxy
unsafeDeposit emits Proxy(depositor, depositNonce, ...), but depositNonce is not declared as a named parameter in the function signature (it is still an unnamed uint256 argument). This introduces an undeclared identifier error and blocks compilation once parsing reaches this line.
Useful? React with 👍 / 👎.
- Add missing comma after depositor param in deposit() - Fix comma placement for depositor param in unsafeDeposit() - Name depositNonce param so it can be referenced in emit - Add .toAddress() conversions for bytes32 -> address event params - Update test to use new Proxy event signature Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
@codex review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c24af1b348
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uint32, // exclusivityParameter | ||
| bytes calldata message | ||
| ) external { | ||
| emit Proxy(depositor.toAddress(), depositNonce, inputToken.toAddress(), inputAmount); |
There was a problem hiding this comment.
The new Proxy log for unsafeDeposit emits only (depositor, depositNonce, inputToken, inputAmount), but Across unsafe deposit IDs are derived from keccak256(msgSender, depositor, depositNonce) (see SpokePool.getUnsafeDepositId). In any context where TransferProxy can be called by more than one contract/EOA (including direct calls), different callers can produce identical Proxy tuples, so indexers cannot uniquely map the event to one deposit ID and can be spoofed by replaying another depositor/nonce pair.
Useful? React with 👍 / 👎.
1 participant
This will help link depositIds to CCTP and other non Across txfers.