Skip to content

Commit

Permalink
Merge pull request #127 from Squads-Protocol/final-review-fixes
Browse files Browse the repository at this point in the history
General Fixes
  • Loading branch information
0xRigel authored Oct 7, 2024
2 parents 7596c1a + ca85338 commit b445684
Show file tree
Hide file tree
Showing 13 changed files with 516 additions and 389 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct TransactionBufferClose<'info> {
SEED_PREFIX,
multisig.key().as_ref(),
SEED_TRANSACTION_BUFFER,
&transaction_buffer.transaction_index.to_le_bytes(),
&transaction_buffer.buffer_index.to_le_bytes()
],
bump
)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use crate::state::*;

#[derive(AnchorSerialize, AnchorDeserialize)]
pub struct TransactionBufferCreateArgs {
/// Index of the buffer account to seed the account derivation
pub buffer_index: u8,
/// Index of the vault this transaction belongs to.
pub vault_index: u8,
/// Hash of the final assembled transaction message.
Expand Down Expand Up @@ -34,7 +36,7 @@ pub struct TransactionBufferCreate<'info> {
SEED_PREFIX,
multisig.key().as_ref(),
SEED_TRANSACTION_BUFFER,
&multisig.transaction_index.checked_add(1).unwrap().to_le_bytes(),
&args.buffer_index.to_le_bytes(),
],
bump
)]
Expand Down Expand Up @@ -88,14 +90,14 @@ impl TransactionBufferCreate<'_> {
let multisig = &ctx.accounts.multisig;
let creator = &mut ctx.accounts.creator;

// Get the transaction index.
let transaction_index = multisig.transaction_index.checked_add(1).unwrap();
// Get the buffer index.
let buffer_index = args.buffer_index;

// Initialize the transaction fields.
transaction_buffer.multisig = multisig.key();
transaction_buffer.creator = creator.key();
transaction_buffer.vault_index = args.vault_index;
transaction_buffer.transaction_index = transaction_index;
transaction_buffer.buffer_index = buffer_index;
transaction_buffer.final_buffer_hash = args.final_buffer_hash;
transaction_buffer.final_buffer_size = args.final_buffer_size;
transaction_buffer.buffer = args.buffer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@ pub struct TransactionBufferExtend<'info> {
mut,
// Only the creator can extend the buffer
constraint = transaction_buffer.creator == creator.key() @ MultisigError::Unauthorized,
// Extending the buffer only work if it still represents the next
// transaction index in the multisig
seeds = [
SEED_PREFIX,
multisig.key().as_ref(),
SEED_TRANSACTION_BUFFER,
&multisig.transaction_index.checked_add(1).unwrap().to_le_bytes(),
&transaction_buffer.buffer_index.to_le_bytes()
],
bump
)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use anchor_lang::prelude::*;

use crate::errors::*;
use crate::instructions::*;
use crate::state::*;
use anchor_lang::{prelude::*, system_program};

