Skip to content

Commit ed951be

Browse files
committed
fix: resolve E2E test failures and verify all scenarios
- Fix failures.rs Scenario 4 (Session Expiry): use warp_to_slot to ensure expiry - Fix failures.rs Scenario 5 (Admin Perms): fix instruction data and account order - Fix cross_wallet_attacks.rs: fix instruction data and signing keypairs - Add warp_to_slot to TestContext - All E2E tests PASS
1 parent 4587573 commit ed951be

4 files changed

Lines changed: 73 additions & 98 deletions

File tree

tests-e2e/TEST_ISSUES.md

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,31 @@
11
# E2E Test Issues
22

3-
## Issue #1: `failures.rs` Scenario 3 - Spender Privilege Escalation Test
4-
5-
**Status**: 🔴 Broken
6-
**Priority**: Medium
7-
**File**: `tests-e2e/src/scenarios/failures.rs` (lines 225-290)
8-
9-
### Problem
10-
Test has incorrect logic - mixes CreateSession and AddAuthority concepts:
11-
- Creates `session_pda` and `session_auth_pda` with session keypair seeds
12-
- But uses AddAuthority instruction discriminator `[1, 3]`
13-
- Account list doesn't match either CreateSession or AddAuthority expected format
14-
- Error: `InvalidInstructionData` (consumed only 113 compute units)
15-
16-
### Root Cause
17-
Test was likely written during refactoring and instruction formats changed. The test:
18-
1. Sets up PDAs for session creation
19-
2. But instruction data is for AddAuthority (discriminator 1)
20-
3. Account order is wrong for both instructions
21-
22-
### Fix Required
23-
Decide what this test should actually verify:
24-
- **Option A**: Test that spender cannot call AddAuthority → fix instruction data and accounts
25-
- **Option B**: Test that expired session cannot be used → rewrite as proper CreateSession + Execute flow
26-
27-
### Affected Code
28-
```rust
29-
// Line 268-272 - Instruction data is mixed up
30-
data: [
31-
vec![1, 3], // AddAuthority(Session) ← WRONG
32-
(now - 100).to_le_bytes().to_vec(), // Expires in past
33-
]
34-
.concat(),
35-
```
36-
37-
---
38-
39-
## Issue #2: Other Skipped Tests Not Running
40-
41-
Tests after scenario 3 failure are skipped:
42-
- Cross Wallet Attacks
43-
- DoS Attack
3+
## Resolved Issues
4+
5+
### Issue #1: `failures.rs` Scenario 3 - Spender Privilege Escalation Test
6+
**Status**: ✅ Fixed
7+
**Fix**: Corrected account order (payer first) and used proper instruction data format.
8+
9+
### Issue #2: `failures.rs` Scenario 4 - Session Expiry
10+
**Status**: ✅ Fixed
11+
**Fix**: Updated CreateSession format, switched to slot-based expiry, and used `warp_to_slot` to ensure expiry.
12+
13+
### Issue #3: `failures.rs` Scenario 5 - Admin Permission Constraints
14+
**Status**: ✅ Fixed
15+
**Fix**: Corrected AddAuthority instruction data and account order.
16+
17+
### Issue #4: `cross_wallet_attacks.rs` Malformed Data
18+
**Status**: ✅ Fixed
19+
**Fix**: Corrected malformed `vec![1,1]` data to full `add_cross_data`.
20+
21+
### Issue #5: `cross_wallet_attacks.rs` Keypair Mismatch
22+
**Status**: ✅ Fixed
23+
**Fix**: Removed unused owner keypair from transaction signing to match instruction accounts.
24+
25+
## Current Status
26+
All E2E scenarios are PASSING.
27+
- Happy Path
28+
- Failures (5/5)
29+
- Cross Wallet (3/3)
30+
- DoS Attack
4431
- Audit Validations
45-
46-
These should run after fixing Issue #1.

tests-e2e/src/common.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ impl TestContext {
7878
.get_account(&addr)
7979
.ok_or_else(|| anyhow!("Account not found"))
8080
}
81+
82+
pub fn warp_to_slot(&mut self, slot: u64) {
83+
self.svm.warp_to_slot(slot);
84+
}
8185
}
8286

8387
pub trait ToAddress {

tests-e2e/src/scenarios/cross_wallet_attacks.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
6868
Some(&Signer::pubkey(&ctx.payer).to_address()),
6969
);
7070
let mut create_a_tx = Transaction::new_unsigned(message_a);
71-
create_a_tx.sign(&[&ctx.payer, &owner_a], ctx.svm.latest_blockhash());
71+
create_a_tx.sign(&[&ctx.payer], ctx.svm.latest_blockhash());
7272

7373
ctx.execute_tx(create_a_tx)?;
7474

@@ -99,7 +99,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
9999
Some(&Signer::pubkey(&ctx.payer).to_address()),
100100
);
101101
let mut create_b_tx = Transaction::new_unsigned(message_b);
102-
create_b_tx.sign(&[&ctx.payer, &owner_b], ctx.svm.latest_blockhash());
102+
create_b_tx.sign(&[&ctx.payer], ctx.svm.latest_blockhash());
103103

104104
ctx.execute_tx(create_b_tx)?;
105105

@@ -132,7 +132,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
132132
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
133133
AccountMeta::new_readonly(Signer::pubkey(&owner_a).to_address(), true), // Owner A signing
134134
],
135-
data: vec![1, 1],
135+
data: add_cross_data,
136136
};
137137

138138
let message_cross = Message::new(

tests-e2e/src/scenarios/failures.rs

Lines changed: 38 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,6 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
143143
],
144144
&ctx.program_id,
145145
);
146-
let (spender_b_auth_pda, _) = Pubkey::find_program_address(
147-
&[
148-
b"authority",
149-
wallet_pda.as_ref(),
150-
Signer::pubkey(&spender_keypair).as_ref(),
151-
],
152-
&ctx.program_id,
153-
);
154146

155147
// Add Spender (by Owner)
156148
let mut add_spender_data = vec![1]; // AddAuthority
@@ -163,14 +155,14 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
163155
let add_spender_ix = Instruction {
164156
program_id: ctx.program_id.to_address(),
165157
accounts: vec![
166-
AccountMeta::new(wallet_pda.to_address(), false),
167-
AccountMeta::new(owner_auth_pda.to_address(), false), // auth
168-
AccountMeta::new(spender_b_auth_pda.to_address(), false), // target
169-
AccountMeta::new(Signer::pubkey(&owner_keypair).to_address(), true), // signer
170158
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
159+
AccountMeta::new(wallet_pda.to_address(), false),
160+
AccountMeta::new_readonly(owner_auth_pda.to_address(), false), // auth
161+
AccountMeta::new(spender_auth_pda.to_address(), false), // target
171162
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
163+
AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true), // signer
172164
],
173-
data: vec![1, 2], // AddAuthority(Spender)
165+
data: add_spender_data,
174166
};
175167

176168
let message = Message::new(
@@ -204,14 +196,14 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
204196
let malicious_ix = Instruction {
205197
program_id: ctx.program_id.to_address(),
206198
accounts: vec![
207-
AccountMeta::new(wallet_pda.to_address(), false),
208-
AccountMeta::new(spender_b_auth_pda.to_address(), false), // Spender auth
209-
AccountMeta::new(owner_auth_pda.to_address(), false), // Target (Owner)
210-
AccountMeta::new(Signer::pubkey(&spender_keypair).to_address(), true), // Signer
211199
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
200+
AccountMeta::new(wallet_pda.to_address(), false),
201+
AccountMeta::new_readonly(spender_auth_pda.to_address(), false), // Spender auth
202+
AccountMeta::new(bad_admin_pda.to_address(), false), // Target (Bad admin)
212203
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
204+
AccountMeta::new_readonly(Signer::pubkey(&spender_keypair).to_address(), true), // Signer
213205
],
214-
data: vec![1, 2], // Try to add Spender (doesn't matter, auth check fails first)
206+
data: malicious_add,
215207
};
216208

217209
let message = Message::new(
@@ -236,40 +228,28 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
236228
],
237229
&ctx.program_id,
238230
);
239-
let (session_auth_pda, _) = Pubkey::find_program_address(
240-
&[
241-
b"authority",
242-
wallet_pda.as_ref(),
243-
session_keypair.pubkey().as_ref(),
244-
],
245-
&ctx.program_id,
246-
);
247-
// Use previously initialized clock
248-
// let clock: solana_clock::Clock = ctx.svm.get_sysvar(); // Already initialized
249-
// Re-get it to be safe if svm advanced
231+
// Use slot-based expiry
250232
let clock: solana_clock::Clock = ctx.svm.get_sysvar();
251-
let now = clock.unix_timestamp as u64;
233+
let current_slot = clock.slot;
234+
let expires_at = current_slot + 50; // Expires in 50 slots
252235

253-
let mut session_create_data = vec![5]; // CreateSession
254-
session_create_data.extend_from_slice(session_keypair.pubkey().as_ref());
255-
session_create_data.extend_from_slice(&0u64.to_le_bytes()); // Expires at 0 (Genesis)
236+
let mut session_data = Vec::new();
237+
session_data.push(5); // CreateSession
238+
session_data.extend_from_slice(session_keypair.pubkey().as_ref());
239+
session_data.extend_from_slice(&expires_at.to_le_bytes());
256240

257241
let create_session_ix = Instruction {
258242
program_id: ctx.program_id.to_address(),
259243
accounts: vec![
260-
AccountMeta::new(wallet_pda.to_address(), false),
261-
AccountMeta::new(owner_auth_pda.to_address(), false),
262-
AccountMeta::new(session_auth_pda.to_address(), false),
263-
AccountMeta::new(Signer::pubkey(&owner_keypair).to_address(), true),
264244
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
245+
AccountMeta::new_readonly(wallet_pda.to_address(), false),
246+
AccountMeta::new_readonly(owner_auth_pda.to_address(), false),
247+
AccountMeta::new(session_pda.to_address(), false),
265248
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
266249
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
250+
AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true),
267251
],
268-
data: [
269-
vec![1, 3], // AddAuthority(Session)
270-
(now - 100).to_le_bytes().to_vec(), // Expires in past
271-
]
272-
.concat(),
252+
data: session_data,
273253
};
274254

275255
let message = Message::new(
@@ -281,16 +261,22 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
281261

282262
ctx.execute_tx(create_session_tx)?;
283263

264+
// Warp to future slot to expire session
265+
ctx.warp_to_slot(current_slot + 100);
266+
284267
// Try to Execute with Expired Session
268+
let mut exec_payload = vec![4]; // Execute
269+
exec_payload.push(0); // Empty compact instructions
285270
let exec_expired_ix = Instruction {
286271
program_id: ctx.program_id.to_address(),
287272
accounts: vec![
288-
AccountMeta::new(wallet_pda.to_address(), false),
289-
AccountMeta::new(session_auth_pda.to_address(), false),
290-
AccountMeta::new(Signer::pubkey(&session_keypair).to_address(), true),
291-
AccountMeta::new_readonly(solana_system_program::id().to_address(), false), // target to invoke (system)
273+
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true), // Payer
274+
AccountMeta::new(wallet_pda.to_address(), false), // Wallet
275+
AccountMeta::new(session_pda.to_address(), false), // Authority (Session PDA)
276+
AccountMeta::new(vault_pda.to_address(), false), // Vault
277+
AccountMeta::new_readonly(Signer::pubkey(&session_keypair).to_address(), true), // Session Signer
292278
],
293-
data: vec![3, 0], // Execute payload (empty for test)
279+
data: exec_payload,
294280
};
295281

296282
let message = Message::new(
@@ -327,14 +313,14 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
327313
let add_admin_ix = Instruction {
328314
program_id: ctx.program_id.to_address(),
329315
accounts: vec![
330-
AccountMeta::new(wallet_pda.to_address(), false),
331-
AccountMeta::new(owner_auth_pda.to_address(), false), // auth
332-
AccountMeta::new(admin_auth_pda.to_address(), false), // target
333-
AccountMeta::new(Signer::pubkey(&owner_keypair).to_address(), true), // signer
334316
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
317+
AccountMeta::new(wallet_pda.to_address(), false),
318+
AccountMeta::new_readonly(owner_auth_pda.to_address(), false), // auth
319+
AccountMeta::new(admin_auth_pda.to_address(), false), // target
335320
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
321+
AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true), // signer
336322
],
337-
data: vec![1, 1], // AddAuthority(Admin)
323+
data: add_admin_data,
338324
};
339325

340326
let message = Message::new(

0 commit comments

Comments
 (0)