Skip to content

Commit 924eb69

Browse files
authored
perf(program): reduce PDA validation compute units (#39)
* perf(program): reduce PDA validation compute units Replace find_program_address in validate_pda with create_program_address since the bump is already provided in instruction data. Override validate_self in Escrow, Receipt, AllowedMint, and EscrowExtensions to use derive_address with the stored bump, avoiding the off-curve check that is unnecessary for accounts validated at creation. Also documents 2-step admin transfer as a future improvement. * fix(program): enforce canonical PDA validation on init Restore canonical bump enforcement for instruction-provided PDA bumps while preserving a fast path for trusted stored bumps. Add integration regressions that verify noncanonical-but-valid PDA addresses are rejected for escrow, allowed mint, receipt, and extensions initialization flows. * chore(program): remove stale PDA CU comment * refactor(program): remove dead `validate_pda_with_bump` trait method No concrete type called this — all override `validate_self` with `Address::derive_address` directly. Default `validate_self` now falls back to `validate_pda` (canonical bump enforcement). --------- Co-authored-by: Jo D <dev-jodee@users.noreply.github.com>
1 parent b4f85ed commit 924eb69

11 files changed

Lines changed: 167 additions & 20 deletions

File tree

docs/IMPROVEMENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,5 @@ The following enhancements could be considered for future iterations of the prog
1313
5. **Receipt Seed Space Optimization** - The current `receipt_seed` uses a 32-byte `Address` type. Two alternatives could save space:
1414
- **Use `u8` counter**: Change to a simple counter (0-255), saving 31 bytes per receipt. Limits to 256 receipts per depositor/escrow/mint combination, which is acceptable for most use cases.
1515
- **Single receipt with `deposit_additional` instruction**: Allow users to add to an existing receipt rather than creating new ones. This would require handling complexities around `deposited_at` timestamps (e.g., weighted average, use latest, or track per-deposit).
16+
17+
6. **Two-Step Admin Transfer** - The current `UpdateAdmin` instruction requires both the current and new admin to sign the same transaction. This is problematic when transferring to/from multisig wallets (e.g., Squads), since both parties must be present in one transaction. A 2-step pattern (`ProposeAdmin``AcceptAdmin`, with optional `CancelAdminTransfer` and a timeout) would allow async coordination between parties and is the standard pattern for admin handoffs in production programs.

program/src/state/allowed_mint.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,14 @@ impl AllowedMint {
6565
mint: &Address,
6666
) -> Result<&'a Self, ProgramError> {
6767
let state = Self::from_bytes(data)?;
68-
let pda = AllowedMintPda::new(escrow, mint);
69-
pda.validate_pda(account, program_id, state.bump)?;
68+
let derived = Address::derive_address(
69+
&[AllowedMintPda::PREFIX, escrow.as_ref(), mint.as_ref()],
70+
Some(state.bump),
71+
program_id,
72+
);
73+
if account.address() != &derived {
74+
return Err(ProgramError::InvalidSeeds);
75+
}
7076
Ok(state)
7177
}
7278
}

program/src/state/escrow.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,15 @@ impl PdaAccount for Escrow {
7979
fn bump(&self) -> u8 {
8080
self.bump
8181
}
82+
83+
#[inline(always)]
84+
fn validate_self(&self, account: &AccountView, program_id: &Address) -> Result<(), ProgramError> {
85+
let derived = Address::derive_address(&[Self::PREFIX, self.escrow_seed.as_ref()], Some(self.bump), program_id);
86+
if account.address() != &derived {
87+
return Err(ProgramError::InvalidSeeds);
88+
}
89+
Ok(())
90+
}
8291
}
8392

