refactor: disable manageUserBalance, as inconsistent with query opera…

…tions

pkg/standalone-utils/contracts/relayer/VaultActions.sol

@@ -32,7 +32,10 @@ import "./IBaseRelayerLibrary.sol";
  * @dev Since the relayer is not expected to hold user funds, we expect the user to be the recipient of any token
  * transfers from the Vault.
  *
- * All functions must be payable so they can be called from a multicall involving ETH
+ * All functions must be payable so they can be called from a multicall involving ETH.
+ *
+ * Note that this is a base contract for VaultQueryActions. Any functions that should not be called in a query context
+ * (e.g., `manageUserBalance`), should be virtual here, and overridden to revert in VaultQueryActions.
  */
 abstract contract VaultActions is IBaseRelayerLibrary {
     using Math for uint256;
@@ -114,7 +117,7 @@ abstract contract VaultActions is IBaseRelayerLibrary {
         IVault.UserBalanceOp[] memory ops,
         uint256 value,
         OutputReference[] calldata outputReferences
-    ) external payable {
+    ) external payable virtual {
         for (uint256 i = 0; i < ops.length; i++) {
             require(ops[i].sender == msg.sender || ops[i].sender == address(this), "Incorrect sender");
 

pkg/standalone-utils/contracts/relayer/VaultQueryActions.sol

@@ -25,6 +25,9 @@ import "./VaultActions.sol";
  * @title VaultQueryActions
  * @notice Allows users to simulate the core functions on the Balancer Vault (swaps/joins/exits), using queries instead
  * of the actual operations.
+ * @dev Inherits from VaultActions to maximize reuse - but also pulls in `manageUserBalance`. This might not hurt
+ * anything, but isn't intended behavior in a query context, so we override and disable it. Anything else added to the
+ * base contract that isn't query-friendly should likewise be disabled.
  */
 abstract contract VaultQueryActions is VaultActions {
     function swap(
@@ -224,4 +227,13 @@ abstract contract VaultQueryActions is VaultActions {
             _require(token == expectedTokens[i], Errors.TOKENS_MISMATCH);
         }
     }
+
+    /// @dev Prevent `vaultActionsQueryMulticall` from calling manageUserBalance.
+    function manageUserBalance(
+        IVault.UserBalanceOp[] memory,
+        uint256,
+        OutputReference[] calldata
+    ) external payable override {
+        _revert(Errors.UNIMPLEMENTED);
+    }
 }

pkg/standalone-utils/test/VaultQueryActions.test.ts

@@ -1,9 +1,9 @@
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/dist/src/signer-with-address';
 import Vault from '@balancer-labs/v2-helpers/src/models/vault/Vault';
-import { FP_ZERO, fp } from '@balancer-labs/v2-helpers/src/numbers';
+import { BigNumberish, FP_ZERO, fp } from '@balancer-labs/v2-helpers/src/numbers';
 import TokenList from '@balancer-labs/v2-helpers/src/models/tokens/TokenList';
 import WeightedPool from '@balancer-labs/v2-helpers/src/models/pools/weighted/WeightedPool';
-import { SwapKind, WeightedPoolEncoder } from '@balancer-labs/balancer-js';
+import { SwapKind, UserBalanceOpKind, WeightedPoolEncoder } from '@balancer-labs/balancer-js';
 import { MAX_UINT112, randomAddress } from '@balancer-labs/v2-helpers/src/constants';
 import { Contract, BigNumber } from 'ethers';
 import { expect } from 'chai';
@@ -17,6 +17,7 @@ import {
   encodeJoinPool,
   encodeExitPool,
   PoolKind,
+  OutputReference,
 } from './VaultActionsRelayer.setup';
 import { sharedBeforeEach } from '@balancer-labs/v2-common/sharedBeforeEach';
 import { deploy } from '@balancer-labs/v2-helpers/src/contract';
@@ -392,4 +393,49 @@ describe('VaultQueryActions', function () {
       }
     });
   });
+
+  describe('user balance ops', () => {
+    const amountDAI = fp(2);
+    const amountSNX = fp(5);
+
+    function encodeManageUserBalance(params: {
+      ops: Array<{
+        kind: UserBalanceOpKind;
+        asset: string;
+        amount: BigNumberish;
+        sender: Account;
+        recipient?: Account;
+      }>;
+      outputReferences?: OutputReference[];
+    }): string {
+      return relayerLibrary.interface.encodeFunctionData('manageUserBalance', [
+        params.ops.map((op) => ({
+          kind: op.kind,
+          asset: op.asset,
+          amount: op.amount,
+          sender: TypesConverter.toAddress(op.sender),
+          recipient: op.recipient ?? TypesConverter.toAddress(recipient),
+        })),
+        0,
+        params.outputReferences ?? [],
+      ]);
+    }
+
+    it('does not allow calls to manageUserBalance', async () => {
+      await expect(
+        relayer.connect(user).vaultActionsQueryMulticall([
+          encodeManageUserBalance({
+            ops: [
+              { kind: UserBalanceOpKind.DepositInternal, asset: tokens.DAI.address, amount: amountDAI, sender: user },
+              { kind: UserBalanceOpKind.DepositInternal, asset: tokens.SNX.address, amount: amountSNX, sender: user },
+            ],
+            outputReferences: [
+              { index: 0, key: toChainedReference(0) },
+              { index: 1, key: toChainedReference(1) },
+            ],
+          }),
+        ])
+      ).to.be.revertedWith('UNIMPLEMENTED');
+    });
+  });
 });