-
Notifications
You must be signed in to change notification settings - Fork 35
allow user claim; allow claim and call #152
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
base: main
Are you sure you want to change the base?
Conversation
/// @notice The address of the `ScrollChain` contract. | ||
address public immutable rollup; |
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.
Need to init in constructor.
// @note callbackTo is the address of the target contract to call. | ||
IScrollGatewayCallback(callbackTo).onScrollGatewayCallback(data); |
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.
So for example to support swaps, we'd need to deploy a "forwarder contract" that implements onScrollGatewayCallback
, decodes the data, and makes the swap?
But how does it work with tokens, when tokens are transferred to to
instead of callbackTo
? Shouldn't we transfer tokens to callbackTo
in this case?
/// @param amount The amount of tokens to withdraw. | ||
/// @param messageHash The hash of the message, which is the corresponding withdraw message hash in L2. | ||
/// @param signature The signature of the message from sequencer. | ||
function claimFastWithdraw( |
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.
Why not just have one function and call if data is non-empty?
One concern with 2 functions is: The user's intention might be to make a call on L2, which executes and re-deposits in an atomic step. But instead, the sequencer might call claimFastWithdraw
so no call is executed.
} | ||
} | ||
|
||
// decode actual validium sender from message. |
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.
Currently I encode the recipient into data
instead of always using the L3 sender as the withdraw recipient. Should we do that here?
if (l1Token == weth) { | ||
IWETH(weth).withdraw(amount); | ||
AddressUpgradeable.sendValue(payable(sender), amount); | ||
} else { | ||
IERC20Upgradeable(l1Token).safeTransfer(sender, amount); | ||
} |
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.
My thinking was a bit different: If the message is never relayed to L2, the user can claim from L1ScrollMessenger
, and not from the liquidity pool here (which might be empty). Is that doable?
No description provided.