Skip to content

Commit d31f514

Browse files
committed
initiate vault spend optimistic proposal won't fail due to reentrancy from squads
1 parent ea1879c commit d31f514

File tree

5 files changed

+538
-156
lines changed

5 files changed

+538
-156
lines changed

CLAUDE.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -196,13 +196,7 @@ await client
196196

197197
Do NOT use `advanceBySlots()` for this purpose - it changes the clock which may affect time-dependent tests.
198198

199-
**Token amounts in tests:** Use easy-to-read round numbers like hundreds or thousands of tokens. Our standard mint decimals is 6, so:
200-
- 100 tokens = `100_000_000` (100 * 10^6)
201-
- 1,000 tokens = `1_000_000_000` (1000 * 10^6)
202-
203-
This makes test assertions and calculations much easier to verify at a glance.
204-
205-
**Isolating tests during development:** When developing or debugging tests, use `.only` to run only the tests you're working on:
199+
**Isolating tests during development:** When writing or editing tests, ALWAYS add `.only` to the `describe`/`it` block you're working on before running. This keeps feedback fast and output clean. Once your changes pass, remove `.only` and run the full suite (`anchor test --skip-build`) to confirm nothing else broke.
206200

207201
```typescript
208202
// Run only this specific test
@@ -216,8 +210,6 @@ describe.only("#split_tokens", function () {
216210
});
217211
```
218212

219-
This significantly speeds up iteration and makes test output easier to read. Remember to remove `.only` before finishing development.
220-
221213
**Assertion messages:** Do not include assertion messages for better readability. The assertion itself should be clear enough:
222214

223215
```typescript
@@ -231,6 +223,22 @@ assert.equal(recipientBalance.toString(), "500000000", "Recipient should have 50
231223

232224
Exceptions: Keep messages in `expectError()` calls and `assert.fail()` within try-catch blocks, since those are part of error handling patterns and help identify which check failed.
233225

226+
227+
**Token amounts in tests:** Use easy-to-read round numbers like hundreds or thousands of tokens. Our standard mint decimals is 6, so:
228+
- 100 tokens = `100_000_000` (100 * 10^6)
229+
- 1,000 tokens = `1_000_000_000` (1000 * 10^6)
230+
231+
This makes test assertions and calculations much easier to verify at a glance.
232+
233+
### Solana Reentrancy Guard
234+
The Solana runtime prevents a program from appearing more than once in the same CPI stack. This affects two patterns in our codebase:
235+
236+
1. **futarchy → squads → futarchy** (e.g., admin-executing a DAO config change): Futarchy cannot CPI into Squads to execute a vault transaction whose inner instructions CPI back into futarchy. Workaround: futarchy only approves/validates the Squads transaction, then the client executes it as a separate top-level transaction.
237+
238+
2. **squads → futarchy → squads** (e.g., a team multisig that is itself a Squads wallet): If a Squads-initiated transaction calls a futarchy instruction that needs to CPI into Squads, the runtime will reject it. Workaround: have futarchy validate pre-created Squads accounts on-chain instead of creating them via CPI.
239+
240+
When designing instructions that involve Squads CPIs, check whether either pattern applies and flag it early. The general solution is: split the operation across multiple transactions — validate/approve in one, execute in another.
241+
234242
## SDK Usage
235243

236244
```typescript

programs/futarchy/src/instructions/initiate_vault_spend_optimistic_proposal.rs

Lines changed: 102 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ pub struct InitiateVaultSpendOptimisticProposalParams {
88
#[derive(Accounts)]
99
#[event_cpi]
1010
pub struct InitiateVaultSpendOptimisticProposal<'info> {
11-
#[account(mut, seeds = [squads_multisig_program::SEED_PREFIX, squads_multisig_program::SEED_MULTISIG, dao.key().as_ref()], bump, seeds::program = squads_program)]
11+
#[account(seeds = [squads_multisig_program::SEED_PREFIX, squads_multisig_program::SEED_MULTISIG, dao.key().as_ref()], bump, seeds::program = squads_program)]
1212
pub squads_multisig: Account<'info, squads_multisig_program::Multisig>,
1313

1414
/// CHECK: The squads multisig vault that executes the transaction
@@ -18,35 +18,26 @@ pub struct InitiateVaultSpendOptimisticProposal<'info> {
1818
#[account(seeds = [squads_multisig_program::SEED_PREFIX, squads_multisig.key().as_ref(), squads_multisig_program::SEED_SPENDING_LIMIT, dao.key().as_ref()], bump, seeds::program = squads_program)]
1919
pub squads_spending_limit: Account<'info, squads_multisig_program::SpendingLimit>,
2020

21-
/// CHECK: Squads multisig proposal, initialized by squads multisig program, checked by squads multisig program
22-
#[account(mut)]
23-
pub squads_proposal: UncheckedAccount<'info>,
21+
#[account(constraint = squads_proposal.multisig == squads_multisig.key() @ FutarchyError::InvalidTransaction)]
22+
pub squads_proposal: Account<'info, squads_multisig_program::Proposal>,
2423

25-
/// CHECK: Squads multisig vault transaction, initialized by squads multisig program, checked by squads multisig program
26-
#[account(mut)]
27-
pub squads_vault_transaction: UncheckedAccount<'info>,
28-
29-
#[account(address = permissionless_account::id())]
30-
pub squads_multisig_permissionless_account: Signer<'info>,
24+
#[account(constraint = squads_vault_transaction.multisig == squads_multisig.key() @ FutarchyError::InvalidTransaction)]
25+
pub squads_vault_transaction: Account<'info, squads_multisig_program::VaultTransaction>,
3126

3227
#[account(mut, has_one = squads_multisig, has_one = squads_multisig_vault)]
3328
pub dao: Box<Account<'info, Dao>>,
34-
#[account(mut, associated_token::mint = dao.quote_mint, associated_token::authority = dao.squads_multisig_vault)]
29+
#[account(associated_token::mint = dao.quote_mint, associated_token::authority = dao.squads_multisig_vault)]
3530
pub dao_quote_vault_account: Account<'info, TokenAccount>,
3631

3732
// Only the team can initiate an optimistic proposal
38-
#[account(mut, address = dao.team_address)]
33+
#[account(address = dao.team_address)]
3934
pub proposer: Signer<'info>,
4035

4136
/// CHECK: Used for constraints
4237
pub recipient: UncheckedAccount<'info>,
43-
#[account(mut, associated_token::mint = dao.quote_mint, associated_token::authority = recipient)]
38+
#[account(associated_token::mint = dao.quote_mint, associated_token::authority = recipient)]
4439
pub recipient_quote_account: Account<'info, TokenAccount>,
4540

46-
#[account(mut)]
47-
pub payer: Signer<'info>,
48-
49-
pub system_program: Program<'info, System>,
5041
pub squads_program: Program<'info, squads_multisig_program::program::SquadsMultisigProgram>,
5142
pub token_program: Program<'info, Token>,
5243
}
@@ -85,6 +76,97 @@ impl InitiateVaultSpendOptimisticProposal<'_> {
8576
FutarchyError::InvalidAmount
8677
);
8778

