diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d05d543a..df3e33be3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,7 @@ All notable changes to this project will be documented in this file. - Add on-chain validation to reject CloseAccountDevice when device has active references (reference_count > 0) - Allow contributor owner to update ops manager key - Add new arguments on create interface cli command + - Restrict serviceability Delete* instructions (Location, Exchange, Contributor, Device, Link, User, MulticastGroup) to accounts in Activated or Suspended status - Serviceability: enforce that resume instructions for locations, exchanges, contributors, devices, links, and users only succeed when the account status is `Suspended`, returning `InvalidStatus` otherwise, and add tests to cover the new behavior. - RequestBanUser: only allow requests when user.status is Activated or Suspended; otherwise return InvalidStatus - Serviceability: require device interfaces to be in `Pending` status before they can be rejected, and add tests to cover the new status check diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/contributor/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/contributor/delete.rs index c78f813f4..f9a411332 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/contributor/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/contributor/delete.rs @@ -72,7 +72,9 @@ pub fn process_delete_contributor( } let contributor = Contributor::try_from(contributor_account)?; - if contributor.status != ContributorStatus::Activated { + if contributor.status != ContributorStatus::Activated + && contributor.status != ContributorStatus::Suspended + { return Err(DoubleZeroError::InvalidStatus.into()); } if contributor.reference_count > 0 { diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/device/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/device/delete.rs index d0a4d951e..66a6ddd78 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/device/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/device/delete.rs @@ -82,6 +82,10 @@ pub fn process_delete_device( let mut device: Device = Device::try_from(device_account)?; + if device.status != DeviceStatus::Activated { + return Err(DoubleZeroError::InvalidStatus.into()); + } + if device.reference_count > 0 { return Err(DoubleZeroError::ReferenceCountNotZero.into()); } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/exchange/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/exchange/delete.rs index b507d143e..913db647f 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/exchange/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/exchange/delete.rs @@ -1,11 +1,8 @@ -use crate::{ - error::DoubleZeroError, - serializer::try_acc_close, - state::{exchange::Exchange, globalstate::GlobalState}, -}; +use core::fmt; + use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; -use core::fmt; + #[cfg(test)] use solana_program::msg; use solana_program::{ @@ -14,6 +11,15 @@ use solana_program::{ pubkey::Pubkey, }; +use crate::{ + error::DoubleZeroError, + serializer::try_acc_close, + state::{ + exchange::{Exchange, *}, + globalstate::GlobalState, + }, +}; + #[derive(BorshSerialize, BorshDeserializeIncremental, PartialEq, Clone, Default)] pub struct ExchangeDeleteArgs {} @@ -70,6 +76,11 @@ pub fn process_delete_exchange( let exchange = Exchange::try_from(exchange_account)?; + if exchange.status != ExchangeStatus::Activated && exchange.status != ExchangeStatus::Suspended + { + return Err(DoubleZeroError::InvalidStatus.into()); + } + if exchange.reference_count > 0 { return Err(DoubleZeroError::ReferenceCountNotZero.into()); } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/link/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/link/delete.rs index 19a763b06..0cd082a32 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/link/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/link/delete.rs @@ -72,10 +72,10 @@ pub fn process_delete_link( return Err(DoubleZeroError::InvalidOwnerPubkey.into()); } - // Any link can be deleted by its contributor or foundation allowlist on any status + // Any link can be deleted by its contributor or foundation allowlist when Activated or Suspended let mut link: Link = Link::try_from(link_account)?; - if !payer_in_foundation && link.contributor_pk != *contributor_account.key { - return Err(DoubleZeroError::NotAllowed.into()); + if link.status != LinkStatus::Activated { + return Err(DoubleZeroError::InvalidStatus.into()); } link.status = LinkStatus::Deleting; diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/location/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/location/delete.rs index 1fcfc64ac..8806cfd07 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/location/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/location/delete.rs @@ -69,7 +69,8 @@ pub fn process_delete_location( AccountType::Location, "Invalid Account Type" ); - if location.status != LocationStatus::Activated { + if location.status != LocationStatus::Activated && location.status != LocationStatus::Suspended + { return Err(DoubleZeroError::InvalidStatus.into()); } if location.reference_count > 0 { diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/delete.rs index 6e4f8e9a2..32d60f5f0 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/delete.rs @@ -68,7 +68,9 @@ pub fn process_delete_multicastgroup( let mut multicastgroup: MulticastGroup = MulticastGroup::try_from(multicastgroup_account)?; - if multicastgroup.status != MulticastGroupStatus::Activated { + if multicastgroup.status != MulticastGroupStatus::Activated + && multicastgroup.status != MulticastGroupStatus::Suspended + { return Err(DoubleZeroError::InvalidStatus.into()); } multicastgroup.status = MulticastGroupStatus::Deleting; diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index 9796c1636..6db9e3e33 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -124,6 +124,10 @@ pub fn process_delete_user( try_acc_write(&accesspass, accesspass_account, payer_account, accounts)?; } + if user.status != UserStatus::Activated && user.status != UserStatus::SuspendedDeprecated { + return Err(DoubleZeroError::InvalidStatus.into()); + } + user.status = UserStatus::Deleting; try_acc_write(&user, user_account, payer_account, accounts)?; diff --git a/smartcontract/programs/doublezero-serviceability/tests/contributor_test.rs b/smartcontract/programs/doublezero-serviceability/tests/contributor_test.rs index 3318359fb..19175ee9d 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/contributor_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/contributor_test.rs @@ -399,12 +399,11 @@ async fn test_contributor() { } #[tokio::test] -async fn test_suspend_contributor_from_suspended_fails() { +async fn test_contributor_delete_from_suspended() { let (mut banks_client, program_id, payer, recent_blockhash) = init_test().await; let (program_config_pubkey, _) = get_program_config_pda(&program_id); let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); - let owner = Pubkey::new_unique(); // Initialize global state execute_transaction( @@ -420,7 +419,7 @@ async fn test_suspend_contributor_from_suspended_fails() { ) .await; - // Create a contributor + let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); let globalstate_account = get_globalstate(&mut banks_client, globalstate_pubkey).await; let (contributor_pubkey, _) = get_contributor_pda(&program_id, globalstate_account.account_index + 1); @@ -430,18 +429,18 @@ async fn test_suspend_contributor_from_suspended_fails() { recent_blockhash, program_id, DoubleZeroInstruction::CreateContributor(ContributorCreateArgs { - code: "test".to_string(), + code: "la".to_string(), }), vec![ AccountMeta::new(contributor_pubkey, false), - AccountMeta::new(owner, false), + AccountMeta::new(payer.pubkey(), false), AccountMeta::new(globalstate_pubkey, false), ], &payer, ) .await; - // First suspend (should succeed) + // Suspend and then delete directly from Suspended execute_transaction( &mut banks_client, recent_blockhash, @@ -455,20 +454,18 @@ async fn test_suspend_contributor_from_suspended_fails() { ) .await; - // Verify contributor is suspended - let contributor = get_account_data(&mut banks_client, contributor_pubkey) + let contributor_la = get_account_data(&mut banks_client, contributor_pubkey) .await .expect("Unable to get Account") .get_contributor() .unwrap(); - assert_eq!(contributor.status, ContributorStatus::Suspended); + assert_eq!(contributor_la.status, ContributorStatus::Suspended); - // Second suspend (should fail with InvalidStatus) - let result = try_execute_transaction( + execute_transaction( &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::SuspendContributor(ContributorSuspendArgs {}), + DoubleZeroInstruction::DeleteContributor(ContributorDeleteArgs {}), vec![ AccountMeta::new(contributor_pubkey, false), AccountMeta::new(globalstate_pubkey, false), @@ -477,12 +474,6 @@ async fn test_suspend_contributor_from_suspended_fails() { ) .await; - assert!(result.is_err()); - let error_string = format!("{:?}", result.unwrap_err()); - assert!( - error_string.contains("Custom(7)"), - "Expected InvalidStatus error (Custom(7)), got: {}", - error_string - ); - println!("✅ Suspending already-suspended contributor correctly fails with InvalidStatus"); + let contributor_la = get_account_data(&mut banks_client, contributor_pubkey).await; + assert_eq!(contributor_la, None); } diff --git a/smartcontract/programs/doublezero-serviceability/tests/exchange_test.rs b/smartcontract/programs/doublezero-serviceability/tests/exchange_test.rs index 8158f3ca0..175c9ad2f 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/exchange_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/exchange_test.rs @@ -243,6 +243,123 @@ async fn test_exchange() { println!("🟢 End test_exchange"); } +#[tokio::test] +async fn test_exchange_delete_from_suspended() { + let (mut banks_client, program_id, payer, recent_blockhash) = init_test().await; + + let (program_config_pubkey, _) = get_program_config_pda(&program_id); + let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); + + 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; + + let (globalconfig_pubkey, _) = get_globalconfig_pda(&program_id); + + let (device_tunnel_block_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::DeviceTunnelBlock); + let (user_tunnel_block_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::UserTunnelBlock); + let (multicastgroup_block_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::MulticastGroupBlock); + let (link_ids_pda, _, _) = get_resource_extension_pda(&program_id, ResourceType::LinkIds); + let (segment_routing_ids_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::SegmentRoutingIds); + + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::SetGlobalConfig(SetGlobalConfigArgs { + local_asn: 65000, + remote_asn: 65001, + device_tunnel_block: "10.0.0.0/24".parse().unwrap(), + user_tunnel_block: "10.0.0.0/24".parse().unwrap(), + multicastgroup_block: "224.0.0.0/16".parse().unwrap(), + next_bgp_community: None, + }), + vec![ + AccountMeta::new(globalconfig_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(device_tunnel_block_pda, false), + AccountMeta::new(user_tunnel_block_pda, false), + AccountMeta::new(multicastgroup_block_pda, false), + AccountMeta::new(link_ids_pda, false), + AccountMeta::new(segment_routing_ids_pda, false), + ], + &payer, + ) + .await; + + let globalstate_account = get_globalstate(&mut banks_client, globalstate_pubkey).await; + let (exchange_pubkey, _) = get_exchange_pda(&program_id, globalstate_account.account_index + 1); + + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateExchange(ExchangeCreateArgs { + code: "la".to_string(), + name: "Los Angeles".to_string(), + lat: 1.234, + lng: 4.567, + reserved: 0, + }), + vec![ + AccountMeta::new(exchange_pubkey, false), + AccountMeta::new(globalconfig_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::SuspendExchange(ExchangeSuspendArgs {}), + vec![ + AccountMeta::new(exchange_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + let exchange_la = get_account_data(&mut banks_client, exchange_pubkey) + .await + .expect("Unable to get Account") + .get_exchange() + .unwrap(); + assert_eq!(exchange_la.status, ExchangeStatus::Suspended); + + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::DeleteExchange(ExchangeDeleteArgs {}), + vec![ + AccountMeta::new(exchange_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + let exchange_la = get_account_data(&mut banks_client, exchange_pubkey).await; + assert_eq!(exchange_la, None); +} + #[tokio::test] async fn test_exchange_owner_and_foundation_can_update_status() { let (mut banks_client, program_id, payer, recent_blockhash) = init_test().await; diff --git a/smartcontract/programs/doublezero-serviceability/tests/location_test.rs b/smartcontract/programs/doublezero-serviceability/tests/location_test.rs index c76424676..44fa432e8 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/location_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/location_test.rs @@ -200,13 +200,13 @@ async fn test_location() { } #[tokio::test] -async fn test_suspend_location_from_suspended_fails() { +async fn test_location_delete_from_suspended() { let (mut banks_client, program_id, payer, recent_blockhash) = init_test().await; let (program_config_pubkey, _) = get_program_config_pda(&program_id); let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); - // Initialize global state + // Init global state execute_transaction( &mut banks_client, recent_blockhash, @@ -220,7 +220,8 @@ async fn test_suspend_location_from_suspended_fails() { ) .await; - // Create and suspend a location + // Create location + let (globalstate_pubkey, _) = get_globalstate_pda(&program_id); let globalstate_account = get_globalstate(&mut banks_client, globalstate_pubkey).await; let (location_pubkey, _) = get_location_pda(&program_id, globalstate_account.account_index + 1); @@ -229,11 +230,11 @@ async fn test_suspend_location_from_suspended_fails() { recent_blockhash, program_id, DoubleZeroInstruction::CreateLocation(LocationCreateArgs { - code: "test".to_string(), - name: "Test Location".to_string(), + code: "la".to_string(), + name: "Los Angeles".to_string(), country: "us".to_string(), - lat: 1.0, - lng: 2.0, + lat: 1.234, + lng: 4.567, loc_id: 0, }), vec![ @@ -244,7 +245,7 @@ async fn test_suspend_location_from_suspended_fails() { ) .await; - // First suspend (should succeed) + // Suspend and then delete directly from Suspended execute_transaction( &mut banks_client, recent_blockhash, @@ -258,20 +259,18 @@ async fn test_suspend_location_from_suspended_fails() { ) .await; - // Verify location is suspended - let location = get_account_data(&mut banks_client, location_pubkey) + let location_la = get_account_data(&mut banks_client, location_pubkey) .await .expect("Unable to get Account") .get_location() .unwrap(); - assert_eq!(location.status, LocationStatus::Suspended); + assert_eq!(location_la.status, LocationStatus::Suspended); - // Second suspend (should fail with InvalidStatus) - let result = try_execute_transaction( + execute_transaction( &mut banks_client, recent_blockhash, program_id, - DoubleZeroInstruction::SuspendLocation(LocationSuspendArgs {}), + DoubleZeroInstruction::DeleteLocation(LocationDeleteArgs {}), vec![ AccountMeta::new(location_pubkey, false), AccountMeta::new(globalstate_pubkey, false), @@ -280,12 +279,6 @@ async fn test_suspend_location_from_suspended_fails() { ) .await; - assert!(result.is_err()); - let error_string = format!("{:?}", result.unwrap_err()); - assert!( - error_string.contains("Custom(7)"), - "Expected InvalidStatus error (Custom(7)), got: {}", - error_string - ); - println!("✅ Suspending already-suspended location correctly fails with InvalidStatus"); + let location_la = get_account_data(&mut banks_client, location_pubkey).await; + assert_eq!(location_la, None); }