Skip to content

Conversation

@patricios-space
Copy link
Collaborator

@patricios-space patricios-space commented Dec 16, 2025

@patricios-space patricios-space force-pushed the fix/onramp/replace-withdrawable-with-withdrawFeeTokens branch 3 times, most recently from c96b00d to 1fa99df Compare December 17, 2025 15:33
Base automatically changed from testing/unit-tests to main December 19, 2025 15:21
@patricios-space patricios-space force-pushed the fix/onramp/replace-withdrawable-with-withdrawFeeTokens branch from efec888 to bcea881 Compare December 19, 2025 17:54
@patricios-space patricios-space marked this pull request as ready for review December 19, 2025 17:58
@patricios-space patricios-space requested a review from a team as a code owner December 19, 2025 17:58
Copilot AI review requested due to automatic review settings December 19, 2025 17:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a new withdrawFeeTokens function for the OnRamp contract that uses the feeAggregator address to collect fees. The implementation replaces the previous withdrawable pattern with a permissionless function that validates token types and enforces balance reserves.

Key changes:

  • Added reserve field to DynamicConfig to specify minimum balance that must remain on contract
  • Replaced Withdrawable_Withdraw message with OnRamp_WithdrawFeeTokens message
  • Added validation for empty fee tokens list and new error codes UnknownToken and InsufficientValue

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pkg/ccip/bindings/onramp/onramp.go Updated DynamicConfig struct to include Reserve field and added WithdrawFeeTokens struct with validation
pkg/ccip/bindings/onramp/onramp_test.go Added Reserve field to test configuration
pkg/ccip/bindings/onramp/exitcode_string.go Updated exit code mappings for new error types
deployment/ccip/sequence/deploy_ccip.go Added reserve field parsing for OnRamp deployment
deployment/ccip/cs_test_helpers.go Added Reserve value to test helper configuration
deployment/ccip/config/deploy.go Added Reserve field to OnRampParams
contracts/wrappers/ccip/OnRamp.ts Implemented withdrawFeeTokens message codec and updated DynamicConfig type
contracts/tests/ccip/router/Router.Setup.ts Added reserve field to OnRamp deployment setup
contracts/tests/ccip/onramp/OnRamp.withdrawFeeToken.spec.ts Added comprehensive test coverage for withdrawFeeTokens functionality
contracts/tests/ccip/onramp/OnRamp.spec.ts Removed Withdrawable tests and updated deployment helper usage
contracts/tests/ccip/onramp/OnRamp.setDynamicConfig.spec.ts Added reserve field to dynamic config tests
contracts/tests/ccip/onramp/OnRamp.send.spec.ts Updated to use new setup function signature
contracts/tests/ccip/onramp/OnRamp.getFee.spec.ts Updated to use new setup function signature
contracts/tests/ccip/onramp/OnRamp.generateMessageID.spec.ts Updated to use new setup function signature
contracts/tests/ccip/onramp/OnRamp.executorExited.spec.ts Updated to use new setup function signature
contracts/tests/ccip/onramp/OnRamp.applyDestChainConfigUpdates.spec.ts Updated to use new setup function signature
contracts/tests/ccip/onramp/OnRamp.Setup.ts Modified deployOnRampContract to return config object and updated setup function
contracts/contracts/ccip/onramp/types.tolk Added reserve field to DynamicConfig struct
contracts/contracts/ccip/onramp/messages.tolk Added OnRamp_WithdrawFeeTokens message and associated cost calculation
contracts/contracts/ccip/onramp/errors.tolk Added UnknownToken and InsufficientValue error types
contracts/contracts/ccip/onramp/contract.tolk Implemented onWithdrawFeetokens handler with reserve validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

struct (0x7052dc75) OnRamp_WithdrawFeeTokens {
feeTokens: SnakedCell<address>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also need amounts.

As this is an async env best option is the sender checks balances off-chain, sends amounts and we try to transfer to fee aggregator.

Copy link
Collaborator Author

@patricios-space patricios-space Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this taking only addresses to match EVM interface. However, Jetton wallets don't have onchain amount getters. We could keep a balance counter here, or accept amount as an argument

krebernisak
krebernisak previously approved these changes Dec 19, 2025
@patricios-space patricios-space force-pushed the fix/onramp/replace-withdrawable-with-withdrawFeeTokens branch from 577e79a to a05f9d5 Compare December 19, 2025 20:43
@patricios-space patricios-space merged commit 897a04f into main Dec 19, 2025
35 checks passed
@patricios-space patricios-space deleted the fix/onramp/replace-withdrawable-with-withdrawFeeTokens branch December 19, 2025 21:07
jadepark-dev added a commit that referenced this pull request Dec 19, 2025
@patricios-space patricios-space changed the title feat: withdrawFeeTokens use feeAggregator [NONEVM-3185] [OnRamp] Replace Withdrawable with WithdrawFeeTokens Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants