Fix verify

package.json

@@ -14,7 +14,7 @@
   "devDependencies": {
     "@codechecks/client": "^0.1.10",
     "@defi-wonderland/smock": "^2.0.7",
-    "@equilibria/root": "2.3.0-rc7",
+    "@equilibria/root": "2.3.0-rc8",
     "@nomicfoundation/hardhat-chai-matchers": "^1.0.6",
     "@nomicfoundation/hardhat-network-helpers": "^1.0.8",
     "@nomicfoundation/hardhat-toolbox": "2.0.2",

packages/perennial-account/contracts/AccountVerifier.sol

@@ -4,6 +4,7 @@ pragma solidity 0.8.24;
 import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
 import { VerifierBase } from "@equilibria/root/verifier/VerifierBase.sol";
+import { IMarketFactory } from "@equilibria/perennial-v2/contracts/interfaces/IMarketFactory.sol";
 
 import { IAccountVerifier } from "./interfaces/IAccountVerifier.sol";
 import { IRelayVerifier } from "./interfaces/IRelayVerifier.sol"; // only needed for docstrings
@@ -21,8 +22,13 @@ import { RelayedAccessUpdateBatch, RelayedAccessUpdateBatchLib } from "./types/R
 /// @title Verifier
 /// @notice ERC712 signed message verifier for the Perennial V2 Collateral Accounts package.
 contract AccountVerifier is VerifierBase, IAccountVerifier {
+    /// @dev market factory to check authorization
+    IMarketFactory internal immutable marketFactory;
+
     /// @dev Initializes the domain separator and parameter caches
-    constructor() EIP712("Perennial V2 Collateral Accounts", "1.0.0") { }
+    constructor(IMarketFactory _marketFactory) EIP712("Perennial V2 Collateral Accounts", "1.0.0") {
+        marketFactory = _marketFactory;
+    }
 
     /// @inheritdoc IAccountVerifier
     function verifyAction(Action calldata action, bytes calldata signature)
@@ -148,4 +154,12 @@ contract AccountVerifier is VerifierBase, IAccountVerifier {
             outerSignature
         )) revert VerifierInvalidSignerError();
     }
+
+    /// @notice Checks whether signer is allowed to sign a message for account
+    /// @param account user to check authorization for (not the collateral account)
+    /// @param signer address which signed a message for the account
+    /// @return true if signer is authorized, otherwise false
+    function _authorized(address account, address signer) internal view override returns (bool) {
+        return super._authorized(account, signer) || marketFactory.signers(account, signer);
+    }
 }

packages/perennial-account/contracts/Controller.sol

