From 830bcdfe84fc14aa1906c1a55a03ebaa325b111d Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 10:47:56 -0700 Subject: [PATCH 01/19] Reduce size of uint states --- contracts/Optimism_SpokePool.sol | 10 +-- contracts/SpokePool.sol | 141 +++++++++++++++---------------- contracts/SpokePoolInterface.sol | 26 +++--- contracts/test/MockSpokePool.sol | 10 +-- 4 files changed, 92 insertions(+), 95 deletions(-) diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index 8cb440fba..7c9e6a46d 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -23,11 +23,11 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool address _crossDomainAdmin, address _hubPool, address _wethAddress, - uint64 _depositQuoteTimeBuffer, - address timerAddress + address timerAddress, + uint32 _depositQuoteTimeBuffer ) CrossDomainEnabled(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER) - SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, _depositQuoteTimeBuffer, timerAddress) + SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress, _depositQuoteTimeBuffer) {} /************************************** @@ -63,13 +63,13 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool function setEnableRoute( address originToken, - uint256 destinationChainId, + uint32 destinationChainId, bool enable ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint64 buffer) + function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyFromCrossDomainAccount(crossDomainAdmin) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index bc262cd7d..4e83031c5 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -42,21 +42,21 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall address public hubPool; // Timestamp when contract was constructed. Relays cannot have a quote time before this. - uint64 public deploymentTime; + uint32 public deploymentTime; // Any deposit quote times greater than or less than this value to the current contract time is blocked. Forces // caller to use an up to date realized fee. - uint64 public depositQuoteTimeBuffer; + uint32 public depositQuoteTimeBuffer; // Use count of deposits as unique deposit identifier. - uint64 public numberOfDeposits; + uint32 public numberOfDeposits; // Address of WETH contract for this network. If an origin token matches this, then the caller can optionally // instruct this contract to wrap ETH when depositing. WETH9 public weth; // Origin token to destination token routings can be turned on or off. - mapping(address => mapping(uint256 => bool)) public enabledDepositRoutes; + mapping(address => mapping(uint32 => bool)) public enabledDepositRoutes; struct RelayerRefund { // Merkle root of slow relays that were not fully filled and whose recipient is still owed funds from the LP pool. @@ -79,14 +79,14 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall ****************************************/ event SetXDomainAdmin(address indexed newAdmin); event SetHubPool(address indexed newHubPool); - event EnabledDepositRoute(address indexed originToken, uint256 indexed destinationChainId, bool enabled); - event SetDepositQuoteTimeBuffer(uint64 newBuffer); + event EnabledDepositRoute(address indexed originToken, uint32 indexed destinationChainId, bool enabled); + event SetDepositQuoteTimeBuffer(uint32 newBuffer); event FundsDeposited( - uint256 destinationChainId, uint256 amount, - uint64 indexed depositId, uint64 relayerFeePct, - uint64 quoteTimestamp, + uint32 destinationChainId, + uint32 indexed depositId, + uint32 quoteTimestamp, address indexed originToken, address recipient, address indexed depositor @@ -96,11 +96,11 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall uint256 totalRelayAmount, uint256 totalFilledAmount, uint256 fillAmount, - uint256 indexed repaymentChain, - uint256 originChainId, - uint64 depositId, uint64 relayerFeePct, uint64 realizedLpFeePct, + uint32 indexed repaymentChain, + uint32 originChainId, + uint32 depositId, address destinationToken, address indexed relayer, address depositor, @@ -111,34 +111,34 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall uint256 totalRelayAmount, uint256 totalFilledAmount, uint256 fillAmount, - uint256 originChainId, - uint64 depositId, uint64 relayerFeePct, uint64 realizedLpFeePct, + uint32 originChainId, + uint32 depositId, address destinationToken, address indexed caller, address depositor, address recipient ); event InitializedRelayerRefund( - uint256 indexed relayerRefundId, + uint32 indexed relayerRefundId, bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayFulfillmentRoot ); event DistributedRelayerRefund( - uint256 indexed relayerRefundId, - uint256 indexed leafId, - uint256 chainId, uint256 amountToReturn, uint256[] refundAmounts, + uint32 indexed relayerRefundId, + uint32 indexed leafId, + uint32 chainId, address l2TokenAddress, address[] refundAddresses, address indexed caller ); event TokensBridged( - uint256 indexed leafId, - uint256 indexed chainId, uint256 amountToReturn, + uint32 indexed leafId, + uint32 indexed chainId, address indexed l2TokenAddress, address caller ); @@ -147,12 +147,12 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall address _crossDomainAdmin, address _hubPool, address _wethAddress, - uint64 _depositQuoteTimeBuffer, - address timerAddress + address timerAddress, + uint32 _depositQuoteTimeBuffer ) Testable(timerAddress) { _setCrossDomainAdmin(_crossDomainAdmin); _setHubPool(_hubPool); - deploymentTime = uint64(getCurrentTime()); + deploymentTime = uint32(getCurrentTime()); depositQuoteTimeBuffer = _depositQuoteTimeBuffer; weth = WETH9(_wethAddress); } @@ -161,7 +161,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall * MODIFIERS * ****************************************/ - modifier onlyEnabledRoute(address originToken, uint256 destinationId) { + modifier onlyEnabledRoute(address originToken, uint32 destinationId) { require(enabledDepositRoutes[originToken][destinationId], "Disabled route"); _; } @@ -184,14 +184,14 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall function _setEnableRoute( address originToken, - uint256 destinationChainId, + uint32 destinationChainId, bool enabled ) internal { enabledDepositRoutes[originToken][destinationChainId] = enabled; emit EnabledDepositRoute(originToken, destinationChainId, enabled); } - function _setDepositQuoteTimeBuffer(uint64 _depositQuoteTimeBuffer) internal { + function _setDepositQuoteTimeBuffer(uint32 _depositQuoteTimeBuffer) internal { depositQuoteTimeBuffer = _depositQuoteTimeBuffer; emit SetDepositQuoteTimeBuffer(_depositQuoteTimeBuffer); } @@ -205,12 +205,12 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall * @dev The caller must first approve this contract to spend `amount` of `originToken`. */ function deposit( + address recipient, address originToken, - uint256 destinationChainId, uint256 amount, - address recipient, uint64 relayerFeePct, - uint64 quoteTimestamp + uint32 destinationChainId, + uint32 quoteTimestamp ) public payable onlyEnabledRoute(originToken, destinationChainId) nonReentrant { // We limit the relay fees to prevent the user spending all their funds on fees. require(relayerFeePct <= 0.5e18, "invalid relayer fee"); @@ -237,10 +237,10 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall } emit FundsDeposited( - destinationChainId, amount, - numberOfDeposits, relayerFeePct, + destinationChainId, + numberOfDeposits, quoteTimestamp, originToken, recipient, @@ -258,13 +258,13 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall address depositor, address recipient, address destinationToken, - uint64 realizedLpFeePct, - uint64 relayerFeePct, - uint64 depositId, - uint256 originChainId, uint256 totalRelayAmount, uint256 maxTokensToSend, - uint256 repaymentChain + uint64 realizedLpFeePct, + uint64 relayerFeePct, + uint32 repaymentChain, + uint32 depositId, + uint32 originChainId ) public nonReentrant { // Each relay attempt is mapped to the hash of data uniquely identifying it, which includes the deposit data // such as the origin chain ID and the deposit ID, and the data in a relay attempt such as who the recipient @@ -273,33 +273,31 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall depositor: depositor, recipient: recipient, destinationToken: destinationToken, + relayAmount: totalRelayAmount, realizedLpFeePct: realizedLpFeePct, relayerFeePct: relayerFeePct, depositId: depositId, - originChainId: originChainId, - relayAmount: totalRelayAmount + originChainId: originChainId }); bytes32 relayHash = _getRelayHash(relayData); - uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, relayerFeePct, maxTokensToSend, false); + uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, maxTokensToSend, relayerFeePct, false); - _emitFillRelay(relayHash, fillAmountPreFees, repaymentChain, relayerFeePct, relayData); + _emitFillRelay(relayHash, fillAmountPreFees, relayerFeePct, repaymentChain, relayData); } - // We overload `fillRelay` logic to allow the relayer to optionally pass in an updated `relayerFeePct` and a signature - // proving that the depositor agreed to the updated fee. function fillRelayWithUpdatedFee( address depositor, address recipient, address destinationToken, + uint256 totalRelayAmount, + uint256 maxTokensToSend, uint64 realizedLpFeePct, uint64 relayerFeePct, uint64 newRelayerFeePct, - uint64 depositId, - uint256 originChainId, - uint256 totalRelayAmount, - uint256 maxTokensToSend, - uint256 repaymentChain, + uint32 repaymentChain, + uint32 depositId, + uint32 originChainId, bytes memory depositorSignature ) public nonReentrant { // Grouping the signature validation logic into brackets to address stack too deep error. @@ -335,39 +333,39 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall depositor: depositor, recipient: recipient, destinationToken: destinationToken, + relayAmount: totalRelayAmount, realizedLpFeePct: realizedLpFeePct, relayerFeePct: relayerFeePct, depositId: depositId, - originChainId: originChainId, - relayAmount: totalRelayAmount + originChainId: originChainId }); bytes32 relayHash = _getRelayHash(relayData); - uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, newRelayerFeePct, maxTokensToSend, false); + uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, maxTokensToSend, newRelayerFeePct, false); - _emitFillRelay(relayHash, fillAmountPreFees, repaymentChain, newRelayerFeePct, relayData); + _emitFillRelay(relayHash, fillAmountPreFees, newRelayerFeePct, repaymentChain, relayData); } function distributeRelaySlow( address depositor, address recipient, address destinationToken, + uint256 totalRelayAmount, uint64 realizedLpFeePct, uint64 relayerFeePct, - uint64 depositId, - uint256 originChainId, - uint256 totalRelayAmount, - uint256 relayerRefundId, + uint32 depositId, + uint32 originChainId, + uint32 relayerRefundId, bytes32[] memory proof ) public nonReentrant { RelayData memory relayData = RelayData({ depositor: depositor, recipient: recipient, destinationToken: destinationToken, + relayAmount: totalRelayAmount, realizedLpFeePct: realizedLpFeePct, relayerFeePct: relayerFeePct, depositId: depositId, - originChainId: originChainId, - relayAmount: totalRelayAmount + originChainId: originChainId }); require( @@ -383,7 +381,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // Note: use relayAmount as the max amount to send, so the relay is always completely filled by the contract's // funds in all cases. - uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, relayerFeePct, relayData.relayAmount, true); + uint256 fillAmountPreFees = _fillRelay(relayHash, relayData, relayData.relayAmount, relayerFeePct, true); _emitDistributeRelaySlow(relayHash, fillAmountPreFees, relayData); } @@ -395,7 +393,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // valid `inclusionProof` to verify that the leaf is contained within the root. The `relayerRefundId` is the index // of the specific distribution root containing the passed in leaf. function distributeRelayerRefund( - uint256 relayerRefundId, + uint32 relayerRefundId, SpokePoolInterface.DestinationDistributionLeaf memory distributionLeaf, bytes32[] memory proof ) public override nonReentrant { @@ -427,24 +425,23 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // If `distributionLeaf.amountToReturn` is positive, then send L2 --> L1 message to bridge tokens back via // chain-specific bridging method. if (distributionLeaf.amountToReturn > 0) { - // Do we need to perform any check about the last time that funds were bridged from L2 to L1? _bridgeTokensToHubPool(distributionLeaf); emit TokensBridged( + distributionLeaf.amountToReturn, distributionLeaf.leafId, distributionLeaf.chainId, - distributionLeaf.amountToReturn, distributionLeaf.l2TokenAddress, msg.sender ); } emit DistributedRelayerRefund( + distributionLeaf.amountToReturn, + distributionLeaf.refundAmounts, relayerRefundId, distributionLeaf.leafId, distributionLeaf.chainId, - distributionLeaf.amountToReturn, - distributionLeaf.refundAmounts, distributionLeaf.l2TokenAddress, distributionLeaf.refundAddresses, msg.sender @@ -467,11 +464,11 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall internal virtual; - function _computeAmountPreFees(uint256 amount, uint256 feesPct) private pure returns (uint256) { + function _computeAmountPreFees(uint256 amount, uint64 feesPct) private pure returns (uint256) { return (1e18 * amount) / (1e18 - feesPct); } - function _computeAmountPostFees(uint256 amount, uint256 feesPct) private pure returns (uint256) { + function _computeAmountPostFees(uint256 amount, uint64 feesPct) private pure returns (uint256) { return (amount * (1e18 - feesPct)) / 1e18; } @@ -498,7 +495,7 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall function _initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayFulfillmentRoot) internal { - uint256 relayerRefundId = relayerRefunds.length; + uint32 relayerRefundId = uint32(relayerRefunds.length); RelayerRefund storage relayerRefund = relayerRefunds.push(); relayerRefund.distributionRoot = relayerRepaymentDistributionRoot; relayerRefund.slowRelayFulfillmentRoot = slowRelayFulfillmentRoot; @@ -508,8 +505,8 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall function _fillRelay( bytes32 relayHash, RelayData memory relayData, - uint64 updatableRelayerFeePct, uint256 maxTokensToSend, + uint64 updatableRelayerFeePct, bool isSlowRelay ) internal returns (uint256 fillAmountPreFees) { // We limit the relay fees to prevent the user spending all their funds on fees. Note that 0.5e18 (i.e. 50%) @@ -560,8 +557,8 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall function _emitFillRelay( bytes32 relayHash, uint256 fillAmount, - uint256 repaymentChain, uint64 relayerFeePct, + uint32 repaymentChain, RelayData memory relayData ) internal { emit FilledRelay( @@ -569,11 +566,11 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall relayData.relayAmount, relayFills[relayHash], fillAmount, + relayerFeePct, + relayData.realizedLpFeePct, repaymentChain, relayData.originChainId, relayData.depositId, - relayerFeePct, - relayData.realizedLpFeePct, relayData.destinationToken, msg.sender, relayData.depositor, @@ -591,10 +588,10 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall relayData.relayAmount, relayFills[relayHash], fillAmount, - relayData.originChainId, - relayData.depositId, relayData.relayerFeePct, relayData.realizedLpFeePct, + relayData.originChainId, + relayData.depositId, relayData.destinationToken, msg.sender, relayData.depositor, diff --git a/contracts/SpokePoolInterface.sol b/contracts/SpokePoolInterface.sol index 7e187c58b..9765d2951 100644 --- a/contracts/SpokePoolInterface.sol +++ b/contracts/SpokePoolInterface.sol @@ -4,20 +4,20 @@ pragma solidity ^0.8.0; interface SpokePoolInterface { // This leaf is meant to be decoded in the SpokePool in order to pay out individual relayers for this bundle. struct DestinationDistributionLeaf { - // Used as the index in the bitmap to track whether this leaf has been executed or not. - uint256 leafId; - // Used to verify that this is being decoded on the correct chainId. - uint256 chainId; // This is the amount to return to the HubPool. This occurs when there is a PoolRebalanceLeaf netSendAmount that is // negative. This is just that value inverted. uint256 amountToReturn; + // This array designates how much each of those addresses should be refunded. + uint256[] refundAmounts; + // Used as the index in the bitmap to track whether this leaf has been executed or not. + uint32 leafId; + // Used to verify that this is being decoded on the correct chainId. + uint32 chainId; // The associated L2TokenAddress that these claims apply to. address l2TokenAddress; // These two arrays must be the same length and are parallel arrays. They should be order by refundAddresses. // This array designates each address that must be refunded. address[] refundAddresses; - // This array designates how much each of those addresses should be refunded. - uint256[] refundAmounts; } // This struct represents the data to fully-specify a relay. If any portion of this data differs, the relay is @@ -30,17 +30,17 @@ interface SpokePoolInterface { address recipient; // The corresponding token address on the destination chain. address destinationToken; + // The total relay amount before fees are taken out. + uint256 relayAmount; // The LP Fee percentage computed by the relayer based on the deposit's quote timestamp // and the HubPool's utilization. uint64 realizedLpFeePct; // The relayer fee percentage specified in the deposit. uint64 relayerFeePct; // The id uniquely identifying this deposit on the origin chain. - uint64 depositId; + uint32 depositId; // Origin chain id. - uint256 originChainId; - // The total relay amount before fees are taken out. - uint256 relayAmount; + uint32 originChainId; } function setCrossDomainAdmin(address newCrossDomainAdmin) external; @@ -49,16 +49,16 @@ interface SpokePoolInterface { function setEnableRoute( address originToken, - uint256 destinationChainId, + uint32 destinationChainId, bool enable ) external; - function setDepositQuoteTimeBuffer(uint64 buffer) external; + function setDepositQuoteTimeBuffer(uint32 buffer) external; function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) external; function distributeRelayerRefund( - uint256 relayerRefundId, + uint32 relayerRefundId, DestinationDistributionLeaf memory distributionLeaf, bytes32[] memory inclusionProof ) external; diff --git a/contracts/test/MockSpokePool.sol b/contracts/test/MockSpokePool.sol index f2e1392c0..b02c88f44 100644 --- a/contracts/test/MockSpokePool.sol +++ b/contracts/test/MockSpokePool.sol @@ -13,9 +13,9 @@ contract MockSpokePool is SpokePoolInterface, SpokePool { address _crossDomainAdmin, address _hubPool, address _wethAddress, - uint64 _depositQuoteTimeBuffer, - address timerAddress - ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, _depositQuoteTimeBuffer, timerAddress) {} + address timerAddress, + uint32 _depositQuoteTimeBuffer + ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress, _depositQuoteTimeBuffer) {} function setCrossDomainAdmin(address newCrossDomainAdmin) public override { _setCrossDomainAdmin(newCrossDomainAdmin); @@ -27,13 +27,13 @@ contract MockSpokePool is SpokePoolInterface, SpokePool { function setEnableRoute( address originToken, - uint256 destinationChainId, + uint32 destinationChainId, bool enable ) public override { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint64 buffer) public override { + function setDepositQuoteTimeBuffer(uint32 buffer) public override { _setDepositQuoteTimeBuffer(buffer); } From cfc18cb6bf20019398ab11a75e53fbed95666469 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 12:07:20 -0700 Subject: [PATCH 02/19] fix tests --- test/HubPool.Fixture.ts | 2 +- test/MerkleLib.Proofs.ts | 2 +- test/SpokePool.Deposit.ts | 44 ++--- test/SpokePool.Fixture.ts | 62 +++++-- test/SpokePool.Relay.ts | 223 ++++++++++-------------- test/SpokePool.SlowRelay.ts | 97 +++++------ test/chain-adapters/Arbitrum_Adapter.ts | 15 +- 7 files changed, 228 insertions(+), 217 deletions(-) diff --git a/test/HubPool.Fixture.ts b/test/HubPool.Fixture.ts index 657449e58..73d2d4af1 100644 --- a/test/HubPool.Fixture.ts +++ b/test/HubPool.Fixture.ts @@ -42,7 +42,7 @@ export const hubPoolFixture = hre.deployments.createFixture(async ({ ethers }) = const mockAdapter = await (await getContractFactory("Mock_Adapter", signer)).deploy(hubPool.address); const mockSpoke = await ( await getContractFactory("MockSpokePool", { signer: signer, libraries: { MerkleLib: merkleLib.address } }) - ).deploy(crossChainAdmin.address, hubPool.address, weth.address, 0, parentFixtureOutput.timer.address); + ).deploy(crossChainAdmin.address, hubPool.address, weth.address, parentFixtureOutput.timer.address, 0); await hubPool.setCrossChainContracts(repaymentChainId, mockAdapter.address, mockSpoke.address); // Deploy mock l2 tokens for each token created before and whitelist the routes. diff --git a/test/MerkleLib.Proofs.ts b/test/MerkleLib.Proofs.ts index 63bd99f70..70b2491c4 100644 --- a/test/MerkleLib.Proofs.ts +++ b/test/MerkleLib.Proofs.ts @@ -64,7 +64,7 @@ describe("MerkleLib Proofs", async function () { } destinationDistributionLeafs.push({ leafId: BigNumber.from(i), - chainId: randomBigNumber(), + chainId: randomBigNumber(2), amountToReturn: randomBigNumber(), l2TokenAddress: randomAddress(), refundAddresses, diff --git a/test/SpokePool.Deposit.ts b/test/SpokePool.Deposit.ts index 08e8c61a6..218afd414 100644 --- a/test/SpokePool.Deposit.ts +++ b/test/SpokePool.Deposit.ts @@ -33,20 +33,20 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime ) ) .to.emit(spokePool, "FundsDeposited") .withArgs( - destinationChainId, amountToDeposit, - 0, depositRelayerFeePct, + destinationChainId, + 0, currentSpokePoolTime, erc20.address, recipient.address, @@ -68,11 +68,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, weth.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime, { value: 1 } ) @@ -82,11 +82,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, weth.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime, { value: amountToDeposit } ) @@ -102,11 +102,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, weth.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime, { value: 0 } ) @@ -121,11 +121,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime ) ).to.be.reverted; @@ -136,11 +136,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, unwhitelistedErc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime ) ).to.be.reverted; @@ -151,11 +151,11 @@ describe("SpokePool Depositor Logic", async function () { spokePool .connect(depositor) .deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, currentSpokePoolTime ) ).to.be.reverted; @@ -165,11 +165,11 @@ describe("SpokePool Depositor Logic", async function () { // Cannot deposit with invalid relayer fee. await expect( spokePool.connect(depositor).deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, toWei("1"), // Fee > 50% + destinationChainId, currentSpokePoolTime ) ).to.be.reverted; @@ -177,21 +177,21 @@ describe("SpokePool Depositor Logic", async function () { // Cannot deposit invalid quote fee. await expect( spokePool.connect(depositor).deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, toBN(currentSpokePoolTime).add(toBN("700")) // > 10 mins in future ) ).to.be.reverted; await expect( spokePool.connect(depositor).deposit( + recipient.address, erc20.address, - destinationChainId, amountToDeposit, - recipient.address, depositRelayerFeePct, + destinationChainId, toBN(currentSpokePoolTime).sub(toBN("700")) // > 10 mins in past ) ).to.be.reverted; diff --git a/test/SpokePool.Fixture.ts b/test/SpokePool.Fixture.ts index 5dfa11aa8..ba43871c7 100644 --- a/test/SpokePool.Fixture.ts +++ b/test/SpokePool.Fixture.ts @@ -25,7 +25,7 @@ export const spokePoolFixture = hre.deployments.createFixture(async ({ ethers }) const merkleLib = await (await getContractFactory("MerkleLib", deployerWallet)).deploy(); const spokePool = await ( await getContractFactory("MockSpokePool", { signer: deployerWallet, libraries: { MerkleLib: merkleLib.address } }) - ).deploy(crossChainAdmin.address, hubPool.address, weth.address, consts.depositQuoteTimeBuffer, timer.address); + ).deploy(crossChainAdmin.address, hubPool.address, weth.address, timer.address, consts.depositQuoteTimeBuffer); return { timer, weth, erc20, spokePool, unwhitelistedErc20, destErc20 }; }); @@ -55,11 +55,11 @@ export async function deposit( await spokePool .connect(depositor) .deposit( + recipient.address, token.address, - consts.destinationChainId, consts.amountToDeposit, - recipient.address, consts.depositRelayerFeePct, + consts.destinationChainId, currentSpokePoolTime ); } @@ -67,11 +67,11 @@ export interface RelayData { depositor: string; recipient: string; destinationToken: string; + relayAmount: string; realizedLpFeePct: string; relayerFeePct: string; depositId: string; originChainId: string; - relayAmount: string; } export function getRelayHash( _depositor: string, @@ -82,31 +82,71 @@ export function getRelayHash( _relayAmount?: string, _realizedLpFeePct?: string, _relayerFeePct?: string -): { relayHash: string; relayData: RelayData; relayDataValues: string[] } { +): { relayHash: string; relayData: RelayData } { const relayData = { depositor: _depositor, recipient: _recipient, destinationToken: _destinationToken, + relayAmount: _relayAmount || consts.amountToDeposit.toString(), realizedLpFeePct: _realizedLpFeePct || consts.realizedLpFeePct.toString(), relayerFeePct: _relayerFeePct || consts.depositRelayerFeePct.toString(), depositId: _depositId.toString(), originChainId: _originChainId.toString(), - relayAmount: _relayAmount || consts.amountToDeposit.toString(), }; - const relayDataValues = Object.values(relayData); const relayHash = ethers.utils.keccak256( defaultAbiCoder.encode( - ["address", "address", "address", "uint64", "uint64", "uint64", "uint256", "uint256"], - relayDataValues + ["address", "address", "address", "uint256", "uint64", "uint64", "uint32", "uint32"], + Object.values(relayData) ) ); return { relayHash, relayData, - relayDataValues, }; } +export function getFillRelayParams( + _relayData: RelayData, + _maxTokensToSend: BigNumber, + _repaymentChain?: number +): string[] { + return [ + _relayData.depositor, + _relayData.recipient, + _relayData.destinationToken, + _relayData.relayAmount, + _maxTokensToSend.toString(), + _relayData.realizedLpFeePct, + _relayData.relayerFeePct, + _repaymentChain ? _repaymentChain.toString() : consts.repaymentChainId.toString(), + _relayData.depositId, + _relayData.originChainId, + ]; +} + +export function getFillRelayUpdatedFeeParams( + _relayData: RelayData, + _maxTokensToSend: BigNumber, + _updatedFee: BigNumber, + _signature: string, + _repaymentChain?: number +): string[] { + return [ + _relayData.depositor, + _relayData.recipient, + _relayData.destinationToken, + _relayData.relayAmount, + _maxTokensToSend.toString(), + _relayData.realizedLpFeePct, + _relayData.relayerFeePct, + _updatedFee.toString(), + _repaymentChain ? _repaymentChain.toString() : consts.repaymentChainId.toString(), + _relayData.depositId, + _relayData.originChainId, + _signature, + ]; +} + export interface UpdatedRelayerFeeData { newRelayerFeePct: string; depositorMessageHash: string; @@ -120,7 +160,7 @@ export async function modifyRelayHelper( ): Promise<{ messageHash: string; signature: string }> { const messageHash = ethers.utils.keccak256( defaultAbiCoder.encode( - ["string", "uint64", "uint64", "uint256"], + ["string", "uint64", "uint32", "uint32"], ["ACROSS-V2-FEE-1.0", modifiedRelayerFeePct, depositId, originChainId] ) ); diff --git a/test/SpokePool.Relay.ts b/test/SpokePool.Relay.ts index bbbec4611..9d9690d1e 100644 --- a/test/SpokePool.Relay.ts +++ b/test/SpokePool.Relay.ts @@ -1,5 +1,12 @@ import { expect, Contract, ethers, SignerWithAddress, seedWallet, toWei, toBN } from "./utils"; -import { spokePoolFixture, enableRoutes, getRelayHash, modifyRelayHelper } from "./SpokePool.Fixture"; +import { + spokePoolFixture, + enableRoutes, + getRelayHash, + modifyRelayHelper, + getFillRelayParams, + getFillRelayUpdatedFeeParams, +} from "./SpokePool.Fixture"; import * as consts from "./constants"; let spokePool: Contract, weth: Contract, erc20: Contract, destErc20: Contract; @@ -24,7 +31,7 @@ describe("SpokePool Relayer Logic", async function () { await enableRoutes(spokePool, [{ originToken: erc20.address }, { originToken: weth.address }]); }); it("Relaying ERC20 tokens correctly pulls tokens and changes contract state", async function () { - const { relayHash, relayData, relayDataValues } = getRelayHash( + const { relayHash, relayData } = getRelayHash( depositor.address, recipient.address, consts.firstDepositId, @@ -32,20 +39,18 @@ describe("SpokePool Relayer Logic", async function () { destErc20.address ); - await expect( - spokePool.connect(relayer).fillRelay(...relayDataValues, consts.amountToRelay, consts.repaymentChainId) - ) + await expect(spokePool.connect(relayer).fillRelay(...getFillRelayParams(relayData, consts.amountToRelay))) .to.emit(spokePool, "FilledRelay") .withArgs( relayHash, relayData.relayAmount, consts.amountToRelayPreFees, consts.amountToRelayPreFees, - consts.repaymentChainId, - relayData.originChainId, - relayData.depositId, relayData.relayerFeePct, relayData.realizedLpFeePct, + consts.repaymentChainId, + toBN(relayData.originChainId), + toBN(relayData.depositId), relayData.destinationToken, relayer.address, relayData.depositor, @@ -63,25 +68,7 @@ describe("SpokePool Relayer Logic", async function () { // pulls exactly enough tokens to complete the relay. const fullRelayAmount = consts.amountToDeposit; const fullRelayAmountPostFees = fullRelayAmount.mul(consts.totalPostFeesPct).div(toBN(consts.oneHundredPct)); - const amountRemainingInRelay = fullRelayAmount.sub(consts.amountToRelayPreFees); - // const amountRemainingInRelayPostFees = amountRemainingInRelay.mul(totalPostFeesPct).div(toBN(oneHundredPct)); - await expect(spokePool.connect(relayer).fillRelay(...relayDataValues, fullRelayAmount, consts.repaymentChainId)) - .to.emit(spokePool, "FilledRelay") - .withArgs( - relayHash, - relayData.relayAmount, - fullRelayAmount, - amountRemainingInRelay, - consts.repaymentChainId, - relayData.originChainId, - relayData.depositId, - relayData.relayerFeePct, - relayData.realizedLpFeePct, - relayData.destinationToken, - relayer.address, - relayData.depositor, - relayData.recipient - ); + await spokePool.connect(relayer).fillRelay(...getFillRelayParams(relayData, fullRelayAmount)); expect(await destErc20.balanceOf(relayer.address)).to.equal( consts.amountToSeedWallets.sub(fullRelayAmountPostFees) ); @@ -91,7 +78,7 @@ describe("SpokePool Relayer Logic", async function () { expect(await spokePool.relayFills(relayHash)).to.equal(fullRelayAmount); }); it("Relaying WETH correctly unwraps into ETH", async function () { - const { relayHash, relayData, relayDataValues } = getRelayHash( + const { relayHash, relayData } = getRelayHash( depositor.address, recipient.address, consts.firstDepositId, @@ -100,25 +87,7 @@ describe("SpokePool Relayer Logic", async function () { ); const startingRecipientBalance = await recipient.getBalance(); - await expect( - spokePool.connect(relayer).fillRelay(...relayDataValues, consts.amountToRelay, consts.repaymentChainId) - ) - .to.emit(spokePool, "FilledRelay") - .withArgs( - relayHash, - relayData.relayAmount, - consts.amountToRelayPreFees, - consts.amountToRelayPreFees, - consts.repaymentChainId, - relayData.originChainId, - relayData.depositId, - relayData.relayerFeePct, - relayData.realizedLpFeePct, - relayData.destinationToken, - relayer.address, - relayData.depositor, - relayData.recipient - ); + await spokePool.connect(relayer).fillRelay(...getFillRelayParams(relayData, consts.amountToRelay)); // The collateral should have unwrapped to ETH and then transferred to recipient. expect(await weth.balanceOf(relayer.address)).to.equal(consts.amountToSeedWallets.sub(consts.amountToRelay)); @@ -133,72 +102,78 @@ describe("SpokePool Relayer Logic", async function () { spokePool .connect(relayer) .fillRelay( - ...getRelayHash( - depositor.address, - recipient.address, - consts.firstDepositId, - consts.originChainId, - destErc20.address, - consts.amountToDeposit.toString(), - toWei("0.5").toString(), - consts.depositRelayerFeePct.toString() - ).relayDataValues, - consts.amountToRelay, - consts.repaymentChainId + ...getFillRelayParams( + getRelayHash( + depositor.address, + recipient.address, + consts.firstDepositId, + consts.originChainId, + destErc20.address, + consts.amountToDeposit.toString(), + toWei("0.5").toString(), + consts.depositRelayerFeePct.toString() + ).relayData, + consts.amountToRelay, + consts.repaymentChainId + ) ) ).to.be.revertedWith("invalid fees"); await expect( spokePool .connect(relayer) .fillRelay( - ...getRelayHash( - depositor.address, - recipient.address, - consts.firstDepositId, - consts.originChainId, - destErc20.address, - consts.amountToDeposit.toString(), - consts.realizedLpFeePct.toString(), - toWei("0.5").toString() - ).relayDataValues, - consts.amountToRelay, - consts.repaymentChainId + ...getFillRelayParams( + getRelayHash( + depositor.address, + recipient.address, + consts.firstDepositId, + consts.originChainId, + destErc20.address, + consts.amountToDeposit.toString(), + consts.realizedLpFeePct.toString(), + toWei("0.5").toString() + ).relayData, + consts.amountToRelay, + consts.repaymentChainId + ) ) ).to.be.revertedWith("invalid fees"); // Relay already filled await spokePool.connect(relayer).fillRelay( - ...getRelayHash( - depositor.address, - recipient.address, - consts.firstDepositId, - consts.originChainId, - destErc20.address - ).relayDataValues, - consts.amountToDeposit, // Send the full relay amount - consts.repaymentChainId + ...getFillRelayParams( + getRelayHash( + depositor.address, + recipient.address, + consts.firstDepositId, + consts.originChainId, + destErc20.address + ).relayData, + consts.amountToDeposit, // Send the full relay amount + consts.repaymentChainId + ) ); await expect( - spokePool - .connect(relayer) - .fillRelay( - ...getRelayHash( + spokePool.connect(relayer).fillRelay( + ...getFillRelayParams( + getRelayHash( depositor.address, recipient.address, consts.firstDepositId, consts.originChainId, destErc20.address - ).relayDataValues, - "1", + ).relayData, + toBN("1"), // relay any amount consts.repaymentChainId ) + ) ).to.be.revertedWith("relay filled"); }); it("Can fill relay with updated fee by including proof of depositor's agreement", async function () { // The relay should succeed just like before with the same amount of tokens pulled from the relayer's wallet, // however the filled amount should have increased since the proportion of the relay filled would increase with a // higher fee. - const { relayHash, relayData, relayDataValues } = getRelayHash( + const { relayHash, relayData } = getRelayHash( depositor.address, recipient.address, consts.firstDepositId, @@ -211,28 +186,10 @@ describe("SpokePool Relayer Logic", async function () { relayData.originChainId, depositor ); - // Note: modifiedRelayFeePct is inserted in-place into middle of the same params passed to fillRelay - relayDataValues.splice(5, 0, consts.modifiedRelayerFeePct.toString()); - await expect( - spokePool - .connect(relayer) - .fillRelayWithUpdatedFee(...relayDataValues, consts.amountToRelay, consts.repaymentChainId, signature) - ) - .to.emit(spokePool, "FilledRelay") - .withArgs( - relayHash, - relayData.relayAmount, - consts.amountToRelayPreModifiedFees, - consts.amountToRelayPreModifiedFees, - consts.repaymentChainId, - relayData.originChainId, - relayData.depositId, - consts.modifiedRelayerFeePct, // The relayer fee % emitted in event should change - relayData.realizedLpFeePct, - relayData.destinationToken, - relayer.address, - relayData.depositor, - relayData.recipient + await spokePool + .connect(relayer) + .fillRelayWithUpdatedFee( + ...getFillRelayUpdatedFeeParams(relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, signature) ); // The collateral should have transferred from relayer to recipient. @@ -243,15 +200,13 @@ describe("SpokePool Relayer Logic", async function () { expect(await spokePool.relayFills(relayHash)).to.equal(consts.amountToRelayPreModifiedFees); }); it("Updating relayer fee signature verification failure cases", async function () { - const { relayDataValues, relayData } = getRelayHash( + const { relayData } = getRelayHash( depositor.address, recipient.address, consts.firstDepositId, consts.originChainId, destErc20.address ); - // Note: modifiedRelayFeePct is inserted in-place into middle of the same params passed to fillRelay - relayDataValues.splice(5, 0, consts.modifiedRelayerFeePct.toString()); // Message hash doesn't contain the modified fee passed as a function param. const { signature: incorrectFeeSignature } = await modifyRelayHelper( @@ -264,16 +219,18 @@ describe("SpokePool Relayer Logic", async function () { spokePool .connect(relayer) .fillRelayWithUpdatedFee( - ...relayDataValues, - consts.amountToRelay, - consts.repaymentChainId, - incorrectFeeSignature + ...getFillRelayUpdatedFeeParams( + relayData, + consts.amountToRelay, + consts.modifiedRelayerFeePct, + incorrectFeeSignature + ) ) ).to.be.revertedWith("invalid signature"); // Relay data depositID and originChainID don't match data included in relay hash const { signature: incorrectDepositIdSignature } = await modifyRelayHelper( - consts.incorrectModifiedRelayerFeePct, + consts.modifiedRelayerFeePct, relayData.depositId + "1", relayData.originChainId, depositor @@ -282,14 +239,16 @@ describe("SpokePool Relayer Logic", async function () { spokePool .connect(relayer) .fillRelayWithUpdatedFee( - ...relayDataValues, - consts.amountToRelay, - consts.repaymentChainId, - incorrectDepositIdSignature + ...getFillRelayUpdatedFeeParams( + relayData, + consts.amountToRelay, + consts.modifiedRelayerFeePct, + incorrectDepositIdSignature + ) ) ).to.be.revertedWith("invalid signature"); const { signature: incorrectChainIdSignature } = await modifyRelayHelper( - consts.incorrectModifiedRelayerFeePct, + consts.modifiedRelayerFeePct, relayData.depositId, relayData.originChainId + "1", depositor @@ -298,10 +257,12 @@ describe("SpokePool Relayer Logic", async function () { spokePool .connect(relayer) .fillRelayWithUpdatedFee( - ...relayDataValues, - consts.amountToRelay, - consts.repaymentChainId, - incorrectChainIdSignature + ...getFillRelayUpdatedFeeParams( + relayData, + consts.amountToRelay, + consts.modifiedRelayerFeePct, + incorrectChainIdSignature + ) ) ).to.be.revertedWith("invalid signature"); @@ -316,10 +277,12 @@ describe("SpokePool Relayer Logic", async function () { spokePool .connect(relayer) .fillRelayWithUpdatedFee( - ...relayDataValues, - consts.amountToRelay, - consts.repaymentChainId, - incorrectSignerSignature + ...getFillRelayUpdatedFeeParams( + relayData, + consts.amountToRelay, + consts.modifiedRelayerFeePct, + incorrectSignerSignature + ) ) ).to.be.revertedWith("invalid signature"); }); diff --git a/test/SpokePool.SlowRelay.ts b/test/SpokePool.SlowRelay.ts index 16dfef43f..942a61abb 100644 --- a/test/SpokePool.SlowRelay.ts +++ b/test/SpokePool.SlowRelay.ts @@ -12,15 +12,12 @@ import { defaultAbiCoder, keccak256, } from "./utils"; -import { spokePoolFixture, enableRoutes, getRelayHash, modifyRelayHelper, RelayData } from "./SpokePool.Fixture"; +import { spokePoolFixture, enableRoutes, RelayData } from "./SpokePool.Fixture"; import { MerkleTree } from "../utils/MerkleTree"; import * as consts from "./constants"; let spokePool: Contract, weth: Contract, erc20: Contract, destErc20: Contract; -let depositor: SignerWithAddress, - recipient: SignerWithAddress, - relayer: SignerWithAddress, - slowRelayer: SignerWithAddress; +let depositor: SignerWithAddress, recipient: SignerWithAddress, relayer: SignerWithAddress; let relays: RelayData[]; let tree: MerkleTree; @@ -28,7 +25,7 @@ const fullRelayAmountPostFees = consts.amountToRelay.mul(consts.totalPostFeesPct describe("SpokePool Slow Relay Logic", async function () { beforeEach(async function () { - [depositor, recipient, relayer, slowRelayer] = await ethers.getSigners(); + [depositor, recipient, relayer] = await ethers.getSigners(); ({ weth, erc20, spokePool, destErc20 } = await spokePoolFixture()); // mint some fresh tokens and deposit ETH for weth for depositor and relayer. @@ -54,11 +51,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor: randomAddress(), recipient: randomAddress(), destinationToken: randomAddress(), + relayAmount: randomBigNumber().toString(), realizedLpFeePct: randomBigNumber(8).toString(), relayerFeePct: randomBigNumber(8).toString(), - depositId: randomBigNumber(8).toString(), - originChainId: randomBigNumber().toString(), - relayAmount: randomBigNumber().toString(), + depositId: randomBigNumber(2).toString(), + originChainId: randomBigNumber(2).toString(), }); } @@ -67,11 +64,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor: depositor.address, recipient: recipient.address, destinationToken: destErc20.address, + relayAmount: consts.amountToRelay.toString(), realizedLpFeePct: consts.realizedLpFeePct.toString(), relayerFeePct: consts.depositRelayerFeePct.toString(), depositId: consts.firstDepositId.toString(), originChainId: consts.originChainId.toString(), - relayAmount: consts.amountToRelay.toString(), }); // WETH @@ -79,11 +76,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor: depositor.address, recipient: recipient.address, destinationToken: weth.address, + relayAmount: consts.amountToRelay.toString(), realizedLpFeePct: consts.realizedLpFeePct.toString(), relayerFeePct: consts.depositRelayerFeePct.toString(), depositId: consts.firstDepositId.toString(), originChainId: consts.originChainId.toString(), - relayAmount: consts.amountToRelay.toString(), }); const paramType = await getParamType("MerkleLib", "verifySlowRelayFulfillment", "slowRelayFulfillment"); @@ -102,11 +99,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === destErc20.address)!) ) @@ -127,11 +124,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === destErc20.address)!) ) @@ -142,10 +139,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.amountToRelay, consts.amountToRelay, consts.amountToRelay, - consts.originChainId, - consts.firstDepositId, consts.depositRelayerFeePct, consts.realizedLpFeePct, + consts.originChainId, + consts.firstDepositId, destErc20.address, relayer.address, depositor.address, @@ -161,11 +158,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -180,11 +177,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -201,11 +198,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -216,10 +213,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.amountToRelay, consts.amountToRelay, consts.amountToRelay, - consts.originChainId, - consts.firstDepositId, consts.depositRelayerFeePct, consts.realizedLpFeePct, + consts.originChainId, + consts.firstDepositId, weth.address, relayer.address, depositor.address, @@ -237,13 +234,13 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, + partialAmountPostFees, consts.realizedLpFeePct, consts.depositRelayerFeePct, + consts.repaymentChainId, consts.firstDepositId, - consts.originChainId, - consts.amountToRelay, - partialAmountPostFees, - consts.repaymentChainId + consts.originChainId ); await expect(() => spokePool @@ -252,11 +249,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === destErc20.address)!) ) @@ -274,13 +271,13 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, + partialAmountPostFees, consts.realizedLpFeePct, consts.depositRelayerFeePct, + consts.repaymentChainId, consts.firstDepositId, - consts.originChainId, - consts.amountToRelay, - partialAmountPostFees, - consts.repaymentChainId + consts.originChainId ); await expect( @@ -290,11 +287,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, destErc20.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === destErc20.address)!) ) @@ -305,10 +302,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.amountToRelay, consts.amountToRelay, leftoverPreFees, - consts.originChainId, - consts.firstDepositId, consts.depositRelayerFeePct, consts.realizedLpFeePct, + consts.originChainId, + consts.firstDepositId, destErc20.address, relayer.address, depositor.address, @@ -326,13 +323,13 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, + partialAmountPostFees, consts.realizedLpFeePct, consts.depositRelayerFeePct, + consts.repaymentChainId, consts.firstDepositId, - consts.originChainId, - consts.amountToRelay, - partialAmountPostFees, - consts.repaymentChainId + consts.originChainId ); await expect(() => @@ -342,11 +339,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -363,13 +360,13 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, + partialAmountPostFees, consts.realizedLpFeePct, consts.depositRelayerFeePct, + consts.repaymentChainId, consts.firstDepositId, - consts.originChainId, - consts.amountToRelay, - partialAmountPostFees, - consts.repaymentChainId + consts.originChainId ); await expect(() => @@ -379,11 +376,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -401,13 +398,13 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, + partialAmountPostFees, consts.realizedLpFeePct, consts.depositRelayerFeePct, + consts.repaymentChainId, consts.firstDepositId, - consts.originChainId, - consts.amountToRelay, - partialAmountPostFees, - consts.repaymentChainId + consts.originChainId ); await expect( @@ -417,11 +414,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay, consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay, 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) @@ -432,10 +429,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.amountToRelay, consts.amountToRelay, leftoverPreFees, - consts.originChainId, - consts.firstDepositId, consts.depositRelayerFeePct, consts.realizedLpFeePct, + consts.originChainId, + consts.firstDepositId, weth.address, relayer.address, depositor.address, @@ -449,11 +446,11 @@ describe("SpokePool Slow Relay Logic", async function () { depositor.address, recipient.address, weth.address, + consts.amountToRelay.sub(1), // Slightly modify the relay data from the expected set. consts.realizedLpFeePct, consts.depositRelayerFeePct, consts.firstDepositId, consts.originChainId, - consts.amountToRelay.sub(1), // Slightly modify the relay data from the expected set. 0, tree.getHexProof(relays.find((relay) => relay.destinationToken === weth.address)!) ) diff --git a/test/chain-adapters/Arbitrum_Adapter.ts b/test/chain-adapters/Arbitrum_Adapter.ts index 87eacde3c..b587900e1 100644 --- a/test/chain-adapters/Arbitrum_Adapter.ts +++ b/test/chain-adapters/Arbitrum_Adapter.ts @@ -76,7 +76,15 @@ describe("Arbitrum Chain Adapter", function () { // Create an action that will send an L1->L2 tokens transfer and bundle. For this, create a relayer repayment bundle // and check that at it's finalization the L2 bridge contracts are called as expected. const { leafs, tree, tokensSendToL2 } = await constructSingleChainTree(dai, 1, arbitrumChainId); - await hubPool.connect(dataWorker).initiateRelayerRefund([3117], 1, tree.getHexRoot(), consts.mockTreeRoot); + await hubPool + .connect(dataWorker) + .initiateRelayerRefund( + [3117], + 1, + tree.getHexRoot(), + consts.mockDestinationDistributionRoot, + consts.mockSlowRelayFulfillmentRoot + ); await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness); await hubPool.connect(dataWorker).executeRelayerRefund(leafs[0], tree.getHexProof(leafs[0])); // The correct functions should have been called on the arbitrum contracts. @@ -99,7 +107,10 @@ describe("Arbitrum Chain Adapter", function () { owner.address, consts.sampleL2Gas, consts.sampleL2GasPrice, - mockSpoke.interface.encodeFunctionData("initializeRelayerRefund", [consts.mockTreeRoot]) + mockSpoke.interface.encodeFunctionData("initializeRelayerRefund", [ + consts.mockDestinationDistributionRoot, + consts.mockSlowRelayFulfillmentRoot, + ]) ); }); }); From d8b055284f8a0cc6d775329b10a9227104049d48 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 12:14:52 -0700 Subject: [PATCH 03/19] Update hardhat.config.ts --- hardhat.config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardhat.config.ts b/hardhat.config.ts index 8e54673f0..14ba21bbd 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -1,6 +1,6 @@ import * as dotenv from "dotenv"; -import { HardhatUserConfig, task } from "hardhat/config"; +import { HardhatUserConfig } from "hardhat/config"; import "@nomiclabs/hardhat-etherscan"; import "@nomiclabs/hardhat-waffle"; import "@typechain/hardhat"; From b95bfbbf8dfc35462f535a27a5bbe0b1489a78a0 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 12:18:52 -0700 Subject: [PATCH 04/19] Update package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5ad3c3657..3579435f8 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "lint": "yarn prettier --list-different", "lint-fix": "yarn prettier --write", "prettier": "prettier .", - "test": "yarn hardhat test" + "test": "REPORT_GAS=true yarn hardhat test" }, "dependencies": { "@defi-wonderland/smock": "^2.0.7", From ebee27e95c97256e2fe5a76367aee1b2fa835287 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 12:39:17 -0700 Subject: [PATCH 05/19] add arbitrum spokepool skeleton --- contracts/Arbitrum_SpokePool.sol | 122 +++++++++++++++++++++++++++++++ contracts/Optimism_SpokePool.sol | 12 ++- 2 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 contracts/Arbitrum_SpokePool.sol diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol new file mode 100644 index 000000000..d6b51e5d7 --- /dev/null +++ b/contracts/Arbitrum_SpokePool.sol @@ -0,0 +1,122 @@ +//SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.0; + +import "@openzeppelin/contracts/access/Ownable.sol"; +import "./SpokePool.sol"; +import "./SpokePoolInterface.sol"; + +interface StandardBridgeLike { + function outboundTransfer( + address _l1Token, + address _to, + uint256 _amount, + bytes calldata _data + ) external payable returns (bytes memory); +} + +/** + * @notice AVM specific SpokePool. + * @dev Uses AVM cross-domain-enabled logic for access control. + */ + +contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { + // Address of the Arbitrum L2 token gateway. + address public l2GatewayRouter; + + event ArbitrumTokensBridged(address indexed l1Token, address target, uint256 numberOfTokensBridged); + event SetL2GatewayRouter(address indexed newL2GatewayRouter); + + constructor( + address _l2GatewayRouter, + address _crossDomainAdmin, + address _hubPool, + address _wethAddress, + address timerAddress, + uint32 _depositQuoteTimeBuffer + ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress, _depositQuoteTimeBuffer) { + _setL2GatewayRouter(_l2GatewayRouter); + } + + modifier onlyFromCrossDomainAccount(address l1Counterpart) { + require(msg.sender == _applyL1ToL2Alias(l1Counterpart), "ONLY_COUNTERPART_GATEWAY"); + _; + } + + /************************************** + * ADMIN FUNCTIONS * + **************************************/ + function setL2GatewayRouter(address newL2GatewayRouter) public onlyOwner nonReentrant { + _setL2GatewayRouter(newL2GatewayRouter); + } + + /************************************** + * CROSS-CHAIN ADMIN FUNCTIONS * + **************************************/ + + /** + * @notice Changes the L1 contract that can trigger admin functions on this contract. + * @dev This should be set to the address of the L1 contract that ultimately relays a cross-domain message, which + * is expected to be the Optimism_Adapter. + * @dev Only callable by the existing admin via the Optimism cross domain messenger. + * @param newCrossDomainAdmin address of the new L1 admin contract. + */ + function setCrossDomainAdmin(address newCrossDomainAdmin) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { + _setCrossDomainAdmin(newCrossDomainAdmin); + } + + function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + _setHubPool(newHubPool); + } + + function setEnableRoute( + address originToken, + uint32 destinationChainId, + bool enable + ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + _setEnableRoute(originToken, destinationChainId, enable); + } + + function setDepositQuoteTimeBuffer(uint32 buffer) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { + _setDepositQuoteTimeBuffer(buffer); + } + + function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { + _initializeRelayerRefund(relayerRepaymentDistributionRoot, slowRelayRoot); + } + + function _bridgeTokensToHubPool(DestinationDistributionLeaf memory distributionLeaf) internal override { + StandardBridgeLike(l2GatewayRouter).outboundTransfer( + // THIS IS A PROBLEM!!, we need the L1 token address not the L2 token address + address(0), // _l1Token. Address of the L1 token to bridge over. + hubPool, // _to. Withdraw, over the bridge, to the l1 hub pool contract. + distributionLeaf.amountToReturn, // _amount. + "" // _data. We don't need to send any data for the bridging action. + ); + emit ArbitrumTokensBridged(address(0), hubPool, distributionLeaf.amountToReturn); + } + + function _setL2GatewayRouter(address _l2GatewayRouter) internal { + l2GatewayRouter = _l2GatewayRouter; + emit SetL2GatewayRouter(l2GatewayRouter); + } + + // l1 addresses are transformed during l1->l2 calls. See https://developer.offchainlabs.com/docs/l1_l2_messages#address-aliasing for more information. + function _applyL1ToL2Alias(address l1Address) internal pure returns (address l2Address) { + l2Address = address(uint160(l1Address) + uint160(0x1111000000000000000000000000000000001111)); + } +} diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index 7c9e6a46d..cd5722221 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -18,6 +18,7 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool uint32 l1Gas = 6_000_000; event OptimismTokensBridged(address indexed l2Token, address target, uint256 numberOfTokensBridged, uint256 l1Gas); + event SetL1Gas(uint32 indexed newL1Gas); constructor( address _crossDomainAdmin, @@ -33,8 +34,8 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool /************************************** * ADMIN FUNCTIONS * **************************************/ - function setL1GasLimit(uint32 newl1Gas) public onlyOwner nonReentrant { - l1Gas = newl1Gas; + function setL1GasLimit(uint32 newl1Gas) public onlyOwner { + _setL1GasLimit(newl1Gas); } /************************************** @@ -87,12 +88,17 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool _initializeRelayerRefund(relayerRepaymentDistributionRoot, slowRelayRoot); } + function _setL1GasLimit(uint32 _l1Gas) internal { + l1Gas = _l1Gas; + emit SetL1Gas(l1Gas); + } + function _bridgeTokensToHubPool(DestinationDistributionLeaf memory distributionLeaf) internal override { // TODO: Handle WETH token unwrapping IL2ERC20Bridge(Lib_PredeployAddresses.L2_STANDARD_BRIDGE).withdrawTo( distributionLeaf.l2TokenAddress, // _l2Token. Address of the L2 token to bridge over. hubPool, // _to. Withdraw, over the bridge, to the l1 pool contract. - distributionLeaf.amountToReturn, // _amount. Send the full balance of the deposit box to bridge. + distributionLeaf.amountToReturn, // _amount. l1Gas, // _l1Gas. Unused, but included for potential forward compatibility considerations "" // _data. We don't need to send any data for the bridging action. ); From a1cd61e6c0772e0e2b62821cdc95d071317f9b0e Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 15:55:46 -0700 Subject: [PATCH 06/19] Add onlyowner methods to spoke pools --- contracts/Arbitrum_SpokePool.sol | 52 ++++++++++++++++---------------- contracts/HubPool.sol | 1 - contracts/Optimism_SpokePool.sol | 37 ++++++++--------------- 3 files changed, 38 insertions(+), 52 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index d6b51e5d7..fa298cdeb 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -23,8 +23,13 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { // Address of the Arbitrum L2 token gateway. address public l2GatewayRouter; + // Admin controlled mapping of arbitrum tokens to L1 counterpart. L1 counterpart addresses + // are neccessary to bridge tokens to L1. + mapping(address => address) public whitelistedTokens; + event ArbitrumTokensBridged(address indexed l1Token, address target, uint256 numberOfTokensBridged); event SetL2GatewayRouter(address indexed newL2GatewayRouter); + event WhitelistedTokens(address indexed l2Token, address indexed l1Token); constructor( address _l2GatewayRouter, @@ -49,27 +54,15 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { _setL2GatewayRouter(newL2GatewayRouter); } - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ + function whitelistToken(address l2Token, address l1Token) public onlyOwner nonReentrant { + _whitelistToken(l2Token, l1Token); + } - /** - * @notice Changes the L1 contract that can trigger admin functions on this contract. - * @dev This should be set to the address of the L1 contract that ultimately relays a cross-domain message, which - * is expected to be the Optimism_Adapter. - * @dev Only callable by the existing admin via the Optimism cross domain messenger. - * @param newCrossDomainAdmin address of the new L1 admin contract. - */ - function setCrossDomainAdmin(address newCrossDomainAdmin) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyOwner nonReentrant { _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + function setHubPool(address newHubPool) public override onlyOwner nonReentrant { _setHubPool(newHubPool); } @@ -77,19 +70,18 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { address originToken, uint32 destinationChainId, bool enable - ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + ) public override onlyOwner nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint32 buffer) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyOwner nonReentrant { _setDepositQuoteTimeBuffer(buffer); } + /************************************** + * CROSS-CHAIN ADMIN FUNCTIONS * + **************************************/ + function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) public override @@ -99,10 +91,13 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { _initializeRelayerRefund(relayerRepaymentDistributionRoot, slowRelayRoot); } + /************************************** + * INTERNAL FUNCTIONS * + **************************************/ + function _bridgeTokensToHubPool(DestinationDistributionLeaf memory distributionLeaf) internal override { StandardBridgeLike(l2GatewayRouter).outboundTransfer( - // THIS IS A PROBLEM!!, we need the L1 token address not the L2 token address - address(0), // _l1Token. Address of the L1 token to bridge over. + whitelistedTokens[distributionLeaf.l2TokenAddress], // _l1Token. Address of the L1 token to bridge over. hubPool, // _to. Withdraw, over the bridge, to the l1 hub pool contract. distributionLeaf.amountToReturn, // _amount. "" // _data. We don't need to send any data for the bridging action. @@ -115,6 +110,11 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { emit SetL2GatewayRouter(l2GatewayRouter); } + function _whitelistToken(address _l2Token, address _l1Token) internal { + whitelistedTokens[_l2Token][_l1Token]; + emit WhitelistedTokens(_l2Token, _l1Token); + } + // l1 addresses are transformed during l1->l2 calls. See https://developer.offchainlabs.com/docs/l1_l2_messages#address-aliasing for more information. function _applyL1ToL2Alias(address l1Address) internal pure returns (address l2Address) { l2Address = address(uint160(l1Address) + uint160(0x1111000000000000000000000000000000001111)); diff --git a/contracts/HubPool.sol b/contracts/HubPool.sol index 5bf23ff2a..563f2eca1 100644 --- a/contracts/HubPool.sol +++ b/contracts/HubPool.sol @@ -184,7 +184,6 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { address originToken, address destinationToken ) public onlyOwner { - // TODO: Should relay message to L2 for destinationChainId and call setEnableRoute(originToken, destinationChainId, true) whitelistedRoutes[originToken][destinationChainId] = destinationToken; emit WhitelistRoute(destinationChainId, originToken, destinationToken); } diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index cd5722221..99eee4625 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -38,27 +38,11 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool _setL1GasLimit(newl1Gas); } - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ - - /** - * @notice Changes the L1 contract that can trigger admin functions on this contract. - * @dev This should be set to the address of the L1 contract that ultimately relays a cross-domain message, which - * is expected to be the Optimism_Adapter. - * @dev Only callable by the existing admin via the Optimism cross domain messenger. - * @param newCrossDomainAdmin address of the new L1 admin contract. - */ - function setCrossDomainAdmin(address newCrossDomainAdmin) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyOwner nonReentrant { _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + function setHubPool(address newHubPool) public override onlyOwner nonReentrant { _setHubPool(newHubPool); } @@ -66,19 +50,18 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool address originToken, uint32 destinationChainId, bool enable - ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + ) public override onlyOwner nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint32 buffer) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyOwner nonReentrant { _setDepositQuoteTimeBuffer(buffer); } + /************************************** + * CROSS-CHAIN ADMIN FUNCTIONS * + **************************************/ + function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) public override @@ -88,6 +71,10 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool _initializeRelayerRefund(relayerRepaymentDistributionRoot, slowRelayRoot); } + /************************************** + * INTERNAL FUNCTIONS * + **************************************/ + function _setL1GasLimit(uint32 _l1Gas) internal { l1Gas = _l1Gas; emit SetL1Gas(l1Gas); From 9c7a71bc2ce64e93b24e7edda088a9cd8d4c527a Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 16:00:20 -0700 Subject: [PATCH 07/19] Update Arbitrum_SpokePool.sol --- contracts/Arbitrum_SpokePool.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index fa298cdeb..eb173af66 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -111,7 +111,7 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { } function _whitelistToken(address _l2Token, address _l1Token) internal { - whitelistedTokens[_l2Token][_l1Token]; + whitelistedTokens[_l2Token] = _l1Token; emit WhitelistedTokens(_l2Token, _l1Token); } From cc034f13b06e05ba4f0fd1bab0c888604960d4a9 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 16:25:27 -0700 Subject: [PATCH 08/19] Fix tests --- contracts/Arbitrum_SpokePool.sol | 40 +++++++++++++++++-------- contracts/HubPool.sol | 19 ++++++++++++ contracts/Optimism_SpokePool.sol | 30 +++++++++++-------- test/HubPool.Admin.ts | 5 ++-- test/HubPool.RefundExecution.ts | 7 +++-- test/chain-adapters/Arbitrum_Adapter.ts | 2 +- 6 files changed, 72 insertions(+), 31 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index eb173af66..167acfcc2 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -1,7 +1,6 @@ //SPDX-License-Identifier: Unlicense pragma solidity ^0.8.0; -import "@openzeppelin/contracts/access/Ownable.sol"; import "./SpokePool.sol"; import "./SpokePoolInterface.sol"; @@ -19,7 +18,7 @@ interface StandardBridgeLike { * @dev Uses AVM cross-domain-enabled logic for access control. */ -contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { +contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { // Address of the Arbitrum L2 token gateway. address public l2GatewayRouter; @@ -48,21 +47,35 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { } /************************************** - * ADMIN FUNCTIONS * + * CROSS-CHAIN ADMIN FUNCTIONS * **************************************/ - function setL2GatewayRouter(address newL2GatewayRouter) public onlyOwner nonReentrant { + + function setL2GatewayRouter(address newL2GatewayRouter) + public + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _setL2GatewayRouter(newL2GatewayRouter); } - function whitelistToken(address l2Token, address l1Token) public onlyOwner nonReentrant { + function whitelistToken(address l2Token, address l1Token) + public + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _whitelistToken(l2Token, l1Token); } - function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyOwner nonReentrant { + function setCrossDomainAdmin(address newCrossDomainAdmin) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override onlyOwner nonReentrant { + function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { _setHubPool(newHubPool); } @@ -70,18 +83,19 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool, Ownable { address originToken, uint32 destinationChainId, bool enable - ) public override onlyOwner nonReentrant { + ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyOwner nonReentrant { + function setDepositQuoteTimeBuffer(uint32 buffer) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _setDepositQuoteTimeBuffer(buffer); } - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ - function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) public override diff --git a/contracts/HubPool.sol b/contracts/HubPool.sol index 563f2eca1..9387a78bb 100644 --- a/contracts/HubPool.sol +++ b/contracts/HubPool.sol @@ -129,6 +129,7 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { int256[] runningBalance, address indexed caller ); + event SpokePoolAdminFunctionTriggered(uint256 indexed chainId, bytes message); event RelayerRefundDisputed(address indexed disputer, uint256 requestTime, bytes disputedAncillaryData); @@ -150,6 +151,15 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { * ADMIN FUNCTIONS * *************************************************/ + function relaySpokePoolAdminFunction(uint256 chainId, bytes memory functionData) public onlyOwner nonReentrant { + AdapterInterface adapter = crossChainContracts[chainId].adapter; + adapter.relayMessage( + crossChainContracts[chainId].spokePool, // target. This should be the spokePool on the L2. + functionData + ); + emit SpokePoolAdminFunctionTriggered(chainId, functionData); + } + function setBond(IERC20 newBondToken, uint256 newBondAmount) public onlyOwner noActiveRequests { bondToken = newBondToken; bondAmount = newBondAmount; @@ -185,6 +195,15 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { address destinationToken ) public onlyOwner { whitelistedRoutes[originToken][destinationChainId] = destinationToken; + relaySpokePoolAdminFunction( + destinationChainId, + abi.encodeWithSignature( + "setEnableRoute(address,uint32,bool)", + originToken, + uint32(destinationChainId), + true + ) + ); emit WhitelistRoute(destinationChainId, originToken, destinationToken); } diff --git a/contracts/Optimism_SpokePool.sol b/contracts/Optimism_SpokePool.sol index 99eee4625..c1cebd483 100644 --- a/contracts/Optimism_SpokePool.sol +++ b/contracts/Optimism_SpokePool.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.0; import "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol"; import "@eth-optimism/contracts/libraries/constants/Lib_PredeployAddresses.sol"; import "@eth-optimism/contracts/L2/messaging/IL2ERC20Bridge.sol"; -import "@openzeppelin/contracts/access/Ownable.sol"; import "./SpokePool.sol"; import "./SpokePoolInterface.sol"; @@ -13,7 +12,7 @@ import "./SpokePoolInterface.sol"; * @dev Uses OVM cross-domain-enabled logic for access control. */ -contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool, Ownable { +contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool { // "l1Gas" parameter used in call to bridge tokens from this contract back to L1 via `IL2ERC20Bridge`. uint32 l1Gas = 6_000_000; @@ -32,17 +31,23 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool {} /************************************** - * ADMIN FUNCTIONS * + * CROSS-CHAIN ADMIN FUNCTIONS * **************************************/ - function setL1GasLimit(uint32 newl1Gas) public onlyOwner { + + function setL1GasLimit(uint32 newl1Gas) public onlyFromCrossDomainAccount(crossDomainAdmin) { _setL1GasLimit(newl1Gas); } - function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyOwner nonReentrant { + function setCrossDomainAdmin(address newCrossDomainAdmin) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override onlyOwner nonReentrant { + function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { _setHubPool(newHubPool); } @@ -50,18 +55,19 @@ contract Optimism_SpokePool is CrossDomainEnabled, SpokePoolInterface, SpokePool address originToken, uint32 destinationChainId, bool enable - ) public override onlyOwner nonReentrant { + ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyOwner nonReentrant { + function setDepositQuoteTimeBuffer(uint32 buffer) + public + override + onlyFromCrossDomainAccount(crossDomainAdmin) + nonReentrant + { _setDepositQuoteTimeBuffer(buffer); } - /************************************** - * CROSS-CHAIN ADMIN FUNCTIONS * - **************************************/ - function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) public override diff --git a/test/HubPool.Admin.ts b/test/HubPool.Admin.ts index 43a1250df..9c18996d4 100644 --- a/test/HubPool.Admin.ts +++ b/test/HubPool.Admin.ts @@ -2,13 +2,13 @@ import { getContractFactory, SignerWithAddress, seedWallet, expect, Contract, et import { destinationChainId, bondAmount, zeroAddress, mockTreeRoot, mockSlowRelayFulfillmentRoot } from "./constants"; import { hubPoolFixture } from "./HubPool.Fixture"; -let hubPool: Contract, weth: Contract, usdc: Contract; +let hubPool: Contract, weth: Contract, usdc: Contract, mockSpoke: Contract, mockAdapter: Contract; let owner: SignerWithAddress, other: SignerWithAddress; describe("HubPool Admin functions", function () { beforeEach(async function () { [owner, other] = await ethers.getSigners(); - ({ weth, hubPool, usdc } = await hubPoolFixture()); + ({ weth, hubPool, usdc, mockAdapter, mockSpoke } = await hubPoolFixture()); }); it("Can add L1 token to whitelisted lpTokens mapping", async function () { @@ -36,6 +36,7 @@ describe("HubPool Admin functions", function () { 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); await expect(hubPool.whitelistRoute(destinationChainId, weth.address, usdc.address)) .to.emit(hubPool, "WhitelistRoute") .withArgs(destinationChainId, weth.address, usdc.address); diff --git a/test/HubPool.RefundExecution.ts b/test/HubPool.RefundExecution.ts index 6af936004..6fff414f0 100644 --- a/test/HubPool.RefundExecution.ts +++ b/test/HubPool.RefundExecution.ts @@ -63,9 +63,10 @@ describe("HubPool Relayer Refund Execution", function () { // Check the mockAdapter was called with the correct arguments for each method. const relayMessageEvents = await mockAdapter.queryFilter(mockAdapter.filters.RelayMessageCalled()); - expect(relayMessageEvents.length).to.equal(1); // Exactly one message send from L1->L2. - expect(relayMessageEvents[0].args?.target).to.equal(mockSpoke.address); - expect(relayMessageEvents[0].args?.message).to.equal( + expect(relayMessageEvents.length).to.equal(4); // Exactly four message send from L1->L2. 3 for each whitelist route + // and 1 for the initiateRelayerRefund. + expect(relayMessageEvents[relayMessageEvents.length - 1].args?.target).to.equal(mockSpoke.address); + expect(relayMessageEvents[relayMessageEvents.length - 1].args?.message).to.equal( mockSpoke.interface.encodeFunctionData("initializeRelayerRefund", [ consts.mockDestinationDistributionRoot, consts.mockSlowRelayFulfillmentRoot, diff --git a/test/chain-adapters/Arbitrum_Adapter.ts b/test/chain-adapters/Arbitrum_Adapter.ts index b587900e1..92e51b62a 100644 --- a/test/chain-adapters/Arbitrum_Adapter.ts +++ b/test/chain-adapters/Arbitrum_Adapter.ts @@ -98,7 +98,7 @@ describe("Arbitrum Chain Adapter", function () { consts.sampleL2GasPrice, "0x" ); - expect(l1Inbox.createRetryableTicket).to.have.been.calledOnce; // only 1 L1->L2 message sent. + expect(l1Inbox.createRetryableTicket).to.have.been.calledThrice; // only 1 L1->L2 message sent. expect(l1Inbox.createRetryableTicket).to.have.been.calledWith( mockSpoke.address, 0, From 3b92a3bf84167bdea92f894bc784f6ba511134c9 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Mon, 14 Feb 2022 16:31:01 -0700 Subject: [PATCH 09/19] use onlyFromCrossDomainAdmin --- contracts/Arbitrum_SpokePool.sol | 36 ++++++++------------------------ contracts/HubPool.sol | 2 ++ 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index 167acfcc2..df93d4d17 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -41,8 +41,8 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { _setL2GatewayRouter(_l2GatewayRouter); } - modifier onlyFromCrossDomainAccount(address l1Counterpart) { - require(msg.sender == _applyL1ToL2Alias(l1Counterpart), "ONLY_COUNTERPART_GATEWAY"); + modifier onlyFromCrossDomainAdmin() { + require(msg.sender == _applyL1ToL2Alias(crossDomainAdmin), "ONLY_COUNTERPART_GATEWAY"); _; } @@ -50,32 +50,19 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { * CROSS-CHAIN ADMIN FUNCTIONS * **************************************/ - function setL2GatewayRouter(address newL2GatewayRouter) - public - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setL2GatewayRouter(address newL2GatewayRouter) public onlyFromCrossDomainAdmin nonReentrant { _setL2GatewayRouter(newL2GatewayRouter); } - function whitelistToken(address l2Token, address l1Token) - public - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function whitelistToken(address l2Token, address l1Token) public onlyFromCrossDomainAdmin nonReentrant { _whitelistToken(l2Token, l1Token); } - function setCrossDomainAdmin(address newCrossDomainAdmin) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setCrossDomainAdmin(address newCrossDomainAdmin) public override onlyFromCrossDomainAdmin nonReentrant { _setCrossDomainAdmin(newCrossDomainAdmin); } - function setHubPool(address newHubPool) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + function setHubPool(address newHubPool) public override onlyFromCrossDomainAdmin nonReentrant { _setHubPool(newHubPool); } @@ -83,23 +70,18 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { address originToken, uint32 destinationChainId, bool enable - ) public override onlyFromCrossDomainAccount(crossDomainAdmin) nonReentrant { + ) public override onlyFromCrossDomainAdmin nonReentrant { _setEnableRoute(originToken, destinationChainId, enable); } - function setDepositQuoteTimeBuffer(uint32 buffer) - public - override - onlyFromCrossDomainAccount(crossDomainAdmin) - nonReentrant - { + function setDepositQuoteTimeBuffer(uint32 buffer) public override onlyFromCrossDomainAdmin nonReentrant { _setDepositQuoteTimeBuffer(buffer); } function initializeRelayerRefund(bytes32 relayerRepaymentDistributionRoot, bytes32 slowRelayRoot) public override - onlyFromCrossDomainAccount(crossDomainAdmin) + onlyFromCrossDomainAdmin nonReentrant { _initializeRelayerRefund(relayerRepaymentDistributionRoot, slowRelayRoot); diff --git a/contracts/HubPool.sol b/contracts/HubPool.sol index 9387a78bb..87d521f37 100644 --- a/contracts/HubPool.sol +++ b/contracts/HubPool.sol @@ -151,6 +151,8 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { * ADMIN FUNCTIONS * *************************************************/ + // This function has permission to call onlyFromCrossChainAdmin functions on the SpokePool, so its imperative + // that this contract only allows the owner to call this method directly or indirectly. function relaySpokePoolAdminFunction(uint256 chainId, bytes memory functionData) public onlyOwner nonReentrant { AdapterInterface adapter = crossChainContracts[chainId].adapter; adapter.relayMessage( From b8de41442cd718804015ddb89e7b72f6e17421f2 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 11:38:16 -0700 Subject: [PATCH 10/19] add relaySpokePoolAdminFunction unit tests --- contracts/HubPool.sol | 1 - contracts/chain-adapters/L1_Adapter.sol | 50 ++++++++++++++++++++ test/chain-adapters/Arbitrum_Adapter.ts | 20 +++++++- test/chain-adapters/L1_Adapter.ts | 62 +++++++++++++++++++++++++ test/chain-adapters/Optimism_Adapter.ts | 14 +++++- 5 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 contracts/chain-adapters/L1_Adapter.sol create mode 100644 test/chain-adapters/L1_Adapter.ts diff --git a/contracts/HubPool.sol b/contracts/HubPool.sol index 87d521f37..ab2289b2e 100644 --- a/contracts/HubPool.sol +++ b/contracts/HubPool.sol @@ -498,7 +498,6 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { uint256[] memory bundleLpFees ) internal { AdapterInterface adapter = crossChainContracts[chainId].adapter; - require(address(adapter) != address(0), "Adapter not set for target chain"); for (uint32 i = 0; i < l1Tokens.length; i++) { // Validate the L1 -> L2 token route is whitelisted. If it is not then the output of the bridging action diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol new file mode 100644 index 000000000..906c3a1f7 --- /dev/null +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.0; + +import "./Base_Adapter.sol"; +import "../interfaces/AdapterInterface.sol"; +import "../interfaces/WETH9.sol"; + +import "@uma/core/contracts/common/implementation/Lockable.sol"; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; + +contract L1_Adapter is Base_Adapter, Lockable { + using SafeERC20 for IERC20; + + 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"); + emit MessageRelayed(target, message); + } + + function relayTokens( + address l1Token, + address l2Token, // l2Token is unused for L1. + uint256 amount, + address to + ) external payable override nonReentrant onlyHubPool { + IERC20(l1Token).safeTransfer(to, amount); + emit TokensRelayed(l1Token, l2Token, amount, to); + } + + // Note: this snippet of code is copied from Governor.sol. + function _executeCall(address to, bytes memory data) private returns (bool) { + // Note: this snippet of code is copied from Governor.sol and modified to not include any "value" field. + // solhint-disable-next-line no-inline-assembly + + bool success; + assembly { + let inputData := add(data, 0x20) + let inputDataSize := mload(data) + // Hardcode value to be 0 for relayed governance calls in order to avoid addressing complexity of bridging + // value cross-chain. + success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) + } + return success; + } + + receive() external payable {} +} diff --git a/test/chain-adapters/Arbitrum_Adapter.ts b/test/chain-adapters/Arbitrum_Adapter.ts index 92e51b62a..ceee963b3 100644 --- a/test/chain-adapters/Arbitrum_Adapter.ts +++ b/test/chain-adapters/Arbitrum_Adapter.ts @@ -1,6 +1,6 @@ import * as consts from "../constants"; import { ethers, expect, Contract, FakeContract, SignerWithAddress, createFake, toWei } from "../utils"; -import { getContractFactory, seedWallet } from "../utils"; +import { getContractFactory, seedWallet, randomAddress } from "../utils"; import { hubPoolFixture, enableTokensForLP } from "../HubPool.Fixture"; import { constructSingleChainTree } from "../MerkleLib.utils"; @@ -72,6 +72,24 @@ describe("Arbitrum Chain Adapter", function () { await arbitrumAdapter.connect(owner).setL2RefundL2Address(liquidityProvider.address); expect(await arbitrumAdapter.callStatic.l2RefundL2Address()).to.equal(liquidityProvider.address); }); + it("relayMessage calls spoke pool functions", async function () { + const newAdmin = randomAddress(); + const functionCallData = mockSpoke.interface.encodeFunctionData("setCrossDomainAdmin", [newAdmin]); + expect(await hubPool.relaySpokePoolAdminFunction(arbitrumChainId, functionCallData)) + .to.emit(arbitrumAdapter, "MessageRelayed") + .withArgs(mockSpoke.address, functionCallData); + expect(l1Inbox.createRetryableTicket).to.have.been.calledThrice; + expect(l1Inbox.createRetryableTicket).to.have.been.calledWith( + mockSpoke.address, + 0, + consts.sampleL2MaxSubmissionCost, + owner.address, + owner.address, + consts.sampleL2Gas, + consts.sampleL2GasPrice, + functionCallData + ); + }); it("Correctly calls appropriate arbitrum bridge functions when making ERC20 cross chain calls", async function () { // Create an action that will send an L1->L2 tokens transfer and bundle. For this, create a relayer repayment bundle // and check that at it's finalization the L2 bridge contracts are called as expected. diff --git a/test/chain-adapters/L1_Adapter.ts b/test/chain-adapters/L1_Adapter.ts new file mode 100644 index 000000000..dcef2ed48 --- /dev/null +++ b/test/chain-adapters/L1_Adapter.ts @@ -0,0 +1,62 @@ +import * as consts from "../constants"; +import { ethers, expect, Contract, SignerWithAddress, randomAddress } from "../utils"; +import { getContractFactory, seedWallet } from "../utils"; +import { hubPoolFixture, enableTokensForLP } from "../HubPool.Fixture"; +import { constructSingleChainTree } from "../MerkleLib.utils"; + +let hubPool: Contract, l1Adapter: Contract, weth: Contract, dai: Contract, mockSpoke: Contract, timer: Contract; +let owner: SignerWithAddress, dataWorker: SignerWithAddress, liquidityProvider: SignerWithAddress; + +const l1ChainId = 1; + +describe("L1 Chain Adapter", function () { + beforeEach(async function () { + [owner, dataWorker, liquidityProvider] = await ethers.getSigners(); + ({ weth, dai, hubPool, mockSpoke, timer } = await hubPoolFixture()); + await seedWallet(dataWorker, [dai], weth, consts.amountToLp); + await seedWallet(liquidityProvider, [dai], weth, consts.amountToLp.mul(10)); + + await enableTokensForLP(owner, hubPool, weth, [weth, dai]); + await weth.connect(liquidityProvider).approve(hubPool.address, consts.amountToLp); + await hubPool.connect(liquidityProvider).addLiquidity(weth.address, consts.amountToLp); + await weth.connect(dataWorker).approve(hubPool.address, consts.bondAmount.mul(10)); + await dai.connect(liquidityProvider).approve(hubPool.address, consts.amountToLp); + await hubPool.connect(liquidityProvider).addLiquidity(dai.address, consts.amountToLp); + await dai.connect(dataWorker).approve(hubPool.address, consts.bondAmount.mul(10)); + + l1Adapter = await (await getContractFactory("L1_Adapter", owner)).deploy(hubPool.address); + + await hubPool.setCrossChainContracts(l1ChainId, l1Adapter.address, mockSpoke.address); + + await hubPool.whitelistRoute(l1ChainId, weth.address, weth.address); + + await hubPool.whitelistRoute(l1ChainId, dai.address, dai.address); + }); + + it("relayMessage calls spoke pool functions", async function () { + const newAdmin = randomAddress(); + const functionCallData = mockSpoke.interface.encodeFunctionData("setCrossDomainAdmin", [newAdmin]); + expect(await hubPool.relaySpokePoolAdminFunction(l1ChainId, functionCallData)) + .to.emit(l1Adapter, "MessageRelayed") + .withArgs(mockSpoke.address, functionCallData); + + const admin = await mockSpoke.crossDomainAdmin(); + expect(admin).to.equal(newAdmin); + }); + it("Correctly transfers tokens when executing pool rebalance", async function () { + const { leafs, tree, tokensSendToL2 } = await constructSingleChainTree(dai, 1, l1ChainId); + await hubPool + .connect(dataWorker) + .initiateRelayerRefund( + [3117], + 1, + tree.getHexRoot(), + consts.mockDestinationDistributionRoot, + consts.mockSlowRelayFulfillmentRoot + ); + await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness); + expect(await hubPool.connect(dataWorker).executeRelayerRefund(leafs[0], tree.getHexProof(leafs[0]))) + .to.emit(l1Adapter, "TokensRelayed") + .withArgs(dai.address, dai.address, tokensSendToL2, mockSpoke.address); + }); +}); diff --git a/test/chain-adapters/Optimism_Adapter.ts b/test/chain-adapters/Optimism_Adapter.ts index 7a4694ed7..fa73e5525 100644 --- a/test/chain-adapters/Optimism_Adapter.ts +++ b/test/chain-adapters/Optimism_Adapter.ts @@ -7,7 +7,7 @@ import { mockSlowRelayFulfillmentRoot, } from "./../constants"; import { ethers, expect, Contract, FakeContract, SignerWithAddress, createFake } from "../utils"; -import { getContractFactory, seedWallet } from "../utils"; +import { getContractFactory, seedWallet, randomAddress } from "../utils"; import { hubPoolFixture, enableTokensForLP } from "../HubPool.Fixture"; import { constructSingleChainTree } from "../MerkleLib.utils"; @@ -51,6 +51,18 @@ describe("Optimism Chain Adapter", function () { await optimismAdapter.connect(owner).setL2GasLimit(sampleL2Gas + 1); expect(await optimismAdapter.callStatic.l2GasLimit()).to.equal(sampleL2Gas + 1); }); + it("relayMessage calls spoke pool functions", async function () { + const newAdmin = randomAddress(); + const functionCallData = mockSpoke.interface.encodeFunctionData("setCrossDomainAdmin", [newAdmin]); + expect(await hubPool.relaySpokePoolAdminFunction(optimismChainId, functionCallData)) + .to.emit(optimismAdapter, "MessageRelayed") + .withArgs(mockSpoke.address, functionCallData); + expect(l1CrossDomainMessenger.sendMessage).to.have.been.calledWith( + mockSpoke.address, + functionCallData, + sampleL2Gas + ); + }); it("Correctly calls appropriate Optimism bridge functions when making ERC20 cross chain calls", async function () { // Create an action that will send an L1->L2 tokens transfer and bundle. For this, create a relayer repayment bundle // and check that at it's finalization the L2 bridge contracts are called as expected. From b0efeb0346281dae7a26c2ad1e4f69733bd48285 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 14:11:20 -0700 Subject: [PATCH 11/19] Update SpokePool.sol --- contracts/SpokePool.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index f02097c68..5d69dfb7d 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -41,6 +41,10 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // Address of the L1 contract that will send tokens to and receive tokens from this contract. address public hubPool; + // Address of WETH contract for this network. If an origin token matches this, then the caller can optionally + // instruct this contract to wrap ETH when depositing. + WETH9 public weth; + // Timestamp when contract was constructed. Relays cannot have a quote time before this. uint32 public deploymentTime; @@ -51,10 +55,6 @@ abstract contract SpokePool is SpokePoolInterface, Testable, Lockable, MultiCall // Use count of deposits as unique deposit identifier. uint32 public numberOfDeposits; - // Address of WETH contract for this network. If an origin token matches this, then the caller can optionally - // instruct this contract to wrap ETH when depositing. - WETH9 public weth; - // Origin token to destination token routings can be turned on or off. mapping(address => mapping(uint32 => bool)) public enabledDepositRoutes; From be206251747122722c3db1b087db5baa0c362b51 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 14:13:38 -0700 Subject: [PATCH 12/19] Update Arbitrum_Adapter.ts --- test/chain-adapters/Arbitrum_Adapter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/chain-adapters/Arbitrum_Adapter.ts b/test/chain-adapters/Arbitrum_Adapter.ts index ceee963b3..77089fac4 100644 --- a/test/chain-adapters/Arbitrum_Adapter.ts +++ b/test/chain-adapters/Arbitrum_Adapter.ts @@ -116,7 +116,8 @@ describe("Arbitrum Chain Adapter", function () { consts.sampleL2GasPrice, "0x" ); - expect(l1Inbox.createRetryableTicket).to.have.been.calledThrice; // only 1 L1->L2 message sent. + expect(l1Inbox.createRetryableTicket).to.have.been.calledThrice; // only 1 L1->L2 message sent. Note that the two + // whitelist transactions already sent two messages. expect(l1Inbox.createRetryableTicket).to.have.been.calledWith( mockSpoke.address, 0, From 7adc5099769bf1530f5ea84e4eb5c4a4a4368360 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 17:47:31 -0700 Subject: [PATCH 13/19] move require to executeCall --- contracts/chain-adapters/L1_Adapter.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index 906c3a1f7..ee849af2b 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -16,7 +16,7 @@ contract L1_Adapter is Base_Adapter, Lockable { 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"); + _executeCall(target, message); emit MessageRelayed(target, message); } @@ -43,6 +43,7 @@ contract L1_Adapter is Base_Adapter, Lockable { // value cross-chain. success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) } + require(success, "execute call failed"); return success; } From d54aa2f38a0359d7c07cfa0e1a57ca05aaddf645 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 18:40:48 -0700 Subject: [PATCH 14/19] Revert "move require to executeCall" This reverts commit 7adc5099769bf1530f5ea84e4eb5c4a4a4368360. --- contracts/chain-adapters/L1_Adapter.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index ee849af2b..906c3a1f7 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -16,7 +16,7 @@ contract L1_Adapter is Base_Adapter, Lockable { constructor(address _hubPool) Base_Adapter(_hubPool) {} function relayMessage(address target, bytes memory message) external payable override nonReentrant onlyHubPool { - _executeCall(target, message); + require(_executeCall(target, message), "execute call failed"); emit MessageRelayed(target, message); } @@ -43,7 +43,6 @@ contract L1_Adapter is Base_Adapter, Lockable { // value cross-chain. success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) } - require(success, "execute call failed"); return success; } From 66fb38bc7ab2aa12adb902671b213ce818d5347d Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 19:07:01 -0700 Subject: [PATCH 15/19] Revert "Revert "move require to executeCall"" This reverts commit d54aa2f38a0359d7c07cfa0e1a57ca05aaddf645. --- contracts/chain-adapters/L1_Adapter.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index 906c3a1f7..ee849af2b 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -16,7 +16,7 @@ contract L1_Adapter is Base_Adapter, Lockable { 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"); + _executeCall(target, message); emit MessageRelayed(target, message); } @@ -43,6 +43,7 @@ contract L1_Adapter is Base_Adapter, Lockable { // value cross-chain. success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) } + require(success, "execute call failed"); return success; } From 3bb8a2abe5728357af5ca6bcb8a8a85df90f8361 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 19:39:44 -0700 Subject: [PATCH 16/19] Revert "move require to executeCall" This reverts commit 7adc5099769bf1530f5ea84e4eb5c4a4a4368360. --- contracts/HubPool.sol | 16 ++++++++++------ contracts/chain-adapters/L1_Adapter.sol | 4 +++- test/HubPool.Fixture.ts | 14 +++++++++++++- test/chain-adapters/L1_Adapter.ts | 11 +++++++---- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/contracts/HubPool.sol b/contracts/HubPool.sol index d0aba25eb..4e8ce1f23 100644 --- a/contracts/HubPool.sol +++ b/contracts/HubPool.sol @@ -167,12 +167,7 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { // This function has permission to call onlyFromCrossChainAdmin functions on the SpokePool, so its imperative // that this contract only allows the owner to call this method directly or indirectly. function relaySpokePoolAdminFunction(uint256 chainId, bytes memory functionData) public onlyOwner nonReentrant { - AdapterInterface adapter = crossChainContracts[chainId].adapter; - adapter.relayMessage( - crossChainContracts[chainId].spokePool, // target. This should be the spokePool on the L2. - functionData - ); - emit SpokePoolAdminFunctionTriggered(chainId, functionData); + _relaySpokePoolAdminFunction(chainId, functionData); } function setProtocolFeeCapture(address newProtocolFeeCaptureAddress, uint256 newProtocolFeeCapturePct) @@ -657,6 +652,15 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable { if (protocolFeesCaptured > 0) unclaimedAccumulatedProtocolFees[l1Token] += protocolFeesCaptured; } + function _relaySpokePoolAdminFunction(uint256 chainId, bytes memory functionData) internal { + AdapterInterface adapter = crossChainContracts[chainId].adapter; + adapter.relayMessage( + crossChainContracts[chainId].spokePool, // target. This should be the spokePool on the L2. + functionData + ); + emit SpokePoolAdminFunctionTriggered(chainId, functionData); + } + // Added to enable the SpokePool to receive ETH. used when unwrapping WETH. receive() external payable {} } diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index ee849af2b..50f2621b1 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -43,7 +43,9 @@ contract L1_Adapter is Base_Adapter, Lockable { // value cross-chain. success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) } - require(success, "execute call failed"); + // TODO: Can't figure out why but this `require` statement reverts even though the call() does seem to work in + // the L1_Adapter test. + // require(success, "execute call failed"); return success; } diff --git a/test/HubPool.Fixture.ts b/test/HubPool.Fixture.ts index 4d4f0da91..4d630c5fa 100644 --- a/test/HubPool.Fixture.ts +++ b/test/HubPool.Fixture.ts @@ -54,7 +54,19 @@ export const hubPoolFixture = hre.deployments.createFixture(async ({ ethers }) = await hubPool.whitelistRoute(repaymentChainId, dai.address, l2Dai); await hubPool.whitelistRoute(repaymentChainId, usdc.address, l2Usdc); - return { weth, usdc, dai, hubPool, mockAdapter, mockSpoke, l2Weth, l2Dai, l2Usdc, ...parentFixtureOutput }; + return { + weth, + usdc, + dai, + hubPool, + mockAdapter, + mockSpoke, + l2Weth, + l2Dai, + l2Usdc, + crossChainAdmin, + ...parentFixtureOutput, + }; }); export async function enableTokensForLP(owner: Signer, hubPool: Contract, weth: Contract, tokens: Contract[]) { diff --git a/test/chain-adapters/L1_Adapter.ts b/test/chain-adapters/L1_Adapter.ts index dcef2ed48..d596ec8c4 100644 --- a/test/chain-adapters/L1_Adapter.ts +++ b/test/chain-adapters/L1_Adapter.ts @@ -5,14 +5,17 @@ import { hubPoolFixture, enableTokensForLP } from "../HubPool.Fixture"; import { constructSingleChainTree } from "../MerkleLib.utils"; let hubPool: Contract, l1Adapter: Contract, weth: Contract, dai: Contract, mockSpoke: Contract, timer: Contract; -let owner: SignerWithAddress, dataWorker: SignerWithAddress, liquidityProvider: SignerWithAddress; +let owner: SignerWithAddress, + dataWorker: SignerWithAddress, + liquidityProvider: SignerWithAddress, + crossChainAdmin: SignerWithAddress; const l1ChainId = 1; describe("L1 Chain Adapter", function () { beforeEach(async function () { [owner, dataWorker, liquidityProvider] = await ethers.getSigners(); - ({ weth, dai, hubPool, mockSpoke, timer } = await hubPoolFixture()); + ({ weth, dai, hubPool, mockSpoke, timer, crossChainAdmin } = await hubPoolFixture()); await seedWallet(dataWorker, [dai], weth, consts.amountToLp); await seedWallet(liquidityProvider, [dai], weth, consts.amountToLp.mul(10)); @@ -34,14 +37,14 @@ describe("L1 Chain Adapter", function () { }); it("relayMessage calls spoke pool functions", async function () { + expect(await mockSpoke.crossDomainAdmin()).to.equal(crossChainAdmin.address); const newAdmin = randomAddress(); const functionCallData = mockSpoke.interface.encodeFunctionData("setCrossDomainAdmin", [newAdmin]); expect(await hubPool.relaySpokePoolAdminFunction(l1ChainId, functionCallData)) .to.emit(l1Adapter, "MessageRelayed") .withArgs(mockSpoke.address, functionCallData); - const admin = await mockSpoke.crossDomainAdmin(); - expect(admin).to.equal(newAdmin); + expect(await mockSpoke.crossDomainAdmin()).to.equal(newAdmin); }); it("Correctly transfers tokens when executing pool rebalance", async function () { const { leafs, tree, tokensSendToL2 } = await constructSingleChainTree(dai, 1, l1ChainId); From a39be0e671cdf073a7fc227113413d9599401be1 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 19:42:11 -0700 Subject: [PATCH 17/19] Update Arbitrum_SpokePool.sol --- contracts/Arbitrum_SpokePool.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/contracts/Arbitrum_SpokePool.sol b/contracts/Arbitrum_SpokePool.sol index df93d4d17..32baf1934 100644 --- a/contracts/Arbitrum_SpokePool.sol +++ b/contracts/Arbitrum_SpokePool.sol @@ -35,9 +35,8 @@ contract Arbitrum_SpokePool is SpokePoolInterface, SpokePool { address _crossDomainAdmin, address _hubPool, address _wethAddress, - address timerAddress, - uint32 _depositQuoteTimeBuffer - ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress, _depositQuoteTimeBuffer) { + address timerAddress + ) SpokePool(_crossDomainAdmin, _hubPool, _wethAddress, timerAddress) { _setL2GatewayRouter(_l2GatewayRouter); } From 815d384572324de7351da13fef5df55fae67bdca Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Tue, 15 Feb 2022 19:54:15 -0700 Subject: [PATCH 18/19] Update L1_Adapter.sol --- contracts/chain-adapters/L1_Adapter.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index 50f2621b1..97e6c0978 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -16,7 +16,7 @@ contract L1_Adapter is Base_Adapter, Lockable { constructor(address _hubPool) Base_Adapter(_hubPool) {} function relayMessage(address target, bytes memory message) external payable override nonReentrant onlyHubPool { - _executeCall(target, message); + require(_executeCall(target, message), "tx execution failed"); emit MessageRelayed(target, message); } From ac5d1fda9d16aa203e74f46b3b4447859904fd55 Mon Sep 17 00:00:00 2001 From: Nick Pai Date: Wed, 16 Feb 2022 06:50:23 -0700 Subject: [PATCH 19/19] Update L1_Adapter.sol --- contracts/chain-adapters/L1_Adapter.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/contracts/chain-adapters/L1_Adapter.sol b/contracts/chain-adapters/L1_Adapter.sol index 97e6c0978..1616d2808 100644 --- a/contracts/chain-adapters/L1_Adapter.sol +++ b/contracts/chain-adapters/L1_Adapter.sol @@ -16,7 +16,7 @@ contract L1_Adapter is Base_Adapter, Lockable { constructor(address _hubPool) Base_Adapter(_hubPool) {} function relayMessage(address target, bytes memory message) external payable override nonReentrant onlyHubPool { - require(_executeCall(target, message), "tx execution failed"); + _executeCall(target, message); emit MessageRelayed(target, message); } @@ -31,7 +31,7 @@ contract L1_Adapter is Base_Adapter, Lockable { } // Note: this snippet of code is copied from Governor.sol. - function _executeCall(address to, bytes memory data) private returns (bool) { + function _executeCall(address to, bytes memory data) private { // Note: this snippet of code is copied from Governor.sol and modified to not include any "value" field. // solhint-disable-next-line no-inline-assembly @@ -43,10 +43,7 @@ contract L1_Adapter is Base_Adapter, Lockable { // value cross-chain. success := call(gas(), to, 0, inputData, inputDataSize, 0, 0) } - // TODO: Can't figure out why but this `require` statement reverts even though the call() does seem to work in - // the L1_Adapter test. - // require(success, "execute call failed"); - return success; + require(success, "execute call failed"); } receive() external payable {}