From 7a53423f8e23e655924b107e2540a64a5b5d3d86 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 14:23:21 -0700 Subject: [PATCH 1/9] serviceability: add AccessPassKind tag for AccessPassType --- .../src/state/accesspass.rs | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs b/smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs index e3ca3491da..7d8c504d81 100644 --- a/smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs +++ b/smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs @@ -86,6 +86,47 @@ impl AccessPassType { } } +/// The `AccessPassType` variant with no payload attached. +/// +/// `AccessPassType` carries a `Pubkey`, a `String` pair or a `Vec` depending on the +/// variant. The close and delete instructions only need to know which variant a pass is, so +/// they use this type: it is `Copy`, it compares in one instruction, and it never reaches the +/// wire. There is deliberately no `Borsh` derive — the kind is chosen by the instruction +/// variant, not sent as an argument (see malbeclabs/infra#2470). +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum AccessPassKind { + Prepaid, + SolanaValidator, + SolanaRPC, + Others, + EdgeSeat, +} + +impl From<&AccessPassType> for AccessPassKind { + fn from(value: &AccessPassType) -> Self { + match value { + AccessPassType::Prepaid => AccessPassKind::Prepaid, + AccessPassType::SolanaValidator(_) => AccessPassKind::SolanaValidator, + AccessPassType::SolanaRPC(_) => AccessPassKind::SolanaRPC, + AccessPassType::Others(_, _) => AccessPassKind::Others, + AccessPassType::EdgeSeat(_) => AccessPassKind::EdgeSeat, + } + } +} + +impl fmt::Display for AccessPassKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match self { + AccessPassKind::Prepaid => "prepaid", + AccessPassKind::SolanaValidator => "solana_validator", + AccessPassKind::SolanaRPC => "solana_rpc", + AccessPassKind::Others => "others", + AccessPassKind::EdgeSeat => "edge_seat", + }; + f.write_str(name) + } +} + impl fmt::Display for AccessPassType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -398,6 +439,52 @@ mod tests { use super::*; + #[test] + fn test_access_pass_kind_from_type() { + let cases = [ + (AccessPassType::Prepaid, AccessPassKind::Prepaid), + ( + AccessPassType::SolanaValidator(Pubkey::new_unique()), + AccessPassKind::SolanaValidator, + ), + ( + AccessPassType::SolanaRPC(Pubkey::new_unique()), + AccessPassKind::SolanaRPC, + ), + ( + AccessPassType::Others("thing".to_string(), "key".to_string()), + AccessPassKind::Others, + ), + (AccessPassType::EdgeSeat(vec![]), AccessPassKind::EdgeSeat), + ]; + + for (pass_type, want) in cases { + assert_eq!(AccessPassKind::from(&pass_type), want, "{pass_type}"); + } + } + + #[test] + fn test_access_pass_kind_display_matches_discriminant_string() { + // The kind and the pass type must print the same name, so an error message + // and a CLI flag value cannot drift apart. + let cases = [ + AccessPassType::Prepaid, + AccessPassType::SolanaValidator(Pubkey::new_unique()), + AccessPassType::SolanaRPC(Pubkey::new_unique()), + AccessPassType::EdgeSeat(vec![]), + ]; + + for pass_type in cases { + assert_eq!( + AccessPassKind::from(&pass_type).to_string(), + pass_type.to_discriminant_string(), + ); + } + // `Others` carries its own type_name in `to_discriminant_string`, so the kind + // prints the fixed name instead. + assert_eq!(AccessPassKind::Others.to_string(), "others"); + } + #[test] fn test_state_compatibility_accesspass() { /* To generate the base64 strings, use the following commands after deploying the program and creating accounts: From 61b1913c32538ae5404ca6d431c7cb677f143133 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 14:27:28 -0700 Subject: [PATCH 2/9] serviceability: add AccessPassTypeMismatch error --- .../doublezero-serviceability/src/error.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/smartcontract/programs/doublezero-serviceability/src/error.rs b/smartcontract/programs/doublezero-serviceability/src/error.rs index b5b5ae01a9..d70be7409d 100644 --- a/smartcontract/programs/doublezero-serviceability/src/error.rs +++ b/smartcontract/programs/doublezero-serviceability/src/error.rs @@ -247,6 +247,8 @@ pub enum DoubleZeroError { IpProofMessageMismatch, // variant 117 #[error("IP ownership proof carries an unsupported layout version")] IpProofVersionUnsupported, // variant 118 + #[error("This instruction is for a different access pass type")] + AccessPassTypeMismatch, // variant 119 } impl From for ProgramError { @@ -371,6 +373,7 @@ impl From for ProgramError { DoubleZeroError::IpProofSignatureMismatch => ProgramError::Custom(116), DoubleZeroError::IpProofMessageMismatch => ProgramError::Custom(117), DoubleZeroError::IpProofVersionUnsupported => ProgramError::Custom(118), + DoubleZeroError::AccessPassTypeMismatch => ProgramError::Custom(119), } } } @@ -496,11 +499,18 @@ impl From for DoubleZeroError { 116 => DoubleZeroError::IpProofSignatureMismatch, 117 => DoubleZeroError::IpProofMessageMismatch, 118 => DoubleZeroError::IpProofVersionUnsupported, + 119 => DoubleZeroError::AccessPassTypeMismatch, _ => DoubleZeroError::Custom(e), } } } +impl From for DoubleZeroError { + fn from(e: u8) -> Self { + Self::from(e as u32) + } +} + impl From for DoubleZeroError { fn from(e: ProgramError) -> Self { match e { @@ -519,6 +529,13 @@ mod tests { use super::*; use strum::IntoEnumIterator; + #[test] + fn test_access_pass_type_mismatch_roundtrip() { + let err = DoubleZeroError::AccessPassTypeMismatch; + assert_eq!(ProgramError::from(err.clone()), ProgramError::Custom(119)); + assert_eq!(DoubleZeroError::from(119u8), err); + } + #[test] fn test_error_enum_conversions() { // Using EnumIter ensures all variants are tested - if a new variant is added From 69240b2f0afb8f9991ba78a3a980879e08c6eefc Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 14:32:33 -0700 Subject: [PATCH 3/9] serviceability: use the existing u32 error conversion in the mismatch test --- .../programs/doublezero-serviceability/src/error.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/smartcontract/programs/doublezero-serviceability/src/error.rs b/smartcontract/programs/doublezero-serviceability/src/error.rs index d70be7409d..c203c4b974 100644 --- a/smartcontract/programs/doublezero-serviceability/src/error.rs +++ b/smartcontract/programs/doublezero-serviceability/src/error.rs @@ -505,12 +505,6 @@ impl From for DoubleZeroError { } } -impl From for DoubleZeroError { - fn from(e: u8) -> Self { - Self::from(e as u32) - } -} - impl From for DoubleZeroError { fn from(e: ProgramError) -> Self { match e { @@ -533,7 +527,9 @@ mod tests { fn test_access_pass_type_mismatch_roundtrip() { let err = DoubleZeroError::AccessPassTypeMismatch; assert_eq!(ProgramError::from(err.clone()), ProgramError::Custom(119)); - assert_eq!(DoubleZeroError::from(119u8), err); + let pe: ProgramError = ProgramError::Custom(119); + let err2: DoubleZeroError = pe.into(); + assert_eq!(err2, err); } #[test] From b9b1fa09923ab2ccc92fee7605fe682477ccb202 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 14:53:22 -0700 Subject: [PATCH 4/9] serviceability: split CloseAccessPass into one instruction per pass type --- .../src/accesspass.rs | 8 +- .../src/entrypoint.rs | 23 +- .../src/instructions.rs | 57 ++++- .../src/processors/accesspass/close.rs | 64 +++--- .../tests/accesspass_test.rs | 4 +- .../tests/close_access_pass_kind_test.rs | 217 ++++++++++++++++++ .../deprecated_removal_instructions_test.rs | 60 +++++ .../tests/test_helpers.rs | 17 ++ 8 files changed, 408 insertions(+), 42 deletions(-) create mode 100644 smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs create mode 100644 smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs diff --git a/crates/doublezero-serviceability-instruction/src/accesspass.rs b/crates/doublezero-serviceability-instruction/src/accesspass.rs index 96dae6bf7d..84560939e8 100644 --- a/crates/doublezero-serviceability-instruction/src/accesspass.rs +++ b/crates/doublezero-serviceability-instruction/src/accesspass.rs @@ -52,17 +52,19 @@ pub fn set_access_pass( ) } -/// `CloseAccessPass` (variant 69). Accounts: `[accesspass, globalstate]`. +/// Deprecated: `CloseAccessPass` (variant 69) now returns `DoubleZeroError::Deprecated`. Use +/// one of the `CloseAccessPass` builders instead. See malbeclabs/infra#2470. +/// Accounts: `[accesspass, globalstate]`. pub fn close_access_pass( program_id: &Pubkey, payer: &Pubkey, accesspass: &Pubkey, - args: CloseAccessPassArgs, + _args: CloseAccessPassArgs, ) -> Instruction { let (globalstate, _) = get_globalstate_pda(program_id); common::build_with_permission( program_id, - DoubleZeroInstruction::CloseAccessPass(args), + DoubleZeroInstruction::CloseAccessPass(), vec![ AccountMeta::new(*accesspass, false), AccountMeta::new(globalstate, false), diff --git a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs index c5dbfc6c5b..640f3bd02c 100644 --- a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs +++ b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs @@ -105,6 +105,7 @@ use crate::{ update::process_update_user, }, }, + state::accesspass::AccessPassKind, }; use solana_program::{ @@ -168,7 +169,8 @@ pub fn process_instruction( | DoubleZeroInstruction::CloseAccountDevice() | DoubleZeroInstruction::DeactivateMulticastGroup() | DoubleZeroInstruction::RemoveDeviceInterface() - | DoubleZeroInstruction::UnlinkDeviceInterface() => { + | DoubleZeroInstruction::UnlinkDeviceInterface() + | DoubleZeroInstruction::CloseAccessPass() => { return Err(DoubleZeroError::Deprecated.into()); } DoubleZeroInstruction::ActivateUser() @@ -305,8 +307,23 @@ pub fn process_instruction( DoubleZeroInstruction::SetAccessPass(value) => { process_set_access_pass(program_id, accounts, &value)? } - DoubleZeroInstruction::CloseAccessPass(value) => { - process_close_access_pass(program_id, accounts, &value)? + DoubleZeroInstruction::ClosePrepaidAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::Prepaid)? + } + DoubleZeroInstruction::CloseSolanaValidatorAccessPass(value) => process_close_access_pass( + program_id, + accounts, + &value, + AccessPassKind::SolanaValidator, + )?, + DoubleZeroInstruction::CloseSolanaRPCAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::SolanaRPC)? + } + DoubleZeroInstruction::CloseOthersAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::Others)? + } + DoubleZeroInstruction::CloseEdgeSeatAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::EdgeSeat)? } DoubleZeroInstruction::CheckStatusAccessPass(value) => { process_check_status_access_pass(program_id, accounts, &value)? diff --git a/smartcontract/programs/doublezero-serviceability/src/instructions.rs b/smartcontract/programs/doublezero-serviceability/src/instructions.rs index a68995bbaa..ed7b0c4300 100644 --- a/smartcontract/programs/doublezero-serviceability/src/instructions.rs +++ b/smartcontract/programs/doublezero-serviceability/src/instructions.rs @@ -189,7 +189,9 @@ pub enum DoubleZeroInstruction { AcceptLink(LinkAcceptArgs), // variant 66 SetAccessPass(SetAccessPassArgs), // variant 67 SetAirdrop(SetAirdropArgs), // variant 68 - CloseAccessPass(CloseAccessPassArgs), // variant 69 + /// Deprecated: handler returns DoubleZeroError::Deprecated. Use + /// `CloseAccessPass` (variants 119-123). See malbeclabs/infra#2470. + CloseAccessPass(), // variant 69 CheckStatusAccessPass(CheckStatusAccessPassArgs), // variant 70 CheckUserAccessPass(CheckUserAccessPassArgs), // variant 71 @@ -258,6 +260,14 @@ pub enum DoubleZeroInstruction { SubscribeFeed(SubscribeFeedArgs), // variant 117 UnsubscribeFeed(UnsubscribeFeedArgs), // variant 118 + + /// One close instruction per `AccessPassType`. Each refuses a pass of any other + /// kind with `AccessPassTypeMismatch`. See malbeclabs/infra#2470. + ClosePrepaidAccessPass(CloseAccessPassArgs), // variant 119 + CloseSolanaValidatorAccessPass(CloseAccessPassArgs), // variant 120 + CloseSolanaRPCAccessPass(CloseAccessPassArgs), // variant 121 + CloseOthersAccessPass(CloseAccessPassArgs), // variant 122 + CloseEdgeSeatAccessPass(CloseAccessPassArgs), // variant 123 } impl DoubleZeroInstruction { @@ -352,7 +362,7 @@ impl DoubleZeroInstruction { 67 => Ok(Self::SetAccessPass(SetAccessPassArgs::try_from(rest).unwrap())), 68 => Ok(Self::SetAirdrop(SetAirdropArgs::try_from(rest).unwrap())), - 69 => Ok(Self::CloseAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 69 => Ok(Self::CloseAccessPass()), 70 => Ok(Self::CheckStatusAccessPass(CheckStatusAccessPassArgs::try_from(rest).unwrap())), 71 => Ok(Self::CheckUserAccessPass(CheckUserAccessPassArgs::try_from(rest).unwrap())), @@ -409,6 +419,12 @@ impl DoubleZeroInstruction { 117 => Ok(Self::SubscribeFeed(SubscribeFeedArgs::try_from(rest).unwrap())), 118 => Ok(Self::UnsubscribeFeed(UnsubscribeFeedArgs::try_from(rest).unwrap())), + 119 => Ok(Self::ClosePrepaidAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 120 => Ok(Self::CloseSolanaValidatorAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 121 => Ok(Self::CloseSolanaRPCAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 122 => Ok(Self::CloseOthersAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 123 => Ok(Self::CloseEdgeSeatAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + _ => Err(ProgramError::InvalidInstructionData), } } @@ -500,7 +516,7 @@ impl DoubleZeroInstruction { Self::AcceptLink(_) => "AcceptLink".to_string(), // variant 66 Self::SetAccessPass(_) => "SetAccessPass".to_string(), // variant 67 Self::SetAirdrop(_) => "SetAirdrop".to_string(), // variant 68 - Self::CloseAccessPass(_) => "CloseAccessPass".to_string(), // variant 69 + Self::CloseAccessPass() => "CloseAccessPass".to_string(), // variant 69 Self::CheckStatusAccessPass(_) => "CheckStatusAccessPass".to_string(), // variant 70 Self::CheckUserAccessPass(_) => "CheckUserAccessPass".to_string(), // variant 71 @@ -560,6 +576,12 @@ impl DoubleZeroInstruction { Self::SetAccessPassFlags(_) => "SetAccessPassFlags".to_string(), // variant 116 Self::SubscribeFeed(_) => "SubscribeFeed".to_string(), // variant 117 Self::UnsubscribeFeed(_) => "UnsubscribeFeed".to_string(), // variant 118 + + Self::ClosePrepaidAccessPass(_) => "ClosePrepaidAccessPass".to_string(), // variant 119 + Self::CloseSolanaValidatorAccessPass(_) => "CloseSolanaValidatorAccessPass".to_string(), // variant 120 + Self::CloseSolanaRPCAccessPass(_) => "CloseSolanaRPCAccessPass".to_string(), // variant 121 + Self::CloseOthersAccessPass(_) => "CloseOthersAccessPass".to_string(), // variant 122 + Self::CloseEdgeSeatAccessPass(_) => "CloseEdgeSeatAccessPass".to_string(), // variant 123 } } @@ -644,7 +666,7 @@ impl DoubleZeroInstruction { Self::AcceptLink(args) => format!("{args:?}"), // variant 66 Self::SetAccessPass(args) => format!("{args:?}"), // variant 67 Self::SetAirdrop(args) => format!("{args:?}"), // variant 68 - Self::CloseAccessPass(args) => format!("{args:?}"), // variant 69 + Self::CloseAccessPass() => "".to_string(), // variant 69 Self::CheckStatusAccessPass(args) => format!("{args:?}"), // variant 70 Self::CheckUserAccessPass(args) => format!("{args:?}"), // variant 71 @@ -704,6 +726,12 @@ impl DoubleZeroInstruction { Self::SetAccessPassFlags(args) => format!("{args:?}"), // variant 116 Self::SubscribeFeed(args) => format!("{args:?}"), // variant 117 Self::UnsubscribeFeed(args) => format!("{args:?}"), // variant 118 + + Self::ClosePrepaidAccessPass(args) => format!("{args:?}"), // variant 119 + Self::CloseSolanaValidatorAccessPass(args) => format!("{args:?}"), // variant 120 + Self::CloseSolanaRPCAccessPass(args) => format!("{args:?}"), // variant 121 + Self::CloseOthersAccessPass(args) => format!("{args:?}"), // variant 122 + Self::CloseEdgeSeatAccessPass(args) => format!("{args:?}"), // variant 123 } } } @@ -1179,9 +1207,26 @@ mod tests { }), "SetAirdrop", ); + test_instruction(DoubleZeroInstruction::CloseAccessPass(), "CloseAccessPass"); + test_instruction( + DoubleZeroInstruction::ClosePrepaidAccessPass(CloseAccessPassArgs {}), + "ClosePrepaidAccessPass", + ); + test_instruction( + DoubleZeroInstruction::CloseSolanaValidatorAccessPass(CloseAccessPassArgs {}), + "CloseSolanaValidatorAccessPass", + ); + test_instruction( + DoubleZeroInstruction::CloseSolanaRPCAccessPass(CloseAccessPassArgs {}), + "CloseSolanaRPCAccessPass", + ); + test_instruction( + DoubleZeroInstruction::CloseOthersAccessPass(CloseAccessPassArgs {}), + "CloseOthersAccessPass", + ); test_instruction( - DoubleZeroInstruction::CloseAccessPass(CloseAccessPassArgs {}), - "CloseAccessPass", + DoubleZeroInstruction::CloseEdgeSeatAccessPass(CloseAccessPassArgs {}), + "CloseEdgeSeatAccessPass", ); test_instruction( DoubleZeroInstruction::CheckStatusAccessPass(CheckStatusAccessPassArgs {}), diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs index e8e7d7a922..7d7110890c 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs @@ -3,7 +3,9 @@ use crate::{ error::DoubleZeroError, serializer::try_acc_close, state::{ - accesspass::AccessPass, accounttype::AccountType, globalstate::GlobalState, + accesspass::{AccessPass, AccessPassKind}, + accounttype::AccountType, + globalstate::GlobalState, permission::permission_flags, }, }; @@ -30,6 +32,7 @@ pub fn process_close_access_pass( program_id: &Pubkey, accounts: &[AccountInfo], _value: &CloseAccessPassArgs, + expected: AccessPassKind, ) -> ProgramResult { let accounts_iter = &mut accounts.iter(); @@ -84,34 +87,39 @@ pub fn process_close_access_pass( permission_flags::ACCESS_PASS_ADMIN, )?; - if let Ok(data) = accesspass_account.try_borrow_data() { - let account_type: AccountType = data[0].into(); - if account_type != AccountType::AccessPass { - msg!("AccountType is not AccessPass, cannot close"); - return Err(DoubleZeroError::InvalidAccountType.into()); - } - let accesspass = AccessPass::try_from(accesspass_account)?; - - // Feed authority can only close access passes they own - if globalstate.feed_authority_pk == *payer_account.key - && accesspass.owner != *payer_account.key - { - msg!("Feed authority can only close access passes they own"); - return Err(DoubleZeroError::NotAllowed.into()); - } - - if accesspass.connection_count != 0 { - msg!( - "AccessPass has {} active connections, cannot close", - accesspass.connection_count - ); - return Err(DoubleZeroError::AccessPassInUse.into()); - } - - msg!("AccountType is AccessPass and there are no active connections, proceeding to close"); - } else { - msg!("Failed to borrow account data, cannot close"); + // These checks used to sit inside `if let Ok(data) = accesspass_account.try_borrow_data()`, + // with an `else` that logged a warning and fell through to the close. A failed borrow + // therefore closed the pass with neither the account-type check nor the connection check + // applied. Read the account once here and let a failed read stop the instruction. + let account_type: AccountType = accesspass_account.try_borrow_data()?[0].into(); + if account_type != AccountType::AccessPass { + msg!("AccountType is not AccessPass, cannot close"); + return Err(DoubleZeroError::InvalidAccountType.into()); } + let accesspass = AccessPass::try_from(accesspass_account)?; + + let actual = AccessPassKind::from(&accesspass.accesspass_type); + if actual != expected { + msg!("this instruction closes a {expected} pass, but the pass is {actual}"); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + } + + // Feed authority can only close access passes they own + if globalstate.feed_authority_pk == *payer_account.key && accesspass.owner != *payer_account.key + { + msg!("Feed authority can only close access passes they own"); + return Err(DoubleZeroError::NotAllowed.into()); + } + + if accesspass.connection_count != 0 { + msg!( + "AccessPass has {} active connections, cannot close", + accesspass.connection_count + ); + return Err(DoubleZeroError::AccessPassInUse.into()); + } + + msg!("AccountType is AccessPass and there are no active connections, proceeding to close"); try_acc_close(accesspass_account, payer_account)?; diff --git a/smartcontract/programs/doublezero-serviceability/tests/accesspass_test.rs b/smartcontract/programs/doublezero-serviceability/tests/accesspass_test.rs index 6f67969151..d53ff8d337 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/accesspass_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/accesspass_test.rs @@ -142,7 +142,7 @@ async fn test_accesspass() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::CloseAccessPass(CloseAccessPassArgs {}), + DoubleZeroInstruction::CloseSolanaValidatorAccessPass(CloseAccessPassArgs {}), vec![ AccountMeta::new(accesspass_pubkey, false), AccountMeta::new(globalstate_pubkey, false), @@ -728,7 +728,7 @@ async fn test_close_accesspass_rejects_nonzero_connection_count() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::CloseAccessPass(CloseAccessPassArgs {}), + DoubleZeroInstruction::ClosePrepaidAccessPass(CloseAccessPassArgs {}), vec![ AccountMeta::new(accesspass_pubkey, false), AccountMeta::new(globalstate_pubkey, false), diff --git a/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs b/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs new file mode 100644 index 0000000000..c90e3dad19 --- /dev/null +++ b/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs @@ -0,0 +1,217 @@ +//! Issue #2470: each `CloseAccessPass` instruction must close a pass of its own kind and +//! refuse a pass of any other kind with `AccessPassTypeMismatch`. + +mod test_helpers; + +use doublezero_serviceability::{ + error::DoubleZeroError, + instructions::DoubleZeroInstruction, + pda::{get_accesspass_pda, get_globalstate_pda, get_program_config_pda}, + processors::accesspass::close::CloseAccessPassArgs, + state::{ + accesspass::{AccessPass, AccessPassStatus, AccessPassType}, + accounttype::AccountType, + }, +}; +use solana_program::rent::Rent; +use solana_program_test::*; +use solana_sdk::{ + account::Account as SolanaAccount, instruction::AccountMeta, pubkey::Pubkey, + signature::Keypair, signer::Signer, +}; +use std::net::Ipv4Addr; +use test_helpers::*; + +/// The close instruction that matches each pass type, and one that does not. +fn close_instructions( + pass_type: &AccessPassType, +) -> (DoubleZeroInstruction, DoubleZeroInstruction) { + let args = CloseAccessPassArgs {}; + match pass_type { + AccessPassType::Prepaid => ( + DoubleZeroInstruction::ClosePrepaidAccessPass(args.clone()), + DoubleZeroInstruction::CloseEdgeSeatAccessPass(args), + ), + AccessPassType::SolanaValidator(_) => ( + DoubleZeroInstruction::CloseSolanaValidatorAccessPass(args.clone()), + DoubleZeroInstruction::ClosePrepaidAccessPass(args), + ), + AccessPassType::SolanaRPC(_) => ( + DoubleZeroInstruction::CloseSolanaRPCAccessPass(args.clone()), + DoubleZeroInstruction::ClosePrepaidAccessPass(args), + ), + AccessPassType::Others(_, _) => ( + DoubleZeroInstruction::CloseOthersAccessPass(args.clone()), + DoubleZeroInstruction::ClosePrepaidAccessPass(args), + ), + AccessPassType::EdgeSeat(_) => ( + DoubleZeroInstruction::CloseEdgeSeatAccessPass(args.clone()), + DoubleZeroInstruction::ClosePrepaidAccessPass(args), + ), + } +} + +/// Starts a fresh `ProgramTest`, runs `InitGlobalState`, and seeds an `AccessPass` account of +/// `pass_type` owned by the payer, with no active connections. The account-building block is +/// lifted from `accesspass_test.rs::test_close_accesspass_rejects_nonzero_connection_count`. +/// +/// Uses `test_payer()` rather than the `Keypair` `ProgramTest::start()` generates: that one +/// isn't known until after `start()`, too late to use as the `AccessPass`'s `owner` field, +/// which must be written into the account added before `start()`. +async fn seed_access_pass( + pass_type: &AccessPassType, +) -> ( + BanksClient, + Keypair, + solana_program::hash::Hash, + Pubkey, + Pubkey, + Pubkey, +) { + let program_id = Pubkey::new_unique(); + let payer = test_payer(); + + let (program_config_pubkey, _) = get_program_config_pda(&program_id); + let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); + + let client_ip = Ipv4Addr::new(101, 0, 0, 1); + let user_payer = Pubkey::new_unique(); + let (accesspass_pubkey, bump_seed) = get_accesspass_pda(&program_id, &client_ip, &user_payer); + + let seeded_accesspass = AccessPass { + account_type: AccountType::AccessPass, + owner: payer.pubkey(), + bump_seed, + accesspass_type: pass_type.clone(), + client_ip, + user_payer, + last_access_epoch: 0, + connection_count: 0, + status: AccessPassStatus::Requested, + mgroup_pub_allowlist: vec![], + mgroup_sub_allowlist: vec![], + flags: 0, + tenant_allowlist: vec![], + unicast_user_count: 0, + max_unicast_users: 1, + multicast_user_count: 0, + max_multicast_users: 1, + }; + + let accesspass_data = borsh::to_vec(&seeded_accesspass).unwrap(); + let rent = Rent::default(); + let lamports = rent.minimum_balance(accesspass_data.len()); + + let mut program_test = ProgramTest::new( + "doublezero_serviceability", + program_id, + processor!(doublezero_serviceability::entrypoint::process_instruction), + ); + program_test.add_account( + accesspass_pubkey, + SolanaAccount { + lamports, + data: accesspass_data, + owner: program_id, + executable: false, + rent_epoch: 0, + }, + ); + // Fund the payer directly so it can sign InitGlobalState and the close instructions below. + program_test.add_account( + payer.pubkey(), + SolanaAccount { + lamports: 10_000_000_000, + data: vec![], + owner: solana_system_interface::program::ID, + executable: false, + rent_epoch: 0, + }, + ); + + let (mut banks_client, _funder, recent_blockhash) = program_test.start().await; + + // Makes `payer` the sole entry in the foundation allowlist, so it holds ACCESS_PASS_ADMIN. + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::InitGlobalState(), + vec![ + AccountMeta::new(program_config_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + ( + banks_client, + payer, + recent_blockhash, + program_id, + accesspass_pubkey, + globalstate_pubkey, + ) +} + +#[tokio::test] +async fn close_refuses_a_pass_of_another_kind() { + for pass_type in [ + AccessPassType::Prepaid, + AccessPassType::SolanaValidator(Pubkey::new_unique()), + AccessPassType::SolanaRPC(Pubkey::new_unique()), + AccessPassType::Others("thing".to_string(), "key".to_string()), + AccessPassType::EdgeSeat(vec![]), + ] { + let ( + mut banks_client, + payer, + recent_blockhash, + program_id, + accesspass_pubkey, + globalstate_pubkey, + ) = seed_access_pass(&pass_type).await; + let accounts = vec![ + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ]; + let (matching, other) = close_instructions(&pass_type); + + let err = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + other, + accounts.clone(), + &payer, + ) + .await + .expect_err("a close for another kind must fail"); + assert_custom_error(&err, DoubleZeroError::AccessPassTypeMismatch); + + assert!( + get_account_data(&mut banks_client, accesspass_pubkey) + .await + .is_some(), + "the pass must survive a refused close: {pass_type}" + ); + + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + matching, + accounts, + &payer, + ) + .await; + + assert!( + get_account_data(&mut banks_client, accesspass_pubkey) + .await + .is_none(), + "the matching close must remove the pass: {pass_type}" + ); + } +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs b/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs new file mode 100644 index 0000000000..f69ab60904 --- /dev/null +++ b/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs @@ -0,0 +1,60 @@ +//! Issue #2470: the general-purpose removal instructions are replaced by one per pass type. +//! Wire discriminants 69 and 42 are kept so an old client hits a deterministic deprecation +//! error instead of an unknown-instruction decode failure. + +use doublezero_serviceability::{ + entrypoint::process_instruction, error::DoubleZeroError, instructions::DoubleZeroInstruction, +}; +use solana_program::program_error::ProgramError; +use solana_program_test::*; +use solana_sdk::{ + instruction::{AccountMeta, Instruction, InstructionError}, + pubkey::Pubkey, + signer::Signer, + transaction::{Transaction, TransactionError}, +}; + +async fn assert_returns_deprecated(instruction: DoubleZeroInstruction) { + let program_id = Pubkey::new_unique(); + let (banks_client, payer, recent_blockhash) = ProgramTest::new( + "doublezero_serviceability", + program_id, + processor!(process_instruction), + ) + .start() + .await; + + let ix = Instruction { + program_id, + accounts: vec![AccountMeta::new(payer.pubkey(), true)], + data: instruction.pack(), + }; + let mut tx = Transaction::new_with_payer(&[ix], Some(&payer.pubkey())); + tx.try_sign(&[&payer], recent_blockhash).unwrap(); + + let err = banks_client + .process_transaction(tx) + .await + .expect_err("expected deprecated instruction to fail"); + + let expected: ProgramError = DoubleZeroError::Deprecated.into(); + let ProgramError::Custom(expected_code) = expected else { + panic!("Deprecated must map to ProgramError::Custom"); + }; + + match err { + BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(code), + )) => assert_eq!( + code, expected_code, + "expected Deprecated (Custom({expected_code})), got Custom({code})" + ), + other => panic!("expected Custom({expected_code}) InstructionError, got {other:?}"), + } +} + +#[tokio::test] +async fn close_access_pass_returns_deprecated() { + assert_returns_deprecated(DoubleZeroInstruction::CloseAccessPass()).await; +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs b/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs index 1fbef24df7..4e69eb9b5a 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs @@ -1,6 +1,7 @@ use borsh::to_vec; use doublezero_serviceability::{ entrypoint::process_instruction, + error::DoubleZeroError, instructions::*, pda::{ get_contributor_pda, get_exchange_pda, get_globalconfig_pda, get_globalstate_pda, @@ -18,6 +19,7 @@ use doublezero_serviceability::{ topology::TopologyConstraint, }, }; +use solana_program::program_error::ProgramError; use solana_program_test::*; use solana_sdk::{ instruction::{AccountMeta, Instruction}, @@ -987,3 +989,18 @@ pub fn custom_error_code(err: &BanksClientError) -> Option { _ => None, } } + +/// Asserts a failed transaction was rejected with the `ProgramError::Custom` code that +/// `expected` maps to. Takes the expected error as a parameter rather than hardcoding one, so +/// any test in this crate can use it to check any `DoubleZeroError`. +#[allow(dead_code)] +pub fn assert_custom_error(err: &BanksClientError, expected: DoubleZeroError) { + let ProgramError::Custom(want) = ProgramError::from(expected.clone()) else { + panic!("{expected:?} must map to ProgramError::Custom"); + }; + assert_eq!( + custom_error_code(err), + Some(want), + "expected Custom({want}), got {err:?}" + ); +} From 6c95762c36d437d96d9c2af7c06b96fd1e97f375 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 15:21:14 -0700 Subject: [PATCH 5/9] serviceability: split DeleteUser into one instruction per pass type --- .../src/user.rs | 25 +- .../src/entrypoint.rs | 22 +- .../src/instructions.rs | 80 +++- .../processors/multicastgroup/subscribe.rs | 4 +- .../src/processors/user/delete.rs | 8 +- .../tests/accesspass_allow_multiple_ip.rs | 2 +- .../tests/create_subscribe_user_test.rs | 2 +- .../tests/delete_user_dynamic_accesspass.rs | 8 +- .../tests/delete_user_kind_test.rs | 394 ++++++++++++++++++ .../deprecated_removal_instructions_test.rs | 5 + .../tests/user_old_test.rs | 2 +- .../tests/user_onchain_allocation_test.rs | 8 +- .../tests/user_tests.rs | 6 +- 13 files changed, 517 insertions(+), 49 deletions(-) create mode 100644 smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs diff --git a/crates/doublezero-serviceability-instruction/src/user.rs b/crates/doublezero-serviceability-instruction/src/user.rs index 7fcfe1ac8e..30646a5c41 100644 --- a/crates/doublezero-serviceability-instruction/src/user.rs +++ b/crates/doublezero-serviceability-instruction/src/user.rs @@ -309,7 +309,8 @@ pub fn update_user( ) } -/// `DeleteUser` (variant 42). +/// Deprecated: `DeleteUser` (variant 42) now returns `DoubleZeroError::Deprecated`. Use one of +/// the `DeleteUser` builders instead. See malbeclabs/infra#2470. /// /// Account layout, before the trailing accounts: /// @@ -339,7 +340,7 @@ pub fn delete_user( dz_prefix_count: u8, tenant: Option, owner: &Pubkey, - mut args: UserDeleteArgs, + _args: UserDeleteArgs, ) -> Instruction { // The processor rejects `dz_prefix_count == 0` as its first statement // (delete.rs) — DeleteUser requires on-chain deallocation — so a zero here can @@ -348,13 +349,6 @@ pub fn delete_user( dz_prefix_count > 0, "dz_prefix_count must be > 0; DeleteUser requires on-chain deallocation" ); - args.dz_prefix_count = dz_prefix_count; - // This builder always emits the `multicast_publisher_block` account, so the - // declared count MUST be > 0 or the processor (which reads that account only - // when `multicast_publisher_count > 0`) would skip it and misread every - // following account. Written back here for the same reason as - // `dz_prefix_count`: keep the declared count and the account list in lockstep. - args.multicast_publisher_count = 1; let (globalstate, _) = get_globalstate_pda(program_id); let (user_tunnel_block, _, _) = @@ -387,7 +381,7 @@ pub fn delete_user( common::build_with_permission( program_id, - DoubleZeroInstruction::DeleteUser(args), + DoubleZeroInstruction::DeleteUser(), accounts, payer, ) @@ -1096,14 +1090,9 @@ mod tests { UserDeleteArgs::default(), ); assert_eq!(ix.data[0], 42); - // The builder always emits the mpb account, so it MUST pin - // multicast_publisher_count > 0 to keep the declared count and the - // account list in lockstep (else the processor skips the mpb slot and - // misreads every following account). - match DoubleZeroInstruction::unpack(&ix.data).unwrap() { - DoubleZeroInstruction::DeleteUser(a) => assert_eq!(a.multicast_publisher_count, 1), - other => panic!("unexpected variant: {other:?}"), - } + // Deprecated: DeleteUser is payload-free (variant 42 always errors with + // Deprecated), so the wire data carries only the discriminant byte. + assert_eq!(ix.data.len(), 1); let (globalstate, _) = get_globalstate_pda(&pid); let (utb, _, _) = get_resource_extension_pda(&pid, ResourceType::UserTunnelBlock); let (mpb, _, _) = get_resource_extension_pda(&pid, ResourceType::MulticastPublisherBlock); diff --git a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs index 640f3bd02c..23aa961dba 100644 --- a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs +++ b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs @@ -176,11 +176,27 @@ pub fn process_instruction( DoubleZeroInstruction::ActivateUser() | DoubleZeroInstruction::RejectUser() | DoubleZeroInstruction::CloseAccountUser() - | DoubleZeroInstruction::BanUser() => { + | DoubleZeroInstruction::BanUser() + | DoubleZeroInstruction::DeleteUser() => { return Err(DoubleZeroError::Deprecated.into()); } - DoubleZeroInstruction::DeleteUser(value) => { - process_delete_user(program_id, accounts, &value)? + DoubleZeroInstruction::DeletePrepaidUser(value) => { + process_delete_user(program_id, accounts, &value, AccessPassKind::Prepaid)? + } + DoubleZeroInstruction::DeleteSolanaValidatorUser(value) => process_delete_user( + program_id, + accounts, + &value, + AccessPassKind::SolanaValidator, + )?, + DoubleZeroInstruction::DeleteSolanaRPCUser(value) => { + process_delete_user(program_id, accounts, &value, AccessPassKind::SolanaRPC)? + } + DoubleZeroInstruction::DeleteOthersUser(value) => { + process_delete_user(program_id, accounts, &value, AccessPassKind::Others)? + } + DoubleZeroInstruction::DeleteEdgeSeatUser(value) => { + process_delete_user(program_id, accounts, &value, AccessPassKind::EdgeSeat)? } DoubleZeroInstruction::DeleteDevice(value) => { process_delete_device(program_id, accounts, &value)? diff --git a/smartcontract/programs/doublezero-serviceability/src/instructions.rs b/smartcontract/programs/doublezero-serviceability/src/instructions.rs index ed7b0c4300..b59ea978cc 100644 --- a/smartcontract/programs/doublezero-serviceability/src/instructions.rs +++ b/smartcontract/programs/doublezero-serviceability/src/instructions.rs @@ -152,7 +152,9 @@ pub enum DoubleZeroInstruction { UpdateUser(UserUpdateArgs), // variant 39 SuspendUser(), // variant 40 ResumeUser(), // variant 41 - DeleteUser(UserDeleteArgs), // variant 42 + /// Deprecated: handler returns DoubleZeroError::Deprecated. Use `DeleteUser` + /// (variants 124-128). See malbeclabs/infra#2470. + DeleteUser(), // variant 42 /// Deprecated: handler returns DoubleZeroError::Deprecated. See #3622. CloseAccountUser(), // variant 43 RequestBanUser(UserRequestBanArgs), // variant 44 @@ -268,6 +270,15 @@ pub enum DoubleZeroInstruction { CloseSolanaRPCAccessPass(CloseAccessPassArgs), // variant 121 CloseOthersAccessPass(CloseAccessPassArgs), // variant 122 CloseEdgeSeatAccessPass(CloseAccessPassArgs), // variant 123 + + /// One delete instruction per `AccessPassType`, keyed on the kind of pass the user + /// holds. Each refuses a user on a pass of any other kind with + /// `AccessPassTypeMismatch`. See malbeclabs/infra#2470. + DeletePrepaidUser(UserDeleteArgs), // variant 124 + DeleteSolanaValidatorUser(UserDeleteArgs), // variant 125 + DeleteSolanaRPCUser(UserDeleteArgs), // variant 126 + DeleteOthersUser(UserDeleteArgs), // variant 127 + DeleteEdgeSeatUser(UserDeleteArgs), // variant 128 } impl DoubleZeroInstruction { @@ -329,7 +340,7 @@ impl DoubleZeroInstruction { 39 => Ok(Self::UpdateUser(UserUpdateArgs::try_from(rest).unwrap())), 40 => Ok(Self::SuspendUser()), 41 => Ok(Self::ResumeUser()), - 42 => Ok(Self::DeleteUser(UserDeleteArgs::try_from(rest).unwrap())), + 42 => Ok(Self::DeleteUser()), 43 => Ok(Self::CloseAccountUser()), 44 => Ok(Self::RequestBanUser(UserRequestBanArgs::try_from(rest).unwrap())), 45 => Ok(Self::BanUser()), @@ -425,6 +436,12 @@ impl DoubleZeroInstruction { 122 => Ok(Self::CloseOthersAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), 123 => Ok(Self::CloseEdgeSeatAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), + 124 => Ok(Self::DeletePrepaidUser(UserDeleteArgs::try_from(rest).unwrap())), + 125 => Ok(Self::DeleteSolanaValidatorUser(UserDeleteArgs::try_from(rest).unwrap())), + 126 => Ok(Self::DeleteSolanaRPCUser(UserDeleteArgs::try_from(rest).unwrap())), + 127 => Ok(Self::DeleteOthersUser(UserDeleteArgs::try_from(rest).unwrap())), + 128 => Ok(Self::DeleteEdgeSeatUser(UserDeleteArgs::try_from(rest).unwrap())), + _ => Err(ProgramError::InvalidInstructionData), } } @@ -479,7 +496,7 @@ impl DoubleZeroInstruction { Self::UpdateUser(_) => "UpdateUser".to_string(), // variant 39 Self::SuspendUser() => "SuspendUser".to_string(), // variant 40 Self::ResumeUser() => "ResumeUser".to_string(), // variant 41 - Self::DeleteUser(_) => "DeleteUser".to_string(), // variant 42 + Self::DeleteUser() => "DeleteUser".to_string(), // variant 42 Self::CloseAccountUser() => "CloseAccountUser".to_string(), // variant 43 Self::RequestBanUser(_) => "RequestBanUser".to_string(), // variant 44 @@ -582,6 +599,12 @@ impl DoubleZeroInstruction { Self::CloseSolanaRPCAccessPass(_) => "CloseSolanaRPCAccessPass".to_string(), // variant 121 Self::CloseOthersAccessPass(_) => "CloseOthersAccessPass".to_string(), // variant 122 Self::CloseEdgeSeatAccessPass(_) => "CloseEdgeSeatAccessPass".to_string(), // variant 123 + + Self::DeletePrepaidUser(_) => "DeletePrepaidUser".to_string(), // variant 124 + Self::DeleteSolanaValidatorUser(_) => "DeleteSolanaValidatorUser".to_string(), // variant 125 + Self::DeleteSolanaRPCUser(_) => "DeleteSolanaRPCUser".to_string(), // variant 126 + Self::DeleteOthersUser(_) => "DeleteOthersUser".to_string(), // variant 127 + Self::DeleteEdgeSeatUser(_) => "DeleteEdgeSeatUser".to_string(), // variant 128 } } @@ -635,7 +658,7 @@ impl DoubleZeroInstruction { Self::UpdateUser(args) => format!("{args:?}"), // variant 39 Self::SuspendUser() => "".to_string(), // variant 40 Self::ResumeUser() => "".to_string(), // variant 41 - Self::DeleteUser(args) => format!("{args:?}"), // variant 42 + Self::DeleteUser() => "".to_string(), // variant 42 Self::CloseAccountUser() => "".to_string(), // variant 43 Self::RequestBanUser(args) => format!("{args:?}"), // variant 44 @@ -732,6 +755,12 @@ impl DoubleZeroInstruction { Self::CloseSolanaRPCAccessPass(args) => format!("{args:?}"), // variant 121 Self::CloseOthersAccessPass(args) => format!("{args:?}"), // variant 122 Self::CloseEdgeSeatAccessPass(args) => format!("{args:?}"), // variant 123 + + Self::DeletePrepaidUser(args) => format!("{args:?}"), // variant 124 + Self::DeleteSolanaValidatorUser(args) => format!("{args:?}"), // variant 125 + Self::DeleteSolanaRPCUser(args) => format!("{args:?}"), // variant 126 + Self::DeleteOthersUser(args) => format!("{args:?}"), // variant 127 + Self::DeleteEdgeSeatUser(args) => format!("{args:?}"), // variant 128 } } } @@ -963,13 +992,7 @@ mod tests { ); test_instruction(DoubleZeroInstruction::SuspendUser(), "SuspendUser"); test_instruction(DoubleZeroInstruction::ResumeUser(), "ResumeUser"); - test_instruction( - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { - dz_prefix_count: 0, - multicast_publisher_count: 0, - }), - "DeleteUser", - ); + test_instruction(DoubleZeroInstruction::DeleteUser(), "DeleteUser"); test_instruction( DoubleZeroInstruction::CloseAccountDevice(), "CloseAccountDevice", @@ -1228,6 +1251,41 @@ mod tests { DoubleZeroInstruction::CloseEdgeSeatAccessPass(CloseAccessPassArgs {}), "CloseEdgeSeatAccessPass", ); + test_instruction( + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeletePrepaidUser", + ); + test_instruction( + DoubleZeroInstruction::DeleteSolanaValidatorUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeleteSolanaValidatorUser", + ); + test_instruction( + DoubleZeroInstruction::DeleteSolanaRPCUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeleteSolanaRPCUser", + ); + test_instruction( + DoubleZeroInstruction::DeleteOthersUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeleteOthersUser", + ); + test_instruction( + DoubleZeroInstruction::DeleteEdgeSeatUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeleteEdgeSeatUser", + ); test_instruction( DoubleZeroInstruction::CheckStatusAccessPass(CheckStatusAccessPassArgs {}), "CheckStatusAccessPass", diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs index a1bd01bafe..935af73fd1 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs @@ -281,8 +281,8 @@ pub fn process_update_multicastgroup_roles( // another owner's pass with the right permission, and the two operations require different // grants: // - Removal-only cleanup (stripping roles as a prerequisite to delete/request-ban) is a - // USER_ADMIN operation, as DeleteUserCommand / RequestBanUserCommand authorize the - // final instruction with the same flag. + // USER_ADMIN operation, as the DeleteUser instructions / RequestBanUserCommand + // authorize the final instruction with the same flag. // - Granting roles (subscribe/publish) on behalf of another owner manages the pass's // entitlements, so it is an ACCESS_PASS_ADMIN operation. This is the path the oracle // uses to subscribe validator-owned users (accesspass.user_payer = validator) once it diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index 46312f0090..241898fbca 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -5,7 +5,7 @@ use crate::{ processors::validation::validate_program_account, serializer::{try_acc_close, try_acc_write}, state::{ - accesspass::{AccessPass, AccessPassStatus}, + accesspass::{AccessPass, AccessPassKind, AccessPassStatus}, device::Device, globalstate::GlobalState, permission::permission_flags, @@ -52,6 +52,7 @@ pub fn process_delete_user( program_id: &Pubkey, accounts: &[AccountInfo], value: &UserDeleteArgs, + expected: AccessPassKind, ) -> ProgramResult { if value.dz_prefix_count == 0 { msg!("dz_prefix_count must be > 0; DeleteUser requires on-chain deallocation"); @@ -154,6 +155,11 @@ pub fn process_delete_user( if !accesspass_account.data_is_empty() { // Read Access Pass let mut accesspass = AccessPass::try_from(accesspass_account)?; + let actual = AccessPassKind::from(&accesspass.accesspass_type); + if actual != expected { + msg!("this instruction deletes a user on a {expected} pass, but the pass is {actual}"); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + } if accesspass.user_payer != user.owner { msg!( "Invalid user_payer accesspass.user_payer: {} = user_payer: {} ", diff --git a/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs b/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs index 0a7a683cf3..707711683d 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs @@ -403,7 +403,7 @@ async fn test_accesspass_allow_multiple_ip() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), diff --git a/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs b/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs index c60f5c626c..61bc125ade 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs @@ -2544,7 +2544,7 @@ async fn test_publisher_disconnect_delete_decrements_publishers_count() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), diff --git a/smartcontract/programs/doublezero-serviceability/tests/delete_user_dynamic_accesspass.rs b/smartcontract/programs/doublezero-serviceability/tests/delete_user_dynamic_accesspass.rs index 6a294c2659..bfb2a9eff9 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/delete_user_dynamic_accesspass.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/delete_user_dynamic_accesspass.rs @@ -357,7 +357,7 @@ async fn test_delete_user_is_dynamic_pass() { &mut env.banks_client, recent_blockhash, env.program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -444,7 +444,7 @@ async fn test_delete_user_allow_multiple_ip_resets_client_ip() { &mut env.banks_client, recent_blockhash, env.program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -515,7 +515,7 @@ async fn test_delete_user_specific_ip_pass() { &mut env.banks_client, recent_blockhash, env.program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -600,7 +600,7 @@ async fn test_delete_multicast_user_dynamic_pass() { &mut env.banks_client, recent_blockhash, env.program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), diff --git a/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs b/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs new file mode 100644 index 0000000000..08ae75f31b --- /dev/null +++ b/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs @@ -0,0 +1,394 @@ +//! Issue #2470: each `DeleteUser` instruction must delete a user whose access pass is of +//! its own kind, and refuse a user on a pass of any other kind with `AccessPassTypeMismatch`. + +mod test_helpers; + +use doublezero_serviceability::{ + error::DoubleZeroError, + instructions::DoubleZeroInstruction, + pda::{get_accesspass_pda, get_device_pda, get_resource_extension_pda, get_user_pda}, + processors::{ + accesspass::set::SetAccessPassArgs, + device::{create::DeviceCreateArgs, update::DeviceUpdateArgs}, + user::{create::UserCreateArgs, delete::UserDeleteArgs}, + }, + resource::ResourceType, + state::{ + accesspass::{AccessPass, AccessPassType, FeedSeat}, + device::{DeviceDesiredStatus, DeviceType}, + user::{User, UserCYOA, UserType}, + }, +}; +use solana_program_test::*; +use solana_sdk::{ + account::AccountSharedData, instruction::AccountMeta, pubkey::Pubkey, signature::Keypair, + signer::Signer, +}; +use std::net::Ipv4Addr; +use test_helpers::*; + +fn delete_args() -> UserDeleteArgs { + UserDeleteArgs { + dz_prefix_count: 1, + multicast_publisher_count: 1, + } +} + +/// The delete instruction that matches each pass type, and one that does not. +fn delete_instructions( + pass_type: &AccessPassType, +) -> (DoubleZeroInstruction, DoubleZeroInstruction) { + match pass_type { + AccessPassType::Prepaid => ( + DoubleZeroInstruction::DeletePrepaidUser(delete_args()), + DoubleZeroInstruction::DeleteEdgeSeatUser(delete_args()), + ), + AccessPassType::SolanaValidator(_) => ( + DoubleZeroInstruction::DeleteSolanaValidatorUser(delete_args()), + DoubleZeroInstruction::DeletePrepaidUser(delete_args()), + ), + AccessPassType::SolanaRPC(_) => ( + DoubleZeroInstruction::DeleteSolanaRPCUser(delete_args()), + DoubleZeroInstruction::DeletePrepaidUser(delete_args()), + ), + AccessPassType::Others(_, _) => ( + DoubleZeroInstruction::DeleteOthersUser(delete_args()), + DoubleZeroInstruction::DeletePrepaidUser(delete_args()), + ), + AccessPassType::EdgeSeat(_) => ( + DoubleZeroInstruction::DeleteEdgeSeatUser(delete_args()), + DoubleZeroInstruction::DeletePrepaidUser(delete_args()), + ), + } +} + +struct TestEnv { + context: ProgramTestContext, + payer: Keypair, + program_id: Pubkey, + globalstate_pubkey: Pubkey, + device_pubkey: Pubkey, +} + +/// GlobalState/Config, Location, Exchange, Contributor and an Activated Device, ready to host +/// users under any access-pass kind. +async fn setup_test_env() -> TestEnv { + let (mut context, program_id, globalstate_pubkey, globalconfig_pubkey) = + setup_program_with_globalconfig_context().await; + let payer = context.payer.insecure_clone(); + let recent_blockhash = context.last_blockhash; + + let (location_pubkey, exchange_pubkey, contributor_pubkey) = setup_device_prerequisites( + &mut context.banks_client, + recent_blockhash, + program_id, + globalstate_pubkey, + globalconfig_pubkey, + &payer, + ) + .await; + + let gs = get_globalstate(&mut context.banks_client, globalstate_pubkey).await; + let (device_pubkey, _) = get_device_pda(&program_id, gs.account_index + 1); + let (tunnel_ids_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::TunnelIds(device_pubkey, 0)); + let (dz_prefix_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::DzPrefixBlock(device_pubkey, 0)); + + execute_transaction( + &mut context.banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateDevice(DeviceCreateArgs { + code: "dev".to_string(), + device_type: DeviceType::Hybrid, + public_ip: [100, 0, 0, 1].into(), + dz_prefixes: "100.1.0.0/23".parse().unwrap(), + metrics_publisher_pk: Pubkey::default(), + mgmt_vrf: "mgmt".to_string(), + desired_status: Some(DeviceDesiredStatus::Activated), + resource_count: 2, + }), + vec![ + AccountMeta::new(device_pubkey, false), + AccountMeta::new(contributor_pubkey, false), + AccountMeta::new(location_pubkey, false), + AccountMeta::new(exchange_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(globalconfig_pubkey, false), + AccountMeta::new(tunnel_ids_pda, false), + AccountMeta::new(dz_prefix_pda, false), + ], + &payer, + ) + .await; + + execute_transaction( + &mut context.banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateDevice(DeviceUpdateArgs { + max_users: Some(128), + ..DeviceUpdateArgs::default() + }), + vec![ + AccountMeta::new(device_pubkey, false), + AccountMeta::new(contributor_pubkey, false), + AccountMeta::new(location_pubkey, false), + AccountMeta::new(location_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + TestEnv { + context, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + } +} + +/// Set an access pass of `pass_type` at `client_ip` and create a user of `user_type` under it. +/// Returns (accesspass_pubkey, user_pubkey). +async fn create_and_activate_user( + env: &mut TestEnv, + client_ip: Ipv4Addr, + user_type: UserType, + pass_type: AccessPassType, +) -> (Pubkey, Pubkey) { + let recent_blockhash = env.context.last_blockhash; + let payer_pk = env.payer.pubkey(); + + let (accesspass_pubkey, _) = get_accesspass_pda(&env.program_id, &client_ip, &payer_pk); + + execute_transaction( + &mut env.context.banks_client, + recent_blockhash, + env.program_id, + DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { + accesspass_type: pass_type, + client_ip, + last_access_epoch: 9999, + allow_multiple_ip: false, + max_unicast_users: 1, + max_multicast_users: 1, + }), + vec![ + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(env.globalstate_pubkey, false), + AccountMeta::new(payer_pk, false), + ], + &env.payer, + ) + .await; + + let (user_pubkey, _) = get_user_pda(&env.program_id, &client_ip, user_type); + let (user_tunnel_block_pda, _, _) = + get_resource_extension_pda(&env.program_id, ResourceType::UserTunnelBlock); + let (multicast_publisher_block_pda, _, _) = + get_resource_extension_pda(&env.program_id, ResourceType::MulticastPublisherBlock); + let (device_tunnel_ids_pda, _, _) = get_resource_extension_pda( + &env.program_id, + ResourceType::TunnelIds(env.device_pubkey, 0), + ); + let (dz_prefix_block_pda, _, _) = get_resource_extension_pda( + &env.program_id, + ResourceType::DzPrefixBlock(env.device_pubkey, 0), + ); + + execute_transaction( + &mut env.context.banks_client, + recent_blockhash, + env.program_id, + DoubleZeroInstruction::CreateUser(UserCreateArgs { + client_ip, + user_type, + cyoa_type: UserCYOA::GREOverDIA, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + ip_proof: None, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(env.device_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(env.globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block_pda, false), + AccountMeta::new(multicast_publisher_block_pda, false), + AccountMeta::new(device_tunnel_ids_pda, false), + AccountMeta::new(dz_prefix_block_pda, false), + ], + &env.payer, + ) + .await; + + (accesspass_pubkey, user_pubkey) +} + +/// Rewrite the EdgeSeat pass to carry one feed seat with a user on it, and give the user that +/// same feed in `feed_pks` — bypassing `SubscribeFeed` so the test only needs a device and a +/// bare multicast user. This is the only way to put a real seat in front of +/// `process_delete_user`'s `release_feed_seats` call; a feedless EdgeSeat pass makes that call a +/// no-op and never exercises the release path a `DeleteEdgeSeatUser` must perform. +async fn seed_feed_seat( + env: &mut TestEnv, + accesspass_pubkey: Pubkey, + user_pubkey: Pubkey, +) -> Pubkey { + let feed_key = Pubkey::new_unique(); + + let mut accesspass_account = env + .context + .banks_client + .get_account(accesspass_pubkey) + .await + .unwrap() + .expect("access pass must exist"); + let mut accesspass = AccessPass::try_from(&accesspass_account.data[..]).unwrap(); + accesspass.accesspass_type = AccessPassType::EdgeSeat(vec![FeedSeat { + feed_key, + max_users: 1, + max_future_users: 1, + current_users: 1, + anniversary_day: 1, + window_end: 4_000_000_000, + terminates_at: 4_100_000_000, + }]); + accesspass_account.data = borsh::to_vec(&accesspass).unwrap(); + env.context.set_account( + &accesspass_pubkey, + &AccountSharedData::from(accesspass_account), + ); + + let mut user_account = env + .context + .banks_client + .get_account(user_pubkey) + .await + .unwrap() + .expect("user must exist"); + let mut user = User::try_from(&user_account.data[..]).unwrap(); + user.feed_pks = vec![feed_key]; + user_account.data = borsh::to_vec(&user).unwrap(); + env.context + .set_account(&user_pubkey, &AccountSharedData::from(user_account)); + + feed_key +} + +#[tokio::test] +async fn delete_refuses_a_user_of_another_kind() { + for (i, pass_type) in [ + AccessPassType::Prepaid, + AccessPassType::SolanaValidator(Pubkey::new_unique()), + AccessPassType::SolanaRPC(Pubkey::new_unique()), + AccessPassType::Others("thing".to_string(), "key".to_string()), + AccessPassType::EdgeSeat(vec![]), + ] + .into_iter() + .enumerate() + { + let mut env = setup_test_env().await; + let client_ip: Ipv4Addr = [100, 0, 0, 10 + i as u8].into(); + let user_type = if matches!(pass_type, AccessPassType::EdgeSeat(_)) { + UserType::Multicast + } else { + UserType::IBRL + }; + + let (accesspass_pubkey, user_pubkey) = + create_and_activate_user(&mut env, client_ip, user_type, pass_type.clone()).await; + + if matches!(pass_type, AccessPassType::EdgeSeat(_)) { + seed_feed_seat(&mut env, accesspass_pubkey, user_pubkey).await; + } + + let (matching, other) = delete_instructions(&pass_type); + + let (user_tunnel_block_pda, _, _) = + get_resource_extension_pda(&env.program_id, ResourceType::UserTunnelBlock); + let (multicast_publisher_block_pda, _, _) = + get_resource_extension_pda(&env.program_id, ResourceType::MulticastPublisherBlock); + let (device_tunnel_ids_pda, _, _) = get_resource_extension_pda( + &env.program_id, + ResourceType::TunnelIds(env.device_pubkey, 0), + ); + let (dz_prefix_pda, _, _) = get_resource_extension_pda( + &env.program_id, + ResourceType::DzPrefixBlock(env.device_pubkey, 0), + ); + let owner = env.payer.pubkey(); + + let accounts = vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(env.globalstate_pubkey, false), + AccountMeta::new(env.device_pubkey, false), + AccountMeta::new(user_tunnel_block_pda, false), + AccountMeta::new(multicast_publisher_block_pda, false), + AccountMeta::new(device_tunnel_ids_pda, false), + AccountMeta::new(dz_prefix_pda, false), + AccountMeta::new(owner, false), + ]; + + let err = try_execute_transaction( + &mut env.context.banks_client, + env.context.last_blockhash, + env.program_id, + other, + accounts.clone(), + &env.payer, + ) + .await + .expect_err("a delete for another kind must fail"); + assert_custom_error(&err, DoubleZeroError::AccessPassTypeMismatch); + + assert!( + get_account_data(&mut env.context.banks_client, user_pubkey) + .await + .is_some(), + "the user must survive a refused delete: {pass_type}" + ); + + execute_transaction( + &mut env.context.banks_client, + env.context.last_blockhash, + env.program_id, + matching, + accounts, + &env.payer, + ) + .await; + + assert!( + get_account_data(&mut env.context.banks_client, user_pubkey) + .await + .is_none(), + "the matching delete must remove the user: {pass_type}" + ); + + // The EdgeSeat case seeded a real feed seat (see `seed_feed_seat`); confirm the matching + // delete actually released it rather than `release_feed_seats` silently no-op'ing. + if matches!(pass_type, AccessPassType::EdgeSeat(_)) { + let accesspass_account = env + .context + .banks_client + .get_account(accesspass_pubkey) + .await + .unwrap() + .expect("access pass survives a user delete"); + let accesspass = AccessPass::try_from(&accesspass_account.data[..]).unwrap(); + assert_eq!( + accesspass.feed_seats(), + [FeedSeat { + current_users: 0, + ..accesspass.feed_seats()[0].clone() + }], + "the matching EdgeSeat delete must release the seat" + ); + } + } +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs b/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs index f69ab60904..93b8be86f0 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs @@ -58,3 +58,8 @@ async fn assert_returns_deprecated(instruction: DoubleZeroInstruction) { async fn close_access_pass_returns_deprecated() { assert_returns_deprecated(DoubleZeroInstruction::CloseAccessPass()).await; } + +#[tokio::test] +async fn delete_user_returns_deprecated() { + assert_returns_deprecated(DoubleZeroInstruction::DeleteUser()).await; +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs b/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs index cc09b14722..100177dcd0 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs @@ -402,7 +402,7 @@ async fn test_old_user() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs b/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs index f874a26e8f..ed287978d2 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs @@ -760,7 +760,7 @@ async fn test_delete_user_atomic_with_deallocation() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -1467,7 +1467,7 @@ async fn test_delete_user_atomic_decrements_multicast_subscribers_count() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -1672,7 +1672,7 @@ async fn test_multicast_publisher_block_deallocation_and_reuse() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -1968,7 +1968,7 @@ async fn test_delete_user_atomic_decrements_subscribers_count_for_non_publisher( &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs b/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs index 4e9b85f28a..2324a17098 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs @@ -685,7 +685,7 @@ async fn test_user() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -1362,7 +1362,7 @@ async fn test_user_delete_from_banned() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), @@ -1461,7 +1461,7 @@ async fn test_user_check_access_pass_expired_epoch_stays_activated_and_delete() &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + DoubleZeroInstruction::DeletePrepaidUser(UserDeleteArgs { dz_prefix_count: 1, multicast_publisher_count: 0, }), From 1af6fdf0660fa43f3c5fb3f19bab7dd1b7913eb1 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 18:00:27 -0700 Subject: [PATCH 6/9] docs: design for per-pass-type access pass removal --- ...9-01-per-pass-type-removal-guard-design.md | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md diff --git a/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md b/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md new file mode 100644 index 0000000000..2683700a90 --- /dev/null +++ b/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md @@ -0,0 +1,235 @@ +# Design: one close and delete instruction per access pass type + +Date: 2026-09-01 +Issue: [malbeclabs/infra#2470](https://github.com/malbeclabs/infra/issues/2470) +Parent: [malbeclabs/infra#2385](https://github.com/malbeclabs/infra/issues/2385) + +## Problem + +Serviceability has one `CloseAccessPass` instruction (variant 69) and one `DeleteUser` +instruction (variant 42). Neither handler looks at `AccessPassType`. Both destroy state. A +caller that means to remove a prepaid pass can remove an EdgeSeat pass by mistake, and the +program accepts it. + +Issue #2470 asks for one instruction per pass type, and states that the pass type must not be +an instruction argument. An earlier version of this document proposed a declared type in the +args instead. Martin rejected that in the issue. This document follows the issue. + +## Shape + +`AccessPassType` has 5 variants, so there are 10 new instructions: + +| Variant | Name | Replaces | +| --- | --- | --- | +| 119 | `ClosePrepaidAccessPass` | `CloseAccessPass` | +| 120 | `CloseSolanaValidatorAccessPass` | `CloseAccessPass` | +| 121 | `CloseSolanaRPCAccessPass` | `CloseAccessPass` | +| 122 | `CloseOthersAccessPass` | `CloseAccessPass` | +| 123 | `CloseEdgeSeatAccessPass` | `CloseAccessPass` | +| 124 | `DeletePrepaidUser` | `DeleteUser` | +| 125 | `DeleteSolanaValidatorUser` | `DeleteUser` | +| 126 | `DeleteSolanaRPCUser` | `DeleteUser` | +| 127 | `DeleteOthersUser` | `DeleteUser` | +| 128 | `DeleteEdgeSeatUser` | `DeleteUser` | + +The highest variant in use today is 118. + +## Components + +### 1. `AccessPassKind` + +A tag enum in `smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs`, +next to `AccessPassType`. + +```rust +#[repr(u8)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum AccessPassKind { + Prepaid, + SolanaValidator, + SolanaRPC, + Others, + EdgeSeat, +} + +impl From<&AccessPassType> for AccessPassKind { /* one arm per variant */ } +impl fmt::Display for AccessPassKind { /* for the error message */ } +``` + +This type never reaches the wire. It is not part of any instruction argument, and it needs no +Borsh derive. It exists so the shared handler body can take the kind it must accept, and so the +CLI and the SDKs have one name for the choice they make. + +`AccessPassType` carries payloads (a `Pubkey`, a `String` pair, a `Vec`). +`AccessPassKind` carries none, so it is `Copy` and cheap to compare. + +The kind check compares the variant only. `AccessPassType::Others(type_name, key)` maps to +`Others` whatever `type_name` holds, so `CloseOthersAccessPass` closes any `Others` pass. +Pinning `type_name` would tie the instruction set to catalog data, so it is left out. + +### 2. The handler bodies stay single + +Ten instructions, two bodies. Close and delete take the same accounts for every kind, so the +work does not differ by kind. Only the accepted kind differs. + +`processors/accesspass/close.rs`: + +```rust +pub fn process_close_access_pass( + program_id: &Pubkey, + accounts: &[AccountInfo], + value: &CloseAccessPassArgs, + expected: AccessPassKind, +) -> ProgramResult +``` + +`processors/user/delete.rs` gains the same trailing `expected: AccessPassKind` parameter. + +Each body checks the kind right after it reads the `AccessPass`: + +```rust +let actual = AccessPassKind::from(&accesspass.accesspass_type); +if actual != expected { + msg!("instruction is for {expected} but the pass is {actual}"); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); +} +``` + +`DoubleZeroError::AccessPassTypeMismatch` is a new variant, number 119. The highest number in +use today is 118. + +The kind is a Rust parameter chosen by the dispatch arm, not an instruction argument. Ten +copies of a 250 line body would be ten places to fix the next bug in it. + +### 3. Dispatch + +`entrypoint.rs` gets 10 arms. Each names its kind: + +```rust +DoubleZeroInstruction::ClosePrepaidAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::Prepaid)? +} +DoubleZeroInstruction::CloseSolanaValidatorAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, AccessPassKind::SolanaValidator)? +} +// ... 8 more +``` + +### 4. The arguments do not change + +All 5 close instructions carry the existing `CloseAccessPassArgs`, which is empty. All 5 delete +instructions carry the existing `UserDeleteArgs`, which holds `dz_prefix_count` and +`multicast_publisher_count`. No new argument types, and no new incremental defaults to get +wrong. + +No account layout changes, so the SDK fixtures do not need regeneration. + +### 5. Variants 69 and 42 are replaced + +They follow the pattern already used for variants 72, 75, 77 and 78. Each loses its payload and +joins the deprecated arm in `entrypoint.rs`: + +```rust +CloseAccessPass(), // variant 69, deprecated: use CloseAccessPass. See #2470. +DeleteUser(), // variant 42, deprecated: use DeleteUser. See #2470. +``` + +Both return `DoubleZeroError::Deprecated`. The discriminants stay reserved, and a caller that +does not upgrade gets a named error rather than a silent removal. + +### 6. Callers + +| Caller | Change | +| --- | --- | +| `doublezero access-pass close` | new required flag `--type ` | +| `doublezero user delete` | new required flag `--access-pass-type`, same values | +| `CloseAccessPassCommand`, `DeleteUserCommand` (Rust SDK) | new required `kind: AccessPassKind` field; picks the instruction | +| `doublezero-serviceability-instruction` | `close_access_pass` and `delete_user` take the kind and build the matching variant | +| `doublezero disconnect` (client daemon) | fills the kind from the pass it already reads | +| `Executor.DeleteUser` (Go SDK) | new `AccessPassKind` parameter; picks the instruction number | + +**Open question, asked on the issue.** The CLI takes only `--pubkey` today. For the SDK command +to pick an instruction it needs the kind from somewhere. If it reads the pass and picks from +what it read, then the on-chain refusal can never fire, because the instruction was chosen from +the same byte it checks. The guard only holds if the operator states the kind. This design +therefore adds a required flag. If Martin wants no flag, the CLI path stays unguarded and the +flag comes out. + +Two callers read the kind rather than declaring it, and both are unavoidable: + +- `doublezero disconnect` in the client daemon. It is a self delete, and the handler already + checks the owner and the client IP, so the kind adds nothing there. Its `delete_users` loop + does not hold the pass, but the `LedgerClient` trait already exposes `get_accesspass`, and the + loop already has `client_ip` and `user.owner`, so the lookup is one line. +- `DeleteTenantCommand` with `allow_delete_users`. It sweeps every user under a tenant, and those + users can hold passes of different kinds, so no single declared kind exists. Forcing one would + turn "delete every user under this tenant" into "delete only users of one kind", which strands + the tenant record, because the command then waits for `reference_count` to reach 0. + +On both paths the program's refusal cannot fire: the value asserted and the value checked come +from the same account. The comment at each call site has to say so, or someone will copy the +pattern into a path where an operator could have declared the kind. + +### 7. A bug to fix in the code being touched + +`close.rs` wraps the account type check and the `connection_count` check in +`if let Ok(data) = accesspass_account.try_borrow_data()`. When the borrow fails, the handler +logs `Failed to borrow account data, cannot close` and then closes the pass anyway. Both checks +are skipped. + +The new kind check must not sit inside that block. The fix is to read the `AccessPass` once, +before the checks, and let a failed read return an error. This is a small change in a file the +work already edits. + +## Error handling + +| Case | Result | +| --- | --- | +| the instruction matches the stored pass | the removal proceeds as it does today | +| the instruction is for another kind | `AccessPassTypeMismatch`, nothing is written | +| variant 69 or 42 | `Deprecated`, nothing is written | +| the account data cannot be read | an error, and the pass is not closed | + +## Testing + +Program tests: + +- for each of the 5 kinds, one accepted close and one accepted delete; +- for each of the 5 kinds, one rejected close and one rejected delete against a pass of a + different kind; +- variant 69 and variant 42 both return `Deprecated`. + +Existing call sites move to the new instructions: `tests/accesspass_test.rs`, +`tests/user_tests.rs`, `tests/delete_user_dynamic_accesspass.rs`, `tests/user_old_test.rs`, +`tests/accesspass_allow_multiple_ip.rs`, `tests/create_subscribe_user_test.rs`, +`tests/multicastgroup_subscribe_test.rs`, `tests/user_onchain_allocation_test.rs`. + +CLI tests: the new flag is required, and an unknown value is rejected. + +Go SDK: `user_crud_test.go` passes the new parameter. + +## Rollout + +This breaks callers that do not upgrade. They get `Deprecated`. + +The program, the instruction crate, the Rust SDK, the CLI and the Go SDK land together in this +repository. + +The oracle lives in `doublezero-shreds`. Parent issue #2385 lists five places there that remove +users: `cleanup_orphaned_users`, the lapsed seat branch of `reconcile_validator_owned_users`, +`converge_retransmit_only_seats`, `process_instant_withdrawal_requests`, and +`access_pass_expiry`. Each one knows which class of user it cranks, so each one has a real kind +to name. That is where the risk in #2385 sits. It needs its own change, shipped with this +program deploy. + +## PR size + +The 10 enum variants touch 5 places each in `instructions.rs` (the enum, the decoder, the name, +the debug format, and the round trip test), plus 10 arms in `entrypoint.rs`. That is mechanical +but wide. If the total goes past the 500 line guideline in `CLAUDE.md`, the work splits into two +PRs: the program and the instruction crate first, then the CLI and the SDKs. + +## Out of scope + +Feed subscription has no access pass or user instructions. Issue #2470 records the rule for +future instructions there. No code changes. From ea2239805bb127c0879dea5bae745ee42fac758e Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 18:45:57 -0700 Subject: [PATCH 7/9] docs: record the access pass removal breaking change --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a29be997d..4aec25a8da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to this project will be documented in this file. ### Breaking +- Serviceability + - `CloseAccessPass` (variant 69) and `DeleteUser` (variant 42) now return `Deprecated`. Callers must use the per-pass-type instructions instead: `ClosePrepaidAccessPass`, `CloseSolanaValidatorAccessPass`, `CloseSolanaRPCAccessPass`, `CloseOthersAccessPass`, `CloseEdgeSeatAccessPass` for close; `DeletePrepaidUser`, `DeleteSolanaValidatorUser`, `DeleteSolanaRPCUser`, `DeleteOthersUser`, `DeleteEdgeSeatUser` for delete. Each new instruction reads the access pass and refuses unless the pass matches the instruction. (#2470) + - The oracle in `doublezero-shreds` must ship its matching change with this program deploy. Its user removals fail until it names the pass type on each instruction. (#2470) + ### Changes From 5da02c5436d03cd24bb33d534434fa60daac6fe0 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Tue, 1 Sep 2026 19:12:44 -0700 Subject: [PATCH 8/9] serviceability: keep the general-purpose removal instructions working --- CHANGELOG.md | 7 +- .../src/accesspass.rs | 8 +-- .../src/user.rs | 25 +++++-- .../src/entrypoint.rs | 46 ++++++++----- .../src/instructions.rs | 33 ++++++---- .../src/processors/accesspass/close.rs | 15 +++-- .../src/processors/user/delete.rs | 17 +++-- .../deprecated_removal_instructions_test.rs | 65 ------------------- 8 files changed, 92 insertions(+), 124 deletions(-) delete mode 100644 smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aec25a8da..97806eef5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,10 @@ All notable changes to this project will be documented in this file. ### Breaking -- Serviceability - - `CloseAccessPass` (variant 69) and `DeleteUser` (variant 42) now return `Deprecated`. Callers must use the per-pass-type instructions instead: `ClosePrepaidAccessPass`, `CloseSolanaValidatorAccessPass`, `CloseSolanaRPCAccessPass`, `CloseOthersAccessPass`, `CloseEdgeSeatAccessPass` for close; `DeletePrepaidUser`, `DeleteSolanaValidatorUser`, `DeleteSolanaRPCUser`, `DeleteOthersUser`, `DeleteEdgeSeatUser` for delete. Each new instruction reads the access pass and refuses unless the pass matches the instruction. (#2470) - - The oracle in `doublezero-shreds` must ship its matching change with this program deploy. Its user removals fail until it names the pass type on each instruction. (#2470) - ### Changes - +- Serviceability + - Ten new instructions remove an access pass or a user for one specific `AccessPassType`: `ClosePrepaidAccessPass`, `CloseSolanaValidatorAccessPass`, `CloseSolanaRPCAccessPass`, `CloseOthersAccessPass`, `CloseEdgeSeatAccessPass`, and `DeletePrepaidUser`, `DeleteSolanaValidatorUser`, `DeleteSolanaRPCUser`, `DeleteOthersUser`, `DeleteEdgeSeatUser`. Each reads the access pass and refuses with `AccessPassTypeMismatch` unless the pass matches the instruction. `CloseAccessPass` and `DeleteUser` keep working unchanged; a follow-up change deprecates them and moves every caller. (#2470) - SDK - The TypeScript and Python `GlobalState` deserializers expose `ip_verifier_authority_pk`, the RFC-27 trust root the Go SDK and the Rust state already carried, so those consumers can read which key signs IP ownership proofs. The field is appended, so an account written before the upgrade decodes it as the default pubkey rather than failing. (#4231) - CI diff --git a/crates/doublezero-serviceability-instruction/src/accesspass.rs b/crates/doublezero-serviceability-instruction/src/accesspass.rs index 84560939e8..96dae6bf7d 100644 --- a/crates/doublezero-serviceability-instruction/src/accesspass.rs +++ b/crates/doublezero-serviceability-instruction/src/accesspass.rs @@ -52,19 +52,17 @@ pub fn set_access_pass( ) } -/// Deprecated: `CloseAccessPass` (variant 69) now returns `DoubleZeroError::Deprecated`. Use -/// one of the `CloseAccessPass` builders instead. See malbeclabs/infra#2470. -/// Accounts: `[accesspass, globalstate]`. +/// `CloseAccessPass` (variant 69). Accounts: `[accesspass, globalstate]`. pub fn close_access_pass( program_id: &Pubkey, payer: &Pubkey, accesspass: &Pubkey, - _args: CloseAccessPassArgs, + args: CloseAccessPassArgs, ) -> Instruction { let (globalstate, _) = get_globalstate_pda(program_id); common::build_with_permission( program_id, - DoubleZeroInstruction::CloseAccessPass(), + DoubleZeroInstruction::CloseAccessPass(args), vec![ AccountMeta::new(*accesspass, false), AccountMeta::new(globalstate, false), diff --git a/crates/doublezero-serviceability-instruction/src/user.rs b/crates/doublezero-serviceability-instruction/src/user.rs index 30646a5c41..7fcfe1ac8e 100644 --- a/crates/doublezero-serviceability-instruction/src/user.rs +++ b/crates/doublezero-serviceability-instruction/src/user.rs @@ -309,8 +309,7 @@ pub fn update_user( ) } -/// Deprecated: `DeleteUser` (variant 42) now returns `DoubleZeroError::Deprecated`. Use one of -/// the `DeleteUser` builders instead. See malbeclabs/infra#2470. +/// `DeleteUser` (variant 42). /// /// Account layout, before the trailing accounts: /// @@ -340,7 +339,7 @@ pub fn delete_user( dz_prefix_count: u8, tenant: Option, owner: &Pubkey, - _args: UserDeleteArgs, + mut args: UserDeleteArgs, ) -> Instruction { // The processor rejects `dz_prefix_count == 0` as its first statement // (delete.rs) — DeleteUser requires on-chain deallocation — so a zero here can @@ -349,6 +348,13 @@ pub fn delete_user( dz_prefix_count > 0, "dz_prefix_count must be > 0; DeleteUser requires on-chain deallocation" ); + args.dz_prefix_count = dz_prefix_count; + // This builder always emits the `multicast_publisher_block` account, so the + // declared count MUST be > 0 or the processor (which reads that account only + // when `multicast_publisher_count > 0`) would skip it and misread every + // following account. Written back here for the same reason as + // `dz_prefix_count`: keep the declared count and the account list in lockstep. + args.multicast_publisher_count = 1; let (globalstate, _) = get_globalstate_pda(program_id); let (user_tunnel_block, _, _) = @@ -381,7 +387,7 @@ pub fn delete_user( common::build_with_permission( program_id, - DoubleZeroInstruction::DeleteUser(), + DoubleZeroInstruction::DeleteUser(args), accounts, payer, ) @@ -1090,9 +1096,14 @@ mod tests { UserDeleteArgs::default(), ); assert_eq!(ix.data[0], 42); - // Deprecated: DeleteUser is payload-free (variant 42 always errors with - // Deprecated), so the wire data carries only the discriminant byte. - assert_eq!(ix.data.len(), 1); + // The builder always emits the mpb account, so it MUST pin + // multicast_publisher_count > 0 to keep the declared count and the + // account list in lockstep (else the processor skips the mpb slot and + // misreads every following account). + match DoubleZeroInstruction::unpack(&ix.data).unwrap() { + DoubleZeroInstruction::DeleteUser(a) => assert_eq!(a.multicast_publisher_count, 1), + other => panic!("unexpected variant: {other:?}"), + } let (globalstate, _) = get_globalstate_pda(&pid); let (utb, _, _) = get_resource_extension_pda(&pid, ResourceType::UserTunnelBlock); let (mpb, _, _) = get_resource_extension_pda(&pid, ResourceType::MulticastPublisherBlock); diff --git a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs index 23aa961dba..084045ccff 100644 --- a/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs +++ b/smartcontract/programs/doublezero-serviceability/src/entrypoint.rs @@ -169,34 +169,38 @@ pub fn process_instruction( | DoubleZeroInstruction::CloseAccountDevice() | DoubleZeroInstruction::DeactivateMulticastGroup() | DoubleZeroInstruction::RemoveDeviceInterface() - | DoubleZeroInstruction::UnlinkDeviceInterface() - | DoubleZeroInstruction::CloseAccessPass() => { + | DoubleZeroInstruction::UnlinkDeviceInterface() => { return Err(DoubleZeroError::Deprecated.into()); } DoubleZeroInstruction::ActivateUser() | DoubleZeroInstruction::RejectUser() | DoubleZeroInstruction::CloseAccountUser() - | DoubleZeroInstruction::BanUser() - | DoubleZeroInstruction::DeleteUser() => { + | DoubleZeroInstruction::BanUser() => { return Err(DoubleZeroError::Deprecated.into()); } + DoubleZeroInstruction::DeleteUser(value) => { + process_delete_user(program_id, accounts, &value, None)? + } DoubleZeroInstruction::DeletePrepaidUser(value) => { - process_delete_user(program_id, accounts, &value, AccessPassKind::Prepaid)? + process_delete_user(program_id, accounts, &value, Some(AccessPassKind::Prepaid))? } DoubleZeroInstruction::DeleteSolanaValidatorUser(value) => process_delete_user( program_id, accounts, &value, - AccessPassKind::SolanaValidator, + Some(AccessPassKind::SolanaValidator), + )?, + DoubleZeroInstruction::DeleteSolanaRPCUser(value) => process_delete_user( + program_id, + accounts, + &value, + Some(AccessPassKind::SolanaRPC), )?, - DoubleZeroInstruction::DeleteSolanaRPCUser(value) => { - process_delete_user(program_id, accounts, &value, AccessPassKind::SolanaRPC)? - } DoubleZeroInstruction::DeleteOthersUser(value) => { - process_delete_user(program_id, accounts, &value, AccessPassKind::Others)? + process_delete_user(program_id, accounts, &value, Some(AccessPassKind::Others))? } DoubleZeroInstruction::DeleteEdgeSeatUser(value) => { - process_delete_user(program_id, accounts, &value, AccessPassKind::EdgeSeat)? + process_delete_user(program_id, accounts, &value, Some(AccessPassKind::EdgeSeat))? } DoubleZeroInstruction::DeleteDevice(value) => { process_delete_device(program_id, accounts, &value)? @@ -323,23 +327,29 @@ pub fn process_instruction( DoubleZeroInstruction::SetAccessPass(value) => { process_set_access_pass(program_id, accounts, &value)? } + DoubleZeroInstruction::CloseAccessPass(value) => { + process_close_access_pass(program_id, accounts, &value, None)? + } DoubleZeroInstruction::ClosePrepaidAccessPass(value) => { - process_close_access_pass(program_id, accounts, &value, AccessPassKind::Prepaid)? + process_close_access_pass(program_id, accounts, &value, Some(AccessPassKind::Prepaid))? } DoubleZeroInstruction::CloseSolanaValidatorAccessPass(value) => process_close_access_pass( program_id, accounts, &value, - AccessPassKind::SolanaValidator, + Some(AccessPassKind::SolanaValidator), + )?, + DoubleZeroInstruction::CloseSolanaRPCAccessPass(value) => process_close_access_pass( + program_id, + accounts, + &value, + Some(AccessPassKind::SolanaRPC), )?, - DoubleZeroInstruction::CloseSolanaRPCAccessPass(value) => { - process_close_access_pass(program_id, accounts, &value, AccessPassKind::SolanaRPC)? - } DoubleZeroInstruction::CloseOthersAccessPass(value) => { - process_close_access_pass(program_id, accounts, &value, AccessPassKind::Others)? + process_close_access_pass(program_id, accounts, &value, Some(AccessPassKind::Others))? } DoubleZeroInstruction::CloseEdgeSeatAccessPass(value) => { - process_close_access_pass(program_id, accounts, &value, AccessPassKind::EdgeSeat)? + process_close_access_pass(program_id, accounts, &value, Some(AccessPassKind::EdgeSeat))? } DoubleZeroInstruction::CheckStatusAccessPass(value) => { process_check_status_access_pass(program_id, accounts, &value)? diff --git a/smartcontract/programs/doublezero-serviceability/src/instructions.rs b/smartcontract/programs/doublezero-serviceability/src/instructions.rs index b59ea978cc..df6c14b13f 100644 --- a/smartcontract/programs/doublezero-serviceability/src/instructions.rs +++ b/smartcontract/programs/doublezero-serviceability/src/instructions.rs @@ -152,9 +152,7 @@ pub enum DoubleZeroInstruction { UpdateUser(UserUpdateArgs), // variant 39 SuspendUser(), // variant 40 ResumeUser(), // variant 41 - /// Deprecated: handler returns DoubleZeroError::Deprecated. Use `DeleteUser` - /// (variants 124-128). See malbeclabs/infra#2470. - DeleteUser(), // variant 42 + DeleteUser(UserDeleteArgs), // variant 42 /// Deprecated: handler returns DoubleZeroError::Deprecated. See #3622. CloseAccountUser(), // variant 43 RequestBanUser(UserRequestBanArgs), // variant 44 @@ -191,9 +189,7 @@ pub enum DoubleZeroInstruction { AcceptLink(LinkAcceptArgs), // variant 66 SetAccessPass(SetAccessPassArgs), // variant 67 SetAirdrop(SetAirdropArgs), // variant 68 - /// Deprecated: handler returns DoubleZeroError::Deprecated. Use - /// `CloseAccessPass` (variants 119-123). See malbeclabs/infra#2470. - CloseAccessPass(), // variant 69 + CloseAccessPass(CloseAccessPassArgs), // variant 69 CheckStatusAccessPass(CheckStatusAccessPassArgs), // variant 70 CheckUserAccessPass(CheckUserAccessPassArgs), // variant 71 @@ -340,7 +336,7 @@ impl DoubleZeroInstruction { 39 => Ok(Self::UpdateUser(UserUpdateArgs::try_from(rest).unwrap())), 40 => Ok(Self::SuspendUser()), 41 => Ok(Self::ResumeUser()), - 42 => Ok(Self::DeleteUser()), + 42 => Ok(Self::DeleteUser(UserDeleteArgs::try_from(rest).unwrap())), 43 => Ok(Self::CloseAccountUser()), 44 => Ok(Self::RequestBanUser(UserRequestBanArgs::try_from(rest).unwrap())), 45 => Ok(Self::BanUser()), @@ -373,7 +369,7 @@ impl DoubleZeroInstruction { 67 => Ok(Self::SetAccessPass(SetAccessPassArgs::try_from(rest).unwrap())), 68 => Ok(Self::SetAirdrop(SetAirdropArgs::try_from(rest).unwrap())), - 69 => Ok(Self::CloseAccessPass()), + 69 => Ok(Self::CloseAccessPass(CloseAccessPassArgs::try_from(rest).unwrap())), 70 => Ok(Self::CheckStatusAccessPass(CheckStatusAccessPassArgs::try_from(rest).unwrap())), 71 => Ok(Self::CheckUserAccessPass(CheckUserAccessPassArgs::try_from(rest).unwrap())), @@ -496,7 +492,7 @@ impl DoubleZeroInstruction { Self::UpdateUser(_) => "UpdateUser".to_string(), // variant 39 Self::SuspendUser() => "SuspendUser".to_string(), // variant 40 Self::ResumeUser() => "ResumeUser".to_string(), // variant 41 - Self::DeleteUser() => "DeleteUser".to_string(), // variant 42 + Self::DeleteUser(_) => "DeleteUser".to_string(), // variant 42 Self::CloseAccountUser() => "CloseAccountUser".to_string(), // variant 43 Self::RequestBanUser(_) => "RequestBanUser".to_string(), // variant 44 @@ -533,7 +529,7 @@ impl DoubleZeroInstruction { Self::AcceptLink(_) => "AcceptLink".to_string(), // variant 66 Self::SetAccessPass(_) => "SetAccessPass".to_string(), // variant 67 Self::SetAirdrop(_) => "SetAirdrop".to_string(), // variant 68 - Self::CloseAccessPass() => "CloseAccessPass".to_string(), // variant 69 + Self::CloseAccessPass(_) => "CloseAccessPass".to_string(), // variant 69 Self::CheckStatusAccessPass(_) => "CheckStatusAccessPass".to_string(), // variant 70 Self::CheckUserAccessPass(_) => "CheckUserAccessPass".to_string(), // variant 71 @@ -658,7 +654,7 @@ impl DoubleZeroInstruction { Self::UpdateUser(args) => format!("{args:?}"), // variant 39 Self::SuspendUser() => "".to_string(), // variant 40 Self::ResumeUser() => "".to_string(), // variant 41 - Self::DeleteUser() => "".to_string(), // variant 42 + Self::DeleteUser(args) => format!("{args:?}"), // variant 42 Self::CloseAccountUser() => "".to_string(), // variant 43 Self::RequestBanUser(args) => format!("{args:?}"), // variant 44 @@ -689,7 +685,7 @@ impl DoubleZeroInstruction { Self::AcceptLink(args) => format!("{args:?}"), // variant 66 Self::SetAccessPass(args) => format!("{args:?}"), // variant 67 Self::SetAirdrop(args) => format!("{args:?}"), // variant 68 - Self::CloseAccessPass() => "".to_string(), // variant 69 + Self::CloseAccessPass(args) => format!("{args:?}"), // variant 69 Self::CheckStatusAccessPass(args) => format!("{args:?}"), // variant 70 Self::CheckUserAccessPass(args) => format!("{args:?}"), // variant 71 @@ -992,7 +988,13 @@ mod tests { ); test_instruction(DoubleZeroInstruction::SuspendUser(), "SuspendUser"); test_instruction(DoubleZeroInstruction::ResumeUser(), "ResumeUser"); - test_instruction(DoubleZeroInstruction::DeleteUser(), "DeleteUser"); + test_instruction( + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + "DeleteUser", + ); test_instruction( DoubleZeroInstruction::CloseAccountDevice(), "CloseAccountDevice", @@ -1230,7 +1232,10 @@ mod tests { }), "SetAirdrop", ); - test_instruction(DoubleZeroInstruction::CloseAccessPass(), "CloseAccessPass"); + test_instruction( + DoubleZeroInstruction::CloseAccessPass(CloseAccessPassArgs {}), + "CloseAccessPass", + ); test_instruction( DoubleZeroInstruction::ClosePrepaidAccessPass(CloseAccessPassArgs {}), "ClosePrepaidAccessPass", diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs index 7d7110890c..ac9d63f6e5 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs @@ -32,7 +32,7 @@ pub fn process_close_access_pass( program_id: &Pubkey, accounts: &[AccountInfo], _value: &CloseAccessPassArgs, - expected: AccessPassKind, + expected: Option, ) -> ProgramResult { let accounts_iter = &mut accounts.iter(); @@ -98,10 +98,15 @@ pub fn process_close_access_pass( } let accesspass = AccessPass::try_from(accesspass_account)?; - let actual = AccessPassKind::from(&accesspass.accesspass_type); - if actual != expected { - msg!("this instruction closes a {expected} pass, but the pass is {actual}"); - return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + // `None` is the deprecated `CloseAccessPass` (variant 69), which predates the + // per-pass-type split and performs no kind check. It is removed, along with this + // `Option`, in the follow-up that moves every caller. See malbeclabs/infra#2470. + if let Some(expected) = expected { + let actual = AccessPassKind::from(&accesspass.accesspass_type); + if actual != expected { + msg!("this instruction closes a {expected} pass, but the pass is {actual}"); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + } } // Feed authority can only close access passes they own diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index 241898fbca..602df3784b 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -52,7 +52,7 @@ pub fn process_delete_user( program_id: &Pubkey, accounts: &[AccountInfo], value: &UserDeleteArgs, - expected: AccessPassKind, + expected: Option, ) -> ProgramResult { if value.dz_prefix_count == 0 { msg!("dz_prefix_count must be > 0; DeleteUser requires on-chain deallocation"); @@ -155,10 +155,17 @@ pub fn process_delete_user( if !accesspass_account.data_is_empty() { // Read Access Pass let mut accesspass = AccessPass::try_from(accesspass_account)?; - let actual = AccessPassKind::from(&accesspass.accesspass_type); - if actual != expected { - msg!("this instruction deletes a user on a {expected} pass, but the pass is {actual}"); - return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + // `None` is the deprecated `DeleteUser` (variant 42), which predates the + // per-pass-type split and performs no kind check. It is removed, along with this + // `Option`, in the follow-up that moves every caller. See malbeclabs/infra#2470. + if let Some(expected) = expected { + let actual = AccessPassKind::from(&accesspass.accesspass_type); + if actual != expected { + msg!( + "this instruction deletes a user on a {expected} pass, but the pass is {actual}" + ); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); + } } if accesspass.user_payer != user.owner { msg!( diff --git a/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs b/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs deleted file mode 100644 index 93b8be86f0..0000000000 --- a/smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs +++ /dev/null @@ -1,65 +0,0 @@ -//! Issue #2470: the general-purpose removal instructions are replaced by one per pass type. -//! Wire discriminants 69 and 42 are kept so an old client hits a deterministic deprecation -//! error instead of an unknown-instruction decode failure. - -use doublezero_serviceability::{ - entrypoint::process_instruction, error::DoubleZeroError, instructions::DoubleZeroInstruction, -}; -use solana_program::program_error::ProgramError; -use solana_program_test::*; -use solana_sdk::{ - instruction::{AccountMeta, Instruction, InstructionError}, - pubkey::Pubkey, - signer::Signer, - transaction::{Transaction, TransactionError}, -}; - -async fn assert_returns_deprecated(instruction: DoubleZeroInstruction) { - let program_id = Pubkey::new_unique(); - let (banks_client, payer, recent_blockhash) = ProgramTest::new( - "doublezero_serviceability", - program_id, - processor!(process_instruction), - ) - .start() - .await; - - let ix = Instruction { - program_id, - accounts: vec![AccountMeta::new(payer.pubkey(), true)], - data: instruction.pack(), - }; - let mut tx = Transaction::new_with_payer(&[ix], Some(&payer.pubkey())); - tx.try_sign(&[&payer], recent_blockhash).unwrap(); - - let err = banks_client - .process_transaction(tx) - .await - .expect_err("expected deprecated instruction to fail"); - - let expected: ProgramError = DoubleZeroError::Deprecated.into(); - let ProgramError::Custom(expected_code) = expected else { - panic!("Deprecated must map to ProgramError::Custom"); - }; - - match err { - BanksClientError::TransactionError(TransactionError::InstructionError( - 0, - InstructionError::Custom(code), - )) => assert_eq!( - code, expected_code, - "expected Deprecated (Custom({expected_code})), got Custom({code})" - ), - other => panic!("expected Custom({expected_code}) InstructionError, got {other:?}"), - } -} - -#[tokio::test] -async fn close_access_pass_returns_deprecated() { - assert_returns_deprecated(DoubleZeroInstruction::CloseAccessPass()).await; -} - -#[tokio::test] -async fn delete_user_returns_deprecated() { - assert_returns_deprecated(DoubleZeroInstruction::DeleteUser()).await; -} From af050272f45b0991108d201d63cb8b9db9139089 Mon Sep 17 00:00:00 2001 From: Ben Marx Date: Wed, 2 Sep 2026 21:07:42 -0400 Subject: [PATCH 9/9] serviceability: address review comments on the removal instructions Remove a stale #[repr(u8)] from the AccessPassKind design-doc snippet (the enum never has one, deliberately). Delete a dead data_is_empty() wrapper in process_delete_user that made the new AccessPassTypeMismatch check look conditional on an early return 45 lines above. Rewrite close_access_pass_kind_test.rs to build its passes with SetAccessPass instead of hand-inserting AccessPass accounts, matching its delete_user_kind_test.rs sibling. Correct delete_user_kind_test.rs's seed_feed_seat comment, which claimed direct seeding was the only way to put a real seat on a user: SetAccessPassFeeds is the real provisioning path, but ticking a user's seat for real needs CreateSubscribeUser/SubscribeFeed and a live MulticastGroup, which this suite does not otherwise stand up. --- ...9-01-per-pass-type-removal-guard-design.md | 1 - .../src/processors/user/delete.rs | 102 ++++++----- .../tests/close_access_pass_kind_test.rs | 161 +++++++----------- .../tests/delete_user_kind_test.rs | 16 +- 4 files changed, 119 insertions(+), 161 deletions(-) diff --git a/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md b/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md index 2683700a90..acba4013d8 100644 --- a/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md +++ b/docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md @@ -42,7 +42,6 @@ A tag enum in `smartcontract/programs/doublezero-serviceability/src/state/access next to `AccessPassType`. ```rust -#[repr(u8)] #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum AccessPassKind { Prepaid, diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index 602df3784b..e18e054439 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -152,63 +152,59 @@ pub fn process_delete_user( "Invalid AccessPass PDA", ); - if !accesspass_account.data_is_empty() { - // Read Access Pass - let mut accesspass = AccessPass::try_from(accesspass_account)?; - // `None` is the deprecated `DeleteUser` (variant 42), which predates the - // per-pass-type split and performs no kind check. It is removed, along with this - // `Option`, in the follow-up that moves every caller. See malbeclabs/infra#2470. - if let Some(expected) = expected { - let actual = AccessPassKind::from(&accesspass.accesspass_type); - if actual != expected { - msg!( - "this instruction deletes a user on a {expected} pass, but the pass is {actual}" - ); - return Err(DoubleZeroError::AccessPassTypeMismatch.into()); - } - } - if accesspass.user_payer != user.owner { - msg!( - "Invalid user_payer accesspass.user_payer: {} = user_payer: {} ", - accesspass.user_payer, - user.owner - ); - return Err(DoubleZeroError::Unauthorized.into()); - } - // Skip IP validation when the pass is stored at the UNSPECIFIED PDA (0.0.0.0): it is - // a dynamic pass valid for any IP by construction. This includes legacy passes which - // have client_ip=0.0.0.0. - if accesspass.client_ip != Ipv4Addr::UNSPECIFIED - && accesspass.client_ip != user.client_ip - && !accesspass.allow_multiple_ip() - { - msg!( - "Invalid client_ip accesspass.{{client_ip: {}}} = {{ client_ip: {} }}", - accesspass.client_ip, - user.client_ip - ); - return Err(DoubleZeroError::Unauthorized.into()); - } - - accesspass.connection_count = accesspass.connection_count.saturating_sub(1); - // Release the per-category seat (EdgeSeat only; no-op otherwise). - accesspass.remove_user(user.user_type); - // Release every feed-scoped seat this user consumed at connect. The feeds are read from the - // User (recorded when the seats were ticked), so each release is bound to exactly that seat - // and cannot be misdirected to another metro's feed by a caller-supplied account. - user.release_feed_seats(&mut accesspass); - accesspass.status = if accesspass.connection_count > 0 { - AccessPassStatus::Connected - } else { - AccessPassStatus::Disconnected - }; - if accesspass.connection_count == 0 && accesspass.allow_multiple_ip() { - accesspass.client_ip = Ipv4Addr::UNSPECIFIED; // reset to allow multiple IPs + // Read Access Pass + let mut accesspass = AccessPass::try_from(accesspass_account)?; + // `None` is the deprecated `DeleteUser` (variant 42), which predates the + // per-pass-type split and performs no kind check. It is removed, along with this + // `Option`, in the follow-up that moves every caller. See malbeclabs/infra#2470. + if let Some(expected) = expected { + let actual = AccessPassKind::from(&accesspass.accesspass_type); + if actual != expected { + msg!("this instruction deletes a user on a {expected} pass, but the pass is {actual}"); + return Err(DoubleZeroError::AccessPassTypeMismatch.into()); } + } + if accesspass.user_payer != user.owner { + msg!( + "Invalid user_payer accesspass.user_payer: {} = user_payer: {} ", + accesspass.user_payer, + user.owner + ); + return Err(DoubleZeroError::Unauthorized.into()); + } + // Skip IP validation when the pass is stored at the UNSPECIFIED PDA (0.0.0.0): it is + // a dynamic pass valid for any IP by construction. This includes legacy passes which + // have client_ip=0.0.0.0. + if accesspass.client_ip != Ipv4Addr::UNSPECIFIED + && accesspass.client_ip != user.client_ip + && !accesspass.allow_multiple_ip() + { + msg!( + "Invalid client_ip accesspass.{{client_ip: {}}} = {{ client_ip: {} }}", + accesspass.client_ip, + user.client_ip + ); + return Err(DoubleZeroError::Unauthorized.into()); + } - try_acc_write(&accesspass, accesspass_account, payer_account, accounts)?; + accesspass.connection_count = accesspass.connection_count.saturating_sub(1); + // Release the per-category seat (EdgeSeat only; no-op otherwise). + accesspass.remove_user(user.user_type); + // Release every feed-scoped seat this user consumed at connect. The feeds are read from the + // User (recorded when the seats were ticked), so each release is bound to exactly that seat + // and cannot be misdirected to another metro's feed by a caller-supplied account. + user.release_feed_seats(&mut accesspass); + accesspass.status = if accesspass.connection_count > 0 { + AccessPassStatus::Connected + } else { + AccessPassStatus::Disconnected + }; + if accesspass.connection_count == 0 && accesspass.allow_multiple_ip() { + accesspass.client_ip = Ipv4Addr::UNSPECIFIED; // reset to allow multiple IPs } + try_acc_write(&accesspass, accesspass_account, payer_account, accounts)?; + if !user.publishers.is_empty() || !user.subscribers.is_empty() { msg!("{:?}", user); return Err(DoubleZeroError::ReferenceCountNotZero.into()); diff --git a/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs b/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs index c90e3dad19..1791e4dd51 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs @@ -7,18 +7,11 @@ use doublezero_serviceability::{ error::DoubleZeroError, instructions::DoubleZeroInstruction, pda::{get_accesspass_pda, get_globalstate_pda, get_program_config_pda}, - processors::accesspass::close::CloseAccessPassArgs, - state::{ - accesspass::{AccessPass, AccessPassStatus, AccessPassType}, - accounttype::AccountType, - }, + processors::accesspass::{close::CloseAccessPassArgs, set::SetAccessPassArgs}, + state::accesspass::AccessPassType, }; -use solana_program::rent::Rent; use solana_program_test::*; -use solana_sdk::{ - account::Account as SolanaAccount, instruction::AccountMeta, pubkey::Pubkey, - signature::Keypair, signer::Signer, -}; +use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Keypair}; use std::net::Ipv4Addr; use test_helpers::*; @@ -51,85 +44,52 @@ fn close_instructions( } } -/// Starts a fresh `ProgramTest`, runs `InitGlobalState`, and seeds an `AccessPass` account of -/// `pass_type` owned by the payer, with no active connections. The account-building block is -/// lifted from `accesspass_test.rs::test_close_accesspass_rejects_nonzero_connection_count`. -/// -/// Uses `test_payer()` rather than the `Keypair` `ProgramTest::start()` generates: that one -/// isn't known until after `start()`, too late to use as the `AccessPass`'s `owner` field, -/// which must be written into the account added before `start()`. -async fn seed_access_pass( - pass_type: &AccessPassType, -) -> ( - BanksClient, - Keypair, - solana_program::hash::Hash, - Pubkey, - Pubkey, - Pubkey, -) { - let program_id = Pubkey::new_unique(); - let payer = test_payer(); - - let (program_config_pubkey, _) = get_program_config_pda(&program_id); - let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); - - let client_ip = Ipv4Addr::new(101, 0, 0, 1); +/// Create an access pass of `pass_type` at `client_ip` via `SetAccessPass`, the real +/// instruction a caller uses, rather than hand-writing an `AccessPass` and inserting it into +/// the test bank. `SetAccessPass` always creates a fresh pass with `connection_count: 0`, +/// which is what the close path requires. Returns its pubkey. +async fn create_access_pass( + banks_client: &mut BanksClient, + recent_blockhash: solana_program::hash::Hash, + program_id: Pubkey, + globalstate_pubkey: Pubkey, + payer: &Keypair, + client_ip: Ipv4Addr, + pass_type: AccessPassType, +) -> Pubkey { let user_payer = Pubkey::new_unique(); - let (accesspass_pubkey, bump_seed) = get_accesspass_pda(&program_id, &client_ip, &user_payer); + let (accesspass_pubkey, _) = get_accesspass_pda(&program_id, &client_ip, &user_payer); - let seeded_accesspass = AccessPass { - account_type: AccountType::AccessPass, - owner: payer.pubkey(), - bump_seed, - accesspass_type: pass_type.clone(), - client_ip, - user_payer, - last_access_epoch: 0, - connection_count: 0, - status: AccessPassStatus::Requested, - mgroup_pub_allowlist: vec![], - mgroup_sub_allowlist: vec![], - flags: 0, - tenant_allowlist: vec![], - unicast_user_count: 0, - max_unicast_users: 1, - multicast_user_count: 0, - max_multicast_users: 1, - }; + execute_transaction( + banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { + accesspass_type: pass_type, + client_ip, + last_access_epoch: 9999, + allow_multiple_ip: false, + max_unicast_users: 1, + max_multicast_users: 1, + }), + vec![ + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_payer, false), + ], + payer, + ) + .await; - let accesspass_data = borsh::to_vec(&seeded_accesspass).unwrap(); - let rent = Rent::default(); - let lamports = rent.minimum_balance(accesspass_data.len()); + accesspass_pubkey +} - let mut program_test = ProgramTest::new( - "doublezero_serviceability", - program_id, - processor!(doublezero_serviceability::entrypoint::process_instruction), - ); - program_test.add_account( - accesspass_pubkey, - SolanaAccount { - lamports, - data: accesspass_data, - owner: program_id, - executable: false, - rent_epoch: 0, - }, - ); - // Fund the payer directly so it can sign InitGlobalState and the close instructions below. - program_test.add_account( - payer.pubkey(), - SolanaAccount { - lamports: 10_000_000_000, - data: vec![], - owner: solana_system_interface::program::ID, - executable: false, - rent_epoch: 0, - }, - ); +#[tokio::test] +async fn close_refuses_a_pass_of_another_kind() { + let (mut banks_client, program_id, payer, recent_blockhash) = init_test().await; - let (mut banks_client, _funder, recent_blockhash) = program_test.start().await; + let (program_config_pubkey, _) = get_program_config_pda(&program_id); + let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); // Makes `payer` the sole entry in the foundation allowlist, so it holds ACCESS_PASS_ADMIN. execute_transaction( @@ -145,33 +105,28 @@ async fn seed_access_pass( ) .await; - ( - banks_client, - payer, - recent_blockhash, - program_id, - accesspass_pubkey, - globalstate_pubkey, - ) -} - -#[tokio::test] -async fn close_refuses_a_pass_of_another_kind() { - for pass_type in [ + for (i, pass_type) in [ AccessPassType::Prepaid, AccessPassType::SolanaValidator(Pubkey::new_unique()), AccessPassType::SolanaRPC(Pubkey::new_unique()), AccessPassType::Others("thing".to_string(), "key".to_string()), AccessPassType::EdgeSeat(vec![]), - ] { - let ( - mut banks_client, - payer, + ] + .into_iter() + .enumerate() + { + let client_ip: Ipv4Addr = [101, 0, 0, 1 + i as u8].into(); + let accesspass_pubkey = create_access_pass( + &mut banks_client, recent_blockhash, program_id, - accesspass_pubkey, globalstate_pubkey, - ) = seed_access_pass(&pass_type).await; + &payer, + client_ip, + pass_type.clone(), + ) + .await; + let accounts = vec![ AccountMeta::new(accesspass_pubkey, false), AccountMeta::new(globalstate_pubkey, false), diff --git a/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs b/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs index 08ae75f31b..c738b8fdb9 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs @@ -229,10 +229,18 @@ async fn create_and_activate_user( } /// Rewrite the EdgeSeat pass to carry one feed seat with a user on it, and give the user that -/// same feed in `feed_pks` — bypassing `SubscribeFeed` so the test only needs a device and a -/// bare multicast user. This is the only way to put a real seat in front of -/// `process_delete_user`'s `release_feed_seats` call; a feedless EdgeSeat pass makes that call a -/// no-op and never exercises the release path a `DeleteEdgeSeatUser` must perform. +/// same feed in `feed_pks`, bypassing the real provisioning path. That real path is +/// `SetAccessPassFeeds` (see `set_access_pass_feeds_test.rs`), but it only puts the seat on the +/// pass — it needs a caller with a permissioned authority (foundation allowlist or +/// `ACCESS_PASS_ADMIN`) and does not tick `current_users` or touch a user. Recording the feed on +/// a user, ticked, is done by `CreateSubscribeUser` or `SubscribeFeed`, and both require a real +/// `MulticastGroup` (its own create instruction, `ResourceExtension` accounts, and onchain +/// allocation), which this suite does not otherwise set up. Standing that up here to seed one +/// feed seat would roughly double this file for no gain in what the delete path itself is +/// tested against, so the seat is seeded directly instead. +/// This is the only way to put a real seat in front of `process_delete_user`'s +/// `release_feed_seats` call without that extra machinery; a feedless EdgeSeat pass makes that +/// call a no-op and never exercises the release path a `DeleteEdgeSeatUser` must perform. async fn seed_feed_seat( env: &mut TestEnv, accesspass_pubkey: Pubkey,