@@ -37,7 +37,7 @@ contract Controller is Factory, IController {
     Token18 public immutable DSU; // solhint-disable-line var-name-mixedcase
 
     /// @inheritdoc IController
-    IMarketFactory public marketFactory;
+    IMarketFactory public immutable marketFactory;
 
     /// @inheritdoc IController
     IAccountVerifier public verifier;
@@ -59,18 +59,17 @@ contract Controller is Factory, IController {
 
     /// @dev Creates instance of Controller
     /// @param implementation_ Collateral account contract initialized with stablecoin addresses
-    constructor(address implementation_) Factory(implementation_) {
+    constructor(address implementation_, IMarketFactory marketFactory_) Factory(implementation_) {
         USDC = Account(implementation_).USDC();
         DSU = Account(implementation_).DSU();
+        marketFactory = marketFactory_;
     }
 
     /// @inheritdoc IController
     function initialize(
-        IMarketFactory marketFactory_,
         IAccountVerifier verifier_
     ) external initializer(1) {
         __Factory__initialize();
-        marketFactory = marketFactory_;
         verifier = verifier_;
     }
 
@@ -170,8 +169,6 @@ contract Controller is Factory, IController {
     function _changeRebalanceConfigWithSignature(RebalanceConfigChange calldata configChange, bytes calldata signature) internal {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRebalanceConfigChange(configChange, signature);
-        _ensureValidSigner(configChange.action.common.account, configChange.action.common.signer);
-
         // sum of the target allocations of all markets in the group
         _updateRebalanceGroup(configChange, configChange.action.common.account);
     }
@@ -181,18 +178,12 @@ contract Controller is Factory, IController {
         emit AccountDeployed(owner, account);
     }
 
-    /// @dev reverts if user is not authorized to sign transactions for the owner
-    function _ensureValidSigner(address owner, address signer) internal view {
-        if (signer != owner && !marketFactory.signers(owner, signer)) revert ControllerInvalidSignerError();
-    }
-
     function _deployAccountWithSignature(
         DeployAccount calldata deployAccount_,
         bytes calldata signature
     ) internal returns (IAccount account) {
         address owner = deployAccount_.action.common.account;
         verifier.verifyDeployAccount(deployAccount_, signature);
-        _ensureValidSigner(owner, deployAccount_.action.common.signer);
 
         // create the account
         account = _createAccount(owner);
@@ -205,7 +196,6 @@ contract Controller is Factory, IController {
     ) internal {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyMarketTransfer(marketTransfer, signature);
-        _ensureValidSigner(marketTransfer.action.common.account, marketTransfer.action.common.signer);
 
         // only Markets with DSU collateral are supported
         IMarket market = IMarket(marketTransfer.market);
@@ -221,7 +211,6 @@ contract Controller is Factory, IController {
     ) internal {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyWithdrawal(withdrawal, signature);
-        _ensureValidSigner(withdrawal.action.common.account, withdrawal.action.common.signer);
 
         // call the account's implementation to push to owner
         account.withdraw(withdrawal.amount, withdrawal.unwrap);

packages/perennial-account/contracts/Controller_Arbitrum.sol

@@ -4,18 +4,21 @@ pragma solidity 0.8.24;
 import { Kept_Arbitrum, Kept } from "@equilibria/root/attribute/Kept/Kept_Arbitrum.sol";
 import { UFixed18 } from "@equilibria/root/number/types/UFixed18.sol";
 import { IVerifierBase } from "@equilibria/root/verifier/interfaces/IVerifierBase.sol";
+import { IMarketFactory } from "@equilibria/perennial-v2/contracts/interfaces/IMarketFactory.sol";
 import { Controller_Incentivized } from "./Controller_Incentivized.sol";
 
 /// @title Controller_Arbitrum
 /// @notice Controller which compensates keepers for handling or relaying messages on Arbitrum L2.
 contract Controller_Arbitrum is Controller_Incentivized, Kept_Arbitrum {
     /// @dev Creates instance of Controller which compensates keepers
     /// @param implementation Pristine collateral account contract
+    /// @param marketFactory Market Factory contract
     /// @param nonceManager Verifier contract to which nonce and group cancellations are relayed
     constructor(
         address implementation,
+        IMarketFactory marketFactory,
         IVerifierBase nonceManager
-    ) Controller_Incentivized(implementation, nonceManager) {}
+    ) Controller_Incentivized(implementation, marketFactory, nonceManager) {}
 
     /// @dev Use the Kept_Arbitrum implementation for calculating the dynamic fee
     function _calldataFee(

packages/perennial-account/contracts/Controller_Incentivized.sol

@@ -46,23 +46,23 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
 
     /// @dev Creates instance of Controller which compensates keepers
     /// @param implementation_ Pristine collateral account contract
+    /// @param marketFactory_ Market factory contract
     /// @param nonceManager_ Verifier contract to which nonce and group cancellations are relayed
     constructor(
         address implementation_,
+        IMarketFactory marketFactory_,
         IVerifierBase nonceManager_
-    ) Controller(implementation_) {
+    ) Controller(implementation_, marketFactory_) {
         nonceManager = nonceManager_;
     }
 
     /// @notice Configures message verification and keeper compensation
-    /// @param marketFactory_ Contract used to validate delegated signers
     /// @param verifier_ Contract used to validate collateral account message signatures
     /// @param chainlinkFeed_ ETH-USD price feed used for calculating keeper compensation
     /// @param keepConfig_ Configuration used for unbuffered keeper compensation
     /// @param keepConfigBuffered_ Configuration used for buffered keeper compensation
     /// @param keepConfigWithdrawal_ Configuration used to compensate keepers for withdrawals
     function initialize(
-        IMarketFactory marketFactory_,
         IAccountVerifier verifier_,
         AggregatorV3Interface chainlinkFeed_,
         KeepConfig memory keepConfig_,
@@ -71,7 +71,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     ) external initializer(1) {
         __Factory__initialize();
         __Kept__initialize(chainlinkFeed_, DSU);
-        marketFactory = marketFactory_;
         verifier = verifier_;
         keepConfig = keepConfig_;
         keepConfigBuffered = keepConfigBuffered_;
@@ -182,7 +181,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRelayedNonceCancellation(message, outerSignature);
-        _ensureValidSigner(message.action.common.account, message.action.common.signer);
 
         // relay the message to Verifier
         nonceManager.cancelNonceWithSignature(message.nonceCancellation, innerSignature);
@@ -205,7 +203,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRelayedGroupCancellation(message, outerSignature);
-        _ensureValidSigner(message.action.common.account, message.action.common.signer);
 
         // relay the message to Verifier
         nonceManager.cancelGroupWithSignature(message.groupCancellation, innerSignature);
@@ -228,7 +225,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRelayedOperatorUpdate(message, outerSignature);
-        _ensureValidSigner(message.action.common.account, message.action.common.signer);
 
         // relay the message to MarketFactory
         marketFactory.updateOperatorWithSignature(message.operatorUpdate, innerSignature);
@@ -251,7 +247,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRelayedSignerUpdate(message, outerSignature);
-        _ensureValidSigner(message.action.common.account, message.action.common.signer);
 
         // relay the message to MarketFactory
         marketFactory.updateSignerWithSignature(message.signerUpdate, innerSignature);
@@ -274,7 +269,6 @@ abstract contract Controller_Incentivized is Controller, IRelayer, Kept {
     {
         // ensure the message was signed by the owner or a delegated signer
         verifier.verifyRelayedAccessUpdateBatch(message, outerSignature);
-        _ensureValidSigner(message.action.common.account, message.action.common.signer);
 
         // relay the message to MarketFactory
         marketFactory.updateAccessBatchWithSignature(message.accessUpdateBatch, innerSignature);

packages/perennial-account/contracts/interfaces/IController.sol

@@ -88,10 +88,8 @@ interface IController {
     function verifier() external view returns (IAccountVerifier);
 
     /// @notice Sets contract addresses used for message verification and token management
-    /// @param marketFactory Contract used to validate delegated signers
     /// @param verifier Contract used to validate collateral account message signatures
     function initialize(
-        IMarketFactory marketFactory,
         IAccountVerifier verifier
     ) external;
 

packages/perennial-account/test/helpers/arbitrumHelpers.ts

@@ -141,23 +141,25 @@ export async function deployAndInitializeController(
   marketFactory: IMarketFactory,
 ): Promise<[IERC20Metadata, IERC20Metadata, Controller]> {
   const [dsu, usdc] = await getStablecoins(owner)
-  const controller = await deployController(owner, usdc.address, dsu.address, DSU_RESERVE)
+  const controller = await deployController(owner, usdc.address, dsu.address, DSU_RESERVE, marketFactory.address)
 
-  const verifier = await new AccountVerifier__factory(owner).deploy()
-  await controller.initialize(marketFactory.address, verifier.address)
+  const verifier = await new AccountVerifier__factory(owner).deploy(marketFactory.address)
+  await controller.initialize(verifier.address)
   return [dsu, usdc, controller]
 }
 
 // deploys an instance of the Controller with Arbitrum-specific keeper compensation mechanisms
 export async function deployControllerArbitrum(
   owner: SignerWithAddress,
+  marketFactory: IMarketFactory,
   nonceManager: IVerifier,
   overrides?: CallOverrides,
 ): Promise<Controller_Arbitrum> {
   const accountImpl = await new Account__factory(owner).deploy(USDC_ADDRESS, DSU_ADDRESS, DSU_RESERVE)
   accountImpl.initialize(constants.AddressZero)
   const controller = await new Controller_Arbitrum__factory(owner).deploy(
     accountImpl.address,
+    marketFactory.address,
     nonceManager.address,
     overrides ?? {},
   )

packages/perennial-account/test/helpers/setupHelpers.ts

@@ -153,10 +153,11 @@ export async function deployController(
   usdcAddress: Address,
   dsuAddress: Address,
   reserveAddress: Address,
+  marketFactoryAddress: Address,
 ): Promise<Controller> {
   const accountImpl = await new Account__factory(owner).deploy(usdcAddress, dsuAddress, reserveAddress)
   accountImpl.initialize(constants.AddressZero)
-  const controller = await new Controller__factory(owner).deploy(accountImpl.address)
+  const controller = await new Controller__factory(owner).deploy(accountImpl.address, marketFactoryAddress)
   return controller
 }
 

packages/perennial-account/test/integration/Controller.ts

@@ -538,7 +538,7 @@ describe('ControllerBase', () => {
       // ensure withdrawal fails
       await expect(
         controller.connect(keeper).marketTransferWithSignature(marketTransferMessage, signature, TX_OVERRIDES),
-      ).to.be.revertedWithCustomError(controller, 'ControllerInvalidSignerError')
+      ).to.be.revertedWithCustomError(verifier, 'VerifierInvalidSignerError')
     })
   })
 
@@ -599,7 +599,7 @@ describe('ControllerBase', () => {
       // ensure withdrawal fails
       await expect(
         controller.connect(keeper).withdrawWithSignature(withdrawalMessage, signature),
-      ).to.be.revertedWithCustomError(controller, 'ControllerInvalidSignerError')
+      ).to.be.revertedWithCustomError(verifier, 'VerifierInvalidSignerError')
     })
   })
 })

packages/perennial-account/test/integration/Controller_Arbitrum.ts

@@ -51,7 +51,7 @@ import {
   signOperatorUpdate,
   signSignerUpdate,
 } from '@equilibria/perennial-v2-verifier/test/helpers/erc712'
-import { Verifier__factory } from '@equilibria/perennial-v2-verifier/types/generated'
+import { Verifier, Verifier__factory } from '@equilibria/perennial-v2-verifier/types/generated'
 import { IVerifier__factory } from '@equilibria/perennial-v2/types/generated'
 import { IKeeperOracle, IOracleFactory, PythFactory } from '@equilibria/perennial-v2-oracle/types/generated'
 
@@ -213,12 +213,15 @@ describe('Controller_Arbitrum', () => {
       bufferCalldata: 35_200,
     }
     const marketVerifier = IVerifier__factory.connect(await marketFactory.verifier(), owner)
-    controller = await deployControllerArbitrum(owner, marketVerifier, { maxFeePerGas: 100000000 })
-    accountVerifier = await new AccountVerifier__factory(owner).deploy({ maxFeePerGas: 100000000 })
+    controller = await deployControllerArbitrum(owner, marketFactory, marketVerifier, {
+      maxFeePerGas: 100000000,
+    })
+    accountVerifier = await new AccountVerifier__factory(owner).deploy(marketFactory.address, {
+      maxFeePerGas: 100000000,
+    })
     // chainlink feed is used by Kept for keeper compensation
     const KeepConfig = '(uint256,uint256,uint256,uint256)'
-    await controller[`initialize(address,address,address,${KeepConfig},${KeepConfig},${KeepConfig})`](
-      marketFactory.address,
+    await controller[`initialize(address,address,${KeepConfig},${KeepConfig},${KeepConfig})`](
       accountVerifier.address,
       CHAINLINK_ETH_USD_FEED,
       keepConfig,
@@ -743,6 +746,7 @@ describe('Controller_Arbitrum', () => {
     beforeEach(async () => {
       await createCollateralAccount(userA, parse6decimal('6'))
       downstreamVerifier = Verifier__factory.connect(await marketFactory.verifier(), owner)
+      downstreamVerifier.initialize(marketFactory.address, TX_OVERRIDES)
     })
 
     afterEach(async () => {

packages/perennial-account/test/unit/Controller.ts

@@ -84,11 +84,11 @@ describe('Controller', () => {
     const usdc = await smock.fake<IERC20>('IERC20')
     const dsu = await smock.fake<IERC20>('IERC20')
     const reserve = await smock.fake<IEmptySetReserve>('IEmptySetReserve')
-    controller = await deployController(owner, usdc.address, dsu.address, reserve.address)
-
     marketFactory = await smock.fake<IMarketFactory>('IMarketFactory')
-    verifier = await new AccountVerifier__factory(owner).deploy()
-    await controller.initialize(marketFactory.address, verifier.address)
+
+    controller = await deployController(owner, usdc.address, dsu.address, reserve.address, marketFactory.address)
+    verifier = await new AccountVerifier__factory(owner).deploy(marketFactory.address)
+    await controller.initialize(verifier.address)
   }
 
   beforeEach(async () => {
@@ -170,7 +170,7 @@ describe('Controller', () => {
 
       await expect(
         controller.connect(keeper).deployAccountWithSignature(deployAccountMessage, signature),
-      ).to.be.revertedWithCustomError(controller, 'ControllerInvalidSignerError')
+      ).to.be.revertedWithCustomError(verifier, 'VerifierInvalidSignerError')
     })
 
     it('account implementation cannot be initialized', async () => {
@@ -697,7 +697,12 @@ describe('Controller', () => {
       const marketTransferMessage = {
         market: market.address,
         amount: utils.parseEther('4'),
-        ...(await createAction(userA.address, userA.address, utils.parseEther('0.3'), 24)),
+        ...(await createAction(
+          userA.address,
+          userA.address,
+          utils.parseEther('0.3'),
+          (await currentBlockTimestamp()) + 12,
+        )),
       }
       const signature = await signMarketTransfer(userA, verifier, marketTransferMessage)
       await expect(
@@ -718,7 +723,7 @@ describe('Controller', () => {
       // controller should revert
       await expect(
         controller.connect(keeper).withdrawWithSignature(withdrawalMessage, signature),
-      ).to.be.revertedWithCustomError(controller, 'ControllerInvalidSignerError')
+      ).to.be.revertedWithCustomError(verifier, 'VerifierInvalidSignerError')
     })
   })
 })