From 93dc482899fe2c9197889f40236be8cd6fb3e13e Mon Sep 17 00:00:00 2001 From: Frederico Sabino <3332770+fmrsabino@users.noreply.github.com> Date: Wed, 3 Jun 2026 23:25:22 +0100 Subject: [PATCH] Allow multiple beneficiaries when setting up Safe Allow for multiple beneficiaries to be provided when setting up a GP Safe Account. If no beneficiary is provided or an empty array is provided. Set the `owner` as the sole beneficiary (this matches the previous behavior). --- .../accounts-actions/accountSetup.ts | 31 ++-- test/accountSetup.spec.ts | 135 +++++++++++++++++- 2 files changed, 153 insertions(+), 13 deletions(-) diff --git a/src/entrypoints/accounts-actions/accountSetup.ts b/src/entrypoints/accounts-actions/accountSetup.ts index 5541ed0..4498cd0 100644 --- a/src/entrypoints/accounts-actions/accountSetup.ts +++ b/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 { 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 */ diff --git a/test/accountSetup.spec.ts b/test/accountSetup.spec.ts index 55308dd..e9c3580 100644 --- a/test/accountSetup.spec.ts +++ b/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; + }); });