Closed skosito closed 1 month ago
[!CAUTION]
Review failed
The pull request is closed.
The overall change involves reordering parameters in the ERC20Custody
contract's withdrawal functions for improved clarity and consistency. This adjustment affects function signatures, emitted events, and related tests across multiple files. The new order prioritizes the recipient address over the token address, enhancing usability and readability while maintaining the contract's underlying logic.
Files | Change Summary |
---|---|
v2/pkg/.../erc20custody.go , v2/src/.../ERC20Custody.sol , v2/test/.../ERC20Custody.t.sol , v2/typechain-types/.../ERC20Custody.ts , v2/typechain-types/.../ERC20CustodyEchidnaTest.ts , v2/typechain-types/.../factories/ERC20Custody__factory.ts |
Parameter order changes in withdrawal methods (withdraw , withdrawAndCall , withdrawAndRevert ) to improve clarity by placing the recipient address first. Updates to emitted events and tests are included. |
v2/test/.../GatewayZEVM.t.sol |
Renaming and reordering test functions for clarity in zero address and zero amount conditions, enhancing maintainability. |
v2/typechain-types/.../factories/GatewayEVMEchidnaTest__factory.ts |
Updates to bytecode and constructor parameters for improved type safety and clarity, reflecting optimizations in the contract's implementation. |
sequenceDiagram
participant User
participant ERC20Custody
participant Token
User->>ERC20Custody: withdraw(to, token, amount)
ERC20Custody->>Token: transfer(to, amount)
ERC20Custody-->>User: Emit Withdraw Event
🐰 In the meadow where tokens play,
A rabbit hops with joy today.
Parameters danced, swapped around,
Clarity found on the ground.
With every change, we cheer and sing,
For simpler calls make our hearts spring! 🌼
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
@skosito another one that I think we could address in this PR: https://github.com/zeta-chain/protocol-contracts/issues/306
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 83.27%. Comparing base (
6486dd8
) to head (1df9eec
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@skosito another one that I think we could address in this PR: #306
@lumtis done, i changed all v2 events to be in past tense
swap token and recipient args in erc20 custody to keep compatibility with v1
did for all custody methods so that contract methods are consistent, no need to swap these args in other contracts imo, but if there is a need we can do it in other issue
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests