diff --git a/CHANGELOG.md b/CHANGELOG.md index 055cb2185..0aeea1613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ All notable changes to this project will be documented in this file. - SDK - SetFeatureFlagCommand added to manage on-chain feature flags for conditional behavior rollouts - Serviceability: CreateUser instruction supports atomic create+allocate+activate when OnchainAllocation feature is enabled + - Serviceability: DeleteUser instruction supports atomic deallocate+closeaccount when OnchainAllocation feature is enabled - Dependencies - Upgrade Solana SDK workspace dependencies from 2.2.7 to 2.3.x (`solana-sdk`, `solana-client`, `solana-program-test`, and others) - Internet telemetry diff --git a/smartcontract/programs/doublezero-serviceability/src/instructions.rs b/smartcontract/programs/doublezero-serviceability/src/instructions.rs index 8394ef3b9..e332c7763 100644 --- a/smartcontract/programs/doublezero-serviceability/src/instructions.rs +++ b/smartcontract/programs/doublezero-serviceability/src/instructions.rs @@ -808,7 +808,10 @@ mod tests { test_instruction(DoubleZeroInstruction::SuspendUser(), "SuspendUser"); test_instruction(DoubleZeroInstruction::ResumeUser(), "ResumeUser"); test_instruction( - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), "DeleteUser", ); test_instruction( diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index c0e4efce6..2607b13cc 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -1,49 +1,120 @@ use crate::{ error::DoubleZeroError, pda::get_accesspass_pda, - serializer::try_acc_write, + serializer::{try_acc_close, try_acc_write}, state::{ accesspass::{AccessPass, AccessPassStatus}, + device::Device, globalstate::GlobalState, + tenant::Tenant, user::*, }, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; use core::fmt; - use solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, msg, + program_error::ProgramError, pubkey::Pubkey, }; use std::net::Ipv4Addr; +use super::resource_onchain_helpers; + #[derive(BorshSerialize, BorshDeserializeIncremental, PartialEq, Clone, Default)] -pub struct UserDeleteArgs {} +pub struct UserDeleteArgs { + /// Number of DzPrefixBlock accounts passed for on-chain deallocation. + /// When 0, legacy behavior (Deleting status). When > 0, atomic delete+deallocate+close. + #[incremental(default = 0)] + pub dz_prefix_count: u8, + /// Whether MulticastPublisherBlock account is passed (1 = yes, 0 = no). + #[incremental(default = 0)] + pub multicast_publisher_count: u8, +} impl fmt::Debug for UserDeleteArgs { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "") + write!( + f, + "dz_prefix_count: {}, multicast_publisher_count: {}", + self.dz_prefix_count, self.multicast_publisher_count + ) } } pub fn process_delete_user( program_id: &Pubkey, accounts: &[AccountInfo], - _value: &UserDeleteArgs, + value: &UserDeleteArgs, ) -> ProgramResult { let accounts_iter = &mut accounts.iter(); let user_account = next_account_info(accounts_iter)?; let accesspass_account = next_account_info(accounts_iter)?; let globalstate_account = next_account_info(accounts_iter)?; + + // Optional: additional accounts for atomic deallocation (between globalstate and payer) + // Account layout WITH deallocation (dz_prefix_count > 0): + // [user, accesspass, globalstate, device, user_tunnel_block, multicast_publisher_block?, device_tunnel_ids, dz_prefix_0..N, optional_tenant, owner, payer, system] + // Account layout WITHOUT (legacy, dz_prefix_count == 0): + // [user, accesspass, globalstate, payer, system] + let deallocation_accounts = if value.dz_prefix_count > 0 { + let device_account = next_account_info(accounts_iter)?; + let user_tunnel_block_ext = next_account_info(accounts_iter)?; + + let multicast_publisher_block_ext = if value.multicast_publisher_count > 0 { + Some(next_account_info(accounts_iter)?) + } else { + None + }; + + let device_tunnel_ids_ext = next_account_info(accounts_iter)?; + + let mut dz_prefix_accounts = Vec::with_capacity(value.dz_prefix_count as usize); + for _ in 0..value.dz_prefix_count { + dz_prefix_accounts.push(next_account_info(accounts_iter)?); + } + + Some(( + device_account, + user_tunnel_block_ext, + multicast_publisher_block_ext, + device_tunnel_ids_ext, + dz_prefix_accounts, + )) + } else { + None + }; + + // For atomic path: check if user has a tenant (we'll peek at user data) + // For legacy path: no tenant handling needed + let tenant_account = if value.dz_prefix_count > 0 { + // Peek at user to check tenant + let user_peek = User::try_from(user_account)?; + if user_peek.tenant_pk != Pubkey::default() { + Some(next_account_info(accounts_iter)?) + } else { + None + } + } else { + None + }; + + // For atomic path, owner account is needed for close + let owner_account = if value.dz_prefix_count > 0 { + Some(next_account_info(accounts_iter)?) + } else { + None + }; + let payer_account = next_account_info(accounts_iter)?; let system_program = next_account_info(accounts_iter)?; #[cfg(test)] - msg!("process_delete_user({:?})", _value); + msg!("process_delete_user({:?})", value); // Check if the payer is a signer assert!(payer_account.is_signer, "Payer must be a signer"); @@ -69,7 +140,7 @@ pub fn process_delete_user( // Check if the account is writable assert!(user_account.is_writable, "PDA Account is not writable"); - let mut user: User = User::try_from(user_account)?; + let user: User = User::try_from(user_account)?; let globalstate = GlobalState::try_from(globalstate_account)?; if !globalstate.foundation_allowlist.contains(payer_account.key) @@ -124,8 +195,17 @@ pub fn process_delete_user( try_acc_write(&accesspass, accesspass_account, payer_account, accounts)?; } - if matches!(user.status, UserStatus::Deleting | UserStatus::Updating) { - return Err(DoubleZeroError::InvalidStatus.into()); + // Status check differs between legacy and atomic paths + if value.dz_prefix_count > 0 { + // Atomic: reject Deleting and Updating + if matches!(user.status, UserStatus::Deleting | UserStatus::Updating) { + return Err(DoubleZeroError::InvalidStatus.into()); + } + } else { + // Legacy: reject Deleting and Updating (same check) + if matches!(user.status, UserStatus::Deleting | UserStatus::Updating) { + return Err(DoubleZeroError::InvalidStatus.into()); + } } if !user.publishers.is_empty() || !user.subscribers.is_empty() { @@ -133,12 +213,86 @@ pub fn process_delete_user( return Err(DoubleZeroError::ReferenceCountNotZero.into()); } - user.status = UserStatus::Deleting; + if let Some(( + device_account, + user_tunnel_block_ext, + multicast_publisher_block_ext, + device_tunnel_ids_ext, + dz_prefix_accounts, + )) = deallocation_accounts + { + let owner_account = owner_account.unwrap(); - try_acc_write(&user, user_account, payer_account, accounts)?; + // Validate additional accounts + assert_eq!( + device_account.owner, program_id, + "Invalid Device Account Owner" + ); - #[cfg(test)] - msg!("Deleting: {:?}", user); + if user.device_pk != *device_account.key { + return Err(ProgramError::InvalidAccountData); + } + if user.owner != *owner_account.key { + return Err(ProgramError::InvalidAccountData); + } + + // Deallocate resources via helper (checks feature flag, validates PDAs) + resource_onchain_helpers::validate_and_deallocate_user_resources( + program_id, + &user, + user_tunnel_block_ext, + multicast_publisher_block_ext.as_ref().map(|a| *a), + device_tunnel_ids_ext, + &dz_prefix_accounts, + &globalstate, + )?; + + // Decrement tenant reference count if user has tenant assigned + if let Some(tenant_acc) = tenant_account { + assert_eq!( + tenant_acc.key, &user.tenant_pk, + "Tenant account doesn't match user's tenant" + ); + assert_eq!(tenant_acc.owner, program_id, "Invalid Tenant Account Owner"); + assert!(tenant_acc.is_writable, "Tenant Account is not writable"); + + let mut tenant = Tenant::try_from(tenant_acc)?; + tenant.reference_count = tenant + .reference_count + .checked_sub(1) + .ok_or(DoubleZeroError::InvalidIndex)?; + + try_acc_write(&tenant, tenant_acc, payer_account, accounts)?; + } + + // Decrement device counters + let mut device = Device::try_from(device_account)?; + device.reference_count = device.reference_count.saturating_sub(1); + device.users_count = device.users_count.saturating_sub(1); + match user.user_type { + UserType::Multicast => { + device.multicast_users_count = device.multicast_users_count.saturating_sub(1); + } + _ => { + device.unicast_users_count = device.unicast_users_count.saturating_sub(1); + } + } + + try_acc_write(&device, device_account, payer_account, accounts)?; + try_acc_close(user_account, owner_account)?; + + #[cfg(test)] + msg!("DeleteUser (atomic): User deallocated and closed"); + } else { + // Legacy path: just mark as Deleting + let mut user: User = User::try_from(user_account)?; + user.status = UserStatus::Deleting; + + try_acc_write(&user, user_account, payer_account, accounts)?; + + #[cfg(test)] + msg!("Deleting: {:?}", user); + } Ok(()) } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/resource_onchain_helpers.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/resource_onchain_helpers.rs index 0e8089d39..bcb3a67f5 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/resource_onchain_helpers.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/resource_onchain_helpers.rs @@ -5,14 +5,17 @@ use crate::{ resource::{allocate_id, allocate_ip, allocate_ip_from_first_available}, validation::validate_program_account, }, - resource::ResourceType, + resource::{IdOrIp, ResourceType}, state::{ feature_flags::{is_feature_enabled, FeatureFlag}, globalstate::GlobalState, + resource_extension::ResourceExtensionBorrowed, user::{User, UserType}, }, }; use doublezero_program_common::types::NetworkV4; +#[cfg(test)] +use solana_program::msg; use solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey}; use std::net::Ipv4Addr; @@ -119,3 +122,135 @@ pub fn validate_and_allocate_user_resources<'a>( Ok(()) } + +/// Validate and deallocate user resources back to ResourceExtension accounts. +/// Deallocates tunnel_net, tunnel_id, and conditionally dz_ip. +/// Deallocation is idempotent. +pub fn validate_and_deallocate_user_resources<'a>( + program_id: &Pubkey, + user: &User, + global_resource_ext: &AccountInfo<'a>, + multicast_publisher_block_ext: Option<&AccountInfo<'a>>, + device_tunnel_ids_ext: &AccountInfo<'a>, + dz_prefix_accounts: &[&AccountInfo<'a>], + globalstate: &GlobalState, +) -> ProgramResult { + // Check feature flag + if !is_feature_enabled(globalstate.feature_flags, FeatureFlag::OnChainAllocation) { + return Err(DoubleZeroError::FeatureNotEnabled.into()); + } + + // Validate global_resource_ext (UserTunnelBlock) + let (expected_user_tunnel_pda, _, _) = + get_resource_extension_pda(program_id, ResourceType::UserTunnelBlock); + validate_program_account!( + global_resource_ext, + program_id, + writable = true, + pda = Some(&expected_user_tunnel_pda), + "UserTunnelBlock" + ); + + // Validate multicast_publisher_block_ext if provided + if let Some(multicast_publisher_ext) = multicast_publisher_block_ext { + let (expected_multicast_publisher_pda, _, _) = + get_resource_extension_pda(program_id, ResourceType::MulticastPublisherBlock); + validate_program_account!( + multicast_publisher_ext, + program_id, + writable = true, + pda = Some(&expected_multicast_publisher_pda), + "MulticastPublisherBlock" + ); + } + + // Validate device_tunnel_ids_ext (TunnelIds) + let (expected_tunnel_ids_pda, _, _) = + get_resource_extension_pda(program_id, ResourceType::TunnelIds(user.device_pk, 0)); + validate_program_account!( + device_tunnel_ids_ext, + program_id, + writable = true, + pda = Some(&expected_tunnel_ids_pda), + "TunnelIds" + ); + + // Validate all DzPrefixBlock accounts + for (idx, dz_prefix_account) in dz_prefix_accounts.iter().enumerate() { + let (expected_dz_prefix_pda, _, _) = get_resource_extension_pda( + program_id, + ResourceType::DzPrefixBlock(user.device_pk, idx), + ); + validate_program_account!( + dz_prefix_account, + program_id, + writable = true, + pda = Some(&expected_dz_prefix_pda), + &format!("DzPrefixBlock[{idx}]") + ); + } + + // Deallocate tunnel_net from global UserTunnelBlock + { + let mut buffer = global_resource_ext.data.borrow_mut(); + let mut resource = ResourceExtensionBorrowed::inplace_from(&mut buffer[..])?; + let _deallocated = resource.deallocate(&IdOrIp::Ip(user.tunnel_net)); + #[cfg(test)] + msg!( + "Deallocated tunnel_net {}: {}", + user.tunnel_net, + _deallocated + ); + } + + // Deallocate tunnel_id from device TunnelIds + { + let mut buffer = device_tunnel_ids_ext.data.borrow_mut(); + let mut resource = ResourceExtensionBorrowed::inplace_from(&mut buffer[..])?; + let _deallocated = resource.deallocate(&IdOrIp::Id(user.tunnel_id)); + #[cfg(test)] + msg!("Deallocated tunnel_id {}: {}", user.tunnel_id, _deallocated); + } + + // Deallocate dz_ip (try MulticastPublisherBlock first, then DzPrefixBlock) + // Only deallocate if dz_ip is allocated (not client_ip and not UNSPECIFIED) + if user.dz_ip != user.client_ip && user.dz_ip != Ipv4Addr::UNSPECIFIED { + if let Ok(dz_ip_net) = NetworkV4::new(user.dz_ip, 32) { + let mut deallocated = false; + + // Try MulticastPublisherBlock first (for publishers) + if let Some(multicast_publisher_ext) = multicast_publisher_block_ext { + let mut buffer = multicast_publisher_ext.data.borrow_mut(); + let mut resource = ResourceExtensionBorrowed::inplace_from(&mut buffer[..])?; + deallocated = resource.deallocate(&IdOrIp::Ip(dz_ip_net)); + #[cfg(test)] + msg!( + "Deallocated dz_ip {} from MulticastPublisherBlock: {}", + dz_ip_net, + deallocated + ); + } + + // Fall back to DzPrefixBlock if not in MulticastPublisherBlock + if !deallocated { + for dz_prefix_account in dz_prefix_accounts.iter() { + let mut buffer = dz_prefix_account.data.borrow_mut(); + let mut resource = ResourceExtensionBorrowed::inplace_from(&mut buffer[..])?; + deallocated = resource.deallocate(&IdOrIp::Ip(dz_ip_net)); + #[cfg(test)] + msg!( + "Deallocated dz_ip {} from DzPrefixBlock {:?}: {}", + dz_ip_net, + dz_prefix_account.key, + deallocated + ); + if deallocated { + break; + } + } + } + } + } + + Ok(()) +} 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 9ae7b70d4..250830829 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs @@ -475,7 +475,10 @@ async fn test_accesspass_allow_multiple_ip() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs b/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs index 10dc503c8..0cda052a2 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs @@ -470,7 +470,10 @@ async fn test_old_user() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), 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 075f128cb..f3fd5d50a 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs @@ -599,7 +599,10 @@ async fn test_closeaccount_user_with_deallocation() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -1614,7 +1617,10 @@ async fn test_multicast_publisher_block_deallocation_and_reuse() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -1904,6 +1910,7 @@ async fn test_activate_user_already_activated_fails() { // ResourceExtension bitmap before attempting activation. This is resource-intensive // and may be better suited for a stress test file. + // ============================================================================ // Atomic Create+Allocate+Activate Tests (Issue 2402) // ============================================================================ @@ -2470,3 +2477,329 @@ async fn test_create_user_atomic_feature_flag_disabled() { println!("[PASS] test_create_user_atomic_feature_flag_disabled"); } + + +// ============================================================================ +// DeleteUser Atomic Tests +// ============================================================================ + +#[tokio::test] +async fn test_delete_user_atomic_with_deallocation() { + println!("[TEST] test_delete_user_atomic_with_deallocation"); + + let client_ip = [100, 0, 0, 10]; + let ( + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + user_pubkey, + accesspass_pubkey, + ( + user_tunnel_block_pubkey, + multicast_publisher_block_pubkey, + tunnel_ids_pubkey, + dz_prefix_block_pubkey, + ), + ) = setup_user_onchain_allocation_test(UserType::IBRLWithAllocatedIP, client_ip).await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Enable OnChainAllocation feature flag + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::SetFeatureFlags(SetFeatureFlagsArgs { + feature_flags: FeatureFlag::OnChainAllocation.to_mask(), + }), + vec![AccountMeta::new(globalstate_pubkey, false)], + &payer, + ) + .await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Activate user with on-chain allocation + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::ActivateUser(UserActivateArgs { + tunnel_id: 0, + tunnel_net: "0.0.0.0/0".parse().unwrap(), + dz_ip: [0, 0, 0, 0].into(), + dz_prefix_count: 1, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block_pubkey, false), + AccountMeta::new(multicast_publisher_block_pubkey, false), + AccountMeta::new(tunnel_ids_pubkey, false), + AccountMeta::new(dz_prefix_block_pubkey, false), + ], + &payer, + ) + .await; + + // Verify allocations exist + let user_tunnel_before = + get_resource_extension_data(&mut banks_client, user_tunnel_block_pubkey) + .await + .expect("UserTunnelBlock should exist"); + let tunnel_ids_before = get_resource_extension_data(&mut banks_client, tunnel_ids_pubkey) + .await + .expect("TunnelIds should exist"); + let dz_prefix_before = get_resource_extension_data(&mut banks_client, dz_prefix_block_pubkey) + .await + .expect("DzPrefixBlock should exist"); + + assert_eq!(user_tunnel_before.iter_allocated().len(), 2); + assert_eq!(tunnel_ids_before.iter_allocated().len(), 1); + assert_eq!(dz_prefix_before.iter_allocated().len(), 2); // reserved first IP + user + + // Verify device counters before delete + let device_before = get_account_data(&mut banks_client, device_pubkey) + .await + .expect("Device should exist") + .get_device() + .unwrap(); + assert_eq!(device_before.users_count, 1); + assert_eq!(device_before.unicast_users_count, 1); + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Atomic DeleteUser: should deallocate resources and close the account + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 1, + multicast_publisher_count: 0, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(user_tunnel_block_pubkey, false), + AccountMeta::new(tunnel_ids_pubkey, false), + AccountMeta::new(dz_prefix_block_pubkey, false), + AccountMeta::new(payer.pubkey(), false), // owner + ], + &payer, + ) + .await; + + // User account should be closed + let user = get_account_data(&mut banks_client, user_pubkey).await; + assert!( + user.is_none(), + "User account should be closed after atomic delete" + ); + + // Verify bitmap bits were deallocated + let user_tunnel_after = + get_resource_extension_data(&mut banks_client, user_tunnel_block_pubkey) + .await + .expect("UserTunnelBlock should exist"); + let tunnel_ids_after = get_resource_extension_data(&mut banks_client, tunnel_ids_pubkey) + .await + .expect("TunnelIds should exist"); + let dz_prefix_after = get_resource_extension_data(&mut banks_client, dz_prefix_block_pubkey) + .await + .expect("DzPrefixBlock should exist"); + + assert!( + user_tunnel_after.iter_allocated().is_empty(), + "UserTunnelBlock should have no allocations after atomic delete" + ); + assert!( + tunnel_ids_after.iter_allocated().is_empty(), + "TunnelIds should have no allocations after atomic delete" + ); + assert_eq!( + dz_prefix_after.iter_allocated().len(), + 1, + "DzPrefixBlock should have only reserved first IP after atomic delete" + ); + + // Verify device counters decremented + let device_after = get_account_data(&mut banks_client, device_pubkey) + .await + .expect("Device should exist") + .get_device() + .unwrap(); + assert_eq!(device_after.users_count, 0); + assert_eq!(device_after.unicast_users_count, 0); + + println!("[PASS] test_delete_user_atomic_with_deallocation"); +} + +#[tokio::test] +async fn test_delete_user_atomic_backward_compat() { + println!("[TEST] test_delete_user_atomic_backward_compat"); + + let client_ip = [100, 0, 0, 11]; + let ( + mut banks_client, + payer, + program_id, + globalstate_pubkey, + _device_pubkey, + user_pubkey, + accesspass_pubkey, + _resource_pubkeys, + ) = setup_user_onchain_allocation_test(UserType::IBRL, client_ip).await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Activate user with legacy path + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::ActivateUser(UserActivateArgs { + tunnel_id: 501, + tunnel_net: "169.254.0.0/25".parse().unwrap(), + dz_ip: [200, 0, 0, 1].into(), + dz_prefix_count: 0, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Legacy delete (dz_prefix_count=0) should set status to Deleting + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + // User should still exist with Deleting status + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should still exist after legacy delete") + .get_user() + .unwrap(); + assert_eq!( + user.status, + UserStatus::Deleting, + "User should be in Deleting status after legacy delete" + ); + + println!("[PASS] test_delete_user_atomic_backward_compat"); +} + +#[tokio::test] +async fn test_delete_user_atomic_feature_flag_disabled() { + println!("[TEST] test_delete_user_atomic_feature_flag_disabled"); + + let client_ip = [100, 0, 0, 12]; + let ( + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + user_pubkey, + accesspass_pubkey, + ( + user_tunnel_block_pubkey, + _multicast_publisher_block_pubkey, + tunnel_ids_pubkey, + dz_prefix_block_pubkey, + ), + ) = setup_user_onchain_allocation_test(UserType::IBRLWithAllocatedIP, client_ip).await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Activate user with legacy path (no feature flag needed for legacy activate) + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::ActivateUser(UserActivateArgs { + tunnel_id: 501, + tunnel_net: "169.254.0.0/25".parse().unwrap(), + dz_ip: [200, 0, 0, 1].into(), + dz_prefix_count: 0, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + + // Atomic delete WITHOUT feature flag enabled should fail + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 1, + multicast_publisher_count: 0, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(user_tunnel_block_pubkey, false), + AccountMeta::new(tunnel_ids_pubkey, false), + AccountMeta::new(dz_prefix_block_pubkey, false), + AccountMeta::new(payer.pubkey(), false), // owner + ], + &payer, + ) + .await; + + assert!( + result.is_err(), + "Atomic delete should fail when OnChainAllocation feature flag is disabled" + ); + + // User should still exist (not closed) + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should still exist after failed atomic delete") + .get_user() + .unwrap(); + assert_eq!( + user.status, + UserStatus::Activated, + "User status should be unchanged after failed atomic delete" + ); + + println!("[PASS] test_delete_user_atomic_feature_flag_disabled"); +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs b/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs index c8d8ab645..3f4449b4d 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_tests.rs @@ -473,7 +473,10 @@ async fn test_user() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -1486,7 +1489,10 @@ async fn test_user_delete_from_pending_ban() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -1550,7 +1556,10 @@ async fn test_user_delete_from_banned() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -1623,7 +1632,10 @@ async fn test_user_delete_from_out_of_credits() { &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 0, + multicast_publisher_count: 0, + }), vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/tenant/delete.rs b/smartcontract/sdk/rs/src/commands/tenant/delete.rs index 99ff67941..df8c29254 100644 --- a/smartcontract/sdk/rs/src/commands/tenant/delete.rs +++ b/smartcontract/sdk/rs/src/commands/tenant/delete.rs @@ -290,7 +290,7 @@ mod tests { client .expect_execute_transaction() .with( - predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs {})), + predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs::default())), predicate::always(), ) .times(1) diff --git a/smartcontract/sdk/rs/src/commands/user/delete.rs b/smartcontract/sdk/rs/src/commands/user/delete.rs index d0eb4522b..38f9686f4 100644 --- a/smartcontract/sdk/rs/src/commands/user/delete.rs +++ b/smartcontract/sdk/rs/src/commands/user/delete.rs @@ -3,6 +3,7 @@ use std::{collections::HashSet, net::Ipv4Addr, time::Duration}; use crate::{ commands::{ accesspass::get::GetAccessPassCommand, + device::get::GetDeviceCommand, globalstate::get::GetGlobalStateCommand, multicastgroup::{ list::ListMulticastGroupCommand, subscribe::SubscribeMulticastGroupCommand, @@ -13,7 +14,11 @@ use crate::{ }; use backon::{BlockingRetryable, ExponentialBuilder}; use doublezero_serviceability::{ - instructions::DoubleZeroInstruction, processors::user::delete::UserDeleteArgs, + instructions::DoubleZeroInstruction, + pda::get_resource_extension_pda, + processors::user::delete::UserDeleteArgs, + resource::ResourceType, + state::feature_flags::{is_feature_enabled, FeatureFlag}, }; use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Signature}; @@ -24,65 +29,72 @@ pub struct DeleteUserCommand { impl DeleteUserCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { - let (globalstate_pubkey, _globalstate) = GetGlobalStateCommand + let (globalstate_pubkey, globalstate) = GetGlobalStateCommand .execute(client) .map_err(|_err| eyre::eyre!("Globalstate not initialized"))?; + let use_onchain_deallocation = + is_feature_enabled(globalstate.feature_flags, FeatureFlag::OnChainAllocation); + let user = client .get(self.pubkey) .map_err(|_| eyre::eyre!("User not found ({})", self.pubkey))? .get_user() .map_err(|e| eyre::eyre!(e))?; - let unique_mgroup_pks: Vec = user - .publishers - .iter() - .chain(user.subscribers.iter()) - .copied() - .collect::>() - .into_iter() - .collect(); - let multicastgroups = ListMulticastGroupCommand {}.execute(client)?; - for mgroup_pk in &unique_mgroup_pks { - if multicastgroups.contains_key(mgroup_pk) { - SubscribeMulticastGroupCommand { - group_pk: *mgroup_pk, - user_pk: self.pubkey, - client_ip: user.client_ip, - publisher: false, - subscriber: false, + // With onchain deallocation, the program handles everything atomically — + // no need for multicast unsubscribe + retry logic. + if !use_onchain_deallocation { + let unique_mgroup_pks: Vec = user + .publishers + .iter() + .chain(user.subscribers.iter()) + .copied() + .collect::>() + .into_iter() + .collect(); + let multicastgroups = ListMulticastGroupCommand {}.execute(client)?; + for mgroup_pk in &unique_mgroup_pks { + if multicastgroups.contains_key(mgroup_pk) { + SubscribeMulticastGroupCommand { + group_pk: *mgroup_pk, + user_pk: self.pubkey, + client_ip: user.client_ip, + publisher: false, + subscriber: false, + } + .execute(client)?; } - .execute(client)?; } - } - if !user.publishers.is_empty() || !user.subscribers.is_empty() { - // timings are set to handle expected worst case activator reactions - let builder = ExponentialBuilder::new() - .with_max_times(8) // 1+2+4+8+16+32+32+32 = 127 seconds max - .with_min_delay(Duration::from_secs(1)) - .with_max_delay(Duration::from_secs(32)); - - // need to wait until activator is done and changes status from Updating - let get_user = || match (GetUserCommand { - pubkey: self.pubkey, - }) - .execute(client) - { - Ok((_, user)) => { - if user.status == UserStatus::Updating { - Err(()) - } else { - Ok(user) + if !user.publishers.is_empty() || !user.subscribers.is_empty() { + // timings are set to handle expected worst case activator reactions + let builder = ExponentialBuilder::new() + .with_max_times(8) // 1+2+4+8+16+32+32+32 = 127 seconds max + .with_min_delay(Duration::from_secs(1)) + .with_max_delay(Duration::from_secs(32)); + + // need to wait until activator is done and changes status from Updating + let get_user = || match (GetUserCommand { + pubkey: self.pubkey, + }) + .execute(client) + { + Ok((_, user)) => { + if user.status == UserStatus::Updating { + Err(()) + } else { + Ok(user) + } } - } - Err(_) => Err(()), - }; + Err(_) => Err(()), + }; - let _ = get_user - .retry(builder) - .call() - .map_err(|_| eyre::eyre!("Timeout waiting for user multicast unsubscribe"))?; + let _ = get_user + .retry(builder) + .call() + .map_err(|_| eyre::eyre!("Timeout waiting for user multicast unsubscribe"))?; + } } let (accesspass_pk, _) = GetAccessPassCommand { @@ -101,13 +113,71 @@ impl DeleteUserCommand { }) .ok_or_else(|| eyre::eyre!("You have no Access Pass"))?; + let mut accounts = vec![ + AccountMeta::new(self.pubkey, false), + AccountMeta::new(accesspass_pk, false), + AccountMeta::new(globalstate_pubkey, false), + ]; + + let (dz_prefix_count, multicast_publisher_count) = if use_onchain_deallocation { + let (_, device) = GetDeviceCommand { + pubkey_or_code: user.device_pk.to_string(), + } + .execute(client) + .map_err(|_| eyre::eyre!("Device not found"))?; + + let count = device.dz_prefixes.len(); + + // Device account (writable) + accounts.push(AccountMeta::new(user.device_pk, false)); + + // UserTunnelBlock (global) + let (user_tunnel_block_ext, _, _) = + get_resource_extension_pda(&client.get_program_id(), ResourceType::UserTunnelBlock); + accounts.push(AccountMeta::new(user_tunnel_block_ext, false)); + + // MulticastPublisherBlock (global) — always include for deallocation + let (multicast_publisher_block_ext, _, _) = get_resource_extension_pda( + &client.get_program_id(), + ResourceType::MulticastPublisherBlock, + ); + accounts.push(AccountMeta::new(multicast_publisher_block_ext, false)); + + // TunnelIds (per-device) + let (device_tunnel_ids_ext, _, _) = get_resource_extension_pda( + &client.get_program_id(), + ResourceType::TunnelIds(user.device_pk, 0), + ); + accounts.push(AccountMeta::new(device_tunnel_ids_ext, false)); + + // DzPrefixBlock accounts (per-device) + for idx in 0..count { + let (dz_prefix_ext, _, _) = get_resource_extension_pda( + &client.get_program_id(), + ResourceType::DzPrefixBlock(user.device_pk, idx), + ); + accounts.push(AccountMeta::new(dz_prefix_ext, false)); + } + + // Optional tenant + if user.tenant_pk != Pubkey::default() { + accounts.push(AccountMeta::new(user.tenant_pk, false)); + } + + // Owner account + accounts.push(AccountMeta::new(user.owner, false)); + + (count as u8, 1u8) + } else { + (0u8, 0u8) + }; + client.execute_transaction( - DoubleZeroInstruction::DeleteUser(UserDeleteArgs {}), - vec![ - AccountMeta::new(self.pubkey, false), - AccountMeta::new(accesspass_pk, false), - AccountMeta::new(globalstate_pubkey, false), - ], + DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count, + multicast_publisher_count, + }), + accounts, ) } } @@ -116,19 +186,26 @@ impl DeleteUserCommand { mod tests { use crate::{ commands::user::delete::DeleteUserCommand, tests::utils::create_test_client, - DoubleZeroClient, + DoubleZeroClient, MockDoubleZeroClient, }; use doublezero_program_common::types::NetworkV4; use doublezero_serviceability::{ instructions::DoubleZeroInstruction, - pda::{get_accesspass_pda, get_globalstate_pda, get_multicastgroup_pda}, + pda::{ + get_accesspass_pda, get_globalstate_pda, get_multicastgroup_pda, + get_resource_extension_pda, + }, processors::{ multicastgroup::subscribe::MulticastGroupSubscribeArgs, user::delete::UserDeleteArgs, }, + resource::ResourceType, state::{ accesspass::{AccessPass, AccessPassStatus, AccessPassType}, accountdata::AccountData, accounttype::AccountType, + device::Device, + feature_flags::FeatureFlag, + globalstate::GlobalState, multicastgroup::{MulticastGroup, MulticastGroupStatus}, user::{User, UserCYOA, UserStatus, UserType}, }, @@ -322,7 +399,7 @@ mod tests { client .expect_execute_transaction() .with( - predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs {})), + predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs::default())), predicate::eq(vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -522,7 +599,7 @@ mod tests { client .expect_execute_transaction() .with( - predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs {})), + predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs::default())), predicate::eq(vec![ AccountMeta::new(user_pubkey, false), AccountMeta::new(accesspass_pubkey, false), @@ -540,4 +617,138 @@ mod tests { assert!(res.is_ok()); } + + #[test] + fn test_delete_user_with_onchain_deallocation() { + let mut client = MockDoubleZeroClient::new(); + + let payer = Pubkey::new_unique(); + client.expect_get_payer().returning(move || payer); + let program_id = Pubkey::new_unique(); + client.expect_get_program_id().returning(move || program_id); + + let (globalstate_pubkey, bump_seed) = get_globalstate_pda(&program_id); + let globalstate = GlobalState { + account_type: AccountType::GlobalState, + bump_seed, + account_index: 0, + foundation_allowlist: vec![], + _device_allowlist: vec![], + _user_allowlist: vec![], + activator_authority_pk: Pubkey::new_unique(), + sentinel_authority_pk: Pubkey::new_unique(), + contributor_airdrop_lamports: 1_000_000_000, + user_airdrop_lamports: 40_000, + health_oracle_pk: Pubkey::new_unique(), + qa_allowlist: vec![], + feature_flags: FeatureFlag::OnChainAllocation.to_mask(), + reservation_authority_pk: Pubkey::default(), + }; + client + .expect_get() + .with(predicate::eq(globalstate_pubkey)) + .returning(move |_| Ok(AccountData::GlobalState(globalstate.clone()))); + + let user_pubkey = Pubkey::new_unique(); + let device_pk = Pubkey::new_unique(); + let client_ip = Ipv4Addr::new(192, 168, 1, 10); + + let user = User { + account_type: AccountType::User, + owner: payer, + bump_seed: 0, + index: 1, + tenant_pk: Pubkey::default(), + user_type: UserType::IBRLWithAllocatedIP, + device_pk, + cyoa_type: UserCYOA::GREOverDIA, + client_ip, + dz_ip: Ipv4Addr::new(10, 0, 0, 1), + tunnel_id: 100, + tunnel_net: "10.1.0.0/31".parse().unwrap(), + status: UserStatus::Activated, + publishers: vec![], + subscribers: vec![], + validator_pubkey: Pubkey::default(), + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + }; + + let owner = user.owner; + client + .expect_get() + .with(predicate::eq(user_pubkey)) + .returning(move |_| Ok(AccountData::User(user.clone()))); + + // Mock AccessPass fetch (UNSPECIFIED IP path) + let (accesspass_pubkey, _) = + get_accesspass_pda(&program_id, &Ipv4Addr::UNSPECIFIED, &payer); + let accesspass = AccessPass { + account_type: AccountType::AccessPass, + bump_seed: 0, + accesspass_type: AccessPassType::Prepaid, + client_ip: Ipv4Addr::UNSPECIFIED, + user_payer: payer, + last_access_epoch: 0, + connection_count: 0, + status: AccessPassStatus::Requested, + owner: payer, + mgroup_pub_allowlist: vec![], + mgroup_sub_allowlist: vec![], + tenant_allowlist: vec![], + flags: 0, + }; + client + .expect_get() + .with(predicate::eq(accesspass_pubkey)) + .returning(move |_| Ok(AccountData::AccessPass(accesspass.clone()))); + + // Mock Device fetch (1 dz_prefix) + let device = Device { + account_type: AccountType::Device, + dz_prefixes: "10.0.0.0/24".parse().unwrap(), + ..Default::default() + }; + client + .expect_get() + .with(predicate::eq(device_pk)) + .returning(move |_| Ok(AccountData::Device(device.clone()))); + + // Compute ResourceExtension PDAs + let (user_tunnel_block_ext, _, _) = + get_resource_extension_pda(&program_id, ResourceType::UserTunnelBlock); + let (multicast_publisher_block_ext, _, _) = + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock); + let (device_tunnel_ids_ext, _, _) = + get_resource_extension_pda(&program_id, ResourceType::TunnelIds(device_pk, 0)); + let (dz_prefix_ext, _, _) = + get_resource_extension_pda(&program_id, ResourceType::DzPrefixBlock(device_pk, 0)); + + client + .expect_execute_transaction() + .with( + predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { + dz_prefix_count: 1, + multicast_publisher_count: 1, + })), + predicate::eq(vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(device_pk, false), + AccountMeta::new(user_tunnel_block_ext, false), + AccountMeta::new(multicast_publisher_block_ext, false), + AccountMeta::new(device_tunnel_ids_ext, false), + AccountMeta::new(dz_prefix_ext, false), + AccountMeta::new(owner, false), + ]), + ) + .returning(|_, _| Ok(Signature::new_unique())); + + let res = DeleteUserCommand { + pubkey: user_pubkey, + } + .execute(&client); + + assert!(res.is_ok()); + } }