From 41f23390b3903684a8c82c354a4707399fb93ad7 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 20 Feb 2023 10:11:58 -0500 Subject: [PATCH 1/7] WIP Signed-off-by: Matt Rice --- contracts/SpokePool.sol | 272 ++++++++++++++++++------------- contracts/SpokePoolInterface.sol | 2 + 2 files changed, 160 insertions(+), 114 deletions(-) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 96f2c210b..1b32d2496 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -16,6 +16,12 @@ import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; import "@openzeppelin/contracts/utils/math/SignedMath.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; + + +interface AcrossMessageHandler { + function handleAcrossMessage(address tokenSent, uint256 amount, bytes memory message) external; +} /** * @title SpokePool @@ -83,6 +89,8 @@ abstract contract SpokePool is uint256 public constant MAX_TRANSFER_SIZE = 1e36; + bytes32 public constant UPDATE_DEPOSIT_DETAILS_HASH = keccak256("UpdateDepositDetails(uint32 depositId,uint256 originChainId,int64 newRelayerFeePct,address newRecipient,bytes newData)"); + /**************************************** * EVENTS * ****************************************/ @@ -99,10 +107,13 @@ abstract contract SpokePool is uint32 quoteTimestamp, address indexed originToken, address recipient, - address indexed depositor + address indexed depositor, + bytes message ); event RequestedSpeedUpDeposit( int64 newRelayerFeePct, + bytes newMessage, + address newRecipient, uint32 indexed depositId, address indexed depositor, bytes depositorSignature @@ -122,6 +133,9 @@ abstract contract SpokePool is address indexed relayer, address indexed depositor, address recipient, + address appliedRecipient, + bytes message, + bytes appliedMessage, bool isSlowRelay ); event RelayedRootBundle( @@ -150,6 +164,19 @@ abstract contract SpokePool is event PausedDeposits(bool isPaused); event PausedFills(bool isPaused); + struct RelayExecution { + RelayData relay; + bytes32 relayHash; + int64 updatedRelayerFee; + address updatedRecipient; + bytes updatedMessage; + uint256 repaymentChain; + uint256 maxTokens; + uint256 maxCount; + bool slowFill; + int256 payoutAdjustment; + } + /** * Do not leave an implementation contract uninitialized. An uninitialized implementation contract can be * taken over by an attacker, which may impact the proxy. To prevent the implementation contract from being @@ -334,6 +361,7 @@ abstract contract SpokePool is uint256 destinationChainId, int64 relayerFeePct, uint32 quoteTimestamp, + bytes memory message, uint256 maxCount ) public payable override nonReentrant unpausedDeposits { // Check that deposit route is enabled. @@ -380,7 +408,8 @@ abstract contract SpokePool is quoteTimestamp, originToken, recipient, - msg.sender + msg.sender, + message ); } @@ -407,15 +436,17 @@ abstract contract SpokePool is address depositor, int64 newRelayerFeePct, uint32 depositId, + bytes memory newMessage, + address newRecipient, bytes memory depositorSignature ) public override nonReentrant { require(newRelayerFeePct < 0.5e18, "invalid relayer fee"); - _verifyUpdateRelayerFeeMessage(depositor, chainId(), newRelayerFeePct, depositId, depositorSignature); + _verifyUpdateDepositMessage(depositor, chainId(), newRelayerFeePct, depositId, newMessage, newRecipient, depositorSignature); // Assuming the above checks passed, a relayer can take the signature and the updated relayer fee information // from the following event to submit a fill with an updated fee %. - emit RequestedSpeedUpDeposit(newRelayerFeePct, depositId, depositor, depositorSignature); + emit RequestedSpeedUpDeposit(newRelayerFeePct, newMessage, newRecipient, depositId, depositor, depositorSignature); } /************************************** @@ -458,37 +489,38 @@ abstract contract SpokePool is int64 realizedLpFeePct, int64 relayerFeePct, uint32 depositId, + bytes memory message, uint256 maxCount ) public nonReentrant unpausedFills { - uint256 thisChain = chainId(); // 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 // is, which chain and currency the recipient wants to receive funds on, and the relay fees. - SpokePoolInterface.RelayData memory relayData = SpokePoolInterface.RelayData({ - depositor: depositor, - recipient: recipient, - destinationToken: destinationToken, - amount: amount, - realizedLpFeePct: realizedLpFeePct, - relayerFeePct: relayerFeePct, - depositId: depositId, - originChainId: originChainId, - destinationChainId: thisChain + RelayExecution memory relayExecution = RelayExecution({ + relay: SpokePoolInterface.RelayData({ + depositor: depositor, + recipient: recipient, + destinationToken: destinationToken, + amount: amount, + realizedLpFeePct: realizedLpFeePct, + relayerFeePct: relayerFeePct, + depositId: depositId, + originChainId: originChainId, + destinationChainId: chainId(), + message: message + }), + relayHash: bytes32(0), + appliedelayerFee: relayerFeePct, + appliedRecipient: recipient, + appliedMessage: message, + repaymentChainId: repaymentChainId, + maxTokensToSend: maxTokensToSend, + slowFill: false, + payoutAdjustment: 0 }); - bytes32 relayHash = _getRelayHash(relayData); + relayExecution.relayHash = _getRelayHash(relayExecution.relay); - uint256 fillAmountPreFees = _fillRelay( - relayHash, - relayData, - maxTokensToSend, - relayerFeePct, - false, - repaymentChainId == thisChain, - maxCount, - 0 - ); - - _emitFillRelay(relayHash, fillAmountPreFees, repaymentChainId, relayerFeePct, relayData, false); + uint256 fillAmountPreFees = _fillRelay(relayExecution); + _emitFillRelay(relayExecution, fillAmountPreFees); } /** @@ -514,9 +546,10 @@ abstract contract SpokePool is * relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the * EIP-712 standard. See more in the _verifyUpdateRelayerFeeMessage() comments. */ - function fillRelayWithUpdatedFee( + function fillRelayWithUpdatedDeposit( address depositor, address recipient, + address newRecipient, address destinationToken, uint256 amount, uint256 maxTokensToSend, @@ -526,38 +559,40 @@ abstract contract SpokePool is int64 relayerFeePct, int64 newRelayerFeePct, uint32 depositId, + bytes memory message, + bytes memory newMessage, bytes memory depositorSignature, uint256 maxCount ) public override nonReentrant unpausedFills { - _verifyUpdateRelayerFeeMessage(depositor, originChainId, newRelayerFeePct, depositId, depositorSignature); - - uint256 thisChain = chainId(); - - // Now follow the default fillRelay flow with the updated fee and the original relay hash. - RelayData memory relayData = RelayData({ - depositor: depositor, - recipient: recipient, - destinationToken: destinationToken, - amount: amount, - realizedLpFeePct: realizedLpFeePct, - relayerFeePct: relayerFeePct, - depositId: depositId, - originChainId: originChainId, - destinationChainId: thisChain + + RelayExecution memory relayExecution = RelayExecution({ + relay: SpokePoolInterface.RelayData({ + depositor: depositor, + recipient: recipient, + destinationToken: destinationToken, + amount: amount, + realizedLpFeePct: realizedLpFeePct, + relayerFeePct: relayerFeePct, + depositId: depositId, + originChainId: originChainId, + destinationChainId: chainId(), + message: message + }), + relayHash: bytes32(0), + appliedelayerFee: newRelayerFeePct, + appliedRecipient: newRecipient, + appliedMessage: newMessage, + repaymentChainId: repaymentChainId, + maxTokensToSend: maxTokensToSend, + slowFill: false, + payoutAdjustment: 0 }); - bytes32 relayHash = _getRelayHash(relayData); - uint256 fillAmountPreFees = _fillRelay( - relayHash, - relayData, - maxTokensToSend, - newRelayerFeePct, - false, - repaymentChainId == thisChain, - maxCount, - 0 - ); + relayExecution.relayHash = _getRelayHash(relayExecution.relay); + - _emitFillRelay(relayHash, fillAmountPreFees, repaymentChainId, newRelayerFeePct, relayData, false); + _verifyUpdateDepositMessage(relayExecution, depositorSignature); + uint256 fillAmountPreFees = _fillRelay(relayExecution); + _emitFillRelay(relayExecution, fillAmountPreFees); } /************************************** @@ -725,48 +760,45 @@ abstract contract SpokePool is int64 relayerFeePct, uint32 depositId, uint32 rootBundleId, + bytes memory message, int256 payoutAdjustment, bytes32[] memory proof ) internal { - RelayData memory relayData = RelayData({ - depositor: depositor, - recipient: recipient, - destinationToken: destinationToken, - amount: amount, - originChainId: originChainId, - destinationChainId: destinationChainId, - realizedLpFeePct: realizedLpFeePct, - relayerFeePct: relayerFeePct, - depositId: depositId + RelayExecution memory relayExecution = RelayExecution({ + relay: SpokePoolInterface.RelayData({ + depositor: depositor, + recipient: recipient, + destinationToken: destinationToken, + amount: amount, + realizedLpFeePct: realizedLpFeePct, + relayerFeePct: relayerFeePct, + depositId: depositId, + originChainId: originChainId, + destinationChainId: chainId(), + message: message + }), + relayHash: bytes32(0), + appliedelayerFee: newRelayerFeePct, + appliedRecipient: newRecipient, + appliedMessage: newMessage, + repaymentChainId: repaymentChainId, + maxTokensToSend: maxTokensToSend, + slowFill: false, + payoutAdjustment: payoutAdjustment }); + relayExecution.relayHash = _getRelayHash(relayExecution.relay); - SlowFill memory slowFill = SlowFill({ relayData: relayData, payoutAdjustment: payoutAdjustment }); - - require( - MerkleLib.verifySlowRelayFulfillment(rootBundles[rootBundleId].slowRelayRoot, slowFill, proof), - "Invalid proof" - ); - - bytes32 relayHash = _getRelayHash(relayData); + _verifySlowFill(relayExecution, rootBundleId, proof); // Note: use relayAmount as the max amount to send, so the relay is always completely filled by the contract's // funds in all cases. As this is a slow relay we set the relayerFeePct to 0. This effectively refunds the // relayer component of the relayerFee thereby only charging the depositor the LpFee. - uint256 fillAmountPreFees = _fillRelay( - relayHash, - relayData, - relayData.amount, - 0, - true, - true, - type(uint256).max, - payoutAdjustment - ); + uint256 fillAmountPreFees = _fillRelay(relayExecution); // Note: Set repayment chain ID to 0 to indicate that there is no repayment to be made. The off-chain data // worker can use repaymentChainId=0 as a signal to ignore such relays for refunds. Also, set the relayerFeePct // to 0 as slow relays do not pay the caller of this method (depositor is refunded this fee). - _emitFillRelay(relayHash, fillAmountPreFees, 0, 0, relayData, true); + _emitFillRelay(relayExecution, fillAmountPreFees); } function _setCrossDomainAdmin(address newCrossDomainAdmin) internal { @@ -784,15 +816,17 @@ abstract contract SpokePool is // Should be overriden by implementing contract depending on how L2 handles sending tokens to L1. function _bridgeTokensToHubPool(SpokePoolInterface.RelayerRefundLeaf memory relayerRefundLeaf) internal virtual; - function _verifyUpdateRelayerFeeMessage( + function _verifyUpdateDepositMessage( address depositor, uint256 originChainId, - int64 newRelayerFeePct, uint32 depositId, + int64 newRelayerFeePct, + bytes memory newMessage, + address newRecipient, bytes memory depositorSignature ) internal view { - // A depositor can request to speed up an un-relayed deposit by signing a hash containing the relayer - // fee % to update to and information uniquely identifying the deposit to relay. This information ensures + // A depositor can request to modify an un-relayed deposit by signing a hash containing the updated + // details and information uniquely identifying the deposit to relay. This information ensures // that this signature cannot be re-used for other deposits. // Note: We use the EIP-712 (https://eips.ethereum.org/EIPS/eip-712) standard for hashing and signing typed data. // Specifically, we use the version of the encoding known as "v4", as implemented by the JSON RPC method @@ -801,23 +835,25 @@ abstract contract SpokePool is // EIP-712 compliant hash struct: https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct keccak256( abi.encode( - keccak256("UpdateRelayerFeeMessage(int64 newRelayerFeePct,uint32 depositId,uint256 originChainId)"), - newRelayerFeePct, + UPDATE_DEPOSIT_DETAILS_HASH, depositId, - originChainId + originChainId, + newRelayerFeePct, + newRecipient, + newMessage ) ), // By passing in the origin chain id, we enable the verification of the signature on a different chain originChainId ); - _verifyDepositorUpdateFeeMessage(depositor, expectedTypedDataV4Hash, depositorSignature); + _verifyDepositorUpdateMessage(depositor, expectedTypedDataV4Hash, depositorSignature); } // This function is isolated and made virtual to allow different L2's to implement chain specific recovery of // signers from signatures because some L2s might not support ecrecover. To be safe, consider always reverting // this function for L2s where ecrecover is different from how it works on Ethereum, otherwise there is the // potential to forge a signature from the depositor using a different private key than the original depositor's. - function _verifyDepositorUpdateFeeMessage( + function _verifyDepositorUpdateMessage( address depositor, bytes32 ethSignedMessageHash, bytes memory depositorSignature @@ -833,6 +869,15 @@ abstract contract SpokePool is require(isValid, "invalid signature"); } + function _verifySlowFill(RelayExecution memory relayExecution, uint32 rootBundleId, bytes32[] memory proof) internal { + SlowFill memory slowFill = SlowFill({ relayData: relayExecution.relay, payoutAdjustment: relayExecution.payoutAdjustment }); + + require( + MerkleLib.verifySlowRelayFulfillment(rootBundles[rootBundleId].slowRelayRoot, slowFill, proof), + "Invalid slow relay proof" + ); + } + function _computeAmountPreFees(uint256 amount, int64 feesPct) private pure returns (uint256) { return (1e18 * amount) / uint256((int256(1e18) - feesPct)); } @@ -867,20 +912,14 @@ abstract contract SpokePool is * The amount to be sent might end up less if there is insufficient relay amount remaining to be sent. */ function _fillRelay( - bytes32 relayHash, - RelayData memory relayData, - uint256 maxTokensToSend, - int64 updatableRelayerFeePct, - bool useContractFunds, - bool localRepayment, - uint256 maxCount, - int256 payoutAdjustment + RelayExecution memory relayExecution ) internal returns (uint256 fillAmountPreFees) { + RelayData memory relayData = relayExecution.relayData; // We limit the relay fees to prevent the user spending all their funds on fees. Note that 0.5e18 (i.e. 50%) // fees are just magic numbers. The important point is to prevent the total fee from being 100%, otherwise // computing the amount pre fees runs into divide-by-0 issues. require( - SignedMath.abs(updatableRelayerFeePct) < 0.5e18 && SignedMath.abs(relayData.realizedLpFeePct) < 0.5e18, + SignedMath.abs(relayExecution.appliedRelayerFeePct) < 0.5e18 && SignedMath.abs(relayData.realizedLpFeePct) < 0.5e18, "invalid fees" ); @@ -902,11 +941,11 @@ abstract contract SpokePool is // of the full relay size, the caller would need to send 10 tokens to the user. fillAmountPreFees = _computeAmountPreFees( maxTokensToSend, - (relayData.realizedLpFeePct + updatableRelayerFeePct) + (relayData.realizedLpFeePct + relayExecution.appliedRelayerFeePct) ); // If user's specified max amount to send is greater than the amount of the relay remaining pre-fees, // we'll pull exactly enough tokens to complete the relay. - uint256 amountToSend = maxTokensToSend; + uint256 amountToSend = relayExecution.maxTokensToSend; uint256 amountRemainingInRelay = relayData.amount - relayFills[relayHash]; if (amountRemainingInRelay < fillAmountPreFees) { fillAmountPreFees = amountRemainingInRelay; @@ -916,7 +955,7 @@ abstract contract SpokePool is // this is a slow relay. amountToSend = _computeAmountPostFees( fillAmountPreFees, - relayData.realizedLpFeePct + updatableRelayerFeePct + relayData.realizedLpFeePct + relayExecution.appliedRelayerFeePct ); if (payoutAdjustment != 0) { @@ -962,6 +1001,10 @@ abstract contract SpokePool is ); else IERC20Upgradeable(relayData.destinationToken).safeTransfer(relayData.recipient, amountToSend); } + + if (Address.isContract(relayData.recipient) && relayData.data.length > 0) { + AcrossMessageHandler(relayData.recipient).handleAcrossMessage(relayData.destinationToken, amountToSend, relayData.message); + } } function _updateCountFromFill( @@ -994,12 +1037,8 @@ abstract contract SpokePool is // The following internal methods emit events with many params to overcome solidity stack too deep issues. function _emitFillRelay( - bytes32 relayHash, - uint256 fillAmount, - uint256 repaymentChainId, - int64 appliedRelayerFeePct, - RelayData memory relayData, - bool isSlowRelay + RelayExecution memory relayExecution, + ) internal { emit FilledRelay( relayData.amount, @@ -1016,6 +1055,9 @@ abstract contract SpokePool is msg.sender, relayData.depositor, relayData.recipient, + appliedRecipient, + relayData.message, + appliedMessage, isSlowRelay ); } @@ -1029,7 +1071,8 @@ abstract contract SpokePool is uint32 quoteTimestamp, address originToken, address recipient, - address depositor + address depositor, + bytes memory message ) internal { emit FundsDeposited( amount, @@ -1040,7 +1083,8 @@ abstract contract SpokePool is quoteTimestamp, originToken, recipient, - depositor + depositor, + message ); } diff --git a/contracts/SpokePoolInterface.sol b/contracts/SpokePoolInterface.sol index 33a28c22a..c39743edf 100644 --- a/contracts/SpokePoolInterface.sol +++ b/contracts/SpokePoolInterface.sol @@ -46,6 +46,8 @@ interface SpokePoolInterface { int64 relayerFeePct; // The id uniquely identifying this deposit on the origin chain. uint32 depositId; + // Data that is forwarded to the recipient. + bytes data; } struct SlowFill { From 68a55e6877616b5a8ce07e352bcc287a5cbe7586 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 20 Feb 2023 14:23:15 -0500 Subject: [PATCH 2/7] DRAFT: data updates Signed-off-by: Matt Rice --- contracts/Ovm_SpokePool.sol | 2 + contracts/Polygon_SpokePool.sol | 2 + contracts/SpokePool.sol | 234 ++++++++++++++++++------------- contracts/SpokePoolInterface.sol | 16 ++- 4 files changed, 153 insertions(+), 101 deletions(-) diff --git a/contracts/Ovm_SpokePool.sol b/contracts/Ovm_SpokePool.sol index 39c61499d..c4826d7d0 100644 --- a/contracts/Ovm_SpokePool.sol +++ b/contracts/Ovm_SpokePool.sol @@ -100,6 +100,7 @@ contract Ovm_SpokePool is SpokePool { int64 relayerFeePct, uint32 depositId, uint32 rootBundleId, + bytes memory message, int256 payoutAdjustment, bytes32[] memory proof ) public override(SpokePool) nonReentrant { @@ -116,6 +117,7 @@ contract Ovm_SpokePool is SpokePool { relayerFeePct, depositId, rootBundleId, + message, payoutAdjustment, proof ); diff --git a/contracts/Polygon_SpokePool.sol b/contracts/Polygon_SpokePool.sol index 8ef169ee6..e897f6cb8 100644 --- a/contracts/Polygon_SpokePool.sol +++ b/contracts/Polygon_SpokePool.sol @@ -192,6 +192,7 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool { int64 relayerFeePct, uint32 depositId, uint32 rootBundleId, + bytes memory message, int256 payoutAdjustment, bytes32[] memory proof ) public virtual override nonReentrant { @@ -207,6 +208,7 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool { relayerFeePct, depositId, rootBundleId, + message, payoutAdjustment, proof ); diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 1b32d2496..879cbe08d 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -18,9 +18,12 @@ import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable. import "@openzeppelin/contracts/utils/math/SignedMath.sol"; import "@openzeppelin/contracts/utils/Address.sol"; - interface AcrossMessageHandler { - function handleAcrossMessage(address tokenSent, uint256 amount, bytes memory message) external; + function handleAcrossMessage( + address tokenSent, + uint256 amount, + bytes memory message + ) external; } /** @@ -89,7 +92,10 @@ abstract contract SpokePool is uint256 public constant MAX_TRANSFER_SIZE = 1e36; - bytes32 public constant UPDATE_DEPOSIT_DETAILS_HASH = keccak256("UpdateDepositDetails(uint32 depositId,uint256 originChainId,int64 newRelayerFeePct,address newRecipient,bytes newData)"); + bytes32 public constant UPDATE_DEPOSIT_DETAILS_HASH = + keccak256( + "UpdateDepositDetails(uint32 depositId,uint256 originChainId,int64 updatedRelayerFeePct,address updatedRecipient,bytes updatedMessage)" + ); /**************************************** * EVENTS * @@ -111,9 +117,9 @@ abstract contract SpokePool is bytes message ); event RequestedSpeedUpDeposit( - int64 newRelayerFeePct, - bytes newMessage, - address newRecipient, + int64 updatedRelayerFeePct, + bytes updatedMessage, + address updatedRecipient, uint32 indexed depositId, address indexed depositor, bytes depositorSignature @@ -167,11 +173,11 @@ abstract contract SpokePool is struct RelayExecution { RelayData relay; bytes32 relayHash; - int64 updatedRelayerFee; + int64 updatedRelayerFeePct; address updatedRecipient; bytes updatedMessage; - uint256 repaymentChain; - uint256 maxTokens; + uint256 repaymentChainId; + uint256 maxTokensToSend; uint256 maxCount; bool slowFill; int256 payoutAdjustment; @@ -426,7 +432,7 @@ abstract contract SpokePool is * @param depositor Signer of the update fee message who originally submitted the deposit. If the deposit doesn't * exist, then the relayer will not be able to fill any relay, so the caller should validate that the depositor * did in fact submit a relay. - * @param newRelayerFeePct New relayer fee that relayers can use. + * @param updatedRelayerFeePct New relayer fee that relayers can use. * @param depositId Deposit to update fee for that originated in this contract. * @param depositorSignature Signed message containing the depositor address, this contract chain ID, the updated * relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the @@ -434,19 +440,34 @@ abstract contract SpokePool is */ function speedUpDeposit( address depositor, - int64 newRelayerFeePct, + int64 updatedRelayerFeePct, uint32 depositId, - bytes memory newMessage, - address newRecipient, + bytes memory updatedMessage, + address updatedRecipient, bytes memory depositorSignature ) public override nonReentrant { - require(newRelayerFeePct < 0.5e18, "invalid relayer fee"); + require(updatedRelayerFeePct < 0.5e18, "invalid relayer fee"); - _verifyUpdateDepositMessage(depositor, chainId(), newRelayerFeePct, depositId, newMessage, newRecipient, depositorSignature); + _verifyUpdateDepositMessage( + depositor, + depositId, + chainId(), + updatedRelayerFeePct, + updatedRecipient, + updatedMessage, + depositorSignature + ); // Assuming the above checks passed, a relayer can take the signature and the updated relayer fee information // from the following event to submit a fill with an updated fee %. - emit RequestedSpeedUpDeposit(newRelayerFeePct, newMessage, newRecipient, depositId, depositor, depositorSignature); + emit RequestedSpeedUpDeposit( + updatedRelayerFeePct, + updatedMessage, + updatedRecipient, + depositId, + depositor, + depositorSignature + ); } /************************************** @@ -509,13 +530,14 @@ abstract contract SpokePool is message: message }), relayHash: bytes32(0), - appliedelayerFee: relayerFeePct, - appliedRecipient: recipient, - appliedMessage: message, + updatedRelayerFeePct: relayerFeePct, + updatedRecipient: recipient, + updatedMessage: message, repaymentChainId: repaymentChainId, maxTokensToSend: maxTokensToSend, slowFill: false, - payoutAdjustment: 0 + payoutAdjustment: 0, + maxCount: maxCount }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); @@ -540,7 +562,7 @@ abstract contract SpokePool is * @param realizedLpFeePct Fee % based on L1 HubPool utilization at deposit quote time. Deterministic based on * quote time. * @param relayerFeePct Original fee % to keep as relayer set by depositor. - * @param newRelayerFeePct New fee % to keep as relayer also specified by depositor. + * @param updatedRelayerFeePct New fee % to keep as relayer also specified by depositor. * @param depositId Unique deposit ID on origin spoke pool. * @param depositorSignature Signed message containing the depositor address, this contract chain ID, the updated * relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the @@ -549,7 +571,7 @@ abstract contract SpokePool is function fillRelayWithUpdatedDeposit( address depositor, address recipient, - address newRecipient, + address updatedRecipient, address destinationToken, uint256 amount, uint256 maxTokensToSend, @@ -557,14 +579,13 @@ abstract contract SpokePool is uint256 originChainId, int64 realizedLpFeePct, int64 relayerFeePct, - int64 newRelayerFeePct, + int64 updatedRelayerFeePct, uint32 depositId, bytes memory message, - bytes memory newMessage, + bytes memory updatedMessage, bytes memory depositorSignature, uint256 maxCount ) public override nonReentrant unpausedFills { - RelayExecution memory relayExecution = RelayExecution({ relay: SpokePoolInterface.RelayData({ depositor: depositor, @@ -579,18 +600,26 @@ abstract contract SpokePool is message: message }), relayHash: bytes32(0), - appliedelayerFee: newRelayerFeePct, - appliedRecipient: newRecipient, - appliedMessage: newMessage, + updatedRelayerFeePct: updatedRelayerFeePct, + updatedRecipient: updatedRecipient, + updatedMessage: updatedMessage, repaymentChainId: repaymentChainId, maxTokensToSend: maxTokensToSend, slowFill: false, - payoutAdjustment: 0 + payoutAdjustment: 0, + maxCount: maxCount }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); - - _verifyUpdateDepositMessage(relayExecution, depositorSignature); + _verifyUpdateDepositMessage( + depositor, + depositId, + originChainId, + updatedRelayerFeePct, + updatedRecipient, + updatedMessage, + depositorSignature + ); uint256 fillAmountPreFees = _fillRelay(relayExecution); _emitFillRelay(relayExecution, fillAmountPreFees); } @@ -627,6 +656,7 @@ abstract contract SpokePool is int64 relayerFeePct, uint32 depositId, uint32 rootBundleId, + bytes memory message, int256 payoutAdjustment, bytes32[] memory proof ) public virtual override nonReentrant { @@ -641,6 +671,7 @@ abstract contract SpokePool is relayerFeePct, depositId, rootBundleId, + message, payoutAdjustment, proof ); @@ -778,13 +809,14 @@ abstract contract SpokePool is message: message }), relayHash: bytes32(0), - appliedelayerFee: newRelayerFeePct, - appliedRecipient: newRecipient, - appliedMessage: newMessage, - repaymentChainId: repaymentChainId, - maxTokensToSend: maxTokensToSend, - slowFill: false, - payoutAdjustment: payoutAdjustment + updatedRelayerFeePct: 0, + updatedRecipient: recipient, + updatedMessage: message, + repaymentChainId: 0, + maxTokensToSend: type(uint256).max, + slowFill: true, + payoutAdjustment: payoutAdjustment, + maxCount: type(uint256).max }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); @@ -818,11 +850,11 @@ abstract contract SpokePool is function _verifyUpdateDepositMessage( address depositor, - uint256 originChainId, uint32 depositId, - int64 newRelayerFeePct, - bytes memory newMessage, - address newRecipient, + uint256 originChainId, + int64 updatedRelayerFeePct, + address updatedRecipient, + bytes memory updatedMessage, bytes memory depositorSignature ) internal view { // A depositor can request to modify an un-relayed deposit by signing a hash containing the updated @@ -838,22 +870,22 @@ abstract contract SpokePool is UPDATE_DEPOSIT_DETAILS_HASH, depositId, originChainId, - newRelayerFeePct, - newRecipient, - newMessage + updatedRelayerFeePct, + updatedRecipient, + updatedMessage ) ), // By passing in the origin chain id, we enable the verification of the signature on a different chain originChainId ); - _verifyDepositorUpdateMessage(depositor, expectedTypedDataV4Hash, depositorSignature); + _verifyDepositorSignature(depositor, expectedTypedDataV4Hash, depositorSignature); } // This function is isolated and made virtual to allow different L2's to implement chain specific recovery of // signers from signatures because some L2s might not support ecrecover. To be safe, consider always reverting // this function for L2s where ecrecover is different from how it works on Ethereum, otherwise there is the // potential to forge a signature from the depositor using a different private key than the original depositor's. - function _verifyDepositorUpdateMessage( + function _verifyDepositorSignature( address depositor, bytes32 ethSignedMessageHash, bytes memory depositorSignature @@ -869,8 +901,15 @@ abstract contract SpokePool is require(isValid, "invalid signature"); } - function _verifySlowFill(RelayExecution memory relayExecution, uint32 rootBundleId, bytes32[] memory proof) internal { - SlowFill memory slowFill = SlowFill({ relayData: relayExecution.relay, payoutAdjustment: relayExecution.payoutAdjustment }); + function _verifySlowFill( + RelayExecution memory relayExecution, + uint32 rootBundleId, + bytes32[] memory proof + ) internal { + SlowFill memory slowFill = SlowFill({ + relayData: relayExecution.relay, + payoutAdjustment: relayExecution.payoutAdjustment + }); require( MerkleLib.verifySlowRelayFulfillment(rootBundles[rootBundleId].slowRelayRoot, slowFill, proof), @@ -911,15 +950,14 @@ abstract contract SpokePool is * @dev Caller must approve this contract to transfer up to maxTokensToSend of the relayData.destinationToken. * The amount to be sent might end up less if there is insufficient relay amount remaining to be sent. */ - function _fillRelay( - RelayExecution memory relayExecution - ) internal returns (uint256 fillAmountPreFees) { - RelayData memory relayData = relayExecution.relayData; + function _fillRelay(RelayExecution memory relayExecution) internal returns (uint256 fillAmountPreFees) { + RelayData memory relayData = relayExecution.relay; // We limit the relay fees to prevent the user spending all their funds on fees. Note that 0.5e18 (i.e. 50%) // fees are just magic numbers. The important point is to prevent the total fee from being 100%, otherwise // computing the amount pre fees runs into divide-by-0 issues. require( - SignedMath.abs(relayExecution.appliedRelayerFeePct) < 0.5e18 && SignedMath.abs(relayData.realizedLpFeePct) < 0.5e18, + SignedMath.abs(relayExecution.updatedRelayerFeePct) < 0.5e18 && + SignedMath.abs(relayData.realizedLpFeePct) < 0.5e18, "invalid fees" ); @@ -927,26 +965,26 @@ abstract contract SpokePool is // Check that the relay has not already been completely filled. Note that the relays mapping will point to // the amount filled so far for a particular relayHash, so this will start at 0 and increment with each fill. - require(relayFills[relayHash] < relayData.amount, "relay filled"); + require(relayFills[relayExecution.relayHash] < relayData.amount, "relay filled"); // This allows the caller to add in frontrunning protection for quote validity. - require(fillCounter[relayData.destinationToken] <= maxCount, "Above max count"); + require(fillCounter[relayData.destinationToken] <= relayExecution.maxCount, "Above max count"); // Stores the equivalent amount to be sent by the relayer before fees have been taken out. - if (maxTokensToSend == 0) return 0; + if (relayExecution.maxTokensToSend == 0) return 0; // Derive the amount of the relay filled if the caller wants to send exactly maxTokensToSend tokens to // the recipient. For example, if the user wants to send 10 tokens to the recipient, the full relay amount // is 100, and the fee %'s total 5%, then this computation would return ~10.5, meaning that to fill 10.5/100 // of the full relay size, the caller would need to send 10 tokens to the user. fillAmountPreFees = _computeAmountPreFees( - maxTokensToSend, - (relayData.realizedLpFeePct + relayExecution.appliedRelayerFeePct) + relayExecution.maxTokensToSend, + (relayData.realizedLpFeePct + relayExecution.updatedRelayerFeePct) ); // If user's specified max amount to send is greater than the amount of the relay remaining pre-fees, // we'll pull exactly enough tokens to complete the relay. uint256 amountToSend = relayExecution.maxTokensToSend; - uint256 amountRemainingInRelay = relayData.amount - relayFills[relayHash]; + uint256 amountRemainingInRelay = relayData.amount - relayFills[relayExecution.relayHash]; if (amountRemainingInRelay < fillAmountPreFees) { fillAmountPreFees = amountRemainingInRelay; @@ -955,30 +993,30 @@ abstract contract SpokePool is // this is a slow relay. amountToSend = _computeAmountPostFees( fillAmountPreFees, - relayData.realizedLpFeePct + relayExecution.appliedRelayerFeePct + relayData.realizedLpFeePct + relayExecution.updatedRelayerFeePct ); - if (payoutAdjustment != 0) { - require(int256(amountToSend) + payoutAdjustment >= 0, "payoutAdjustment too small"); - amountToSend = uint256(int256(amountToSend) + payoutAdjustment); + if (relayExecution.payoutAdjustment != 0) { + require(int256(amountToSend) + relayExecution.payoutAdjustment >= 0, "payoutAdjustment too small"); + amountToSend = uint256(int256(amountToSend) + relayExecution.payoutAdjustment); } } // Update fill counter. _updateCountFromFill( - relayFills[relayHash], - relayFills[relayHash] + fillAmountPreFees, + relayFills[relayExecution.relayHash], + relayFills[relayExecution.relayHash] + fillAmountPreFees, relayData.amount, relayData.realizedLpFeePct, relayData.destinationToken, - localRepayment, - useContractFunds + relayData.destinationChainId == relayExecution.repaymentChainId, + relayExecution.slowFill ); // relayFills keeps track of pre-fee fill amounts as a convenience to relayers who want to specify round // numbers for the maxTokensToSend parameter or convenient numbers like 100 (i.e. relayers who will fully // fill any relay up to 100 tokens, and partial fill with 100 tokens for larger relays). - relayFills[relayHash] += fillAmountPreFees; + relayFills[relayExecution.relayHash] += fillAmountPreFees; // If relay token is wrappedNativeToken then unwrap and send native token. if (relayData.destinationToken == address(wrappedNativeToken)) { @@ -987,13 +1025,13 @@ abstract contract SpokePool is // recipient wants wrappedNativeToken, then we can assume that wrappedNativeToken is already in the // contract, otherwise we'll need the user to send wrappedNativeToken to this contract. Regardless, we'll // need to unwrap it to native token before sending to the user. - if (!useContractFunds) + if (!relayExecution.slowFill) IERC20Upgradeable(relayData.destinationToken).safeTransferFrom(msg.sender, address(this), amountToSend); _unwrapwrappedNativeTokenTo(payable(relayData.recipient), amountToSend); // Else, this is a normal ERC20 token. Send to recipient. } else { // Note: Similar to note above, send token directly from the contract to the user in the slow relay case. - if (!useContractFunds) + if (!relayExecution.slowFill) IERC20Upgradeable(relayData.destinationToken).safeTransferFrom( msg.sender, relayData.recipient, @@ -1002,8 +1040,12 @@ abstract contract SpokePool is else IERC20Upgradeable(relayData.destinationToken).safeTransfer(relayData.recipient, amountToSend); } - if (Address.isContract(relayData.recipient) && relayData.data.length > 0) { - AcrossMessageHandler(relayData.recipient).handleAcrossMessage(relayData.destinationToken, amountToSend, relayData.message); + if (Address.isContract(relayData.recipient) && relayData.message.length > 0) { + AcrossMessageHandler(relayData.recipient).handleAcrossMessage( + relayData.destinationToken, + amountToSend, + relayData.message + ); } } @@ -1036,30 +1078,28 @@ abstract contract SpokePool is } // The following internal methods emit events with many params to overcome solidity stack too deep issues. - function _emitFillRelay( - RelayExecution memory relayExecution, - - ) internal { - emit FilledRelay( - relayData.amount, - relayFills[relayHash], - fillAmount, - repaymentChainId, - relayData.originChainId, - relayData.destinationChainId, - relayData.relayerFeePct, - appliedRelayerFeePct, - relayData.realizedLpFeePct, - relayData.depositId, - relayData.destinationToken, - msg.sender, - relayData.depositor, - relayData.recipient, - appliedRecipient, - relayData.message, - appliedMessage, - isSlowRelay - ); + function _emitFillRelay(RelayExecution memory relayExecution, uint256 fillAmountPreFees) internal { + // Stack too deep -- working on a fix. + // emit FilledRelay( + // relayExecution.relay.amount, + // relayFills[relayExecution.relayHash], + // fillAmountPreFees, + // relayExecution.repaymentChainId, + // relayExecution.relay.originChainId, + // relayExecution.relay.destinationChainId, + // relayExecution.relay.relayerFeePct, + // relayExecution.updatedRelayerFeePct, + // relayExecution.relay.realizedLpFeePct, + // relayExecution.relay.depositId, + // relayExecution.relay.destinationToken, + // msg.sender, + // relayExecution.relay.depositor, + // relayExecution.relay.recipient, + // relayExecution.updatedRecipient, + // relayExecution.relay.message, + // relayExecution.updatedMessage, + // relayExecution.slowFill + // ); } function _emitDeposit( diff --git a/contracts/SpokePoolInterface.sol b/contracts/SpokePoolInterface.sol index c39743edf..5aebd7ceb 100644 --- a/contracts/SpokePoolInterface.sol +++ b/contracts/SpokePoolInterface.sol @@ -47,7 +47,7 @@ interface SpokePoolInterface { // The id uniquely identifying this deposit on the origin chain. uint32 depositId; // Data that is forwarded to the recipient. - bytes data; + bytes message; } struct SlowFill { @@ -94,13 +94,16 @@ interface SpokePoolInterface { uint256 destinationChainId, int64 relayerFeePct, uint32 quoteTimestamp, + bytes memory message, uint256 maxCount ) external payable; function speedUpDeposit( address depositor, - int64 newRelayerFeePct, + int64 updatedRelayerFeePct, uint32 depositId, + bytes memory updatedMessage, + address updatedRecipient, bytes memory depositorSignature ) external; @@ -115,12 +118,14 @@ interface SpokePoolInterface { int64 realizedLpFeePct, int64 relayerFeePct, uint32 depositId, + bytes memory message, uint256 maxCount ) external; - function fillRelayWithUpdatedFee( + function fillRelayWithUpdatedDeposit( address depositor, address recipient, + address updatedRecipient, address destinationToken, uint256 amount, uint256 maxTokensToSend, @@ -128,8 +133,10 @@ interface SpokePoolInterface { uint256 originChainId, int64 realizedLpFeePct, int64 relayerFeePct, - int64 newRelayerFeePct, + int64 updatedRelayerFeePct, uint32 depositId, + bytes memory message, + bytes memory updatedMessage, bytes memory depositorSignature, uint256 maxCount ) external; @@ -144,6 +151,7 @@ interface SpokePoolInterface { int64 relayerFeePct, uint32 depositId, uint32 rootBundleId, + bytes memory message, int256 payoutAdjustment, bytes32[] memory proof ) external; From e72d7138cf80382eddfa7016402bf82d8235af17 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 20 Feb 2023 17:23:41 -0500 Subject: [PATCH 3/7] WIP Signed-off-by: Matt Rice --- contracts/SpokePool.sol | 63 +++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 879cbe08d..46e43fd8a 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -132,17 +132,14 @@ abstract contract SpokePool is uint256 originChainId, uint256 destinationChainId, int64 relayerFeePct, - int64 appliedRelayerFeePct, int64 realizedLpFeePct, uint32 indexed depositId, address destinationToken, address indexed relayer, address indexed depositor, address recipient, - address appliedRecipient, bytes message, - bytes appliedMessage, - bool isSlowRelay + RelayExecutionInfo updatableRelayData ); event RelayedRootBundle( uint32 indexed rootBundleId, @@ -183,6 +180,13 @@ abstract contract SpokePool is int256 payoutAdjustment; } + struct RelayExecutionInfo { + address recipient; + bytes message; + int64 relayerFeePct; + bool isSlowRelay; + } + /** * Do not leave an implementation contract uninitialized. An uninitialized implementation contract can be * taken over by an attacker, which may impact the proxy. To prevent the implementation contract from being @@ -1079,27 +1083,36 @@ abstract contract SpokePool is // The following internal methods emit events with many params to overcome solidity stack too deep issues. function _emitFillRelay(RelayExecution memory relayExecution, uint256 fillAmountPreFees) internal { - // Stack too deep -- working on a fix. - // emit FilledRelay( - // relayExecution.relay.amount, - // relayFills[relayExecution.relayHash], - // fillAmountPreFees, - // relayExecution.repaymentChainId, - // relayExecution.relay.originChainId, - // relayExecution.relay.destinationChainId, - // relayExecution.relay.relayerFeePct, - // relayExecution.updatedRelayerFeePct, - // relayExecution.relay.realizedLpFeePct, - // relayExecution.relay.depositId, - // relayExecution.relay.destinationToken, - // msg.sender, - // relayExecution.relay.depositor, - // relayExecution.relay.recipient, - // relayExecution.updatedRecipient, - // relayExecution.relay.message, - // relayExecution.updatedMessage, - // relayExecution.slowFill - // ); + RelayExecutionInfo memory relayExecutionInfo; + // Stack too deep + { + relayExecutionInfo = RelayExecutionInfo({ + relayerFeePct: relayExecution.updatedRelayerFeePct, + recipient: relayExecution.updatedRecipient, + message: relayExecution.updatedMessage, + isSlowRelay: relayExecution.slowFill + }); + } + + { + emit FilledRelay( + relayExecution.relay.amount, + relayFills[relayExecution.relayHash], + fillAmountPreFees, + relayExecution.repaymentChainId, + relayExecution.relay.originChainId, + relayExecution.relay.destinationChainId, + relayExecution.relay.relayerFeePct, + relayExecution.relay.realizedLpFeePct, + relayExecution.relay.depositId, + relayExecution.relay.destinationToken, + msg.sender, + relayExecution.relay.depositor, + relayExecution.relay.recipient, + relayExecution.relay.message, + relayExecutionInfo + ); + } } function _emitDeposit( From 0da43d383c4a54d57fd147265e5997e489f965fb Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 21 Feb 2023 05:03:54 -0500 Subject: [PATCH 4/7] WIP Signed-off-by: Matt Rice --- contracts/SpokePool.sol | 75 ++++-- contracts/SpokePoolInterface.sol | 4 +- contracts/test/AcrossMessageHandlerMock.sol | 10 + test/SpokePool.Deposit.ts | 3 +- test/SpokePool.Relay.ts | 218 +++++++++++++++--- test/SpokePool.SlowRelay.ts | 50 +++- test/fixtures/SpokePool.Fixture.ts | 37 ++- .../SpokePool.SlowRelayLeafExecution.ts | 9 +- 8 files changed, 327 insertions(+), 79 deletions(-) create mode 100644 contracts/test/AcrossMessageHandlerMock.sol diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 46e43fd8a..a77e4bdde 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -16,8 +16,8 @@ import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; import "@openzeppelin/contracts/utils/math/SignedMath.sol"; -import "@openzeppelin/contracts/utils/Address.sol"; +// This interface is expected to be implemented by any contract that expects to recieve messages from the SpokePool. interface AcrossMessageHandler { function handleAcrossMessage( address tokenSent, @@ -118,10 +118,10 @@ abstract contract SpokePool is ); event RequestedSpeedUpDeposit( int64 updatedRelayerFeePct, - bytes updatedMessage, - address updatedRecipient, uint32 indexed depositId, address indexed depositor, + address updatedRecipient, + bytes updatedMessage, bytes depositorSignature ); event FilledRelay( @@ -141,6 +141,7 @@ abstract contract SpokePool is bytes message, RelayExecutionInfo updatableRelayData ); + event RelayedRootBundle( uint32 indexed rootBundleId, bytes32 indexed relayerRefundRoot, @@ -177,7 +178,7 @@ abstract contract SpokePool is uint256 maxTokensToSend; uint256 maxCount; bool slowFill; - int256 payoutAdjustment; + int256 payoutAdjustmentPct; } struct RelayExecutionInfo { @@ -185,6 +186,15 @@ abstract contract SpokePool is bytes message; int64 relayerFeePct; bool isSlowRelay; + int256 payoutAdjustmentPct; + } + + struct DepositUpdate { + uint32 depositId; + uint256 originChainId; + int64 updatedRelayerFeePct; + address updatedRecipient; + bytes updatedMessage; } /** @@ -363,6 +373,9 @@ abstract contract SpokePool is * @param relayerFeePct % of deposit amount taken out to incentivize a fast relayer. * @param quoteTimestamp Timestamp used by relayers to compute this deposit's realizedLPFeePct which is paid * to LP pool on HubPool. + * @param message Arbitrary data that can be used to pass additional information to the recipient along with the tokens. + * Note: this is intended to be used to pass along instructions for how a contract should use or allocate the tokens. + * @param maxCount used to protect the depositor from frontrunning to guarantee their quote remains valid. */ function deposit( address recipient, @@ -438,6 +451,8 @@ abstract contract SpokePool is * did in fact submit a relay. * @param updatedRelayerFeePct New relayer fee that relayers can use. * @param depositId Deposit to update fee for that originated in this contract. + * @param updatedRecipient New recipient address that should receive the tokens. + * @param updatedMessage New message that should be provided to the recipient. * @param depositorSignature Signed message containing the depositor address, this contract chain ID, the updated * relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the * EIP-712 standard. See more in the _verifyUpdateRelayerFeeMessage() comments. @@ -446,8 +461,8 @@ abstract contract SpokePool is address depositor, int64 updatedRelayerFeePct, uint32 depositId, - bytes memory updatedMessage, address updatedRecipient, + bytes memory updatedMessage, bytes memory depositorSignature ) public override nonReentrant { require(updatedRelayerFeePct < 0.5e18, "invalid relayer fee"); @@ -466,10 +481,10 @@ abstract contract SpokePool is // from the following event to submit a fill with an updated fee %. emit RequestedSpeedUpDeposit( updatedRelayerFeePct, - updatedMessage, - updatedRecipient, depositId, depositor, + updatedRecipient, + updatedMessage, depositorSignature ); } @@ -502,6 +517,8 @@ abstract contract SpokePool is * quote time. * @param relayerFeePct Fee % to keep as relayer, specified by depositor. * @param depositId Unique deposit ID on origin spoke pool. + * @param message Message to send to recipient along with tokens. + * @param maxCount Max count to protect the relayer from frontrunning. */ function fillRelay( address depositor, @@ -540,7 +557,7 @@ abstract contract SpokePool is repaymentChainId: repaymentChainId, maxTokensToSend: maxTokensToSend, slowFill: false, - payoutAdjustment: 0, + payoutAdjustmentPct: 0, maxCount: maxCount }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); @@ -568,9 +585,12 @@ abstract contract SpokePool is * @param relayerFeePct Original fee % to keep as relayer set by depositor. * @param updatedRelayerFeePct New fee % to keep as relayer also specified by depositor. * @param depositId Unique deposit ID on origin spoke pool. + * @param message Original message that was sent along with this deposit. + * @param updatedMessage Modified message that the depositor signed when updating parameters. * @param depositorSignature Signed message containing the depositor address, this contract chain ID, the updated * relayer fee %, and the deposit ID. This signature is produced by signing a hash of data according to the * EIP-712 standard. See more in the _verifyUpdateRelayerFeeMessage() comments. + * @param maxCount Max fill count to protect the relayer from frontrunning. */ function fillRelayWithUpdatedDeposit( address depositor, @@ -610,7 +630,7 @@ abstract contract SpokePool is repaymentChainId: repaymentChainId, maxTokensToSend: maxTokensToSend, slowFill: false, - payoutAdjustment: 0, + payoutAdjustmentPct: 0, maxCount: maxCount }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); @@ -648,6 +668,9 @@ abstract contract SpokePool is * @param relayerFeePct Original fee % to keep as relayer set by depositor. * @param depositId Unique deposit ID on origin spoke pool. * @param rootBundleId Unique ID of root bundle containing slow relay root that this leaf is contained in. + * @param message Message to send to the recipient if the recipient is a contract. + * @param payoutAdjustment Adjustment to the payout amount. Can be used to increase or decrease the payout to allow + * for rewards or penalties. * @param proof Inclusion proof for this leaf in slow relay root in root bundle. */ function executeSlowRelayLeaf( @@ -796,7 +819,7 @@ abstract contract SpokePool is uint32 depositId, uint32 rootBundleId, bytes memory message, - int256 payoutAdjustment, + int256 payoutAdjustmentPct, bytes32[] memory proof ) internal { RelayExecution memory relayExecution = RelayExecution({ @@ -809,7 +832,7 @@ abstract contract SpokePool is relayerFeePct: relayerFeePct, depositId: depositId, originChainId: originChainId, - destinationChainId: chainId(), + destinationChainId: destinationChainId, message: message }), relayHash: bytes32(0), @@ -817,9 +840,9 @@ abstract contract SpokePool is updatedRecipient: recipient, updatedMessage: message, repaymentChainId: 0, - maxTokensToSend: type(uint256).max, + maxTokensToSend: amount, slowFill: true, - payoutAdjustment: payoutAdjustment, + payoutAdjustmentPct: payoutAdjustmentPct, maxCount: type(uint256).max }); relayExecution.relayHash = _getRelayHash(relayExecution.relay); @@ -876,7 +899,7 @@ abstract contract SpokePool is originChainId, updatedRelayerFeePct, updatedRecipient, - updatedMessage + keccak256(updatedMessage) ) ), // By passing in the origin chain id, we enable the verification of the signature on a different chain @@ -909,10 +932,10 @@ abstract contract SpokePool is RelayExecution memory relayExecution, uint32 rootBundleId, bytes32[] memory proof - ) internal { + ) internal view { SlowFill memory slowFill = SlowFill({ relayData: relayExecution.relay, - payoutAdjustment: relayExecution.payoutAdjustment + payoutAdjustmentPct: relayExecution.payoutAdjustmentPct }); require( @@ -925,7 +948,7 @@ abstract contract SpokePool is return (1e18 * amount) / uint256((int256(1e18) - feesPct)); } - function _computeAmountPostFees(uint256 amount, int64 feesPct) private pure returns (uint256) { + function _computeAmountPostFees(uint256 amount, int256 feesPct) private pure returns (uint256) { return (amount * uint256(int256(1e18) - feesPct)) / 1e18; } @@ -1000,9 +1023,14 @@ abstract contract SpokePool is relayData.realizedLpFeePct + relayExecution.updatedRelayerFeePct ); - if (relayExecution.payoutAdjustment != 0) { - require(int256(amountToSend) + relayExecution.payoutAdjustment >= 0, "payoutAdjustment too small"); - amountToSend = uint256(int256(amountToSend) + relayExecution.payoutAdjustment); + if (relayExecution.payoutAdjustmentPct != 0) { + // If payoutAdjustmentPct is positive, then the recipient will receive more than the amount they + // were originally expecting. If it is negative, then the recipient will receive less. + // -1e18 is -100%. Because we cannot pay out negative values, that is the minimum. + require(relayExecution.payoutAdjustmentPct >= -1e18, "payoutAdjustmentPct too small"); + + // Note: since _computeAmountPostFees is typicaly intended for fees, the signage must be reversed. + amountToSend = _computeAmountPostFees(amountToSend, -relayExecution.payoutAdjustmentPct); } } @@ -1044,7 +1072,7 @@ abstract contract SpokePool is else IERC20Upgradeable(relayData.destinationToken).safeTransfer(relayData.recipient, amountToSend); } - if (Address.isContract(relayData.recipient) && relayData.message.length > 0) { + if (relayData.recipient.isContract() && relayData.message.length > 0) { AcrossMessageHandler(relayData.recipient).handleAcrossMessage( relayData.destinationToken, amountToSend, @@ -1084,13 +1112,14 @@ abstract contract SpokePool is // The following internal methods emit events with many params to overcome solidity stack too deep issues. function _emitFillRelay(RelayExecution memory relayExecution, uint256 fillAmountPreFees) internal { RelayExecutionInfo memory relayExecutionInfo; - // Stack too deep + // This separate struct is created entirely to avoid stack too deep errors. { relayExecutionInfo = RelayExecutionInfo({ relayerFeePct: relayExecution.updatedRelayerFeePct, recipient: relayExecution.updatedRecipient, message: relayExecution.updatedMessage, - isSlowRelay: relayExecution.slowFill + isSlowRelay: relayExecution.slowFill, + payoutAdjustmentPct: relayExecution.payoutAdjustmentPct }); } diff --git a/contracts/SpokePoolInterface.sol b/contracts/SpokePoolInterface.sol index 5aebd7ceb..6d43bf6cf 100644 --- a/contracts/SpokePoolInterface.sol +++ b/contracts/SpokePoolInterface.sol @@ -52,7 +52,7 @@ interface SpokePoolInterface { struct SlowFill { RelayData relayData; - int256 payoutAdjustment; + int256 payoutAdjustmentPct; } // Stores collection of merkle roots that can be published to this contract from the HubPool, which are referenced @@ -102,8 +102,8 @@ interface SpokePoolInterface { address depositor, int64 updatedRelayerFeePct, uint32 depositId, - bytes memory updatedMessage, address updatedRecipient, + bytes memory updatedMessage, bytes memory depositorSignature ) external; diff --git a/contracts/test/AcrossMessageHandlerMock.sol b/contracts/test/AcrossMessageHandlerMock.sol new file mode 100644 index 000000000..eea29820e --- /dev/null +++ b/contracts/test/AcrossMessageHandlerMock.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: AGPL-3.0-only +import "../SpokePool.sol"; + +contract AcrossMessageHandlerMock is AcrossMessageHandler { + function handleAcrossMessage( + address tokenSent, + uint256 amount, + bytes memory message + ) external override {} +} diff --git a/test/SpokePool.Deposit.ts b/test/SpokePool.Deposit.ts index 3e09d14fd..308202fce 100644 --- a/test/SpokePool.Deposit.ts +++ b/test/SpokePool.Deposit.ts @@ -73,7 +73,8 @@ describe("SpokePool Depositor Logic", async function () { currentSpokePoolTime, erc20.address, recipient.address, - depositor.address + depositor.address, + "0x" ); // The collateral should have transferred from depositor to contract. diff --git a/test/SpokePool.Relay.ts b/test/SpokePool.Relay.ts index de308ebd7..e5250538d 100644 --- a/test/SpokePool.Relay.ts +++ b/test/SpokePool.Relay.ts @@ -1,6 +1,11 @@ -import { expect, Contract, ethers, SignerWithAddress, seedWallet, toWei, toBN, BigNumber } from "./utils"; -import { spokePoolFixture, getRelayHash, modifyRelayHelper } from "./fixtures/SpokePool.Fixture"; -import { getFillRelayParams, getFillRelayUpdatedFeeParams } from "./fixtures/SpokePool.Fixture"; +import { expect, Contract, ethers, SignerWithAddress, seedWallet, toWei, toBN, BigNumber, createFake } from "./utils"; +import { + spokePoolFixture, + getRelayHash, + modifyRelayHelper, + getFillRelayParams, + getFillRelayUpdatedFeeParams, +} from "./fixtures/SpokePool.Fixture"; import * as consts from "./constants"; let spokePool: Contract, weth: Contract, erc20: Contract, destErc20: Contract, erc1271: Contract; @@ -47,14 +52,14 @@ describe("SpokePool Relayer Logic", async function () { toBN(relayData.originChainId), toBN(relayData.destinationChainId), relayData.relayerFeePct, - relayData.relayerFeePct, relayData.realizedLpFeePct, toBN(relayData.depositId), relayData.destinationToken, relayer.address, relayData.depositor, relayData.recipient, - false + relayData.message, + [relayData.recipient, relayData.message, relayData.relayerFeePct, false] ); // The collateral should have transferred from relayer to recipient. @@ -125,6 +130,33 @@ describe("SpokePool Relayer Logic", async function () { // Fill amount should be set. expect(await spokePool.relayFills(relayHash)).to.equal(consts.amountToRelayPreFees); }); + it("Relaying to contract recipient correctly calls contract and sends tokens", async function () { + const acrossMessageHandler = await createFake("AcrossMessageHandlerMock"); + const { relayData } = getRelayHash( + depositor.address, + acrossMessageHandler.address, + consts.firstDepositId, + consts.originChainId, + consts.destinationChainId, + weth.address, + undefined, + undefined, + undefined, + "0x1234" + ); + + await spokePool.connect(relayer).fillRelay(...getFillRelayParams(relayData, consts.amountToRelay)); + + expect(acrossMessageHandler.handleAcrossMessage).to.have.been.calledOnce; + expect(acrossMessageHandler.handleAcrossMessage).to.have.been.calledOnceWith( + weth.address, + consts.amountToRelay, + "0x1234" + ); + + // The collateral should have not unwrapped to ETH and then transferred to recipient. + expect(await weth.balanceOf(acrossMessageHandler.address)).to.equal(consts.amountToRelay); + }); it("General failure cases", async function () { // Fees set too high. await expect( @@ -226,10 +258,10 @@ describe("SpokePool Relayer Logic", async function () { await testUpdatedFeeSignaling(erc1271.address); }); it("Can fill relay with updated fee by including proof of depositor's agreement", async function () { - await testFillRelayWithUpdatedFee(depositor.address); + await testfillRelayWithUpdatedDeposit(depositor.address); }); it("EIP1271 - Can fill relay with updated fee by including proof of depositor's agreement", async function () { - await testFillRelayWithUpdatedFee(erc1271.address); + await testfillRelayWithUpdatedDeposit(erc1271.address); }); it("Updating relayer fee signature verification failure cases", async function () { await testUpdatedFeeSignatureFailCases(depositor.address); @@ -241,49 +273,105 @@ describe("SpokePool Relayer Logic", async function () { async function testUpdatedFeeSignaling(depositorAddress: string) { const spokePoolChainId = await spokePool.chainId(); + const updatedMessage = "0x1234"; + const updatedRecipient = depositor.address; const { signature } = await modifyRelayHelper( consts.modifiedRelayerFeePct, consts.firstDepositId.toString(), spokePoolChainId.toString(), - depositor + depositor, + updatedRecipient, + updatedMessage ); // Cannot set new relayer fee pct >= 50% await expect( - spokePool.connect(relayer).speedUpDeposit(depositorAddress, toWei("0.5"), consts.firstDepositId, signature) + spokePool + .connect(relayer) + .speedUpDeposit( + depositorAddress, + toWei("0.5"), + consts.firstDepositId, + updatedRecipient, + updatedMessage, + signature + ) ).to.be.revertedWith("invalid relayer fee"); await expect( spokePool .connect(relayer) - .speedUpDeposit(depositorAddress, consts.modifiedRelayerFeePct, consts.firstDepositId, signature) + .speedUpDeposit( + depositorAddress, + consts.modifiedRelayerFeePct, + consts.firstDepositId, + updatedRecipient, + updatedMessage, + signature + ) ) .to.emit(spokePool, "RequestedSpeedUpDeposit") - .withArgs(consts.modifiedRelayerFeePct, consts.firstDepositId, depositorAddress, signature); + .withArgs( + consts.modifiedRelayerFeePct, + consts.firstDepositId, + depositorAddress, + updatedRecipient, + updatedMessage, + signature + ); // Reverts if any param passed to function is changed. await expect( spokePool .connect(relayer) - .speedUpDeposit(relayer.address, consts.modifiedRelayerFeePct, consts.firstDepositId, signature) + .speedUpDeposit( + relayer.address, + consts.modifiedRelayerFeePct, + consts.firstDepositId, + updatedRecipient, + updatedMessage, + signature + ) + ).to.be.reverted; + + await expect( + spokePool + .connect(relayer) + .speedUpDeposit(depositorAddress, "0", consts.firstDepositId, updatedRecipient, updatedMessage, signature) ).to.be.reverted; - await expect(spokePool.connect(relayer).speedUpDeposit(depositorAddress, "0", consts.firstDepositId, signature)).to.be - .reverted; + await expect( spokePool .connect(relayer) - .speedUpDeposit(depositorAddress, consts.modifiedRelayerFeePct, consts.firstDepositId + 1, signature) + .speedUpDeposit( + depositorAddress, + consts.modifiedRelayerFeePct, + consts.firstDepositId + 1, + updatedRecipient, + updatedMessage, + signature + ) ).to.be.reverted; + await expect( spokePool .connect(relayer) - .speedUpDeposit(depositorAddress, consts.modifiedRelayerFeePct, consts.firstDepositId, "0xrandombytes") + .speedUpDeposit( + depositorAddress, + consts.modifiedRelayerFeePct, + consts.firstDepositId, + updatedRecipient, + updatedMessage, + "0xrandombytes" + ) ).to.be.reverted; const { signature: incorrectOriginChainIdSignature } = await modifyRelayHelper( consts.modifiedRelayerFeePct, consts.firstDepositId.toString(), consts.originChainId.toString(), - depositor + depositor, + updatedRecipient, + updatedMessage ); await expect( spokePool @@ -292,12 +380,14 @@ async function testUpdatedFeeSignaling(depositorAddress: string) { depositorAddress, consts.modifiedRelayerFeePct, consts.firstDepositId, + updatedRecipient, + updatedMessage, incorrectOriginChainIdSignature ) ).to.be.reverted; } -async function testFillRelayWithUpdatedFee(depositorAddress: string) { +async function testfillRelayWithUpdatedDeposit(depositorAddress: string) { // 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. @@ -309,17 +399,31 @@ async function testFillRelayWithUpdatedFee(depositorAddress: string) { consts.destinationChainId, destErc20.address ); + + const updatedRecipient = depositor.address; + const updatedMessage = "0x1234"; + const { signature } = await modifyRelayHelper( consts.modifiedRelayerFeePct, relayData.depositId, relayData.originChainId, - depositor + depositor, + updatedRecipient, + updatedMessage ); await expect( spokePool .connect(relayer) - .fillRelayWithUpdatedFee( - ...getFillRelayUpdatedFeeParams(relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, signature) + .fillRelayWithUpdatedDeposit( + ...getFillRelayUpdatedFeeParams( + relayData, + consts.amountToRelay, + consts.modifiedRelayerFeePct, + signature, + undefined, + updatedRecipient, + updatedMessage + ) ) ) .to.emit(spokePool, "FilledRelay") @@ -331,16 +435,37 @@ async function testFillRelayWithUpdatedFee(depositorAddress: string) { toBN(relayData.originChainId), toBN(relayData.destinationChainId), relayData.relayerFeePct, - consts.modifiedRelayerFeePct, // Applied relayer fee % should be diff from original fee %. relayData.realizedLpFeePct, toBN(relayData.depositId), relayData.destinationToken, relayer.address, relayData.depositor, relayData.recipient, - false + relayData.message, + [ + updatedRecipient, + updatedMessage, + consts.modifiedRelayerFeePct, // Applied relayer fee % should be diff from original fee %. + false, + ] ); + // relayExecution.relay.amount, + // relayFills[relayExecution.relayHash], + // fillAmountPreFees, + // relayExecution.repaymentChainId, + // relayExecution.relay.originChainId, + // relayExecution.relay.destinationChainId, + // relayExecution.relay.relayerFeePct, + // relayExecution.relay.realizedLpFeePct, + // relayExecution.relay.depositId, + // relayExecution.relay.destinationToken, + // msg.sender, + // relayExecution.relay.depositor, + // relayExecution.relay.recipient, + // relayExecution.relay.message, + // relayExecutionInfo + // The collateral should have transferred from relayer to recipient. expect(await destErc20.balanceOf(relayer.address)).to.equal(consts.amountToSeedWallets.sub(consts.amountToRelay)); expect(await destErc20.balanceOf(recipient.address)).to.equal(consts.amountToRelay); @@ -359,22 +484,30 @@ async function testUpdatedFeeSignatureFailCases(depositorAddress: string) { destErc20.address ); + const updatedRecipient = depositor.address; + const updatedMessage = "0x1234"; + // Message hash doesn't contain the modified fee passed as a function param. const { signature: incorrectFeeSignature } = await modifyRelayHelper( consts.incorrectModifiedRelayerFeePct, relayData.depositId, relayData.originChainId, - depositor + depositor, + updatedRecipient, + updatedMessage ); await expect( spokePool .connect(relayer) - .fillRelayWithUpdatedFee( + .fillRelayWithUpdatedDeposit( ...getFillRelayUpdatedFeeParams( relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, - incorrectFeeSignature + incorrectFeeSignature, + undefined, + updatedRecipient, + updatedMessage ) ) ).to.be.revertedWith("invalid signature"); @@ -384,17 +517,22 @@ async function testUpdatedFeeSignatureFailCases(depositorAddress: string) { consts.modifiedRelayerFeePct, relayData.depositId + "1", relayData.originChainId, - depositor + depositor, + updatedRecipient, + updatedMessage ); await expect( spokePool .connect(relayer) - .fillRelayWithUpdatedFee( + .fillRelayWithUpdatedDeposit( ...getFillRelayUpdatedFeeParams( relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, - incorrectDepositIdSignature + incorrectDepositIdSignature, + undefined, + updatedRecipient, + updatedMessage ) ) ).to.be.revertedWith("invalid signature"); @@ -402,17 +540,22 @@ async function testUpdatedFeeSignatureFailCases(depositorAddress: string) { consts.modifiedRelayerFeePct, relayData.depositId, relayData.originChainId + "1", - depositor + depositor, + updatedRecipient, + updatedMessage ); await expect( spokePool .connect(relayer) - .fillRelayWithUpdatedFee( + .fillRelayWithUpdatedDeposit( ...getFillRelayUpdatedFeeParams( relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, - incorrectChainIdSignature + incorrectChainIdSignature, + undefined, + updatedRecipient, + updatedMessage ) ) ).to.be.revertedWith("invalid signature"); @@ -422,17 +565,22 @@ async function testUpdatedFeeSignatureFailCases(depositorAddress: string) { consts.modifiedRelayerFeePct, relayData.depositId, relayData.originChainId, - relayer + relayer, + updatedRecipient, + updatedMessage ); await expect( spokePool .connect(relayer) - .fillRelayWithUpdatedFee( + .fillRelayWithUpdatedDeposit( ...getFillRelayUpdatedFeeParams( relayData, consts.amountToRelay, consts.modifiedRelayerFeePct, - incorrectSignerSignature + incorrectSignerSignature, + undefined, + updatedRecipient, + updatedMessage ) ) ).to.be.revertedWith("invalid signature"); diff --git a/test/SpokePool.SlowRelay.ts b/test/SpokePool.SlowRelay.ts index 9c6c13abd..8e2594f46 100644 --- a/test/SpokePool.SlowRelay.ts +++ b/test/SpokePool.SlowRelay.ts @@ -29,6 +29,12 @@ let tree: MerkleTree; const OTHER_DESTINATION_CHAIN_ID = (consts.destinationChainId + 666).toString(); const ZERO = BigNumber.from(0); +// Random message for ERC20 case. +const erc20Message = randomBigNumber(100).toHexString(); + +// Random message for WETH case. +const wethMessage = randomBigNumber(100).toHexString(); + // Relay fees for slow relay are only the realizedLpFee; the depositor should be re-funded the relayer fee // for any amount sent by a slow relay. const fullRelayAmountPostFees = consts.amountToRelay @@ -71,8 +77,9 @@ describe("SpokePool Slow Relay Logic", async function () { realizedLpFeePct: randomBigNumber(8, true), relayerFeePct: randomBigNumber(8, true), depositId: randomBigNumber(2).toString(), + message: randomBigNumber(100).toHexString(), }, - payoutAdjustment: "0", + payoutAdjustmentPct: "0", }); } @@ -88,8 +95,9 @@ describe("SpokePool Slow Relay Logic", async function () { realizedLpFeePct: consts.realizedLpFeePct, relayerFeePct: consts.depositRelayerFeePct, depositId: consts.firstDepositId.toString(), + message: erc20Message, }, - payoutAdjustment: "0", + payoutAdjustmentPct: "0", }); // WETH @@ -104,8 +112,9 @@ describe("SpokePool Slow Relay Logic", async function () { realizedLpFeePct: consts.realizedLpFeePct, relayerFeePct: consts.depositRelayerFeePct, depositId: consts.firstDepositId.toString(), + message: wethMessage, }, - payoutAdjustment: "0", + payoutAdjustmentPct: "0", }); tree = await buildSlowRelayTree(slowFills); @@ -127,6 +136,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + erc20Message, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === destErc20.address)!) ) @@ -155,6 +165,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + erc20Message, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === destErc20.address)!) ) @@ -169,14 +180,19 @@ describe("SpokePool Slow Relay Logic", async function () { consts.originChainId, consts.destinationChainId, consts.depositRelayerFeePct, - 0, // Should not have an applied relayerFeePct for slow relay fills. consts.realizedLpFeePct, consts.firstDepositId, destErc20.address, relayer.address, depositor.address, recipient.address, - true + erc20Message, + [ + recipient.address, + erc20Message, + 0, // Should not have an applied relayerFeePct for slow relay fills. + true, + ] ); }); @@ -195,6 +211,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + wethMessage, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === weth.address)!) ) @@ -217,6 +234,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + wethMessage, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === weth.address)!) ) @@ -243,7 +261,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.originChainId, consts.destinationChainId, destErc20.address, - consts.amountToRelay + consts.amountToRelay, + undefined, + undefined, + erc20Message ).relayData, partialAmountPostFees ) @@ -262,6 +283,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + erc20Message, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === destErc20.address)!) ) @@ -286,7 +308,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.originChainId, consts.destinationChainId, weth.address, - consts.amountToRelay + consts.amountToRelay, + undefined, + undefined, + wethMessage ).relayData, partialAmountPostFees ) @@ -306,6 +331,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + wethMessage, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === weth.address)!) ) @@ -330,7 +356,10 @@ describe("SpokePool Slow Relay Logic", async function () { consts.originChainId, consts.destinationChainId, weth.address, - consts.amountToRelay + consts.amountToRelay, + undefined, + undefined, + wethMessage ).relayData, partialAmountPostFees ) @@ -350,6 +379,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + wethMessage, ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === weth.address)!) ) @@ -377,11 +407,12 @@ describe("SpokePool Slow Relay Logic", async function () { toBN(slowFill.relayData.relayerFeePct), Number(slowFill.relayData.depositId), 0, + slowFill.relayData.message, ZERO, tree.getHexProof(slowFill!) ) ) - ).to.be.revertedWith("Invalid proof"); + ).to.be.revertedWith("Invalid slow relay proof"); }); it("Bad proof: Relay data besides destination chain ID is not included in merkle root", async function () { @@ -397,6 +428,7 @@ describe("SpokePool Slow Relay Logic", async function () { consts.depositRelayerFeePct, consts.firstDepositId, 0, + "0x1234", ZERO, tree.getHexProof(slowFills.find((slowFill) => slowFill.relayData.destinationToken === weth.address)!) ) diff --git a/test/fixtures/SpokePool.Fixture.ts b/test/fixtures/SpokePool.Fixture.ts index 8ff6f7fe0..9bd7c673f 100644 --- a/test/fixtures/SpokePool.Fixture.ts +++ b/test/fixtures/SpokePool.Fixture.ts @@ -179,11 +179,12 @@ export interface RelayData { depositId: string; originChainId: string; destinationChainId: string; + message: string; } export interface SlowFill { relayData: RelayData; - payoutAdjustment: string; + payoutAdjustmentPct: string; } export function getRelayHash( @@ -195,7 +196,8 @@ export function getRelayHash( _destinationToken: string, _amount?: BigNumber, _realizedLpFeePct?: BigNumber, - _relayerFeePct?: BigNumber + _relayerFeePct?: BigNumber, + _message?: string ): { relayHash: string; relayData: RelayData } { const relayData = { depositor: _depositor, @@ -207,11 +209,15 @@ export function getRelayHash( realizedLpFeePct: _realizedLpFeePct || consts.realizedLpFeePct, relayerFeePct: _relayerFeePct || consts.depositRelayerFeePct, depositId: _depositId.toString(), + message: _message || "0x", }; + const relayHash = ethers.utils.keccak256( defaultAbiCoder.encode( - ["address", "address", "address", "uint256", "uint256", "uint256", "uint64", "uint64", "uint32"], - Object.values(relayData) + [ + "tuple(address depositor, address recipient, address destinationToken, uint256 amount, uint256 originChainId, uint256 destinationChainId, int64 realizedLpFeePct, int64 relayerFeePct, uint32 depositId, bytes message)", + ], + [relayData] ) ); return { relayHash, relayData }; @@ -233,6 +239,7 @@ export function getDepositParams( _destinationChainId.toString(), _relayerFeePct.toString(), _quoteTime.toString(), + "0x", _maxCount ? _maxCount.toString() : consts.maxUint256.toString(), ]; } @@ -254,6 +261,7 @@ export function getFillRelayParams( _relayData.realizedLpFeePct.toString(), _relayData.relayerFeePct.toString(), _relayData.depositId, + _relayData.message || "0x", _maxCount ? _maxCount.toString() : consts.maxUint256.toString(), ]; } @@ -264,11 +272,14 @@ export function getFillRelayUpdatedFeeParams( _updatedFee: BigNumber, _signature: string, _repaymentChain?: number, + _updatedRecipient?: string, + _updatedMessage?: string, _maxCount?: BigNumber ): string[] { return [ _relayData.depositor, _relayData.recipient, + _updatedRecipient || _relayData.recipient, _relayData.destinationToken, _relayData.amount.toString(), _maxTokensToSend.toString(), @@ -278,6 +289,8 @@ export function getFillRelayUpdatedFeeParams( _relayData.relayerFeePct.toString(), _updatedFee.toString(), _relayData.depositId, + _relayData.message, + _updatedMessage || _relayData.message, _signature, _maxCount ? _maxCount.toString() : consts.maxUint256.toString(), ]; @@ -293,6 +306,7 @@ export function getExecuteSlowRelayParams( _relayerFeePct: BigNumber, _depositId: number, _relayerRefundId: number, + _message: string, _payoutAdjustment: BigNumber, _proof: string[] ): (string | string[])[] { @@ -306,6 +320,7 @@ export function getExecuteSlowRelayParams( _relayerFeePct.toString(), _depositId.toString(), _relayerRefundId.toString(), + _message, _payoutAdjustment.toString(), _proof, ]; @@ -320,14 +335,18 @@ export async function modifyRelayHelper( modifiedRelayerFeePct: BigNumber, depositId: string, originChainId: string, - depositor: SignerWithAddress + depositor: SignerWithAddress, + updatedRecipient: string, + updatedMessage: string ): Promise<{ signature: string }> { const typedData = { types: { - UpdateRelayerFeeMessage: [ - { name: "newRelayerFeePct", type: "int64" }, + UpdateDepositDetails: [ { name: "depositId", type: "uint32" }, { name: "originChainId", type: "uint256" }, + { name: "updatedRelayerFeePct", type: "int64" }, + { name: "updatedRecipient", type: "address" }, + { name: "updatedMessage", type: "bytes" }, ], }, domain: { @@ -336,9 +355,11 @@ export async function modifyRelayHelper( chainId: Number(originChainId), }, message: { - newRelayerFeePct: modifiedRelayerFeePct, depositId, originChainId, + updatedRelayerFeePct: modifiedRelayerFeePct, + updatedRecipient, + updatedMessage, }, }; const signature = await depositor._signTypedData(typedData.domain, typedData.types, typedData.message); diff --git a/test/gas-analytics/SpokePool.SlowRelayLeafExecution.ts b/test/gas-analytics/SpokePool.SlowRelayLeafExecution.ts index f63051f1f..43d55962b 100644 --- a/test/gas-analytics/SpokePool.SlowRelayLeafExecution.ts +++ b/test/gas-analytics/SpokePool.SlowRelayLeafExecution.ts @@ -47,8 +47,9 @@ async function constructSimpleTree( realizedLpFeePct: toBN(FEE_PCT), relayerFeePct: toBN(FEE_PCT), depositId: i.toString(), + message: "0x", }, - payoutAdjustment: "0", + payoutAdjustmentPct: "0", }); } const tree = await buildSlowRelayTree(slowFills); @@ -111,6 +112,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, "0", "0", + "0x", "0", initTree.tree.getHexProof(initTree.leaves[0]) ); @@ -148,6 +150,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, "0", "1", + "0x", "0", tree.getHexProof(leaves[leafIndexToExecute]) ); @@ -172,6 +175,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, i, "1", + "0x", "0", tree.getHexProof(leaves[i]) ) @@ -198,6 +202,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, i, "1", + "0x", "0", tree.getHexProof(leaf), ]); @@ -233,6 +238,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, "0", "0", + "0x", "0", initTree.tree.getHexProof(initTree.leaves[0]) ); @@ -265,6 +271,7 @@ describe("Gas Analytics: SpokePool Slow Relay Root Execution", function () { FEE_PCT, "0", "1", + "0x", "0", tree.getHexProof(leaves[leafIndexToExecute]) ); From 07aef1e5b470d1cd8477a0373166d6d658864bac Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 21 Feb 2023 05:27:32 -0500 Subject: [PATCH 5/7] WIP Signed-off-by: Matt Rice --- scripts/buildSampleTree.ts | 1 + test/SpokePool.Relay.ts | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/buildSampleTree.ts b/scripts/buildSampleTree.ts index 451b1d4aa..b7e8dec4d 100644 --- a/scripts/buildSampleTree.ts +++ b/scripts/buildSampleTree.ts @@ -132,6 +132,7 @@ async function main() { realizedLpFeePct: toBN(0), relayerFeePct: toBN(0), depositId: i.toString(), + message: "0x", }); console.group(); console.log(`- slowRelayLeaf ID#${i}: `, leaves[i]); diff --git a/test/SpokePool.Relay.ts b/test/SpokePool.Relay.ts index e5250538d..49a08fc04 100644 --- a/test/SpokePool.Relay.ts +++ b/test/SpokePool.Relay.ts @@ -147,7 +147,6 @@ describe("SpokePool Relayer Logic", async function () { await spokePool.connect(relayer).fillRelay(...getFillRelayParams(relayData, consts.amountToRelay)); - expect(acrossMessageHandler.handleAcrossMessage).to.have.been.calledOnce; expect(acrossMessageHandler.handleAcrossMessage).to.have.been.calledOnceWith( weth.address, consts.amountToRelay, From 4fb12f442f2d9357411ada260af321b54939d30a Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 21 Feb 2023 09:01:29 -0500 Subject: [PATCH 6/7] WIP Signed-off-by: Matt Rice --- contracts/SpokePool.sol | 23 ++++------------------- test/SpokePool.Relay.ts | 5 ++--- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index a77e4bdde..5a4b02d85 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -1041,7 +1041,6 @@ abstract contract SpokePool is relayData.amount, relayData.realizedLpFeePct, relayData.destinationToken, - relayData.destinationChainId == relayExecution.repaymentChainId, relayExecution.slowFill ); @@ -1087,26 +1086,12 @@ abstract contract SpokePool is uint256 totalFillAmount, int64 realizedLPFeePct, address token, - bool localRepayment, bool useContractFunds ) internal { - // If this is a slow fill or it's an initial 0-fill, do nothing, as these should not impact the count. - if (useContractFunds || endingFillAmount == 0) return; - - // If this is the first fill and it's partial, assume the rest of the fill will be slow filled (refunded on this chain). - if (startingFillAmount == 0 && totalFillAmount - endingFillAmount > 0) { - fillCounter[token] += _computeAmountPostFees(totalFillAmount - endingFillAmount, realizedLPFeePct); - } - - // If this is not the first fill, remove the partial fill that was previously assumed. - if (startingFillAmount != 0) { - fillCounter[token] -= _computeAmountPostFees(endingFillAmount - startingFillAmount, realizedLPFeePct); - } - - // If the repayment is local, add the fill amount to the running fill count. - if (localRepayment) { - fillCounter[token] += _computeAmountPostFees(endingFillAmount - startingFillAmount, realizedLPFeePct); - } + // If this is a slow fill, it's an initial 0-fill, or a partial fill has already happened, do nothing, as these + // should not impact the count. + if (useContractFunds || endingFillAmount == 0 || startingFillAmount > 0) return; + fillCounter[token] += _computeAmountPostFees(totalFillAmount, realizedLPFeePct); } // The following internal methods emit events with many params to overcome solidity stack too deep issues. diff --git a/test/SpokePool.Relay.ts b/test/SpokePool.Relay.ts index 49a08fc04..d54e26a91 100644 --- a/test/SpokePool.Relay.ts +++ b/test/SpokePool.Relay.ts @@ -69,10 +69,9 @@ describe("SpokePool Relayer Logic", async function () { // Fill amount should be set. expect(await spokePool.relayFills(relayHash)).to.equal(consts.amountToRelayPreFees); - // Fill count should be set. Note: because the refund is not taken on this chain, this will only include the amount - // left _after_ the fill, which is assumed to be a slow fill. + // Fill count should be set. Note: even though this is a partial fill, the fill count should be set to the full + // amount because we don't know if the rest of the relay will be slow filled or not. const fillCountIncrement = relayData.amount - .sub(consts.amountToRelayPreFees) .mul(consts.oneHundredPct.sub(relayData.realizedLpFeePct)) .div(consts.oneHundredPct); expect(await spokePool.fillCounter(relayData.destinationToken)).to.equal(fillCountIncrement); From 70b5db5a499c014944a8d36fa9b70b0c96144ba4 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 21 Feb 2023 09:02:12 -0500 Subject: [PATCH 7/7] WIP Signed-off-by: Matt Rice --- contracts/test/AcrossMessageHandlerMock.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/test/AcrossMessageHandlerMock.sol b/contracts/test/AcrossMessageHandlerMock.sol index eea29820e..cda26868c 100644 --- a/contracts/test/AcrossMessageHandlerMock.sol +++ b/contracts/test/AcrossMessageHandlerMock.sol @@ -1,4 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.0; + import "../SpokePool.sol"; contract AcrossMessageHandlerMock is AcrossMessageHandler {