#[derive(Accounts)]
pub struct VaultTransactionCreateFromBuffer<'info> {
Expand All @@ -16,11 +15,7 @@ pub struct VaultTransactionCreateFromBuffer<'info> {
SEED_PREFIX,
vault_transaction_create.multisig.key().as_ref(),
SEED_TRANSACTION_BUFFER,
&vault_transaction_create.multisig
.transaction_index
.checked_add(1)
.unwrap()
.to_le_bytes(),
&transaction_buffer.buffer_index.to_le_bytes(),
],
bump
)]
Expand Down Expand Up @@ -70,6 +65,8 @@ impl<'info> VaultTransactionCreateFromBuffer<'info> {
.rent_payer
.to_account_info();

let system_program = &ctx.accounts.vault_transaction_create.system_program.to_account_info();

// Read-only accounts
let transaction_buffer = &ctx.accounts.transaction_buffer;

Expand All @@ -85,9 +82,15 @@ impl<'info> VaultTransactionCreateFromBuffer<'info> {
let top_up_lamports =
rent_exempt_lamports.saturating_sub(vault_transaction_account_info.lamports());

// Top up the account with the difference, paid by the rent payer
**rent_payer_account_info.try_borrow_mut_lamports()? -= top_up_lamports;
**vault_transaction_account_info.try_borrow_mut_lamports()? += top_up_lamports;
// System Transfer the remaining difference to the vault transaction account
let transfer_context = CpiContext::new(
system_program.to_account_info(),
system_program::Transfer {
from: rent_payer_account_info.clone(),
to: vault_transaction_account_info.clone(),
},
);
system_program::transfer(transfer_context, top_up_lamports)?;

// Reallocate the vault transaction account to the new length of the
// actual transaction message
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ pub struct TransactionBuffer {
pub multisig: Pubkey,
/// Member of the Multisig who created the TransactionBuffer.
pub creator: Pubkey,
/// Index to seed address derivation
pub buffer_index: u8,
/// Vault index of the transaction this buffer belongs to.
pub vault_index: u8,
/// Index of the transaction this buffer belongs to.
pub transaction_index: u64,
/// Hash of the final assembled transaction message.
pub final_buffer_hash: [u8; 32],
/// The size of the final assembled transaction message.
Expand All @@ -34,8 +34,8 @@ impl TransactionBuffer {
8 + // anchor account discriminator
32 + // multisig
32 + // creator
8 + // buffer_index
8 + // vault_index
8 + // transaction_index
32 + // transaction_message_hash
2 + // final_buffer_size
4 + // vec length bytes
Expand Down
17 changes: 12 additions & 5 deletions sdk/multisig/idl/squads_multisig_program.json
Original file line number Diff line number Diff line change
Expand Up @@ -2113,18 +2113,18 @@
"type": "publicKey"
},
{
"name": "vaultIndex",
"name": "bufferIndex",
"docs": [
"Vault index of the transaction this buffer belongs to."
"Index to seed address derivation"
],
"type": "u8"
},
{
"name": "transactionIndex",
"name": "vaultIndex",
"docs": [
"Index of the transaction this buffer belongs to."
"Vault index of the transaction this buffer belongs to."
],
"type": "u64"
"type": "u8"
},
{
"name": "finalBufferHash",
Expand Down Expand Up @@ -2771,6 +2771,13 @@
"type": {
"kind": "struct",
"fields": [
{
"name": "bufferIndex",
"docs": [
"Index of the buffer account to seed the account derivation"
],
"type": "u8"
},
{
"name": "vaultIndex",
"docs": [
Expand Down
22 changes: 6 additions & 16 deletions sdk/multisig/src/generated/accounts/TransactionBuffer.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions tests/suites/examples/transaction-buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
VaultTransactionCreateFromBufferInstructionArgs,
} from "@sqds/multisig/lib/generated";
import assert from "assert";
import { BN } from "bn.js";
import * as crypto from "crypto";
import {
TestMembers,
Expand Down Expand Up @@ -74,6 +73,7 @@ describe("Examples / Transaction Buffers", () => {

it("set buffer, extend, and create", async () => {
const transactionIndex = 1n;
const bufferIndex = 0;

const testIx = createTestTransferInstruction(vaultPda, vaultPda, 1);

Expand Down Expand Up @@ -103,7 +103,7 @@ describe("Examples / Transaction Buffers", () => {
Buffer.from("multisig"),
multisigPda.toBuffer(),
Buffer.from("transaction_buffer"),
new BN(Number(transactionIndex)).toBuffer("le", 8),
Buffer.from([bufferIndex])
],
programId
);
Expand All @@ -122,10 +122,10 @@ describe("Examples / Transaction Buffers", () => {
transactionBuffer,
creator: members.almighty.publicKey,
rentPayer: members.almighty.publicKey,
systemProgram: SystemProgram.programId,
},
{
args: {
bufferIndex: bufferIndex,
vaultIndex: 0,
// Must be a SHA256 hash of the message buffer.
finalBufferHash: Array.from(messageHash),
Expand Down
12 changes: 6 additions & 6 deletions tests/suites/instructions/transactionBufferClose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ describe("Instructions / transaction_buffer_close", () => {
let vaultPda: PublicKey;
let transactionBuffer: PublicKey;

const createKey = Keypair.generate();

before(async () => {
members = await generateMultisigMembers(connection);

const createKey = Keypair.generate();
multisigPda = (await createAutonomousMultisigV2({
connection,
createKey,
Expand All @@ -59,7 +59,7 @@ describe("Instructions / transaction_buffer_close", () => {
);
await connection.confirmTransaction(signature);

const transactionIndex = 1n;
const bufferIndex = 0;
const testIx = await createTestTransferInstruction(
vaultPda,
Keypair.generate().publicKey,
Expand All @@ -78,16 +78,15 @@ describe("Instructions / transaction_buffer_close", () => {
vaultPda,
});

[transactionBuffer] = await PublicKey.findProgramAddressSync(
[transactionBuffer] = PublicKey.findProgramAddressSync(
[
Buffer.from("multisig"),
multisigPda.toBuffer(),
Buffer.from("transaction_buffer"),
new BN(Number(transactionIndex)).toBuffer("le", 8),
Uint8Array.from([bufferIndex])
],
programId
);

const messageHash = crypto
.createHash("sha256")
.update(messageBuffer)
Expand All @@ -103,6 +102,7 @@ describe("Instructions / transaction_buffer_close", () => {
},
{
args: {
bufferIndex: Number(bufferIndex),
vaultIndex: 0,
finalBufferHash: Array.from(messageHash),
finalBufferSize: messageBuffer.length,
Expand All @@ -121,7 +121,7 @@ describe("Instructions / transaction_buffer_close", () => {
const createTx = new VersionedTransaction(createMessage);
createTx.sign([members.proposer]);

const createSig = await connection.sendTransaction(createTx, { skipPreflight: true });
const createSig = await connection.sendRawTransaction(createTx.serialize(), { skipPreflight: true });
await connection.confirmTransaction(createSig);
});

Expand Down
Loading

0 comments on commit b445684

Please sign in to comment.