Multichain #105
Multichain #105
Conversation
# Conflicts: # contracts/error/Errors.sol
dmax10
force-pushed
the
multichain
branch
2 times, most recently
from
70dba6c to
6f03e4f
Compare
| // TODO: confirm virtual account is used correctly | ||
| address virtualAccount = MultichainUtils.getVirtualAccount(account, sourceChainId); | ||
|
|
||
| return _createDeposit(relayParams.tokenPermits, relayParams.fee, params, virtualAccount); |
There was a problem hiding this comment.
should use _createDeposit(..., account, chainId) here instead of virtualAccount
the GM tokens should be sent to MultichainVault and the account's multichain GM balance should be increased
so would need a deposit.chainId as well
in execute deposit, if the deposit.chainId is "0" this means that the deposit would be a normal deposit, so the GM tokens would be sent directly to the user, if the deposit.chainId is not zero, then the user's multichain GM balance should be increased
There was a problem hiding this comment.
Updated. Named it srcChainId though.
| eventEmitter: eventEmitter, | ||
| // TODO: confirm Contracts struct can be modified --> replace `OrderVault orderVault;` field with `StrictBank vault;` | ||
| // otherwise, should probably overridde _handleRelay | ||
| orderVault: OrderVault(payable(multichainVault)) |
There was a problem hiding this comment.
@gdev8317 is it ok for Max to change Contracts { OrderVault orderVault } into Contracts { StrictBank bank }?
|
|
||
| // TODO: confirm where funds are bridged. Are they sent to this contract, to MultichainProvider, or directly to MultichainVault? | ||
| // if funds are sent directly to MultichainVault, _sendTokens would not be needed, but then where is the relay fee being paid from? | ||
| _sendTokens( |
There was a problem hiding this comment.
funds should be bridged to MultichainVault directly
if funds are sent directly to MultichainVault, _sendTokens would not be needed
I don't quite understand this, to create a deposit, the tokens would still need to be sent from MultichainVault to DepositVault?
then where is the relay fee being paid from?
relay fee should be sent from MultichainVault, from the user's multichain balance
residual fees should be sent to the deposit vault
There was a problem hiding this comment.
I don't quite understand this, to create a deposit, the tokens would still need to be sent from MultichainVault to DepositVault?
I thought so. DepositUtils.createDeposit does a recordTransferIn, which assumes tokens have already been transferred to DepositVault
| @@ -7,6 +7,6 @@ pragma solidity ^0.8.0; | |||
| */ | |||
| library MultichainUtils { | |||
| function getVirtualAccount(address account, uint256 sourceChainId) internal pure returns (address) { | |||
There was a problem hiding this comment.
should remove this function
… it in MultichainRouter
…eOrder, createShift, createGlvDeposit, createGlvWithdrawal
| import "./MultichainUtils.sol"; | ||
|
|
||
| contract MultichainRouter is GelatoRelayRouter { | ||
| struct GaslessCreateDepositParams { |
There was a problem hiding this comment.
i would suggest to call it MulticallCreateDepositParams (and use multichain instead of gasless in otehr names) because GelatoRelayRouter is also gasless
There was a problem hiding this comment.
and maybe chainId, longTokenAmount and shortTokenAmount can be passed as a separate arguments and remove the extra nesting? just thinking, at first glance it could simplify the contract interface
There was a problem hiding this comment.
Changed to:
struct MultichainCreateDepositParams {
uint256 desChainId;
uint256 longTokenAmount;
uint256 shortTokenAmount;
DepositUtils.CreateDepositParams createDepositParams;
}
and replaced gasless with multichain as well.
There was a problem hiding this comment.
Didn't change the nesting yet, but could do that as well if needed.
| tokenPermits, | ||
| fee, | ||
| account, | ||
| NonceUtils.getKey(contracts.dataStore, NonceUtils.getCurrentNonce(dataStore) + 1), // calculate next key without incrementing |
There was a problem hiding this comment.
would it be the same as NonceUtils.getNextKey(dataStore)?
There was a problem hiding this comment.
Yes, same logic, the difference is that getNextKey also writes the incremented value (it is named "get" but it does "set" as well).
There was a problem hiding this comment.
oh right, not ideal naming, would be good to change it sometime
| function getVirtualAccount(address account, uint256 sourceChainId) internal pure returns (address) { | ||
| return address(uint160(uint256(keccak256(abi.encode("GMX Multichain", account, sourceChainId))))); | ||
| function increaseBalance(DataStore dataStore, uint256 chainId, address account, address token, uint256 amount) internal { | ||
| dataStore.decrementUint(Keys.multichainBalanceKey(chainId, account, token), amount); |
There was a problem hiding this comment.
i guess it should be incrementUint instead of decrementUint
| Contracts memory contracts = Contracts({ | ||
| dataStore: dataStore, | ||
| eventEmitter: eventEmitter, | ||
| // TODO: confirm Contracts struct can be modified --> replace `OrderVault orderVault;` field with `StrictBank vault;` |
There was a problem hiding this comment.
i think it's okay to change type to StrictBank, yes
It was removed as a temporary fix to reduce the contract size and it's not needed anymore. Different refactoring was implemented
…chainTransferRouter
…ize, add gasless validation
…onality It's actually inherited into BaseGelatoRelayRouter instead of MultichainTransferRouter due to inheritance structure. Since SubaccountGelatoRelayRouter was getting too big, some of it's logic is moved into a new external lib SubaccountRelayUtils
…al was used to pay the execution fee, small refactor to solve stack too deep error in OrderUtils
No description provided.