-
Notifications
You must be signed in to change notification settings - Fork 5
TokenTransfer onramp flow skeleton #754
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?
Changes from all commits
bc1ea95
2edcda9
42c9b66
bd48c10
9150791
0d9fe92
11f137b
f198c41
8b19471
4111963
7b60147
c2733ff
83de367
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,9 @@ import "../../lib/utils" | |
| import "../router/messages" | ||
| import "../fee_quoter/types" | ||
| import "@stdlib/gas-payments" | ||
| import "../common/types" | ||
| import "../token_registry/messages" | ||
| import "../test/tokenPool/messages" | ||
|
|
||
| tolk 1.4.1 | ||
|
|
||
|
|
@@ -46,6 +49,17 @@ fun onInternalMessage(in: InMessage) { | |
| assert(this.addresses.load().feeQuoter == in.senderAddress, CCIPSendExecutor_Error.Unauthorized); | ||
| this.onMessageValidationFailed(msg); | ||
| } | ||
| // Sender must be TokenRegistry | ||
| TokenRegistry_ReturnTokenInfo => { | ||
| var this = CCIPSendExecutor<CCIPSendExecutor_State_TokenRegistryAccess>.load(); | ||
| assert(this.addresses.load().tokenRegistry! == in.senderAddress, CCIPSendExecutor_Error.Unauthorized); | ||
| this.onTokenInfoReceived(msg); | ||
| } | ||
| MockTokenPool_NotifySuccessfulLockOrBurn => { | ||
| var this = CCIPSendExecutor<CCIPSendExecutor_State_TokenTransfer>.load(); | ||
| assert(this.state.load().tokenPool == in.senderAddress, CCIPSendExecutor_Error.Unauthorized); | ||
| this.onConfirmLockOrBurn(msg); | ||
| } | ||
| else => { | ||
| assert (in.body.isEmpty()) throw 0xFFFF; | ||
| }, | ||
|
|
@@ -59,9 +73,16 @@ fun onBouncedMessage(in: InMessageBounced) { | |
| val this = CCIPSendExecutor<CCIPSendExecutor_State_OnGoingFeeValidation>.load(); | ||
| this.exitWithError(CCIPSendExecutor_Error.FeeQuoterBounce); | ||
| } | ||
| //TODO: We should handle bounced messages from the TokenRegistry, as that means the token is not enabled on the lane | ||
| } | ||
| } | ||
|
|
||
| //TODO: Figure out how to maintain backwards compatibility for messages sent between updating the Executor code in the OnRamp and upgrading the OnRamp itself. | ||
| // Option 1: split the initialization in two messages to maintain backwards compatibility. | ||
| // 1. CCIPSendExecutor_Execute stays the same as in 1.6.1 (that means not changing Config) | ||
| // 2. CCIPSendExecutor_InitTokenTransfer passes the token registry address and other necessary configs | ||
| // That way we can upgrade the SendExecutor code in the OnRamp and then upgrade the OnRamp itself, if an executor is deployed by the OnRamp between upgrades the new SendExecutor will still be able to handle the initialization message. | ||
| // Option 2: Create a new message type CCIPSendExecutor_ExecuteV2 which has the new config element. Make the new executor able to handle both this version and the original. | ||
| fun init(onrampSend: OnRamp_Send, config: CCIPSendExecutor_Config): CCIPSendExecutor<CCIPSendExecutor_State_Initialized> { | ||
| val st = lazy CCIPSendExecutor_InitialData.fromCell(contract.getData()); | ||
| return CCIPSendExecutor<CCIPSendExecutor_State_Initialized> { | ||
|
|
@@ -70,6 +91,7 @@ fun init(onrampSend: OnRamp_Send, config: CCIPSendExecutor_Config): CCIPSendExec | |
| addresses: CCIPSendExecutor_Addresses { | ||
| onramp: st.onramp, | ||
| feeQuoter: config.feeQuoter, | ||
| tokenRegistry: config.tokenRegistry | ||
| }.toCell(), | ||
| state: CCIPSendExecutor_State_Initialized{ | ||
| }.toCell(), | ||
|
|
@@ -102,13 +124,73 @@ fun getValidatedFee(feeQuoter: address, onrampSend: OnRamp_Send) { | |
| } | ||
|
|
||
| fun CCIPSendExecutor<CCIPSendExecutor_State_OnGoingFeeValidation>.onMessageValidated(mutate self, msg: FeeQuoter_MessageValidated<RemainingBitsAndRefs>) { | ||
| val message = Router_CCIPSend.fromCell(self.onrampSend.msg); | ||
| val tokenAmounts = message.tokenAmounts; | ||
| if (msg.fee.feeTokenAmount + Router_Costs.CCIPSend() > self.onrampSend.metadata.value) { | ||
| self.exitWithError(CCIPSendExecutor_Error.InsufficientFunds); | ||
| return; | ||
| } | ||
| // If token amounts is empty finalize the message | ||
| if (tokenAmounts.empty()) { | ||
| setGasLimitToMaximum(); | ||
| self.exitSuccessfully(msg.fee); | ||
| return; | ||
| } | ||
|
|
||
| // Else if there are token transfers continue querying the tokenRegistry | ||
| val tokenRegistry = self.addresses.load().tokenRegistry!; | ||
| val queryMsg = createMessage({ | ||
| bounce: true, | ||
| value: 0, | ||
| dest: tokenRegistry, | ||
| body: TokenRegistry_GetTokenInfo { | ||
| } | ||
| }); | ||
| queryMsg.send(SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE); | ||
| val newState = CCIPSendExecutor<CCIPSendExecutor_State_TokenRegistryAccess> { | ||
| id: self.id, | ||
| onrampSend: self.onrampSend, | ||
| addresses: self.addresses, | ||
| state: CCIPSendExecutor_State_TokenRegistryAccess { | ||
| fee: msg.fee, | ||
| }.toCell(), | ||
| }; | ||
| newState.store(); | ||
| } | ||
|
|
||
| fun CCIPSendExecutor<CCIPSendExecutor_State_TokenRegistryAccess>.onTokenInfoReceived(mutate self, msg: TokenRegistry_ReturnTokenInfo) { | ||
| //TODO validate sender wallet address by querying the jettonMinter | ||
| assert(msg.tokenInfo.enabled) throw CCIPSendExecutor_Error.TokenNotEnabled; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use the type system to represent this. Maybe the registry should return a different message if the token is not enabled OR it returns an optional TokenInfo. In this way, we don't even have the tokenPool address here, preventing us from trying to lock anyway
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I prefer having just one request and response instead of two different response types, but I can do the two messages if you think it's important to leave that Maybe we should also handle bounced messages from Executor->Registry which would indicate that the token has not been enabled either. I think handling this could be part of the scope of the task to handle the failure path on the CCIPSend TokenTransfer flow. I'll add a TODO |
||
| val onrampSend = lazy self.onrampSend.msg.load(); | ||
| val tokenAmount = onrampSend.tokenAmounts.iter().next(); | ||
| val addresses = lazy self.addresses.load(); | ||
| val requestLockOrBurn = createMessage({ | ||
| bounce: true, | ||
| value: 0, | ||
| dest: addresses.onramp, | ||
| body: OnRamp_ExecutorRequestsLockOrBurn { | ||
| tokenAmount, | ||
| tokenPool: msg.tokenInfo.tokenPool, | ||
| destChainSelector: onrampSend.destChainSelector, | ||
| executorID: self.id, | ||
| } | ||
| }); | ||
| requestLockOrBurn.send(SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE); | ||
| val newState = CCIPSendExecutor<CCIPSendExecutor_State_TokenTransfer> { | ||
| id: self.id, | ||
| onrampSend: self.onrampSend, | ||
| addresses: self.addresses, | ||
| state: CCIPSendExecutor_State_TokenTransfer { | ||
| tokenPool: msg.tokenInfo.tokenPool, | ||
| fee: self.state.load().fee, | ||
| }.toCell(), | ||
| }; | ||
| newState.store(); | ||
| } | ||
|
|
||
| fun CCIPSendExecutor<CCIPSendExecutor_State_TokenTransfer>.onConfirmLockOrBurn(mutate self, msg: MockTokenPool_NotifySuccessfulLockOrBurn) { | ||
| setGasLimitToMaximum(); | ||
|
|
||
| self.exitSuccessfully(msg.fee); | ||
| self.exitSuccessfully(self.state.load().fee); | ||
| } | ||
|
|
||
| fun CCIPSendExecutor<T>.exitSuccessfully(self, fee: Fee) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,4 +7,5 @@ enum CCIPSendExecutor_Error { | |
| InsufficientFunds | ||
| InsufficientFee | ||
| FeeQuoterBounce | ||
| TokenNotEnabled | ||
| } | ||
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.
SendExecutor doesn't load this config lazily, so I think this is going to be a breaking change. I think this should be good any way because old SendExecutors are not going to receive new messages from the OnRamp. It's only the new OnRamp that will receive messages from the old SendExecutors
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 guess the update order would be:
Issue there is if the sendExecutor code is updated, and before the onRamp code is updated the onRamp tries to deploy a sendExecutor with the outdated config (that does not have the tokenRegistry)
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.
Maybe we can separate the initialization in two steps...
The new SendExecutor receives
CCIPSendExecutor_Executefirst, and we don't change that message type.But, on the new handler if a TokenAmount is present then the executor blocks and waits for another message
CCIPSendExecutor_InitTokenTransferwhich passes the token registry address.The Onramp sends only
Executewhen there are no token amounts , andExecutefollowed byInitTokenTransferwhen there are token amountsThere 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.
The other option is making the executor able to handle both the old and the new config, we keep CCIPSendExecutor_Execute and add CCIPSendExecutor_ExecuteV2. The new SendExecutor is able to handle both but does not do any tokenTransfer related execution with the first one
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 think I lean towards the second option