79+
// Validate the squads proposal is Active
80+
require!(
81+
matches!(
82+
self.squads_proposal.status,
83+
squads_multisig_program::ProposalStatus::Active { .. }
84+
),
85+
FutarchyError::InvalidSquadsProposalStatus
86+
);
87+
88+
// Validate the proposal references the vault transaction
89+
require_eq!(
90+
self.squads_proposal.transaction_index,
91+
self.squads_vault_transaction.index,
92+
FutarchyError::InvalidTransaction
93+
);
94+
95+
// Validate the vault transaction message contains exactly the expected SPL transfer
96+
let message = &self.squads_vault_transaction.message;
97+
98+
// Must have exactly 1 instruction
99+
require!(
100+
message.instructions.len() == 1,
101+
FutarchyError::InvalidTransaction
102+
);
103+
104+
let instruction = &message.instructions[0];
105+
106+
// Program ID must be SPL Token
107+
let program_id = message
108+
.account_keys
109+
.get(instruction.program_id_index as usize)
110+
.ok_or(error!(FutarchyError::InvalidTransaction))?;
111+
require_keys_eq!(
112+
*program_id,
113+
self.token_program.key(),
114+
FutarchyError::InvalidTransaction
115+
);
116+
117+
// Instruction data must be a Transfer: discriminator [3] + amount as u64 LE (9 bytes total)
118+
require!(
119+
instruction.data.len() == 9,
120+
FutarchyError::InvalidTransaction
121+
);
122+
require!(instruction.data[0] == 3, FutarchyError::InvalidTransaction);
123+
let encoded_amount = u64::from_le_bytes(
124+
instruction.data[1..9]
125+
.try_into()
126+
.map_err(|_| error!(FutarchyError::InvalidTransaction))?,
127+
);
128+
require_eq!(
129+
encoded_amount,
130+
params.amount,
131+
FutarchyError::InvalidTransaction
132+
);
133+
134+
// Validate the transfer accounts: source, destination, authority
135+
require!(
136+
instruction.account_indexes.len() >= 3,
137+
FutarchyError::InvalidTransaction
138+
);
139+
140+
let source = message
141+
.account_keys
142+
.get(instruction.account_indexes[0] as usize)
143+
.ok_or(error!(FutarchyError::InvalidTransaction))?;
144+
require_keys_eq!(
145+
*source,
146+
self.dao_quote_vault_account.key(),
147+
FutarchyError::InvalidTransaction
148+
);
149+
150+
let destination = message
151+
.account_keys
152+
.get(instruction.account_indexes[1] as usize)
153+
.ok_or(error!(FutarchyError::InvalidTransaction))?;
154+
require_keys_eq!(
155+
*destination,
156+
self.recipient_quote_account.key(),
157+
FutarchyError::InvalidTransaction
158+
);
159+
160+
let authority = message
161+
.account_keys
162+
.get(instruction.account_indexes[2] as usize)
163+
.ok_or(error!(FutarchyError::InvalidTransaction))?;
164+
require_keys_eq!(
165+
*authority,
166+
self.squads_multisig_vault.key(),
167+
FutarchyError::InvalidTransaction
168+
);
169+
88170
Ok(())
89171
}
90172

@@ -97,86 +179,18 @@ impl InitiateVaultSpendOptimisticProposal<'_> {
97179
squads_multisig_vault,
98180
squads_spending_limit: _,
99181
squads_proposal,
100-
squads_vault_transaction,
182+
squads_vault_transaction: _,
101183
dao,
102-
payer: _,
103-
system_program,
104184
event_authority: _,
105185
program: _,
106-
squads_program,
186+
squads_program: _,
107187
proposer,
108188
recipient: _,
109189
recipient_quote_account,
110-
squads_multisig_permissionless_account,
111-
token_program,
190+
token_program: _,
112191
dao_quote_vault_account,
113192
} = ctx.accounts;
114193

115-
// Prepare the transfer instruction
116-
let transfer_ix = anchor_spl::token::spl_token::instruction::transfer(
117-
&token_program.key(),
118-
&dao_quote_vault_account.key(),
119-
&recipient_quote_account.key(),
120-
&squads_multisig_vault.key(),
121-
&[&squads_multisig_vault.key()],
122-
params.amount,
123-
)?;
124-
125-
// Compile the transaction message in Squads' format
126-
let transaction_message =
127-
compile_squads_transaction_message(&squads_multisig_vault.key(), &[transfer_ix])?;
128-
129-
let transaction_message_bytes = transaction_message.try_to_vec()?;
130-
131-
let dao_nonce = &dao.nonce.to_le_bytes();
132-
let dao_creator_key = dao.dao_creator.as_ref();
133-
let dao_seeds = &[b"dao".as_ref(), dao_creator_key, dao_nonce, &[dao.pda_bump]];
134-
135-
let dao_signer = &[&dao_seeds[..]];
136-
137-
// Create the squads transaction
138-
squads_multisig_program::cpi::vault_transaction_create(
139-
CpiContext::new(
140-
squads_program.to_account_info(),
141-
squads_multisig_program::cpi::accounts::VaultTransactionCreate {
142-
creator: squads_multisig_permissionless_account.to_account_info(),
143-
multisig: squads_multisig.to_account_info(),
144-
rent_payer: proposer.to_account_info(),
145-
system_program: system_program.to_account_info(),
146-
transaction: squads_vault_transaction.to_account_info(),
147-
},
148-
),
149-
squads_multisig_program::VaultTransactionCreateArgs {
150-
ephemeral_signers: 0,
151-
vault_index: 0,
152-
transaction_message: transaction_message_bytes,
153-
memo: None,
154-
},
155-
)?;
156-
157-
// Reload the squads multisig account to get the latest transaction index
158-
squads_multisig.reload()?;
159-
let transaction_index = squads_multisig.transaction_index;
160-
161-
// Create the squads proposal
162-
squads_multisig_program::cpi::proposal_create(
163-
CpiContext::new_with_signer(
164-
squads_program.to_account_info(),
165-
squads_multisig_program::cpi::accounts::ProposalCreate {
166-
creator: squads_multisig_permissionless_account.to_account_info(),
167-
multisig: squads_multisig.to_account_info(),
168-
rent_payer: proposer.to_account_info(),
169-
system_program: system_program.to_account_info(),
170-
proposal: squads_proposal.to_account_info(),
171-
},
172-
dao_signer,
173-
),
174-
squads_multisig_program::ProposalCreateArgs {
175-
transaction_index,
176-
draft: false,
177-
},
178-
)?;
179-
180194
// Update the DAO state
181195
let clock = Clock::get()?;
182196

