Merge remote-tracking branch 'origin/audit-zkstack-usdc' into pxrl/n01-zksync

Author: pxrl

contracts/ZkSync_SpokePool.sol

@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.0;
 
-import "./SpokePool.sol";
+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import { CircleCCTPAdapter, CircleDomainIds, ITokenMessenger } from "./libraries/CircleCCTPAdapter.sol";
 import { CrossDomainAddressUtils } from "./libraries/CrossDomainAddressUtils.sol";
+import "./SpokePool.sol";
 
 // https://github.com/matter-labs/era-contracts/blob/6391c0d7bf6184d7f6718060e3991ba6f0efe4a7/zksync/contracts/bridge/L2ERC20Bridge.sol#L104
 interface ZkBridgeLike {
@@ -24,6 +25,8 @@ interface IL2ETH {
  * @custom:security-contact [email protected]
  */
 contract ZkSync_SpokePool is SpokePool, CircleCCTPAdapter {
+    using SafeERC20 for IERC20;
+
     // On Ethereum, avoiding constructor parameters and putting them into constants reduces some of the gas cost
     // upon contract deployment. On zkSync the opposite is true: deploying the same bytecode for contracts,
     // while changing only constructor parameters can lead to substantial fee savings. So, the following params
@@ -152,6 +155,7 @@ contract ZkSync_SpokePool is SpokePool, CircleCCTPAdapter {
                 _transferUsdc(withdrawalRecipient, amountToReturn);
             } else {
                 // Matter Labs custom USDC bridge for Circle Bridged (upgradable) USDC.
+                IERC20(l2TokenAddress).forceApprove(address(zkUSDCBridge), amountToReturn);
                 zkUSDCBridge.withdraw(withdrawalRecipient, l2TokenAddress, amountToReturn);
             }
         } else {

test/evm/hardhat/chain-specific-spokepools/ZkSync_SpokePool.ts

@@ -202,9 +202,16 @@ describe("ZkSync Spoke Pool", function () {
       await zkSyncSpokePool.callStatic.chainId()
     );
     await zkSyncSpokePool.connect(crossDomainAlias).relayRootBundle(tree.getHexRoot(), mockTreeRoot);
+    let allowance = await usdc.allowance(zkSyncSpokePool.address, zkUSDCBridge.address);
+    expect(allowance.isZero()).to.be.true;
+
     await zkSyncSpokePool.connect(relayer).executeRelayerRefundLeaf(0, leaves[0], tree.getHexProof(leaves[0]));
 
-    // This should have sent tokens back to L1. Check the correct methods on the gateway are correctly called.
+    // This should have called withdraw() to pull tokens back to L1. Check the correct methods on the gateway are correctly called.
+    // zkUSDCBridge is a mocked contract, so the tokens are not actually moved and the approval is intact.
+    allowance = await usdc.allowance(zkSyncSpokePool.address, zkUSDCBridge.address);
+    expect(allowance.eq(amountToReturn)).to.be.true;
+
     expect(zkUSDCBridge.withdraw).to.have.been.calledOnce;
     expect(zkUSDCBridge.withdraw).to.have.been.calledWith(hubPool.address, usdc.address, amountToReturn);
   });