Allow multiple beneficiaries when setting up Safe

src/entrypoints/accounts-actions/accountSetup.ts

@@ -1,4 +1,4 @@
-import { AbiCoder, ZeroAddress, getAddress } from "ethers";
+import { AbiCoder, getAddress, ZeroAddress } from "ethers";
 
 import { IERC20__factory } from "../../../typechain-types";
 import { SPENDING_ALLOWANCE_KEY, SPENDING_ROLE_KEY } from "../../constants";
@@ -49,9 +49,10 @@ type AccountSetupParameters = {
    */
   nonce: number;
   /*
-   * (optional) The beneficiary address of the Safe that is to be configured
+   * (optional) The beneficiary addresses to enable as account owners (enabled
+   * modules) on the Delay Modifier. Defaults to `[owner]`.
    */
-  beneficiary?: string;
+  beneficiaries?: string[];
 };
 
 /**
@@ -90,18 +91,24 @@ type AccountSetupParameters = {
  * await relayer.sendTransaction(transaction);
  */
 export default async function populateAccountSetup(
-  { account, owner, chainId, nonce, beneficiary }: AccountSetupParameters,
+  { account, owner, chainId, nonce, beneficiaries }: AccountSetupParameters,
   config: SetupConfig,
   sign: SignTypedDataCallback
 ): Promise<TransactionRequest> {
   account = getAddress(account);
   owner = getAddress(owner);
-  beneficiary = getAddress(beneficiary ?? owner);
+
+  // defaults to the owner when no beneficiaries are provided
+  if (beneficiaries === undefined || beneficiaries.length === 0) {
+    beneficiaries = [owner];
+  }
+
+  const beneficiariesSet = new Set(beneficiaries.map(getAddress));
 
   const { iface } = deployments.safeMastercopy;
 
   const { to, data, value, operation } = createInnerTransaction(
-    { account, owner, beneficiary },
+    { account, owner, beneficiaries: Array.from(beneficiariesSet) },
     config
   );
 
@@ -134,8 +141,8 @@ function createInnerTransaction(
   {
     account,
     owner,
-    beneficiary,
-  }: { account: string; owner: string; beneficiary: string },
+    beneficiaries,
+  }: { account: string; owner: string; beneficiaries: string[] },
   {
     spender,
     receiver,
@@ -203,12 +210,12 @@ function createInnerTransaction(
       value: 0,
       data: delayMod.iface.encodeFunctionData("setTxExpiration", [expiration]),
     },
-    // enable owner on the delay as module
-    {
+    // enable each owner on the delay as a module
+    ...beneficiaries.map((b) => ({
       to: delayMod.address,
       value: 0,
-      data: delayMod.iface.encodeFunctionData("enableModule", [beneficiary]),
-    },
+      data: delayMod.iface.encodeFunctionData("enableModule", [b]),
+    })),
     /**
      * DEPLOY AND CONFIG ROLES MODIFIER
      */

test/accountSetup.spec.ts

@@ -241,7 +241,13 @@ describe("account-setup", () => {
     });
 
     const setupTx = await populateAccountSetup(
-      { owner: user.address, account, chainId: 31337, nonce: 0, beneficiary },
+      {
+        owner: user.address,
+        account,
+        chainId: 31337,
+        nonce: 0,
+        beneficiaries: [beneficiary],
+      },
       config,
       ({ domain, types, message }) => user.signTypedData(domain, types, message)
     );
@@ -286,4 +292,131 @@ describe("account-setup", () => {
       "0x0000000000000000000000000000000000000002",
     ]);
   });
+
+  it("correctly enables multiple beneficiaries on the Delay", async () => {
+    const beneficiaryA = "0x000000000000000000000000000000000000000A";
+    const beneficiaryB = "0x000000000000000000000000000000000000000B";
+
+    const { user, account, spender, receiver, relayer, delayMod } =
+      await loadFixture(createAccount);
+
+    const COOLDOWN = 60 * 3;
+    const EXPIRATION = 60 * 30;
+
+    const config = createSetupConfig({
+      spender: spender.address,
+      receiver: receiver.address,
+      cooldown: COOLDOWN,
+      expiration: EXPIRATION,
+    });
+
+    const setupTx = await populateAccountSetup(
+      {
+        owner: user.address,
+        account,
+        chainId: 31337,
+        nonce: 0,
+        beneficiaries: [beneficiaryA, beneficiaryB],
+      },
+      config,
+      ({ domain, types, message }) => user.signTypedData(domain, types, message)
+    );
+
+    await relayer.sendTransaction(setupTx);
+
+    expect(await delayMod.isModuleEnabled(beneficiaryA)).to.be.true;
+    expect(await delayMod.isModuleEnabled(beneficiaryB)).to.be.true;
+    // when an explicit list is provided the owner account is not enabled
+    expect(await delayMod.isModuleEnabled(user.address)).to.be.false;
+    expect(await delayMod.isModuleEnabled(spender.address)).to.be.false;
+
+    expect(await delayMod.owner()).to.equal(account);
+  });
+
+  it("deduplicates repeated beneficiaries (case-insensitive)", async () => {
+    const beneficiary = "0x000000000000000000000000000000000000000A";
+
+    const { user, account, spender, receiver, relayer, delayMod } =
+      await loadFixture(createAccount);
+
+    const config = createSetupConfig({
+      spender: spender.address,
+      receiver: receiver.address,
+      cooldown: 60 * 3,
+      expiration: 60 * 30,
+    });
+
+    const setupTx = await populateAccountSetup(
+      {
+        owner: user.address,
+        account,
+        chainId: 31337,
+        nonce: 0,
+        beneficiaries: [beneficiary, beneficiary.toLowerCase()],
+      },
+      config,
+      ({ domain, types, message }) => user.signTypedData(domain, types, message)
+    );
+
+    await relayer.sendTransaction(setupTx);
+
+    expect(await delayMod.isModuleEnabled(beneficiary)).to.be.true;
+    expect(await delayMod.owner()).to.equal(account);
+  });
+
+  it("defaults to the owner when beneficiaries is empty", async () => {
+    const { user, account, spender, receiver, relayer, delayMod } =
+      await loadFixture(createAccount);
+
+    const config = createSetupConfig({
+      spender: spender.address,
+      receiver: receiver.address,
+      cooldown: 60 * 3,
+      expiration: 60 * 30,
+    });
+
+    const setupTx = await populateAccountSetup(
+      {
+        owner: user.address,
+        account,
+        chainId: 31337,
+        nonce: 0,
+        beneficiaries: [],
+      },
+      config,
+      ({ domain, types, message }) => user.signTypedData(domain, types, message)
+    );
+
+    await relayer.sendTransaction(setupTx);
+
+    expect(await delayMod.isModuleEnabled(user.address)).to.be.true;
+    expect(await delayMod.owner()).to.equal(account);
+  });
+
+  it("rejects an invalid beneficiary address", async () => {
+    const { user, account, spender, receiver } =
+      await loadFixture(createAccount);
+
+    const config = createSetupConfig({
+      spender: spender.address,
+      receiver: receiver.address,
+      cooldown: 60 * 3,
+      expiration: 60 * 30,
+    });
+
+    await expect(
+      populateAccountSetup(
+        {
+          owner: user.address,
+          account,
+          chainId: 31337,
+          nonce: 0,
+          beneficiaries: ["0xnot-an-address"],
+        },
+        config,
+        ({ domain, types, message }) =>
+          user.signTypedData(domain, types, message)
+      )
+    ).to.be.rejected;
+  });
 });