sdk/src/v0.7/FutarchyClient.ts

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
import { ConditionalVaultClient } from "./ConditionalVaultClient.js";
5454
import {
5555
createAssociatedTokenAccountIdempotentInstruction,
56+
createTransferInstruction,
5657
getAssociatedTokenAddressSync,
5758
unpackMint,
5859
TOKEN_PROGRAM_ID,
@@ -1160,6 +1161,48 @@ export class FutarchyClient {
11601161
index: transactionIndex,
11611162
})[0];
11621163

1164+
const daoQuoteVaultAccount = getAssociatedTokenAddressSync(
1165+
quoteMint,
1166+
squadsMultisigVault,
1167+
true,
1168+
);
1169+
const recipientQuoteAccount = getAssociatedTokenAddressSync(
1170+
quoteMint,
1171+
recipient,
1172+
true,
1173+
);
1174+
1175+
// Build the SPL token transfer instruction for the vault transaction
1176+
const transferIx = createTransferInstruction(
1177+
daoQuoteVaultAccount,
1178+
recipientQuoteAccount,
1179+
squadsMultisigVault,
1180+
BigInt(amount.toString()),
1181+
);
1182+
1183+
const transactionMessage = new TransactionMessage({
1184+
payerKey: payer,
1185+
recentBlockhash: "",
1186+
instructions: [transferIx],
1187+
});
1188+
1189+
const vaultTxCreate = multisig.instructions.vaultTransactionCreate({
1190+
multisigPda,
1191+
transactionIndex,
1192+
creator: PERMISSIONLESS_ACCOUNT.publicKey,
1193+
rentPayer: payer,
1194+
vaultIndex: 0,
1195+
ephemeralSigners: 0,
1196+
transactionMessage,
1197+
});
1198+
1199+
const proposalCreate = multisig.instructions.proposalCreate({
1200+
multisigPda,
1201+
transactionIndex,
1202+
creator: PERMISSIONLESS_ACCOUNT.publicKey,
1203+
rentPayer: payer,
1204+
});
1205+
11631206
return this.autocrat.methods
11641207
.initiateVaultSpendOptimisticProposal({ amount })
11651208
.accounts({
@@ -1168,32 +1211,23 @@ export class FutarchyClient {
11681211
squadsSpendingLimit,
11691212
squadsProposal,
11701213
squadsVaultTransaction,
1171-
squadsMultisigPermissionlessAccount: PERMISSIONLESS_ACCOUNT.publicKey,
11721214
dao,
1173-
daoQuoteVaultAccount: getAssociatedTokenAddressSync(
1174-
quoteMint,
1175-
squadsMultisigVault,
1176-
true,
1177-
),
1215+
daoQuoteVaultAccount,
11781216
proposer,
11791217
recipient,
1180-
recipientQuoteAccount: getAssociatedTokenAddressSync(
1181-
quoteMint,
1182-
recipient,
1183-
true,
1184-
),
1185-
payer,
1186-
systemProgram: SystemProgram.programId,
1218+
recipientQuoteAccount,
11871219
squadsProgram: SQUADS_PROGRAM_ID,
11881220
tokenProgram: TOKEN_PROGRAM_ID,
11891221
})
11901222
.preInstructions([
11911223
createAssociatedTokenAccountIdempotentInstruction(
11921224
payer,
1193-
getAssociatedTokenAddressSync(quoteMint, recipient, true),
1225+
recipientQuoteAccount,
11941226
recipient,
11951227
quoteMint,
11961228
),
1229+
vaultTxCreate,
1230+
proposalCreate,
11971231
]);
11981232
}
11991233

0 commit comments

Comments
 (0)