improve: Destructure RelayData params in FilledRelay event

contracts/SpokePool.sol

@@ -44,7 +44,7 @@ abstract contract SpokePool is Testable, Lockable, MultiCaller {
     mapping(address => mapping(uint256 => bool)) public enabledDepositRoutes;
 
     struct RelayData {
-        address sender;
+        address depositor;
         address recipient;
         address destinationToken;
         uint64 realizedLpFeePct;
@@ -72,15 +72,22 @@ abstract contract SpokePool is Testable, Lockable, MultiCaller {
         uint64 quoteTimestamp,
         address indexed originToken,
         address recipient,
-        address indexed sender
+        address indexed depositor
     );
     event FilledRelay(
         bytes32 indexed relayHash,
-        uint256 newFilledAmount,
+        uint256 totalRelayAmount,
+        uint256 totalFilledAmount,
+        uint256 fillAmount,
         uint256 indexed repaymentChain,
-        uint256 amountSentToRecipient,
+        uint256 originChainId,
+        uint64 depositId,
+        uint64 relayerFeePct,
+        uint64 realizedLpFeePct,
+        address destinationToken,
         address indexed relayer,
-        RelayData relayData
+        address depositor,
+        address recipient
     );
 
     constructor(
@@ -175,7 +182,7 @@ abstract contract SpokePool is Testable, Lockable, MultiCaller {
     }
 
     function fillRelay(
-        address sender,
+        address depositor,
         address recipient,
         address destinationToken,
         uint64 realizedLpFeePct,
@@ -196,7 +203,7 @@ abstract contract SpokePool is Testable, Lockable, MultiCaller {
         // 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.
         RelayData memory relayData = RelayData({
-            sender: sender,
+            depositor: depositor,
             recipient: recipient,
             destinationToken: destinationToken,
             realizedLpFeePct: realizedLpFeePct,
@@ -212,34 +219,42 @@ abstract contract SpokePool is Testable, Lockable, MultiCaller {
         // so this will start at 0 and increment with each fill.
         require(maxTokensToSend > 0 && relayFills[relayHash] < totalRelayAmount, "Cannot send 0, or relay filled");
 
-        // Compute the equivalent amount to be sent by the relayer before fees have been taken out. This is the amount
-        // that we'll add to the `relayFills` counter, and we do this math here in the contract for the user's
-        // convenience so that they don't have to do this math before calling this function. The user can simply
-        // pass in `maxTokensToSend` and assume that the contract will pull exactly that amount of tokens (or revert).
+        // Stores the equivalent amount to be sent by the relayer before fees have been taken out.
         uint256 fillAmountPreFees = _computeAmountPreFees(maxTokensToSend, (realizedLpFeePct + relayerFeePct));
 
-        // 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;
-        if (totalRelayAmount - relayFills[relayHash] < fillAmountPreFees) {
-            amountToSend = _computeAmountPostFees(
-                totalRelayAmount - relayFills[relayHash],
-                (realizedLpFeePct + relayerFeePct)
-            );
-            relayFills[relayHash] = totalRelayAmount;
-        } else {
-            amountToSend = maxTokensToSend;
+        // Adding brackets "stack too deep" solidity error.
+        {
+            // 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;
+            if (totalRelayAmount - relayFills[relayHash] < fillAmountPreFees) {
+                fillAmountPreFees = totalRelayAmount - relayFills[relayHash];
+                amountToSend = _computeAmountPostFees(fillAmountPreFees, (realizedLpFeePct + relayerFeePct));
+            }
             relayFills[relayHash] += fillAmountPreFees;
+            // If relay token is weth then unwrap and send eth.
+            if (destinationToken == address(weth)) {
+                IERC20(destinationToken).safeTransferFrom(msg.sender, address(this), amountToSend);
+                _unwrapWETHTo(payable(recipient), amountToSend);
+                // Else, this is a normal ERC20 token. Send to recipient.
+            } else IERC20(destinationToken).safeTransferFrom(msg.sender, recipient, amountToSend);
         }
 
-        // If relay token is weth then unwrap and send eth.
-        if (destinationToken == address(weth)) {
-            IERC20(destinationToken).safeTransferFrom(msg.sender, address(this), amountToSend);
-            _unwrapWETHTo(payable(recipient), amountToSend);
-            // Else, this is a normal ERC20 token. Send to recipient.
-        } else IERC20(destinationToken).safeTransferFrom(msg.sender, recipient, amountToSend);
-
-        emit FilledRelay(relayHash, relayFills[relayHash], repaymentChain, amountToSend, msg.sender, relayData);
+        emit FilledRelay(
+            relayHash,
+            relayData.relayAmount,
+            relayFills[relayHash],
+            fillAmountPreFees,
+            repaymentChain,
+            relayData.originChainId,
+            relayData.depositId,
+            relayData.relayerFeePct,
+            relayData.realizedLpFeePct,
+            relayData.destinationToken,
+            msg.sender,
+            relayData.depositor,
+            relayData.recipient
+        );
     }
 
     function initializeRelayerRefund(bytes32 relayerRepaymentDistributionProof) public {}

test/SpokePool.Fixture.ts

@@ -69,34 +69,46 @@ export async function deposit(
       currentSpokePoolTime
     );
 }
+export interface RelayData {
+  depositor: string;
+  recipient: string;
+  destinationToken: string;
+  realizedLpFeePct: string;
+  relayerFeePct: string;
+  depositId: string;
+  originChainId: string;
+  relayAmount: string;
+}
 export function getRelayHash(
-  sender: string,
-  recipient: string,
-  depositId: number,
-  originChainId: number,
-  destinationToken: string,
-  relayAmount?: string,
+  _depositor: string,
+  _recipient: string,
+  _depositId: number,
+  _originChainId: number,
+  _destinationToken: string,
+  _relayAmount?: string,
   _realizedLpFeePct?: string,
-  relayerFeePct?: string
-): { relayHash: string; relayData: string[] } {
-  const relayData = [
-    sender,
-    recipient,
-    destinationToken,
-    _realizedLpFeePct || realizedLpFeePct.toString(),
-    relayerFeePct || depositRelayerFeePct.toString(),
-    depositId.toString(),
-    originChainId.toString(),
-    relayAmount || amountToDeposit.toString(),
-  ];
+  _relayerFeePct?: string
+): { relayHash: string; relayData: RelayData; relayDataValues: string[] } {
+  const relayData = {
+    depositor: _depositor,
+    recipient: _recipient,
+    destinationToken: _destinationToken,
+    realizedLpFeePct: _realizedLpFeePct || realizedLpFeePct.toString(),
+    relayerFeePct: _relayerFeePct || depositRelayerFeePct.toString(),
+    depositId: _depositId.toString(),
+    originChainId: _originChainId.toString(),
+    relayAmount: _relayAmount || amountToDeposit.toString(),
+  };
+  const relayDataValues = Object.values(relayData);
   const relayHash = keccak256(
     defaultAbiCoder.encode(
       ["address", "address", "address", "uint64", "uint64", "uint64", "uint256", "uint256"],
-      relayData
+      relayDataValues
     )
   );
   return {
     relayHash,
     relayData,
+    relayDataValues,
   };
 }

test/SpokePool.Relay.ts

@@ -44,17 +44,31 @@ describe("SpokePool Relayer Logic", async function () {
     ]);
   });
   it("Relaying ERC20 tokens correctly pulls tokens and changes contract state", async function () {
-    const { relayHash, relayData } = getRelayHash(
+    const { relayHash, relayData, relayDataValues } = getRelayHash(
       depositor.address,
       recipient.address,
       firstDepositId,
       originChainId,
       destErc20.address
     );
 
-    await expect(spokePool.connect(relayer).fillRelay(...relayData, amountToRelay, repaymentChainId))
+    await expect(spokePool.connect(relayer).fillRelay(...relayDataValues, amountToRelay, repaymentChainId))
       .to.emit(spokePool, "FilledRelay")
-      .withArgs(relayHash, amountToRelayPreFees, repaymentChainId, amountToRelay, relayer.address, relayData);
+      .withArgs(
+        relayHash,
+        relayData.relayAmount,
+        amountToRelayPreFees,
+        amountToRelayPreFees,
+        repaymentChainId,
+        relayData.originChainId,
+        relayData.depositId,
+        relayData.relayerFeePct,
+        relayData.realizedLpFeePct,
+        relayData.destinationToken,
+        relayer.address,
+        relayData.depositor,
+        relayData.recipient
+      );
 
     // The collateral should have transferred from relayer to recipient.
     expect(await destErc20.balanceOf(relayer.address)).to.equal(amountToSeedWallets.sub(amountToRelay));
@@ -68,16 +82,23 @@ describe("SpokePool Relayer Logic", async function () {
     const fullRelayAmount = amountToDeposit;
     const fullRelayAmountPostFees = fullRelayAmount.mul(totalPostFeesPct).div(toBN(oneHundredPct));
     const amountRemainingInRelay = fullRelayAmount.sub(amountToRelayPreFees);
-    const amountRemainingInRelayPostFees = amountRemainingInRelay.mul(totalPostFeesPct).div(toBN(oneHundredPct));
-    await expect(spokePool.connect(relayer).fillRelay(...relayData, fullRelayAmount, repaymentChainId))
+    // const amountRemainingInRelayPostFees = amountRemainingInRelay.mul(totalPostFeesPct).div(toBN(oneHundredPct));
+    await expect(spokePool.connect(relayer).fillRelay(...relayDataValues, fullRelayAmount, repaymentChainId))
       .to.emit(spokePool, "FilledRelay")
       .withArgs(
         relayHash,
+        relayData.relayAmount,
         fullRelayAmount,
+        amountRemainingInRelay,
         repaymentChainId,
-        amountRemainingInRelayPostFees,
+        relayData.originChainId,
+        relayData.depositId,
+        relayData.relayerFeePct,
+        relayData.realizedLpFeePct,
+        relayData.destinationToken,
         relayer.address,
-        relayData
+        relayData.depositor,
+        relayData.recipient
       );
     expect(await destErc20.balanceOf(relayer.address)).to.equal(amountToSeedWallets.sub(fullRelayAmountPostFees));
     expect(await destErc20.balanceOf(recipient.address)).to.equal(fullRelayAmountPostFees);
@@ -86,7 +107,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 } = getRelayHash(
+    const { relayHash, relayData, relayDataValues } = getRelayHash(
       depositor.address,
       recipient.address,
       firstDepositId,
@@ -95,9 +116,23 @@ describe("SpokePool Relayer Logic", async function () {
     );
 
     const startingRecipientBalance = await recipient.getBalance();
-    await expect(spokePool.connect(relayer).fillRelay(...relayData, amountToRelay, repaymentChainId))
+    await expect(spokePool.connect(relayer).fillRelay(...relayDataValues, amountToRelay, repaymentChainId))
       .to.emit(spokePool, "FilledRelay")
-      .withArgs(relayHash, amountToRelayPreFees, repaymentChainId, amountToRelay, relayer.address, relayData);
+      .withArgs(
+        relayHash,
+        relayData.relayAmount,
+        amountToRelayPreFees,
+        amountToRelayPreFees,
+        repaymentChainId,
+        relayData.originChainId,
+        relayData.depositId,
+        relayData.relayerFeePct,
+        relayData.realizedLpFeePct,
+        relayData.destinationToken,
+        relayer.address,
+        relayData.depositor,
+        relayData.recipient
+      );
 
     // The collateral should have unwrapped to ETH and then transferred to recipient.
     expect(await weth.balanceOf(relayer.address)).to.equal(amountToSeedWallets.sub(amountToRelay));
@@ -121,7 +156,7 @@ describe("SpokePool Relayer Logic", async function () {
             amountToDeposit.toString(),
             toWei("0.51").toString(),
             toWei("0.5").toString()
-          ).relayData,
+          ).relayDataValues,
           amountToRelay,
           repaymentChainId
         )
@@ -139,7 +174,7 @@ describe("SpokePool Relayer Logic", async function () {
             amountToDeposit.toString(),
             toWei("0.5").toString(),
             toWei("0.51").toString()
-          ).relayData,
+          ).relayDataValues,
           amountToRelay,
           repaymentChainId
         )
@@ -157,7 +192,7 @@ describe("SpokePool Relayer Logic", async function () {
             amountToDeposit.toString(),
             toWei("0.5").toString(),
             toWei("0.5").toString()
-          ).relayData,
+          ).relayDataValues,
           amountToRelay,
           repaymentChainId
         )
@@ -169,15 +204,16 @@ describe("SpokePool Relayer Logic", async function () {
         .connect(relayer)
         .fillRelay(
           ...getRelayHash(depositor.address, recipient.address, firstDepositId, originChainId, destErc20.address)
-            .relayData,
+            .relayDataValues,
           "0",
           repaymentChainId
         )
     ).to.be.reverted;
 
     // Relay already filled
     await spokePool.connect(relayer).fillRelay(
-      ...getRelayHash(depositor.address, recipient.address, firstDepositId, originChainId, destErc20.address).relayData,
+      ...getRelayHash(depositor.address, recipient.address, firstDepositId, originChainId, destErc20.address)
+        .relayDataValues,
       amountToDeposit, // Send the full relay amount
       repaymentChainId
     );
@@ -186,7 +222,7 @@ describe("SpokePool Relayer Logic", async function () {
         .connect(relayer)
         .fillRelay(
           ...getRelayHash(depositor.address, recipient.address, firstDepositId, originChainId, destErc20.address)
-            .relayData,
+            .relayDataValues,
           "1",
           repaymentChainId
         )