From adaa39d3a24e50bb22ed173d0a6150ad9cec4943 Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Tue, 14 Jul 2026 01:36:42 +0530 Subject: [PATCH 1/4] fix: Fix signer role indexing in post-delegation actions --- dlp-api/src/compact/mod.rs | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/dlp-api/src/compact/mod.rs b/dlp-api/src/compact/mod.rs index 27954f9c..fd888b05 100644 --- a/dlp-api/src/compact/mod.rs +++ b/dlp-api/src/compact/mod.rs @@ -471,4 +471,53 @@ mod tests { } } } + + #[test] + fn test_cleartext_with_insertable_reuses_inserted_signer_as_non_signer() { + let shared = pk(1); + let program_id = pk(2); + + let insertable = PostDelegationActions { + inserted_signers: 0, + inserted_non_signers: 0, + signers: vec![shared.to_bytes()], + non_signers: vec![], + instructions: vec![], + }; + + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new_readonly(shared, false)], + data: vec![1, 2, 3], + }; + + let actions = vec![ix].cleartext_with_insertable(insertable, 0); + + assert_eq!(actions.inserted_signers, 1); + assert_eq!(actions.inserted_non_signers, 0); + assert_eq!(actions.signers, vec![shared.to_bytes()]); + + let new_ix = &actions.instructions[0]; + assert_cleartext_meta(&new_ix.accounts[0], 0, false); + + // This mirrors process_delegate_with_actions() validation and + // currently fails because index 0 is in the signer section. + let index = actions + .validate_index(match &new_ix.accounts[0] { + MaybeEncryptedAccountMeta::ClearText(meta) => meta.key(), + MaybeEncryptedAccountMeta::Encrypted(_) => { + panic!("expected cleartext account meta") + } + }) + .unwrap(); + assert_eq!( + match &new_ix.accounts[0] { + MaybeEncryptedAccountMeta::ClearText(meta) => meta.is_signer(), + MaybeEncryptedAccountMeta::Encrypted(_) => { + panic!("expected cleartext account meta") + } + }, + actions.is_signer(index).unwrap() + ); + } } From 37a8ae7f9339911fb921a1d59f93a7c7c88febae Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Tue, 14 Jul 2026 02:11:28 +0530 Subject: [PATCH 2/4] add the actual fix and updated validation --- dlp-api/src/compact/mod.rs | 28 +++++++++++---------- src/processor/fast/delegate_with_actions.rs | 19 +++++++++----- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/dlp-api/src/compact/mod.rs b/dlp-api/src/compact/mod.rs index fd888b05..4f32107d 100644 --- a/dlp-api/src/compact/mod.rs +++ b/dlp-api/src/compact/mod.rs @@ -456,7 +456,9 @@ mod tests { assert_cleartext_meta(&new_ix.accounts[2], 7, false); assert_cleartext_meta(&new_ix.accounts[3], 8, false); - // a similar code is used in process_delegate_with_actions() to early validate actions + // A similar check is used in process_delegate_with_actions() to + // validate the signer security model: signer metas must resolve to + // signer storage, and signer-storage pubkeys must be signed. for ix in actions.instructions.iter() { actions.validate_index(ix.program_id).unwrap(); @@ -467,7 +469,9 @@ mod tests { continue; }; let index = actions.validate_index(meta.key()).unwrap(); - assert_eq!(meta.is_signer(), actions.is_signer(index).unwrap()); + if meta.is_signer() { + assert!(actions.is_signer(index).unwrap()); + } } } } @@ -500,8 +504,9 @@ mod tests { let new_ix = &actions.instructions[0]; assert_cleartext_meta(&new_ix.accounts[0], 0, false); - // This mirrors process_delegate_with_actions() validation and - // currently fails because index 0 is in the signer section. + // A non-signer meta may reuse signer storage. The signer account is + // available to the action set, but this instruction does not request + // signer privilege for this account. let index = actions .validate_index(match &new_ix.accounts[0] { MaybeEncryptedAccountMeta::ClearText(meta) => meta.key(), @@ -510,14 +515,11 @@ mod tests { } }) .unwrap(); - assert_eq!( - match &new_ix.accounts[0] { - MaybeEncryptedAccountMeta::ClearText(meta) => meta.is_signer(), - MaybeEncryptedAccountMeta::Encrypted(_) => { - panic!("expected cleartext account meta") - } - }, - actions.is_signer(index).unwrap() - ); + let MaybeEncryptedAccountMeta::ClearText(meta) = &new_ix.accounts[0] + else { + panic!("expected cleartext account meta"); + }; + assert!(!meta.is_signer()); + assert!(actions.is_signer(index).unwrap()); } } diff --git a/src/processor/fast/delegate_with_actions.rs b/src/processor/fast/delegate_with_actions.rs index d78bfd0d..9448895d 100644 --- a/src/processor/fast/delegate_with_actions.rs +++ b/src/processor/fast/delegate_with_actions.rs @@ -1,4 +1,4 @@ -use dlp_api::{compat::borsh, require_eq}; +use dlp_api::{compat::borsh, require}; use pinocchio::{ address::address_eq, cpi::{Seed, Signer}, @@ -99,6 +99,12 @@ pub fn process_delegate_with_actions( // Validate clear-text indices early when possible. Encrypted // AccountMetas are skipped here because they can only be validated // after decryption by the ER validator. + // + // Signer security model: + // 1. For every signer meta, require its compact index to resolve to + // signer storage. + // 2. For every signer-storage pubkey, require the real AccountInfo to + // be present and signed. for ix in args.actions.instructions.iter() { args.actions.validate_index(ix.program_id)?; @@ -109,11 +115,12 @@ pub fn process_delegate_with_actions( continue; }; let index = args.actions.validate_index(meta.key())?; - require_eq!( - meta.is_signer(), - args.actions.is_signer(index)?, - ProgramError::InvalidInstructionData - ); + if meta.is_signer() { + require!( + args.actions.is_signer(index)?, + ProgramError::InvalidInstructionData + ); + } } } From 47fcb60cb293c243307dcc7d8e8217deb3e4854b Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Tue, 14 Jul 2026 03:30:48 +0530 Subject: [PATCH 3/4] fix algo: shared-key in non-signer, then in signer scenario --- dlp-api/src/compact/mod.rs | 93 ++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/dlp-api/src/compact/mod.rs b/dlp-api/src/compact/mod.rs index 4f32107d..a06b3a2e 100644 --- a/dlp-api/src/compact/mod.rs +++ b/dlp-api/src/compact/mod.rs @@ -172,6 +172,16 @@ impl ClearTextWithInsertable for Vec { "PostDelegationActions does not support multiple merge/insert" ); + let old_signers_len = insertable.signers.len(); + let old_non_signers_len = insertable.non_signers.len(); + let old_total = old_signers_len + old_non_signers_len; + + let skipable_signer_pubkeys: Vec> = insertable + .signers + .iter() + .map(|signer| Some((*signer).into())) + .collect(); + // add keys from actions (pre-encrypted instructions) let skipable_pubkeys: Vec> = { let mut skipable: Vec> = vec![]; @@ -198,7 +208,8 @@ impl ClearTextWithInsertable for Vec { let add_to = |metas: &mut Vec, - meta: &solana_instruction::AccountMeta| { + meta: &solana_instruction::AccountMeta, + skipable_pubkeys: &[Option
]| { if skipable_pubkeys.contains(&Some(meta.pubkey)) { return; } @@ -217,7 +228,7 @@ impl ClearTextWithInsertable for Vec { .flat_map(|ix| ix.accounts.iter()) .filter(|meta| meta.is_signer) { - add_to(&mut signers, signer_meta); + add_to(&mut signers, signer_meta, &skipable_signer_pubkeys); } for ix in self.iter() { @@ -227,6 +238,7 @@ impl ClearTextWithInsertable for Vec { ix.program_id, false, ), + &skipable_pubkeys, ); for non_signer_meta in ix.accounts.iter().filter(|meta| !meta.is_signer) @@ -239,12 +251,16 @@ impl ClearTextWithInsertable for Vec { // update its is_writable only. signer.is_writable |= non_signer_meta.is_writable; } else { - add_to(&mut non_signers, non_signer_meta); + add_to( + &mut non_signers, + non_signer_meta, + &skipable_pubkeys, + ); } } } - if signers.len() + non_signers.len() + if old_total + signers.len() + non_signers.len() > crate::compact::MAX_PUBKEYS as usize { panic!( @@ -253,10 +269,6 @@ impl ClearTextWithInsertable for Vec { ); } - let old_signers_len = insertable.signers.len(); - let old_non_signers_len = insertable.non_signers.len(); - let old_total = old_signers_len + old_non_signers_len; - let index_of = |pk: &solana_address::Address| -> u8 { // // The final list will be this as per PostDelegationActions: @@ -292,6 +304,18 @@ impl ClearTextWithInsertable for Vec { as u8 }; + let index_of_signer = |pk: &solana_address::Address| -> u8 { + if let Some(index) = skipable_signer_pubkeys + .iter() + .position(|pubkey| pubkey == &Some(*pk)) + { + return index as u8; + } + + (old_total + signers.iter().position(|s| &s.pubkey == pk).unwrap()) + as u8 + }; + let mut compact_instructions: Vec = self .into_iter() .map(|ix| MaybeEncryptedInstruction { @@ -301,7 +325,11 @@ impl ClearTextWithInsertable for Vec { .accounts .into_iter() .map(|meta| { - let index = index_of(&meta.pubkey); + let index = if meta.is_signer { + index_of_signer(&meta.pubkey) + } else { + index_of(&meta.pubkey) + }; crate::compact::AccountMeta::try_new( index, meta.is_signer, @@ -522,4 +550,51 @@ mod tests { assert!(!meta.is_signer()); assert!(actions.is_signer(index).unwrap()); } + + #[test] + fn test_cleartext_with_insertable_adds_signer_for_inserted_non_signer() { + let shared = pk(1); + let program_id = pk(2); + + let insertable = PostDelegationActions { + inserted_signers: 0, + inserted_non_signers: 0, + signers: vec![], + non_signers: vec![MaybeEncryptedPubkey::ClearText( + shared.to_bytes(), + )], + instructions: vec![], + }; + + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new_readonly(shared, true)], + data: vec![1, 2, 3], + }; + + let actions = vec![ix].cleartext_with_insertable(insertable, 0); + + assert_eq!(actions.inserted_signers, 0); + assert_eq!(actions.inserted_non_signers, 1); + assert_eq!(actions.signers, vec![shared.to_bytes()]); + assert_eq!( + actions.non_signers, + vec![ + MaybeEncryptedPubkey::ClearText(shared.to_bytes()), + MaybeEncryptedPubkey::ClearText(program_id.to_bytes()), + ] + ); + + let new_ix = &actions.instructions[0]; + assert_eq!(new_ix.program_id, 2); + assert_cleartext_meta(&new_ix.accounts[0], 1, true); + + let MaybeEncryptedAccountMeta::ClearText(meta) = &new_ix.accounts[0] + else { + panic!("expected cleartext account meta"); + }; + let index = actions.validate_index(meta.key()).unwrap(); + assert!(meta.is_signer()); + assert!(actions.is_signer(index).unwrap()); + } } From 874b3e7c8e23fb8d0138c675f7e22fcdff692c77 Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Wed, 15 Jul 2026 22:42:40 +0530 Subject: [PATCH 4/4] make rabbit happy --- dlp-api/src/compact/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dlp-api/src/compact/mod.rs b/dlp-api/src/compact/mod.rs index a06b3a2e..0c530998 100644 --- a/dlp-api/src/compact/mod.rs +++ b/dlp-api/src/compact/mod.rs @@ -312,8 +312,15 @@ impl ClearTextWithInsertable for Vec { return index as u8; } - (old_total + signers.iter().position(|s| &s.pubkey == pk).unwrap()) - as u8 + // This lookup is only used for metas from `self` that have + // `is_signer = true`. The earlier signer collection loop inserts + // every such pubkey unless existing inserted signer storage can + // satisfy it, which returns above. + let Some(index) = signers.iter().position(|s| &s.pubkey == pk) + else { + unreachable!("signer meta must resolve to inserted or new signer storage"); + }; + (old_total + index) as u8 }; let mut compact_instructions: Vec = self