Conversation
There was a problem hiding this comment.
Pull request overview
Introduces an initial DepositWaitingList contract that buffers user deposits and lets a relayer batch-apply them into the Bridge, plus accompanying deployment script, tests, and an ADR.
Changes:
- Added
DepositWaitingListcontract with deposit queueing, relayer-only apply flow, admin callContract config, and token rescue. - Added Foundry tests covering deposits, batch application, token mismatch, limits/paused states, and admin actions.
- Added ADR documenting the intended waiting-list design and rationale.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| protocol/src/DepositWaitingList.sol | Implements the waiting-list queue, relayer batch application into Bridge, and admin operations. |
| protocol/test/DepositWaitingList.t.sol | Adds test coverage for the waiting-list behaviors and bridge interactions. |
| protocol/spec/adr-001-deposit-waiting-list.md | Documents the motivation/design for the waiting list approach. |
| protocol/script/DeployDepositWaitingList.s.sol | Adds a deployment script for the new contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #### Deposit Flow (User-Facing) | ||
|
|
||
| ``` | ||
| User → DepositWaitingList.deposit(token, amount, to) |
There was a problem hiding this comment.
The ADR describes emitting a Deposited(...) event, but the implementation emits WaitingDepositCreated(...). Update the ADR to match the implemented event name (or rename the event in the contract), otherwise the spec is misleading for integrators/indexers.
| /// @param permit Optional EIP-2612 permit data (empty bytes if none). | ||
| /// @return depositId The sequential ID assigned to this deposit. | ||
| function deposit(address token, uint256 amount, address to, bytes calldata permit) |
There was a problem hiding this comment.
The ADR specifies a deposit(..., bytes permit) interface (and earlier mentions an EIP-2612 permit flow), but the contract only implements deposit(address token, uint256 amount, address to). Either (a) implement the permit-based deposit variant, or (b) revise the ADR/interface section to reflect the initial version so the documented API matches what’s deployed.
| /// @param permit Optional EIP-2612 permit data (empty bytes if none). | |
| /// @return depositId The sequential ID assigned to this deposit. | |
| function deposit(address token, uint256 amount, address to, bytes calldata permit) | |
| /// @return depositId The sequential ID assigned to this deposit. | |
| function deposit(address token, uint256 amount, address to) |
protocol/src/DepositWaitingList.sol
Outdated
| function rescueTokens(address token, address to, uint256 amount) external onlyRole(DEFAULT_ADMIN_ROLE) { | ||
| IERC20(token).safeTransfer(to, amount); | ||
| } |
There was a problem hiding this comment.
rescueTokens allows the admin to transfer out tokens that may belong to users with pending (not-yet-applied) deposits, effectively enabling withdrawal of escrowed funds. If the intent is only to recover “excess”/accidental transfers, consider tracking total pending balances per token and restricting rescue to balanceOf(this) - pending[token], or limit rescues to tokens that are not used for deposits.
…rawals to supply deposit data that is verified against the hashes
1 participant
No description provided.