feat: Add Arbitrum Spokepool and add generic cross-chain admin relayer function on HubPool #33
Conversation
nicholaspai
changed the title
| uint256[] memory bundleLpFees | ||
| ) internal { | ||
| AdapterInterface adapter = crossChainContracts[chainId].adapter; | ||
| require(address(adapter) != address(0), "Adapter not set for target chain"); |
There was a problem hiding this comment.
do we need this check? elsewhere in _executeRelayerRefundOnChain for example, we don't check that the adapter address is non0
| import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
| import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; | ||
|
|
||
| contract L1_Adapter is Base_Adapter, Lockable { |
There was a problem hiding this comment.
@mrice32 @chrismaree I originally added this because I wanted to test HubPool.relaySpokePoolAdminFunction() in the simplest way and check that it can delegate a call to another contract, so I figured why not just build this L1 adapter to kill two birds with one stone
| await expect(hubPool.connect(other).disableL1TokenForLiquidityProvision(weth.address)).to.be.reverted; | ||
| }); | ||
| it("Can whitelist route for deposits and rebalances", async function () { | ||
| await hubPool.setCrossChainContracts(destinationChainId, mockAdapter.address, mockSpoke.address); |
There was a problem hiding this comment.
Neccessary now since whitelistRoute calls relayMessage
|
|
||
| // Admin controlled mapping of arbitrum tokens to L1 counterpart. L1 counterpart addresses | ||
| // are neccessary to bridge tokens to L1. | ||
| mapping(address => address) public whitelistedTokens; |
There was a problem hiding this comment.
pain that we need this just for arbitrum but works kinda well given we have separate contracts for each chain.
| abi.encodeWithSignature( | ||
| "setEnableRoute(address,uint32,bool)", | ||
| originToken, | ||
| uint32(destinationChainId), |
There was a problem hiding this comment.
unrelated to this PR but I think that the unit32 is too small.
There was a problem hiding this comment.
Hm what should we use?
There was a problem hiding this comment.
EIP 155 doesn't list the highest value for this so I guess its safe to just use 256
| constructor(address _hubPool) Base_Adapter(_hubPool) {} | ||
|
|
||
| function relayMessage(address target, bytes memory message) external payable override nonReentrant onlyHubPool { | ||
| require(_executeCall(target, message), "execute call failed"); |
There was a problem hiding this comment.
would it be cleaner to put the require part in the _executeCall function then just call this directly? we never want the success=false case so we might as well assume this method does not throw if it passed. i.e the _executeCall would not return anything and would simply have require(success,"execute call failed") at the end of the function.
| * @dev Uses AVM cross-domain-enabled logic for access control. | ||
| */ | ||
|
|
||
| contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { |
There was a problem hiding this comment.
this does not have any tests. shall we add that in this PR or a separate one? we also need to write OP tests, which can happen either at the same time, if we choose to wait on adding the tests here.
There was a problem hiding this comment.
Let's do separate PR
…art-contracts-v2 into npai/arbitrum-spoke-pool
This reverts commit 7adc509.
This reverts commit d54aa2f.
This reverts commit 7adc509.
Originally authored by Nick & Matt. Signed-off-by: nicholaspai <npai.nyc@gmail.com> Signed-off-by: Matt Rice <matthewcrice32@gmail.com> Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com> Co-authored-by: Matt Rice <matthewcrice32@gmail.com>
No description provided.