-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Supplementary checks for sufficient native token fees on MockRouter #828
Conversation
LCOV of commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quick note, ok for this one, going forward, this repo is public, don't put ticket numbers in description or branch names
} | ||
|
||
function test_ccipSendWithInsufficientNativeTokens_Revert() public { | ||
//Should revert because did not include sufficient eth to pay for fees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: //[space]comments
, same above and below
@@ -28,6 +28,8 @@ contract MockCCIPRouter is IRouter, IRouterClient { | |||
uint16 public constant GAS_FOR_CALL_EXACT_CHECK = 5_000; | |||
uint64 public constant DEFAULT_GAS_LIMIT = 200_000; | |||
|
|||
uint256 internal s_MockFeeTokenAmount; //use setFee() to change to non-zero to test fees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: s_[lowercase]
uint256 feeTokenAmount = getFee(destinationChainSelector, message); | ||
if (msg.value < feeTokenAmount) revert InsufficientFeeTokenAmount(); | ||
} else { | ||
if (msg.value > 0) revert InvalidMsgValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's not part of original scope, if user is sending non-native fee token, e.g. LINK, we'd want to make sure their test covers token approvals, can we add something like IERC20(message.feeToken).safeTransferFrom(msg.sender, address(this), feeTokenAmount);
and corresponding tests
Pull request was closed
c4ad9e4
to
3160ab3
Compare
…896) ## Motivation Clean PR with every commit signed from #828 ## Solution Added support in MockCCIPRouter contract for supplementary fee-checks in ccipSend. Function now includes toggle-able fees for users to check their contract interactions. If feeToken is the native gas token of the chain, supplementary check is included that msg.value > getFee()
Motivation
Issue #755
Solution
Added support in MockCCIPRouter contract for supplementary fee-checks in
ccipSend
. Function now includes toggle-able fees for users to check their contract interactions. IffeeToken
is the native gas token of the chain, supplementary check is included thatmsg.value > getFee()
constant
FEE_ENABLED
in MockRouter.sol can be enabled for testing with fees of0.1 ether
for native gas Tokens and1e18
otherwise.