feat: updated deposit to trigger #212
Conversation
WalkthroughThe deposit API transitions from multi-parameter to payload-based architecture. IFeesManager interface signature changes to accept bytes calldata payload. Credit.sol decodes payload parameters and converts failed native withdrawals to credits. FeesPlug.sol encodes deposit data, sends through socket for payloadId tracking, and emits updated FeesDeposited event. Tests updated to encode parameters. Changes
Sequence DiagramsequenceDiagram
participant Test as Test/Client
participant FeesPlug
participant Socket
participant Credit as Credit/Vault
participant FeesPool
Test->>FeesPlug: _deposit(token, receiver, creditAmount, nativeAmount)
FeesPlug->>Socket: chainSlug = chainSlug()
FeesPlug->>FeesPlug: payload = encode(chainSlug, token, receiver, creditAmount, nativeAmount)
FeesPlug->>Socket: payloadId = sendPayload(payload)
activate Socket
Socket-->>FeesPlug: payloadId (bytes32)
deactivate Socket
FeesPlug->>FeesPlug: emit FeesDeposited(..., payloadId)
Note over FeesPlug,Credit: Later: Cross-chain delivery
FeesPlug->>Credit: deposit(encoded_payload)
activate Credit
Credit->>Credit: decode payload
Credit->>Credit: mint creditAmount to receiver
alt nativeAmount > 0
Credit->>FeesPool: withdraw(nativeAmount)
alt Withdrawal succeeds
FeesPool-->>Credit: nativeAmount transferred
else Withdrawal fails
Credit->>Credit: mint nativeAmount credits (additional)
Credit->>Credit: nativeAmount = 0
end
end
Credit->>Credit: emit Deposited(updated amounts)
deactivate Credit
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
contracts/evmx/interfaces/IFeesManager.sol (1)
6-6: Document the expected payload format.The interface should document the expected payload structure for implementers. Consider adding a NatSpec comment specifying the encoding format.
Apply this diff:
interface IFeesManager { + /// @param payload_ Encoded as: (uint32 chainSlug, address token, address receiver, uint256 creditAmount, uint256 nativeAmount) function deposit(bytes calldata payload_) external;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
contracts/evmx/fees/Credit.sol(2 hunks)contracts/evmx/interfaces/IFeesManager.sol(1 hunks)contracts/evmx/interfaces/IFeesPlug.sol(1 hunks)contracts/evmx/plugs/FeesPlug.sol(1 hunks)test/SetupTest.t.sol(2 hunks)
🔇 Additional comments (4)
contracts/evmx/interfaces/IFeesPlug.sol (1)
6-12: LGTM - Breaking change for event listeners.The addition of
payloadIdto theFeesDepositedevent enables tracking of deposit payloads. Off-chain systems listening to this event will need to be updated to handle the new parameter.test/SetupTest.t.sol (1)
221-221: LGTM - EVMX configuration added.The
setEvmxConfigcall enables trigger payload handling for the EVMX slug, which is necessary for the new payload-based deposit flow.contracts/evmx/fees/Credit.sol (1)
134-144: LGTM - Robust fallback for failed native withdrawals.The logic correctly handles failed native withdrawals by converting them to credits. This ensures users always receive value even if the native transfer fails, and the emitted event accurately reflects the final state.
contracts/evmx/plugs/FeesPlug.sol (1)
71-81: Transfer-before-payload sequencing exposes tokens to temporary lock if Socket fails; recovery path exists but should be validated.The concern is valid: line 70 transfers tokens unconditionally before line 81 calls
socket__.sendPayload(). IfsendPayload()reverts (via_verifyPlugSwitchboardorISwitchboard.processPayloadfailures), tokens sit locked in the contract until manually rescued.This is by design—the contract functions as a fee escrow with async Socket synchronization. Recovery is available via
rescueFunds()(line 147), protected byRESCUE_ROLE, but depends on:
- The RESCUE_ROLE being properly managed
- Admins monitoring and acting on failed deposits
- The role not being revoked unexpectedly
Verify: Is this transfer-first pattern intentional and are the RESCUE_ROLE permissions maintained? If
sendPayload()failures are expected to be rare, the current flow is acceptable with the recovery mechanism in place. If failures are common, consider wrappingsendPayload()in try/catch or reordering to create payload first.
| emit Deposited(chainSlug_, address(token), user_, credits_, native_); | ||
| hoax(watcherEOA); | ||
| feesManager.deposit(chainSlug_, address(token), user_, native_, credits_); | ||
| feesManager.deposit(abi.encode(chainSlug_, address(token), user_, native_, credits_)); |
There was a problem hiding this comment.
Fix critical parameter order mismatch.
The test encodes parameters as (chainSlug_, address(token), user_, native_, credits_) with native_ before credits_, but FeesPlug.sol line 76 encodes as (chainSlug_, token_, receiver_, creditAmount_, nativeAmount_) with creditAmount_ before nativeAmount_. This mismatch will cause the decoder in Credit.sol to assign values to the wrong variables, breaking the deposit flow.
Apply this diff to fix the parameter order:
- feesManager.deposit(abi.encode(chainSlug_, address(token), user_, native_, credits_));
+ feesManager.deposit(abi.encode(chainSlug_, address(token), user_, credits_, native_));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| feesManager.deposit(abi.encode(chainSlug_, address(token), user_, native_, credits_)); | |
| feesManager.deposit(abi.encode(chainSlug_, address(token), user_, credits_, native_)); |
🤖 Prompt for AI Agents
In test/SetupTest.t.sol around line 434, the abi.encode call passes parameters
as (chainSlug_, address(token), user_, native_, credits_) but FeesPlug.sol
expects (chainSlug_, token_, receiver_, creditAmount_, nativeAmount_); swap the
last two arguments so the call becomes (chainSlug_, address(token), user_,
credits_, native_) to match FeesPlug/Credit.sol decoding and ensure
creditAmount_ and nativeAmount_ map correctly.
Summary by CodeRabbit
Release Notes
New Features
Improvements