From e81feb61f812df0f1ad7ab5afaa3bd656a20cd58 Mon Sep 17 00:00:00 2001 From: Vladimir Guguiev <1524432+vovacodes@users.noreply.github.com> Date: Fri, 24 Nov 2023 13:26:56 +0100 Subject: [PATCH] feat(rent-reclamation): add ConfigAction::SetRentCollector and implement its handling --- .../config_transaction_execute.rs | 35 ++- .../src/state/config_transaction.rs | 2 + sdk/multisig/idl/squads_multisig_program.json | 11 + .../src/generated/types/ConfigAction.ts | 13 + .../suites/instructions/batchAccountsClose.ts | 2 +- .../instructions/configTransactionExecute.ts | 276 ++++++++++++++++++ tests/suites/multisig-sdk.ts | 157 +--------- 7 files changed, 337 insertions(+), 159 deletions(-) create mode 100644 tests/suites/instructions/configTransactionExecute.ts diff --git a/programs/squads_multisig_program/src/instructions/config_transaction_execute.rs b/programs/squads_multisig_program/src/instructions/config_transaction_execute.rs index 7825cd78..b135d456 100644 --- a/programs/squads_multisig_program/src/instructions/config_transaction_execute.rs +++ b/programs/squads_multisig_program/src/instructions/config_transaction_execute.rs @@ -109,9 +109,32 @@ impl<'info> ConfigTransactionExecute<'info> { let rent = Rent::get()?; // Check applying the config actions will require reallocation of space for the multisig account. + + // Handle growing members vector. let new_members_length = members_length_after_actions(multisig.members.len(), &transaction.actions); - if new_members_length > multisig.members.len() { + let needs_members_allocation = new_members_length > multisig.members.len(); + + // Handle growing rent_collector changing from None -> Some(Pubkey). + // The `rent_collector` field after applying the actions will be: + // - the `new_rent_collector` of the last `SetRentCollector` action, if any present among the actions. + // - the current `rent_collector` if no `SetRentCollector` action is present among the actions. + let new_rent_collector = transaction + .actions + .iter() + .rev() + .find_map(|action| { + if let ConfigAction::SetRentCollector { new_rent_collector } = action { + Some(*new_rent_collector) + } else { + None + } + }) + .unwrap_or(multisig.rent_collector); + let needs_rent_collector_allocation = + multisig.rent_collector.is_none() && new_rent_collector.is_some(); + + if needs_members_allocation || needs_rent_collector_allocation { let rent_payer = &ctx .accounts .rent_payer @@ -126,7 +149,7 @@ impl<'info> ConfigTransactionExecute<'info> { let reallocated = Multisig::realloc_if_needed( multisig.to_account_info(), new_members_length, - multisig.rent_collector.is_some(), + new_rent_collector.is_some(), rent_payer.to_account_info(), system_program.to_account_info(), )?; @@ -281,6 +304,13 @@ impl<'info> ConfigTransactionExecute<'info> { // We don't need to invalidate prior transactions here because adding // a spending limit doesn't affect the consensus parameters of the multisig. } + + ConfigAction::SetRentCollector { new_rent_collector } => { + multisig.rent_collector = *new_rent_collector; + + // We don't need to invalidate prior transactions here because changing + // `rent_collector` doesn't affect the consensus parameters of the multisig. + } } } @@ -313,6 +343,7 @@ fn members_length_after_actions(members_length: usize, actions: &[ConfigAction]) ConfigAction::SetTimeLock { .. } => acc, ConfigAction::AddSpendingLimit { .. } => acc, ConfigAction::RemoveSpendingLimit { .. } => acc, + ConfigAction::SetRentCollector { .. } => acc, }); let abs_members_delta = diff --git a/programs/squads_multisig_program/src/state/config_transaction.rs b/programs/squads_multisig_program/src/state/config_transaction.rs index 71011120..24fa069a 100644 --- a/programs/squads_multisig_program/src/state/config_transaction.rs +++ b/programs/squads_multisig_program/src/state/config_transaction.rs @@ -73,4 +73,6 @@ pub enum ConfigAction { }, /// Remove a spending limit from the multisig. RemoveSpendingLimit { spending_limit: Pubkey }, + /// Set the `rent_collector` config parameter of the multisig. + SetRentCollector { new_rent_collector: Option }, } diff --git a/sdk/multisig/idl/squads_multisig_program.json b/sdk/multisig/idl/squads_multisig_program.json index 882f825c..d8bc4b81 100644 --- a/sdk/multisig/idl/squads_multisig_program.json +++ b/sdk/multisig/idl/squads_multisig_program.json @@ -2426,6 +2426,17 @@ "type": "publicKey" } ] + }, + { + "name": "SetRentCollector", + "fields": [ + { + "name": "newRentCollector", + "type": { + "option": "publicKey" + } + } + ] } ] } diff --git a/sdk/multisig/src/generated/types/ConfigAction.ts b/sdk/multisig/src/generated/types/ConfigAction.ts index 278b5657..6a277758 100644 --- a/sdk/multisig/src/generated/types/ConfigAction.ts +++ b/sdk/multisig/src/generated/types/ConfigAction.ts @@ -34,6 +34,7 @@ export type ConfigActionRecord = { destinations: web3.PublicKey[] } RemoveSpendingLimit: { spendingLimit: web3.PublicKey } + SetRentCollector: { newRentCollector: beet.COption } } /** @@ -70,6 +71,10 @@ export const isConfigActionRemoveSpendingLimit = ( x: ConfigAction ): x is ConfigAction & { __kind: 'RemoveSpendingLimit' } => x.__kind === 'RemoveSpendingLimit' +export const isConfigActionSetRentCollector = ( + x: ConfigAction +): x is ConfigAction & { __kind: 'SetRentCollector' } => + x.__kind === 'SetRentCollector' /** * @category userTypes @@ -131,4 +136,12 @@ export const configActionBeet = beet.dataEnum([ 'ConfigActionRecord["RemoveSpendingLimit"]' ), ], + + [ + 'SetRentCollector', + new beet.FixableBeetArgsStruct( + [['newRentCollector', beet.coption(beetSolana.publicKey)]], + 'ConfigActionRecord["SetRentCollector"]' + ), + ], ]) as beet.FixableBeet diff --git a/tests/suites/instructions/batchAccountsClose.ts b/tests/suites/instructions/batchAccountsClose.ts index a4204a4d..4d949008 100644 --- a/tests/suites/instructions/batchAccountsClose.ts +++ b/tests/suites/instructions/batchAccountsClose.ts @@ -18,7 +18,7 @@ import { TestMembers, } from "../../utils"; -const { Multisig, Proposal } = multisig.accounts; +const { Multisig } = multisig.accounts; const programId = getTestProgramId(); const connection = createLocalhostConnection(); diff --git a/tests/suites/instructions/configTransactionExecute.ts b/tests/suites/instructions/configTransactionExecute.ts new file mode 100644 index 00000000..f14dfd6a --- /dev/null +++ b/tests/suites/instructions/configTransactionExecute.ts @@ -0,0 +1,276 @@ +import * as multisig from "@sqds/multisig"; +import assert from "assert"; +import { + createAutonomousMultisig, + createLocalhostConnection, + generateMultisigMembers, + getTestProgramId, + TestMembers, +} from "../../utils"; + +const { Multisig, Proposal } = multisig.accounts; + +const programId = getTestProgramId(); +const connection = createLocalhostConnection(); + +describe("Instructions / config_transaction_execute", () => { + let members: TestMembers; + + before(async () => { + members = await generateMultisigMembers(connection); + }); + + it("error: invalid proposal status (Rejected)", async () => { + // Create new autonomous multisig. + const multisigPda = ( + await createAutonomousMultisig({ + connection, + members, + threshold: 2, + timeLock: 0, + rentCollector: null, + programId, + }) + )[0]; + + // Create a config transaction. + const transactionIndex = 1n; + let signature = await multisig.rpc.configTransactionCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer.publicKey, + actions: [{ __kind: "ChangeThreshold", newThreshold: 3 }], + programId, + }); + await connection.confirmTransaction(signature); + + // Create a proposal for the transaction. + signature = await multisig.rpc.proposalCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer, + programId, + }); + await connection.confirmTransaction(signature); + + // Reject the proposal by a member. + // Our threshold is 2 out of 2 voting members, so the cutoff is 1. + signature = await multisig.rpc.proposalReject({ + connection, + feePayer: members.voter, + multisigPda, + transactionIndex, + member: members.voter, + programId, + }); + await connection.confirmTransaction(signature); + + // Attempt to execute a transaction with a rejected proposal. + await assert.rejects( + () => + multisig.rpc.configTransactionExecute({ + connection, + feePayer: members.almighty, + multisigPda, + transactionIndex, + rentPayer: members.almighty, + member: members.almighty, + programId, + }), + /Invalid proposal status/ + ); + }); + + it("execute config transaction with ChangeThreshold action", async () => { + // Create new autonomous multisig. + const multisigPda = ( + await createAutonomousMultisig({ + connection, + members, + threshold: 1, + timeLock: 0, + rentCollector: null, + programId, + }) + )[0]; + + // Create a config transaction. + const transactionIndex = 1n; + let signature = await multisig.rpc.configTransactionCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer.publicKey, + actions: [{ __kind: "ChangeThreshold", newThreshold: 1 }], + programId, + }); + await connection.confirmTransaction(signature); + + // Create a proposal for the transaction. + signature = await multisig.rpc.proposalCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer, + programId, + }); + await connection.confirmTransaction(signature); + + // Approve the proposal. + signature = await multisig.rpc.proposalApprove({ + connection, + feePayer: members.voter, + multisigPda, + transactionIndex, + member: members.voter, + programId, + }); + await connection.confirmTransaction(signature); + + // Execute the approved config transaction. + signature = await multisig.rpc.configTransactionExecute({ + connection, + feePayer: members.almighty, + multisigPda, + transactionIndex, + member: members.almighty, + rentPayer: members.almighty, + programId, + }); + await connection.confirmTransaction(signature); + + // Verify the proposal account. + const [proposalPda] = multisig.getProposalPda({ + multisigPda, + transactionIndex, + programId, + }); + const proposalAccount = await Proposal.fromAccountAddress( + connection, + proposalPda + ); + assert.ok(multisig.types.isProposalStatusExecuted(proposalAccount.status)); + + // Verify the multisig account. + const multisigAccount = await Multisig.fromAccountAddress( + connection, + multisigPda + ); + // The threshold should have been updated. + assert.strictEqual(multisigAccount.threshold, 1); + // The stale transaction index should be updated and set to 1. + assert.strictEqual(multisigAccount.staleTransactionIndex.toString(), "1"); + }); + + it("execute config transaction with SetRentCollector action", async () => { + // Create new autonomous multisig without rent_collector. + const multisigPda = ( + await createAutonomousMultisig({ + connection, + members, + threshold: 1, + timeLock: 0, + rentCollector: null, + programId, + }) + )[0]; + + const multisigAccountInfoPreExecution = await connection.getAccountInfo( + multisigPda + )!; + + const vaultPda = multisig.getVaultPda({ + multisigPda, + index: 0, + programId, + })[0]; + + // Create a config transaction. + const transactionIndex = 1n; + let signature = await multisig.rpc.configTransactionCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer.publicKey, + actions: [{ __kind: "SetRentCollector", newRentCollector: vaultPda }], + programId, + }); + await connection.confirmTransaction(signature); + + // Create a proposal for the transaction (Approved). + signature = await multisig.rpc.proposalCreate({ + connection, + feePayer: members.proposer, + multisigPda, + transactionIndex, + creator: members.proposer, + programId, + }); + await connection.confirmTransaction(signature); + + // Approve the proposal. + signature = await multisig.rpc.proposalApprove({ + connection, + feePayer: members.voter, + multisigPda, + transactionIndex, + member: members.voter, + programId, + }); + await connection.confirmTransaction(signature); + + // Execute the approved config transaction. + signature = await multisig.rpc.configTransactionExecute({ + connection, + feePayer: members.almighty, + multisigPda, + transactionIndex, + member: members.almighty, + rentPayer: members.almighty, + programId, + }); + await connection.confirmTransaction(signature); + + // Verify the proposal account. + const [proposalPda] = multisig.getProposalPda({ + multisigPda, + transactionIndex, + programId, + }); + const proposalAccount = await Proposal.fromAccountAddress( + connection, + proposalPda + ); + assert.ok(multisig.types.isProposalStatusExecuted(proposalAccount.status)); + + // Verify the multisig account. + const multisigAccountInfoPostExecution = await connection.getAccountInfo( + multisigPda + ); + const [multisigAccountPostExecution] = Multisig.fromAccountInfo( + multisigAccountInfoPostExecution! + ); + // The rentCollector should be updated. + assert.strictEqual( + multisigAccountPostExecution.rentCollector?.toBase58(), + vaultPda.toBase58() + ); + // The stale transaction index should NOT be updated and remain 0. + assert.strictEqual( + multisigAccountPostExecution.staleTransactionIndex.toString(), + "0" + ); + // multisig space should be reallocated: increased by at least 32 bytes of new rent_collector. + assert.ok( + multisigAccountInfoPostExecution!.data.length >= + multisigAccountInfoPreExecution!.data.length + 32 + ); + }); +}); diff --git a/tests/suites/multisig-sdk.ts b/tests/suites/multisig-sdk.ts index de913475..c28a2471 100644 --- a/tests/suites/multisig-sdk.ts +++ b/tests/suites/multisig-sdk.ts @@ -3,7 +3,6 @@ import { LAMPORTS_PER_SOL, PublicKey, TransactionMessage, - VersionedTransaction, } from "@solana/web3.js"; import * as multisig from "@sqds/multisig"; import * as assert from "assert"; @@ -26,6 +25,7 @@ const { Permission, Permissions } = multisig.types; const programId = getTestProgramId(); +import "./instructions/configTransactionExecute"; import "./instructions/configTransactionAccountsClose"; import "./instructions/vaultBatchTransactionAccountClose"; import "./instructions/batchAccountsClose"; @@ -2533,161 +2533,6 @@ describe("Multisig SDK", () => { it("error: execute reentrancy"); }); - describe("config_transaction_execute", () => { - let multisigPda: PublicKey; - const approvedTransactionIndex = 1n; - const rejectedTransactionIndex = 2n; - - before(async () => { - // Create new autonomous multisig. - multisigPda = ( - await createAutonomousMultisig({ - connection, - members, - threshold: 2, - timeLock: 0, - rentCollector: null, - programId, - }) - )[0]; - - // Create a config transaction (Approved). - let signature = await multisig.rpc.configTransactionCreate({ - connection, - feePayer: members.proposer, - multisigPda, - transactionIndex: approvedTransactionIndex, - creator: members.proposer.publicKey, - actions: [{ __kind: "ChangeThreshold", newThreshold: 1 }], - programId, - }); - await connection.confirmTransaction(signature); - - // Create a proposal for the transaction (Approved). - signature = await multisig.rpc.proposalCreate({ - connection, - feePayer: members.proposer, - multisigPda, - transactionIndex: approvedTransactionIndex, - creator: members.proposer, - programId, - }); - await connection.confirmTransaction(signature); - - // Approve the proposal by the first member. - signature = await multisig.rpc.proposalApprove({ - connection, - feePayer: members.voter, - multisigPda, - transactionIndex: approvedTransactionIndex, - member: members.voter, - programId, - }); - await connection.confirmTransaction(signature); - - // Approve the proposal by the second member. - signature = await multisig.rpc.proposalApprove({ - connection, - feePayer: members.almighty, - multisigPda, - transactionIndex: approvedTransactionIndex, - member: members.almighty, - programId, - }); - await connection.confirmTransaction(signature); - - // Create a config transaction (Rejected). - signature = await multisig.rpc.configTransactionCreate({ - connection, - feePayer: members.proposer, - multisigPda, - transactionIndex: rejectedTransactionIndex, - creator: members.proposer.publicKey, - actions: [{ __kind: "ChangeThreshold", newThreshold: 3 }], - programId, - }); - await connection.confirmTransaction(signature); - - // Create a proposal for the transaction (Rejected). - signature = await multisig.rpc.proposalCreate({ - connection, - feePayer: members.proposer, - multisigPda, - transactionIndex: rejectedTransactionIndex, - creator: members.proposer, - programId, - }); - await connection.confirmTransaction(signature); - - // Reject the proposal by a member. - // Our threshold is 2 out of 2 voting members, so the cutoff is 1. - signature = await multisig.rpc.proposalReject({ - connection, - feePayer: members.voter, - multisigPda, - transactionIndex: rejectedTransactionIndex, - member: members.voter, - programId, - }); - await connection.confirmTransaction(signature); - }); - - it("execute a config transaction", async () => { - // Execute the approved config transaction. - const transactionIndex = 1n; - - const signature = await multisig.rpc.configTransactionExecute({ - connection, - feePayer: members.almighty, - multisigPda, - transactionIndex, - member: members.almighty, - rentPayer: members.almighty, - programId, - }); - await connection.confirmTransaction(signature); - - // Verify the proposal account. - const [proposalPda] = multisig.getProposalPda({ - multisigPda, - transactionIndex, - programId, - }); - const proposalAccount = await Proposal.fromAccountAddress( - connection, - proposalPda - ); - assert.ok( - multisig.types.isProposalStatusExecuted(proposalAccount.status) - ); - - // Verify the multisig account. - const multisigAccount = await Multisig.fromAccountAddress( - connection, - multisigPda - ); - // The threshold should have been updated. - assert.strictEqual(multisigAccount.threshold, 1); - }); - - it("error: invalid proposal status (Rejected)", async () => { - // Attempt to execute a transaction with a rejected proposal. - await assert.rejects( - () => - multisig.rpc.configTransactionExecute({ - connection, - feePayer: members.almighty, - multisigPda, - transactionIndex: rejectedTransactionIndex, - rentPayer: members.almighty, - member: members.almighty, - programId, - }), - /Invalid proposal status/ - ); - }); - }); - describe("utils", () => { describe("getAvailableMemoSize", () => { it("provides estimates for available size to use for memo", async () => {