8493
impl Escrow {

program/src/state/escrow_extensions.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -374,21 +374,25 @@ pub fn update_or_append_extension<const N: usize>(
374374
///
375375
/// Returns `Ok(())` if:
376376
/// - Extensions account is the correct PDA for this escrow
377-
/// - If data exists, the stored bump matches the canonical bump
377+
/// - If data exists, the stored bump matches the derived address
378378
pub fn validate_extensions_pda(escrow: &AccountView, extensions: &AccountView, program_id: &Address) -> ProgramResult {
379-
let extensions_pda = ExtensionsPda::new(escrow.address());
380-
let expected_bump = extensions_pda.validate_pda_address(extensions, program_id)?;
381-
382379
if extensions.data_len() > 0 {
383380
if !extensions.owned_by(program_id) {
384381
return Err(ProgramError::InvalidAccountOwner);
385382
}
386383

387384
let data = extensions.try_borrow()?;
388385
let header = EscrowExtensionsHeader::from_bytes(&data)?;
389-
if header.bump != expected_bump {
386+
387+
let derived =
388+
Address::derive_address(&[ExtensionsPda::PREFIX, escrow.address().as_ref()], Some(header.bump), program_id);
389+
if extensions.address() != &derived {
390390
return Err(ProgramError::InvalidSeeds);
391391
}
392+
} else {
393+
// Extensions account not yet created — derive canonical bump via find_program_address
394+
let extensions_pda = ExtensionsPda::new(escrow.address());
395+
extensions_pda.validate_pda_address(extensions, program_id)?;
392396
}
393397

394398
Ok(())

program/src/state/receipt.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,25 @@ impl PdaAccount for Receipt {
124124
fn bump(&self) -> u8 {
125125
self.bump
126126
}
127+
128+
#[inline(always)]
129+
fn validate_self(&self, account: &AccountView, program_id: &Address) -> Result<(), ProgramError> {
130+
let derived = Address::derive_address(
131+
&[
132+
Self::PREFIX,
133+
self.escrow.as_ref(),
134+
self.depositor.as_ref(),
135+
self.mint.as_ref(),
136+
self.receipt_seed.as_ref(),
137+
],
138+
Some(self.bump),
139+
program_id,
140+
);
141+
if account.address() != &derived {
142+
return Err(ProgramError::InvalidSeeds);
143+
}
144+
Ok(())
145+
}
127146
}
128147

129148
impl Receipt {

program/src/traits/pda.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ pub trait PdaSeeds {
2121
Address::find_program_address(&seeds, program_id)
2222
}
2323

24-
/// Validate that account matches derived PDA
24+
/// Validate that account matches the canonical PDA for these seeds.
25+
///
26+
/// This enforces the canonical bump (highest valid bump) returned by
27+
/// `find_program_address`.
2528
#[inline(always)]
2629
fn validate_pda(&self, account: &AccountView, program_id: &Address, expected_bump: u8) -> Result<(), ProgramError> {
2730
let (derived, bump) = self.derive_address(program_id);
@@ -34,7 +37,10 @@ pub trait PdaSeeds {
3437
Ok(())
3538
}
3639

37-
/// Validate that account address matches derived PDA, returns canonical bump
40+
/// Validate that account address matches derived PDA and return the canonical bump.
41+
///
42+
/// Uses `find_program_address` — only call this when the bump is not already known.
43+
/// When bump is available, prefer `validate_pda` or `validate_self`.
3844
#[inline(always)]
3945
fn validate_pda_address(&self, account: &AccountView, program_id: &Address) -> Result<u8, ProgramError> {
4046
let (derived, bump) = self.derive_address(program_id);

tests/integration-tests/src/test_add_timelock.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use crate::{
22
fixtures::{AddTimelockFixture, CreateEscrowFixture, SetImmutableFixture},
33
utils::{
44
assert_escrow_error, assert_extensions_header, assert_instruction_error, assert_timelock_extension,
5-
find_escrow_pda, find_extensions_pda, test_empty_data, test_missing_signer, test_not_writable,
6-
test_truncated_data, test_wrong_account, test_wrong_current_program, test_wrong_system_program, EscrowError,
7-
InstructionTestFixture, TestContext, RANDOM_PUBKEY,
5+
find_escrow_pda, find_extensions_pda, find_noncanonical_program_address, test_empty_data, test_missing_signer,
6+
test_not_writable, test_truncated_data, test_wrong_account, test_wrong_current_program,
7+
test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext, RANDOM_PUBKEY,
88
},
99
};
1010
use solana_sdk::{instruction::InstructionError, signature::Signer};
@@ -53,6 +53,24 @@ fn test_add_timelock_invalid_extensions_bump() {
5353
assert_instruction_error(error, InstructionError::InvalidSeeds);
5454
}
5555

56+
#[test]
57+
fn test_add_timelock_noncanonical_extensions_pda_rejected() {
58+
let mut ctx = TestContext::new();
59+
let test_ix = AddTimelockFixture::build_valid(&mut ctx);
60+
let escrow = test_ix.instruction.accounts[2].pubkey;
61+
let program_id = test_ix.instruction.program_id;
62+
63+
let (noncanonical_extensions, noncanonical_bump) =
64+
find_noncanonical_program_address(&[b"extensions", escrow.as_ref()], &program_id)
65+
.expect("expected at least one noncanonical bump");
66+
67+
let error = test_ix
68+
.with_account_at(3, noncanonical_extensions)
69+
.with_data_byte_at(1, noncanonical_bump)
70+
.send_expect_error(&mut ctx);
71+
assert_instruction_error(error, InstructionError::InvalidSeeds);
72+
}
73+
5674
#[test]
5775
fn test_add_timelock_empty_data() {
5876
let mut ctx = TestContext::new();

tests/integration-tests/src/test_allow_mint.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ use crate::{
22
fixtures::{AllowMintFixture, AllowMintSetup},
33
utils::{
44
assert_account_exists, assert_allowed_mint_account, assert_escrow_error, assert_instruction_error,
5-
find_allowed_mint_pda, test_missing_signer, test_not_writable, test_wrong_current_program,
6-
test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext, RANDOM_PUBKEY,
5+
find_allowed_mint_pda, find_noncanonical_program_address, test_missing_signer, test_not_writable,
6+
test_wrong_current_program, test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext,
7+
RANDOM_PUBKEY,
78
},
89
};
910
use escrow_program_client::instructions::{AllowMintBuilder, SetImmutableBuilder};
@@ -64,6 +65,26 @@ fn test_allow_mint_invalid_bump() {
6465
assert_instruction_error(error, InstructionError::InvalidSeeds);
6566
}
6667

68+
#[test]
69+
fn test_allow_mint_noncanonical_pda_rejected() {
70+
let mut ctx = TestContext::new();
71+
let setup = AllowMintSetup::new(&mut ctx);
72+
let valid_ix = setup.build_instruction(&ctx);
73+
let program_id = valid_ix.instruction.program_id;
74+
75+
let (noncanonical_allowed_mint, noncanonical_bump) = find_noncanonical_program_address(
76+
&[b"allowed_mint", setup.escrow_pda.as_ref(), setup.mint_pubkey.as_ref()],
77+
&program_id,
78+
)
79+
.expect("expected at least one noncanonical bump");
80+
81+
let error = valid_ix
82+
.with_account_at(5, noncanonical_allowed_mint)
83+
.with_data_byte_at(1, noncanonical_bump)
84+
.send_expect_error(&mut ctx);
85+
assert_instruction_error(error, InstructionError::InvalidSeeds);
86+
}
87+
6788
#[test]
6889
fn test_allow_mint_wrong_admin() {
6990
let mut ctx = TestContext::new();

tests/integration-tests/src/test_create_escrow.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::{
22
fixtures::CreateEscrowFixture,
33
utils::{
4-
assert_escrow_account, assert_escrow_mutability, assert_instruction_error, test_empty_data,
5-
test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program,
4+
assert_escrow_account, assert_escrow_mutability, assert_instruction_error, find_noncanonical_program_address,
5+
test_empty_data, test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program,
66
test_wrong_system_program, InstructionTestFixture, TestContext,
77
},
88
};
@@ -70,6 +70,25 @@ fn test_create_escrow_invalid_bump() {
7070
assert_instruction_error(error, InstructionError::InvalidSeeds);
7171
}
7272

73+
#[test]
74+
fn test_create_escrow_noncanonical_bump_rejected() {
75+
let mut ctx = TestContext::new();
76+
let valid_ix = CreateEscrowFixture::build_valid(&mut ctx);
77+
78+
let escrow_seed = valid_ix.instruction.accounts[2].pubkey;
79+
let program_id = valid_ix.instruction.program_id;
80+
let (noncanonical_escrow, noncanonical_bump) =
81+
find_noncanonical_program_address(&[b"escrow", escrow_seed.as_ref()], &program_id)
82+
.expect("expected at least one noncanonical bump");
83+
84+
let error = valid_ix
85+
.with_account_at(3, noncanonical_escrow)
86+
.with_data_byte_at(1, noncanonical_bump)
87+
.send_expect_error(&mut ctx);
88+
89+
assert_instruction_error(error, InstructionError::InvalidSeeds);
90+
}
91+
7392
#[test]
7493
fn test_create_escrow_empty_data() {
7594
let mut ctx = TestContext::new();

tests/integration-tests/src/test_deposit.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ use crate::{
44
DEFAULT_DEPOSIT_AMOUNT,
55
},
66
utils::{
7-
assert_custom_error, assert_escrow_error, assert_instruction_error, find_receipt_pda, test_empty_data,
8-
test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program, test_wrong_owner,
9-
test_wrong_system_program, test_wrong_token_program, EscrowError, TestContext, TestInstruction,
10-
TEST_HOOK_ALLOW_ID, TEST_HOOK_DENY_ERROR, TEST_HOOK_DENY_ID,
7+
assert_custom_error, assert_escrow_error, assert_instruction_error, find_noncanonical_program_address,
8+
find_receipt_pda, test_empty_data, test_missing_signer, test_not_writable, test_wrong_account,
9+
test_wrong_current_program, test_wrong_owner, test_wrong_system_program, test_wrong_token_program, EscrowError,
10+
TestContext, TestInstruction, TEST_HOOK_ALLOW_ID, TEST_HOOK_DENY_ERROR, TEST_HOOK_DENY_ID,
1111
},
1212
};
1313
use escrow_program_client::instructions::DepositBuilder;
@@ -84,6 +84,29 @@ fn test_deposit_invalid_bump() {
8484
assert_instruction_error(error, InstructionError::InvalidSeeds);
8585
}
8686

87+
#[test]
88+
fn test_deposit_noncanonical_receipt_pda_rejected() {
89+
let mut ctx = TestContext::new();
90+
let setup = DepositSetup::new(&mut ctx);
91+
let valid_ix = setup.build_instruction(&ctx);
92+
let program_id = valid_ix.instruction.program_id;
93+
94+
let depositor = setup.depositor.pubkey();
95+
let mint = setup.mint.pubkey();
96+
let receipt_seed = setup.receipt_seed.pubkey();
97+
let (noncanonical_receipt, noncanonical_bump) = find_noncanonical_program_address(
98+
&[b"receipt", setup.escrow_pda.as_ref(), depositor.as_ref(), mint.as_ref(), receipt_seed.as_ref()],
99+
&program_id,
100+
)
101+
.expect("expected at least one noncanonical bump");
102+
103+
let error = valid_ix
104+
.with_account_at(5, noncanonical_receipt)
105+
.with_data_byte_at(1, noncanonical_bump)
106+
.send_expect_error(&mut ctx);
107+
assert_instruction_error(error, InstructionError::InvalidSeeds);
108+
}
109+
87110
#[test]
88111
fn test_deposit_wrong_token_program() {
89112
let mut ctx = TestContext::new();

0 commit comments

Comments
 (0)