From fb37e06c15b1f5770fd38e71d1f6afab6e1e5cc6 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Mon, 9 Mar 2026 17:11:30 +0000 Subject: [PATCH 01/11] smartcontract: scaffold Permission account state and instruction variants --- .../programs/doublezero-serviceability/src/state/permission.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs index a6c12376e9..6af4f7d830 100644 --- a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs +++ b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs @@ -57,6 +57,7 @@ pub enum PermissionStatus { None = 0, Activated = 1, Suspended = 2, + Deleting = 3, } impl From for PermissionStatus { @@ -65,6 +66,7 @@ impl From for PermissionStatus { 0 => PermissionStatus::None, 1 => PermissionStatus::Activated, 2 => PermissionStatus::Suspended, + 3 => PermissionStatus::Deleting, _ => PermissionStatus::None, } } @@ -76,6 +78,7 @@ impl fmt::Display for PermissionStatus { PermissionStatus::None => write!(f, "none"), PermissionStatus::Activated => write!(f, "activated"), PermissionStatus::Suspended => write!(f, "suspended"), + PermissionStatus::Deleting => write!(f, "deleting"), } } } From 4c489d7f61e535857a0944e208e7fcd20484c9d3 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Mon, 9 Mar 2026 17:43:26 +0000 Subject: [PATCH 02/11] smartcontract: implement Permission account CRUD and authorize() mechanism --- .../programs/doublezero-serviceability/src/state/permission.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs index 6af4f7d830..a6c12376e9 100644 --- a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs +++ b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs @@ -57,7 +57,6 @@ pub enum PermissionStatus { None = 0, Activated = 1, Suspended = 2, - Deleting = 3, } impl From for PermissionStatus { @@ -66,7 +65,6 @@ impl From for PermissionStatus { 0 => PermissionStatus::None, 1 => PermissionStatus::Activated, 2 => PermissionStatus::Suspended, - 3 => PermissionStatus::Deleting, _ => PermissionStatus::None, } } @@ -78,7 +76,6 @@ impl fmt::Display for PermissionStatus { PermissionStatus::None => write!(f, "none"), PermissionStatus::Activated => write!(f, "activated"), PermissionStatus::Suspended => write!(f, "suspended"), - PermissionStatus::Deleting => write!(f, "deleting"), } } } From 97fc8bfe8f7f40fd01afbc3b01c8a5e4d4d815bd Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Mon, 9 Mar 2026 17:28:55 +0000 Subject: [PATCH 03/11] smartcontract: enforce Permission-based authorization in existing instructions --- CHANGELOG.md | 6 ++ .../src/processors/accesspass/close.rs | 20 +++-- .../src/processors/accesspass/set.rs | 33 +++++---- .../src/processors/user/delete.rs | 15 +++- .../src/processors/user/requestban.rs | 13 +++- smartcontract/sdk/rs/src/client.rs | 73 ++++++++++++++++++- .../sdk/rs/src/commands/accesspass/close.rs | 4 +- .../sdk/rs/src/commands/accesspass/set.rs | 4 +- .../sdk/rs/src/commands/permission/create.rs | 4 +- .../sdk/rs/src/commands/permission/delete.rs | 4 +- .../sdk/rs/src/commands/permission/resume.rs | 4 +- .../sdk/rs/src/commands/permission/suspend.rs | 4 +- .../sdk/rs/src/commands/permission/update.rs | 4 +- .../sdk/rs/src/commands/tenant/delete.rs | 12 +-- .../sdk/rs/src/commands/user/delete.rs | 10 +-- .../sdk/rs/src/commands/user/requestban.rs | 4 +- smartcontract/sdk/rs/src/doublezeroclient.rs | 9 +++ 17 files changed, 164 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e285d2140..cec3e7c919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -565,6 +565,12 @@ All notable changes to this project will be documented in this file. - Add onchain parent DZD discovery to geoprobe-agent: periodically queries the Geolocation program for this probe's parent devices and resolves their metrics publisher keys from Serviceability, replacing the need for static `--parent-dzd` CLI flags. Static parents from CLI are merged with onchain parents, with onchain taking precedence for duplicate keys. - Optimize inbound probe-measured RTT accuracy: pre-sign both TWAMP probes before network I/O so probe 1 fires immediately after reply 0 with no signing delay, measure Tx-to-Rx interval (reply 0 Tx → probe 1 Rx) instead of Rx-to-Rx to exclude processing overhead on both sides, use kernel `SO_TIMESTAMPNS` receive timestamps on the reflector, and add a 15ms busy-poll window on the sender to avoid scheduler wakeup latency - Optimize outbound probe RTT accuracy: send a staggered warmup probe on a separate socket 2ms before the measurement probe to wake the reflector's thread, then take the min RTT of both +- Onchain Programs + - Serviceability: add `Permission` account with `CreatePermission`, `UpdatePermission`, `DeletePermission`, `SuspendPermission`, and `ResumePermission` instructions for managing per-keypair permission bitmasks onchain +- SDK + - Split `execute_transaction` into `execute_transaction` (no auth) and `execute_authorized_transaction` (injects Permission PDA) to avoid breaking processors that use `accounts.len()` for optional-account detection +- CLI + - Add `permission get`, `permission list`, and `permission set` commands with table and JSON output; `permission set` supports incremental `--add` / `--remove` flags and creates or updates the account as needed ## [v0.11.0](https://github.com/malbeclabs/doublezero/compare/client/v0.10.0...client/v0.11.0) - 2026-03-12 diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs index 89d2dde6d0..3a4b1f47b1 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs @@ -1,7 +1,11 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, serializer::try_acc_close, - state::{accesspass::AccessPass, accounttype::AccountType, globalstate::GlobalState}, + state::{ + accesspass::AccessPass, accounttype::AccountType, globalstate::GlobalState, + permission::permission_flags, + }, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -70,13 +74,15 @@ pub fn process_close_access_pass( "Invalid System Program Account Owner" ); - // Parse the global state account & check if the payer is in the allowlist + // Parse the global state account & check authorization let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) - && globalstate.feed_authority_pk != *payer_account.key - { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::ACCESS_PASS_ADMIN, + )?; if let Ok(data) = accesspass_account.try_borrow_data() { let account_type: AccountType = data[0].into(); diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs index 71a6f313ac..0f876fad6e 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs @@ -1,4 +1,5 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::*, processors::accesspass::airdrop_user_credits, @@ -8,6 +9,7 @@ use crate::{ accesspass::{AccessPass, AccessPassStatus, AccessPassType, ALLOW_MULTIPLE_IP}, accounttype::AccountType, globalstate::GlobalState, + permission::permission_flags, tenant::Tenant, }, }; @@ -111,17 +113,12 @@ pub fn process_set_access_pass( "Invalid System Program Account Owner" ); - // Parse the global state account & resolve authorization. A caller is allowed if any of: - // - they are the sentinel authority, - // - they are the feed authority, - // - they are in the foundation allowlist, or + // Parse the global state account & resolve authorization. A caller is allowed if either: + // - they pass the ACCESS_PASS_ADMIN permission check (foundation allowlist, sentinel + // authority, feed authority, or a Permission account granting ACCESS_PASS_ADMIN), or // - they are an administrator of the tenant being added (tenant_add_account). let globalstate = GlobalState::try_from(globalstate_account)?; - let is_privileged = globalstate.sentinel_authority_pk == *payer_account.key - || globalstate.feed_authority_pk == *payer_account.key - || globalstate.foundation_allowlist.contains(payer_account.key); - // Pre-deserialize the tenant_add account when present so we can both authorize the caller // and later increment its reference_count without double-reading it. let tenant_add_pre = match tenant_add_account { @@ -137,14 +134,20 @@ pub fn process_set_access_pass( .map(|t| t.administrators.contains(payer_account.key)) .unwrap_or(false); + // A caller is "privileged" when they pass the ACCESS_PASS_ADMIN permission check + // (foundation allowlist, sentinel authority, feed authority, or a Permission account + // granting ACCESS_PASS_ADMIN). Privileged callers retain unrestricted authority for the + // tenant_remove path below; a tenant administrator is only authorized for their own tenant. + let is_privileged = authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::ACCESS_PASS_ADMIN, + ) + .is_ok(); + if !is_privileged && !is_tenant_admin { - msg!( - "sentinel_authority_pk: {} feed_authority_pk: {} payer: {} foundation_allowlist: {:?}", - globalstate.sentinel_authority_pk, - globalstate.feed_authority_pk, - payer_account.key, - globalstate.foundation_allowlist - ); return Err(DoubleZeroError::NotAllowed.into()); } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs index 022429104b..f1a6c18945 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs @@ -1,4 +1,5 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::get_accesspass_pda, processors::validation::validate_program_account, @@ -7,6 +8,7 @@ use crate::{ accesspass::{AccessPass, AccessPassStatus}, device::Device, globalstate::GlobalState, + permission::permission_flags, tenant::Tenant, user::*, }, @@ -128,10 +130,15 @@ pub fn process_delete_user( let user: User = User::try_from(user_account)?; let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) - && user.owner != *payer_account.key - { - return Err(DoubleZeroError::NotAllowed.into()); + // The user owner can always delete their own account without a Permission account. + if user.owner != *payer_account.key { + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::USER_ADMIN, + )?; } let (accesspass_pda, _) = get_accesspass_pda(program_id, &user.client_ip, &user.owner); diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/requestban.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/requestban.rs index 5fce939be9..5d92d22337 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/requestban.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/requestban.rs @@ -1,8 +1,9 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, processors::validation::validate_program_account, serializer::try_acc_write, - state::{globalstate::GlobalState, user::*}, + state::{globalstate::GlobalState, permission::permission_flags, user::*}, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -100,9 +101,13 @@ pub fn process_request_ban_user( ); let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::USER_ADMIN, + )?; let mut user: User = User::try_from(user_account)?; if !can_request_ban(user.status) { diff --git a/smartcontract/sdk/rs/src/client.rs b/smartcontract/sdk/rs/src/client.rs index 93e9938c04..cee214a208 100644 --- a/smartcontract/sdk/rs/src/client.rs +++ b/smartcontract/sdk/rs/src/client.rs @@ -6,7 +6,8 @@ use std::time::Duration; use crate::config::default_program_id; use doublezero_serviceability::{ - error::DoubleZeroError, instructions::*, state::accounttype::AccountType, + error::DoubleZeroError, instructions::*, pda::get_permission_pda, + state::accounttype::AccountType, }; use eyre::{bail, eyre, OptionExt}; use log::debug; @@ -482,6 +483,66 @@ impl DZClient { Ok(errors) } + + fn build_and_send( + &self, + instruction: DoubleZeroInstruction, + accounts: Vec, + with_permission: bool, + ) -> eyre::Result { + let payer = self + .payer + .as_ref() + .ok_or_eyre("No default signer found, run \"doublezero keygen\" to create a new one")?; + let data = instruction.pack(); + + let mut trailing = vec![ + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(program::id(), false), + ]; + if with_permission { + let (permission_pda, _) = get_permission_pda(&self.program_id, &payer.pubkey()); + if self.client.get_account(&permission_pda).is_ok() { + trailing.push(AccountMeta::new_readonly(permission_pda, false)); + } + } + + let mut transaction = Transaction::new_with_payer( + &[Instruction::new_with_bytes( + self.program_id, + &data, + [accounts, trailing].concat(), + )], + Some(&payer.pubkey()), + ); + + let blockhash = self.client.get_latest_blockhash().map_err(|e| eyre!(e))?; + transaction.sign(&[&payer], blockhash); + + debug!("Simulating transaction: {transaction:?}"); + + let result = self.client.simulate_transaction(&transaction)?; + if result.value.err.is_some() { + eprintln!("Program Logs:"); + if let Some(logs) = result.value.logs { + for log in logs { + eprintln!("{log}"); + } + } + } + + if let Some(TransactionError::InstructionError(_index, InstructionError::Custom(number))) = + result.value.err + { + return Err(eyre!(DoubleZeroError::from(number))); + } else if let Some(err) = result.value.err { + return Err(eyre!(err)); + } + + self.client + .send_and_confirm_transaction(&transaction) + .map_err(|e| eyre!(e)) + } } impl DoubleZeroClient for DZClient { @@ -572,7 +633,7 @@ impl DoubleZeroClient for DZClient { instruction: DoubleZeroInstruction, accounts: Vec, ) -> eyre::Result { - self.execute_transaction_inner(instruction, accounts, false) + self.build_and_send(instruction, accounts, false) } fn execute_transaction_quiet( @@ -583,6 +644,14 @@ impl DoubleZeroClient for DZClient { self.execute_transaction_inner(instruction, accounts, true) } + fn execute_authorized_transaction( + &self, + instruction: DoubleZeroInstruction, + accounts: Vec, + ) -> eyre::Result { + self.build_and_send(instruction, accounts, true) + } + fn gets(&self, account_type: AccountType) -> eyre::Result> { let account_type = account_type as u8; let filters = vec![RpcFilterType::Memcmp(Memcmp::new( diff --git a/smartcontract/sdk/rs/src/commands/accesspass/close.rs b/smartcontract/sdk/rs/src/commands/accesspass/close.rs index 364676a6c4..15cc77b179 100644 --- a/smartcontract/sdk/rs/src/commands/accesspass/close.rs +++ b/smartcontract/sdk/rs/src/commands/accesspass/close.rs @@ -15,7 +15,7 @@ impl CloseAccessPassCommand { .execute(client) .map_err(|_err| eyre::eyre!("Globalstate not initialized"))?; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::CloseAccessPass(CloseAccessPassArgs {}), vec![ AccountMeta::new(self.pubkey, false), @@ -50,7 +50,7 @@ mod tests { let (pda_pubkey, _) = get_accesspass_pda(&client.get_program_id(), &client_ip, &payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::CloseAccessPass( CloseAccessPassArgs {}, diff --git a/smartcontract/sdk/rs/src/commands/accesspass/set.rs b/smartcontract/sdk/rs/src/commands/accesspass/set.rs index 6a16d22ffd..227b420e2a 100644 --- a/smartcontract/sdk/rs/src/commands/accesspass/set.rs +++ b/smartcontract/sdk/rs/src/commands/accesspass/set.rs @@ -67,7 +67,7 @@ impl SetAccessPassCommand { accounts.push(AccountMeta::new(self.tenant, false)); } - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { accesspass_type: self.accesspass_type.clone(), client_ip: self.client_ip, @@ -145,7 +145,7 @@ mod tests { .returning(|_| Err(eyre::eyre!("account not found"))); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { accesspass_type: AccessPassType::Prepaid, diff --git a/smartcontract/sdk/rs/src/commands/permission/create.rs b/smartcontract/sdk/rs/src/commands/permission/create.rs index 3c8b78d50e..af5887d448 100644 --- a/smartcontract/sdk/rs/src/commands/permission/create.rs +++ b/smartcontract/sdk/rs/src/commands/permission/create.rs @@ -20,7 +20,7 @@ impl CreatePermissionCommand { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &self.user_payer); client - .execute_transaction( + .execute_authorized_transaction( DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { user_payer: self.user_payer, permissions: self.permissions, @@ -58,7 +58,7 @@ mod tests { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &user_payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::CreatePermission( PermissionCreateArgs { diff --git a/smartcontract/sdk/rs/src/commands/permission/delete.rs b/smartcontract/sdk/rs/src/commands/permission/delete.rs index 928481de0b..2aa709d5e2 100644 --- a/smartcontract/sdk/rs/src/commands/permission/delete.rs +++ b/smartcontract/sdk/rs/src/commands/permission/delete.rs @@ -14,7 +14,7 @@ impl DeletePermissionCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::DeletePermission(PermissionDeleteArgs {}), vec![ AccountMeta::new(self.permission_pda, false), @@ -47,7 +47,7 @@ mod tests { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &user_payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeletePermission( PermissionDeleteArgs {}, diff --git a/smartcontract/sdk/rs/src/commands/permission/resume.rs b/smartcontract/sdk/rs/src/commands/permission/resume.rs index e855259455..38e3dd7fd4 100644 --- a/smartcontract/sdk/rs/src/commands/permission/resume.rs +++ b/smartcontract/sdk/rs/src/commands/permission/resume.rs @@ -14,7 +14,7 @@ impl ResumePermissionCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::ResumePermission(PermissionResumeArgs {}), vec![ AccountMeta::new(self.permission_pda, false), @@ -47,7 +47,7 @@ mod tests { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &user_payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::ResumePermission( PermissionResumeArgs {}, diff --git a/smartcontract/sdk/rs/src/commands/permission/suspend.rs b/smartcontract/sdk/rs/src/commands/permission/suspend.rs index 814c8930c1..36d8ff8e11 100644 --- a/smartcontract/sdk/rs/src/commands/permission/suspend.rs +++ b/smartcontract/sdk/rs/src/commands/permission/suspend.rs @@ -14,7 +14,7 @@ impl SuspendPermissionCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::SuspendPermission(PermissionSuspendArgs {}), vec![ AccountMeta::new(self.permission_pda, false), @@ -47,7 +47,7 @@ mod tests { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &user_payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::SuspendPermission( PermissionSuspendArgs {}, diff --git a/smartcontract/sdk/rs/src/commands/permission/update.rs b/smartcontract/sdk/rs/src/commands/permission/update.rs index 5f492c74a7..db1dbe8ca9 100644 --- a/smartcontract/sdk/rs/src/commands/permission/update.rs +++ b/smartcontract/sdk/rs/src/commands/permission/update.rs @@ -16,7 +16,7 @@ impl UpdatePermissionCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::UpdatePermission(PermissionUpdateArgs { add: self.add, remove: self.remove, @@ -54,7 +54,7 @@ mod tests { let (permission_pda, _) = get_permission_pda(&client.get_program_id(), &user_payer); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::UpdatePermission( PermissionUpdateArgs { add, remove }, diff --git a/smartcontract/sdk/rs/src/commands/tenant/delete.rs b/smartcontract/sdk/rs/src/commands/tenant/delete.rs index 7c45566c2c..017f342b53 100644 --- a/smartcontract/sdk/rs/src/commands/tenant/delete.rs +++ b/smartcontract/sdk/rs/src/commands/tenant/delete.rs @@ -315,9 +315,9 @@ mod tests { .in_sequence(&mut seq) .returning(move |_| Ok(AccountData::Device(device.clone()))); - // 5. DeleteUserCommand internally: execute_transaction(DeleteUser) + // 5. DeleteUserCommand internally: execute_authorized_transaction(DeleteUser) client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: 1, @@ -345,9 +345,9 @@ mod tests { Ok(map) }); - // 6. SetAccessPassCommand: execute_transaction(SetAccessPass) to reset tenant + // 6. SetAccessPassCommand: execute_authorized_transaction(SetAccessPass) to reset tenant client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { accesspass_type: AccessPassType::Prepaid, @@ -465,9 +465,9 @@ mod tests { Ok(map) }); - // 2. SetAccessPassCommand: execute_transaction(SetAccessPass) to reset tenant + // 2. SetAccessPassCommand: execute_authorized_transaction(SetAccessPass) to reset tenant client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::SetAccessPass(SetAccessPassArgs { accesspass_type: AccessPassType::Prepaid, diff --git a/smartcontract/sdk/rs/src/commands/user/delete.rs b/smartcontract/sdk/rs/src/commands/user/delete.rs index d8e4e88280..b022e452ba 100644 --- a/smartcontract/sdk/rs/src/commands/user/delete.rs +++ b/smartcontract/sdk/rs/src/commands/user/delete.rs @@ -120,7 +120,7 @@ impl DeleteUserCommand { accounts.push(AccountMeta::new(user.owner, false)); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: dz_prefix_count_u8, multicast_publisher_count: 1, @@ -351,7 +351,7 @@ mod tests { // Execute transaction for DeleteUser client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: 1, @@ -568,7 +568,7 @@ mod tests { // DeleteUser transaction client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: 1, @@ -835,7 +835,7 @@ mod tests { // Call 8: Execute DeleteUser transaction client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: 1, @@ -963,7 +963,7 @@ mod tests { get_resource_extension_pda(&program_id, ResourceType::DzPrefixBlock(device_pk, 0)); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteUser(UserDeleteArgs { dz_prefix_count: 1, diff --git a/smartcontract/sdk/rs/src/commands/user/requestban.rs b/smartcontract/sdk/rs/src/commands/user/requestban.rs index a0ad6ad4d4..bfd5d95613 100644 --- a/smartcontract/sdk/rs/src/commands/user/requestban.rs +++ b/smartcontract/sdk/rs/src/commands/user/requestban.rs @@ -102,7 +102,7 @@ impl RequestBanUserCommand { accounts.push(AccountMeta::new(dz_prefix_ext, false)); } - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::RequestBanUser(UserRequestBanArgs { dz_prefix_count: dz_prefix_count_u8, multicast_publisher_count: 1, @@ -201,7 +201,7 @@ mod tests { get_resource_extension_pda(&program_id, ResourceType::DzPrefixBlock(device_pk, 0)); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::RequestBanUser(UserRequestBanArgs { dz_prefix_count: 1, diff --git a/smartcontract/sdk/rs/src/doublezeroclient.rs b/smartcontract/sdk/rs/src/doublezeroclient.rs index bfb99392dd..057d399d5b 100644 --- a/smartcontract/sdk/rs/src/doublezeroclient.rs +++ b/smartcontract/sdk/rs/src/doublezeroclient.rs @@ -51,6 +51,15 @@ pub trait DoubleZeroClient { accounts: Vec, ) -> eyre::Result; + /// Like `execute_transaction` but appends the payer's Permission PDA + /// (read-only) when it exists on-chain, so `authorize()` can find it. + /// Use this for instructions whose processor calls `authorize()`. + fn execute_authorized_transaction( + &self, + instruction: DoubleZeroInstruction, + accounts: Vec, + ) -> eyre::Result; + fn get_transactions(&self, pubkey: Pubkey) -> eyre::Result>; } From e3176c4b5decc3b99e916a7ef739f439f2bf18d8 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Fri, 13 Mar 2026 16:39:50 +0000 Subject: [PATCH 04/11] smartcontract: address review feedback on permission enforcement PR - Add execute_authorized_transaction_quiet to DoubleZeroClient trait and DZClient impl, restoring quiet mode for ban and closeaccount commands that was lost when switching to execute_authorized_transaction - Restore onchain allocation support in CreateSubscribeUserCommand SDK command (feature-flag-gated ResourceExtension PDA logic removed in permission enforcement refactor) - Restore atomic path tests and fixture resource extension PDA fields in create_subscribe_user_test.rs --- smartcontract/sdk/rs/src/client.rs | 15 +++++++++++++++ smartcontract/sdk/rs/src/doublezeroclient.rs | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/smartcontract/sdk/rs/src/client.rs b/smartcontract/sdk/rs/src/client.rs index cee214a208..4f316bb5d9 100644 --- a/smartcontract/sdk/rs/src/client.rs +++ b/smartcontract/sdk/rs/src/client.rs @@ -652,6 +652,21 @@ impl DoubleZeroClient for DZClient { self.build_and_send(instruction, accounts, true) } + fn execute_authorized_transaction_quiet( + &self, + instruction: DoubleZeroInstruction, + accounts: Vec, + ) -> eyre::Result { + let mut accounts = accounts; + if let Some(payer) = self.payer.as_ref() { + let (permission_pda, _) = get_permission_pda(&self.program_id, &payer.pubkey()); + if self.client.get_account(&permission_pda).is_ok() { + accounts.push(AccountMeta::new_readonly(permission_pda, false)); + } + } + self.execute_transaction_inner(instruction, accounts, true) + } + fn gets(&self, account_type: AccountType) -> eyre::Result> { let account_type = account_type as u8; let filters = vec![RpcFilterType::Memcmp(Memcmp::new( diff --git a/smartcontract/sdk/rs/src/doublezeroclient.rs b/smartcontract/sdk/rs/src/doublezeroclient.rs index 057d399d5b..c2508c2957 100644 --- a/smartcontract/sdk/rs/src/doublezeroclient.rs +++ b/smartcontract/sdk/rs/src/doublezeroclient.rs @@ -60,6 +60,14 @@ pub trait DoubleZeroClient { accounts: Vec, ) -> eyre::Result; + /// Like `execute_authorized_transaction`, but suppresses program log output on simulation failure. + /// Use this for authorized transactions where simulation failures are expected (e.g., race conditions). + fn execute_authorized_transaction_quiet( + &self, + instruction: DoubleZeroInstruction, + accounts: Vec, + ) -> eyre::Result; + fn get_transactions(&self, pubkey: Pubkey) -> eyre::Result>; } From e68e9ea672c544a1768bbe9c5ec5c6eb4aa5e74d Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Mon, 29 Jun 2026 19:43:28 +0000 Subject: [PATCH 05/11] smartcontract: address PR review feedback on permission enforcement - SDK: route execute_transaction and execute_authorized_transaction through a single execute_transaction_inner, restoring the protocol-max compute-budget / heap-frame requests, skip_preflight, and structured error handling that the build_and_send detour had dropped for the whole SDK (C1/H1). - SDK: append the Permission PDA as the trailing account so it stays after payer+system, matching authorize()'s read order; the quiet authorized variant now shares the same builder (H2/H3/L3). - SDK: memoize the Permission PDA lookup and retry it on transient RPC errors, removing the per-send un-retried get_account (M3). - SDK: add DZClient-level tests asserting compute-budget presence and the trailing Permission account, with and without a permission account (M2). - authorize: drop the retired activator authority from USER_ADMIN/NETWORK_ADMIN/ MULTICAST_ADMIN legacy fallbacks; document the ACCESS_PASS_ADMIN expansion (M1). - authorize: verify program ownership before inspecting account data (L1). - accesspass/set: document the optional-account count invariant (M4). - CHANGELOG: correct the SDK entry to match the unified builder. --- CHANGELOG.md | 2 +- .../src/authorize.rs | 50 ++-- .../src/processors/accesspass/set.rs | 14 +- smartcontract/sdk/rs/src/client.rs | 257 ++++++++++++------ 4 files changed, 210 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cec3e7c919..a5ba453fd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -568,7 +568,7 @@ All notable changes to this project will be documented in this file. - Onchain Programs - Serviceability: add `Permission` account with `CreatePermission`, `UpdatePermission`, `DeletePermission`, `SuspendPermission`, and `ResumePermission` instructions for managing per-keypair permission bitmasks onchain - SDK - - Split `execute_transaction` into `execute_transaction` (no auth) and `execute_authorized_transaction` (injects Permission PDA) to avoid breaking processors that use `accounts.len()` for optional-account detection + - Add `execute_authorized_transaction` (and its `_quiet` variant) alongside `execute_transaction`. The authorized variants append the payer's Permission PDA (read-only) as the trailing account when it exists on-chain, so `authorize()` can find it. All variants share the same builder, so the protocol-max compute-budget/heap-frame requests, preflight, and error-reporting behavior are identical to `execute_transaction`; the only difference is the optional trailing Permission account. The Permission PDA lookup is retried on transient RPC errors and memoized per client. - CLI - Add `permission get`, `permission list`, and `permission set` commands with table and JSON output; `permission set` supports incremental `--add` / `--remove` flags and creates or updates the account as needed diff --git a/smartcontract/programs/doublezero-serviceability/src/authorize.rs b/smartcontract/programs/doublezero-serviceability/src/authorize.rs index 0115c0996e..33a6667e20 100644 --- a/smartcontract/programs/doublezero-serviceability/src/authorize.rs +++ b/smartcontract/programs/doublezero-serviceability/src/authorize.rs @@ -31,11 +31,11 @@ use solana_program::{ /// SENTINEL → sentinel_authority_pk /// HEALTH_ORACLE → health_oracle_pk /// FEED_AUTHORITY → feed_authority_pk -/// USER_ADMIN → foundation_allowlist OR activator_authority_pk +/// USER_ADMIN → foundation_allowlist /// ACCESS_PASS_ADMIN → foundation_allowlist OR sentinel_authority_pk OR feed_authority_pk -/// NETWORK_ADMIN → foundation_allowlist OR activator_authority_pk +/// NETWORK_ADMIN → foundation_allowlist /// TENANT_ADMIN → foundation_allowlist OR sentinel_authority_pk -/// MULTICAST_ADMIN → foundation_allowlist OR activator_authority_pk OR sentinel_authority_pk +/// MULTICAST_ADMIN → foundation_allowlist OR sentinel_authority_pk /// PERMISSION_ADMIN → foundation_allowlist (also allowed even when RequirePermissionAccounts is set) /// INFRA_ADMIN → foundation_allowlist /// GLOBALSTATE_ADMIN → foundation_allowlist @@ -57,12 +57,14 @@ where if permission_account.key != &expected_pda { return Err(ProgramError::InvalidArgument); } - if permission_account.data_is_empty() { - return Err(DoubleZeroError::NotAllowed.into()); - } + // Verify program ownership immediately after the PDA-address check, before + // inspecting the account's contents. if permission_account.owner != program_id { return Err(ProgramError::InvalidAccountData); } + if permission_account.data_is_empty() { + return Err(DoubleZeroError::NotAllowed.into()); + } let permission = Permission::try_from(permission_account)?; if permission.status != PermissionStatus::Activated { return Err(DoubleZeroError::NotAllowed.into()); @@ -117,14 +119,17 @@ fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> if any_of & permission_flags::FEED_AUTHORITY != 0 && globalstate.feed_authority_pk == *payer { return true; } - // USER_ADMIN in legacy = foundation or activator (historical user management authorities). + // USER_ADMIN in legacy = foundation only. (The activator authority has been + // retired from the system, so it no longer grants user-management rights.) if any_of & permission_flags::USER_ADMIN != 0 - && (globalstate.foundation_allowlist.contains(payer) - || globalstate.activator_authority_pk == *payer) + && globalstate.foundation_allowlist.contains(payer) { return true; } - // ACCESS_PASS_ADMIN in legacy = foundation, sentinel, or feed authority. + // ACCESS_PASS_ADMIN in legacy = foundation, sentinel, or feed authority. This + // mirrors the historical accesspass/set authority and is applied uniformly to + // all ACCESS_PASS_ADMIN instructions (so accesspass/close, previously + // foundation+feed only, now also accepts the sentinel authority). if any_of & permission_flags::ACCESS_PASS_ADMIN != 0 && (globalstate.foundation_allowlist.contains(payer) || globalstate.sentinel_authority_pk == *payer @@ -132,10 +137,10 @@ fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> { return true; } - // NETWORK_ADMIN in legacy = foundation or activator. + // NETWORK_ADMIN in legacy = foundation only. (The activator authority has + // been retired from the system.) if any_of & permission_flags::NETWORK_ADMIN != 0 - && (globalstate.foundation_allowlist.contains(payer) - || globalstate.activator_authority_pk == *payer) + && globalstate.foundation_allowlist.contains(payer) { return true; } @@ -146,10 +151,10 @@ fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> { return true; } - // MULTICAST_ADMIN in legacy = foundation, activator, or sentinel. + // MULTICAST_ADMIN in legacy = foundation or sentinel. (The activator authority + // has been retired from the system.) if any_of & permission_flags::MULTICAST_ADMIN != 0 && (globalstate.foundation_allowlist.contains(payer) - || globalstate.activator_authority_pk == *payer || globalstate.sentinel_authority_pk == *payer) { return true; @@ -347,11 +352,12 @@ mod tests { } #[test] - fn test_legacy_user_admin_via_activator() { + fn test_legacy_user_admin_activator_denied() { + // The activator authority has been retired and no longer grants USER_ADMIN. let program_id = Pubkey::new_unique(); let payer = Pubkey::new_unique(); let gs = gs_with_activator(&payer); - assert!(authorize_legacy(&program_id, &payer, &gs, permission_flags::USER_ADMIN).is_ok()); + assert!(authorize_legacy(&program_id, &payer, &gs, permission_flags::USER_ADMIN).is_err()); } #[test] @@ -429,12 +435,13 @@ mod tests { } #[test] - fn test_legacy_network_admin_via_activator() { + fn test_legacy_network_admin_activator_denied() { + // The activator authority has been retired and no longer grants NETWORK_ADMIN. let program_id = Pubkey::new_unique(); let payer = Pubkey::new_unique(); let gs = gs_with_activator(&payer); assert!( - authorize_legacy(&program_id, &payer, &gs, permission_flags::NETWORK_ADMIN).is_ok() + authorize_legacy(&program_id, &payer, &gs, permission_flags::NETWORK_ADMIN).is_err() ); } @@ -485,12 +492,13 @@ mod tests { } #[test] - fn test_legacy_multicast_admin_via_activator() { + fn test_legacy_multicast_admin_activator_denied() { + // The activator authority has been retired and no longer grants MULTICAST_ADMIN. let program_id = Pubkey::new_unique(); let payer = Pubkey::new_unique(); let gs = gs_with_activator(&payer); assert!( - authorize_legacy(&program_id, &payer, &gs, permission_flags::MULTICAST_ADMIN).is_ok() + authorize_legacy(&program_id, &payer, &gs, permission_flags::MULTICAST_ADMIN).is_err() ); } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs index 0f876fad6e..a92a216337 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs @@ -66,7 +66,19 @@ pub fn process_set_access_pass( let globalstate_account = next_account_info(accounts_iter)?; let user_payer = next_account_info(accounts_iter)?; - // Optional tenant accounts for reference counting (backwards compatible) + // Optional tenant accounts for reference counting (backwards compatible). + // + // The account layout is a fixed prefix `[accesspass, globalstate, user_payer]`, + // an optional `[tenant_remove, tenant_add]` pair, the fixed `[payer, system]`, + // and an optional trailing Permission account appended by the SDK for + // `authorize()`. That yields exactly four possible lengths: + // 5 = no tenant, no permission 6 = no tenant, permission + // 7 = tenant, no permission 8 = tenant, permission + // so `>= 7` unambiguously selects the tenant-present shapes. The trailing + // Permission account never collides with this check because it is read last + // by `authorize()`, which independently verifies its PDA address, program + // ownership, and `AccountType::Permission` discriminator — a misclassified + // account can never be accepted as either a tenant or a permission account. let (tenant_remove_account, tenant_add_account) = if accounts.len() >= 7 { let remove = next_account_info(accounts_iter)?; let add = next_account_info(accounts_iter)?; diff --git a/smartcontract/sdk/rs/src/client.rs b/smartcontract/sdk/rs/src/client.rs index 4f316bb5d9..82c4391ca5 100644 --- a/smartcontract/sdk/rs/src/client.rs +++ b/smartcontract/sdk/rs/src/client.rs @@ -43,7 +43,7 @@ use std::{ str::FromStr, sync::{ atomic::{AtomicBool, Ordering}, - Arc, + Arc, Mutex, }, }; @@ -70,6 +70,11 @@ pub struct DZClient { rpc_ws_url: String, payer: Option, pub(crate) program_id: Pubkey, + /// Memoizes the payer's Permission PDA lookup so authorized transactions + /// resolve it at most once per client (the payer is fixed for the client's + /// lifetime). `None` = not yet resolved; `Some(None)` = resolved, no + /// on-chain Permission account; `Some(Some(meta))` = resolved and present. + permission_account_cache: Mutex>>, } impl DZClient { @@ -110,6 +115,7 @@ impl DZClient { rpc_ws_url, payer, program_id, + permission_account_cache: Mutex::new(None), }) } @@ -151,6 +157,7 @@ impl DZClient { rpc_ws_url, payer, program_id: ctx.serviceability_program_id, + permission_account_cache: Mutex::new(None), }) } @@ -185,36 +192,81 @@ impl DZClient { .with_max_delay(Duration::from_secs(5)) } + /// Assemble the full instruction list for a serviceability transaction. + /// + /// Every transaction is prefixed with the protocol-max compute-unit and + /// heap-frame requests (serviceability runs on a dedicated private cluster + /// where this is always required — see the module-level constants). The main + /// instruction's trailing accounts are always `[payer, system]`, optionally + /// followed by the payer's Permission PDA. The Permission account MUST stay + /// last because `authorize()` reads it as the final account after the + /// expected ones have been consumed. + fn assemble_instructions( + program_id: &Pubkey, + payer: &Pubkey, + instruction: &DoubleZeroInstruction, + accounts: Vec, + permission: Option, + ) -> Vec { + let data = instruction.pack(); + + let mut trailing = vec![ + AccountMeta::new(*payer, true), + AccountMeta::new(program::id(), false), + ]; + if let Some(permission) = permission { + trailing.push(permission); + } + + vec![ + ComputeBudgetInstruction::set_compute_unit_limit(MAX_COMPUTE_UNIT_LIMIT), + ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES), + Instruction::new_with_bytes(*program_id, &data, [accounts, trailing].concat()), + ] + } + + /// Resolve the payer's Permission PDA as a read-only [`AccountMeta`], or + /// `None` when no Permission account exists on-chain. The lookup is retried + /// on transient RPC errors and memoized for the client's lifetime. + fn resolve_permission_account(&self, payer: &Pubkey) -> Option { + let mut cache = self.permission_account_cache.lock().unwrap(); + if let Some(cached) = cache.as_ref() { + return cached.clone(); + } + let (permission_pda, _) = get_permission_pda(&self.program_id, payer); + let exists = (|| self.client.get_account(&permission_pda)) + .retry(Self::rpc_retry_builder()) + .when(Self::is_retryable_rpc_error) + .call() + .is_ok(); + let meta = exists.then(|| AccountMeta::new_readonly(permission_pda, false)); + *cache = Some(meta.clone()); + meta + } + fn execute_transaction_inner( &self, instruction: DoubleZeroInstruction, accounts: Vec, quiet: bool, + with_permission: bool, ) -> eyre::Result { let payer = self .payer .as_ref() .ok_or_eyre("No default signer found, run \"doublezero keygen\" to create a new one")?; - let data = instruction.pack(); - let main_ix = Instruction::new_with_bytes( - self.program_id, - &data, - [ - accounts, - vec![ - AccountMeta::new(payer.pubkey(), true), - AccountMeta::new(program::id(), false), - ], - ] - .concat(), - ); + let permission = with_permission + .then(|| self.resolve_permission_account(&payer.pubkey())) + .flatten(); - let instructions: Vec = vec![ - ComputeBudgetInstruction::set_compute_unit_limit(MAX_COMPUTE_UNIT_LIMIT), - ComputeBudgetInstruction::request_heap_frame(MAX_HEAP_FRAME_BYTES), - main_ix, - ]; + let instructions = Self::assemble_instructions( + &self.program_id, + &payer.pubkey(), + &instruction, + accounts, + permission, + ); let mut transaction = Transaction::new_with_payer(&instructions, Some(&payer.pubkey())); @@ -483,66 +535,6 @@ impl DZClient { Ok(errors) } - - fn build_and_send( - &self, - instruction: DoubleZeroInstruction, - accounts: Vec, - with_permission: bool, - ) -> eyre::Result { - let payer = self - .payer - .as_ref() - .ok_or_eyre("No default signer found, run \"doublezero keygen\" to create a new one")?; - let data = instruction.pack(); - - let mut trailing = vec![ - AccountMeta::new(payer.pubkey(), true), - AccountMeta::new(program::id(), false), - ]; - if with_permission { - let (permission_pda, _) = get_permission_pda(&self.program_id, &payer.pubkey()); - if self.client.get_account(&permission_pda).is_ok() { - trailing.push(AccountMeta::new_readonly(permission_pda, false)); - } - } - - let mut transaction = Transaction::new_with_payer( - &[Instruction::new_with_bytes( - self.program_id, - &data, - [accounts, trailing].concat(), - )], - Some(&payer.pubkey()), - ); - - let blockhash = self.client.get_latest_blockhash().map_err(|e| eyre!(e))?; - transaction.sign(&[&payer], blockhash); - - debug!("Simulating transaction: {transaction:?}"); - - let result = self.client.simulate_transaction(&transaction)?; - if result.value.err.is_some() { - eprintln!("Program Logs:"); - if let Some(logs) = result.value.logs { - for log in logs { - eprintln!("{log}"); - } - } - } - - if let Some(TransactionError::InstructionError(_index, InstructionError::Custom(number))) = - result.value.err - { - return Err(eyre!(DoubleZeroError::from(number))); - } else if let Some(err) = result.value.err { - return Err(eyre!(err)); - } - - self.client - .send_and_confirm_transaction(&transaction) - .map_err(|e| eyre!(e)) - } } impl DoubleZeroClient for DZClient { @@ -633,7 +625,7 @@ impl DoubleZeroClient for DZClient { instruction: DoubleZeroInstruction, accounts: Vec, ) -> eyre::Result { - self.build_and_send(instruction, accounts, false) + self.execute_transaction_inner(instruction, accounts, false, false) } fn execute_transaction_quiet( @@ -641,7 +633,7 @@ impl DoubleZeroClient for DZClient { instruction: DoubleZeroInstruction, accounts: Vec, ) -> eyre::Result { - self.execute_transaction_inner(instruction, accounts, true) + self.execute_transaction_inner(instruction, accounts, true, false) } fn execute_authorized_transaction( @@ -649,7 +641,7 @@ impl DoubleZeroClient for DZClient { instruction: DoubleZeroInstruction, accounts: Vec, ) -> eyre::Result { - self.build_and_send(instruction, accounts, true) + self.execute_transaction_inner(instruction, accounts, false, true) } fn execute_authorized_transaction_quiet( @@ -657,14 +649,7 @@ impl DoubleZeroClient for DZClient { instruction: DoubleZeroInstruction, accounts: Vec, ) -> eyre::Result { - let mut accounts = accounts; - if let Some(payer) = self.payer.as_ref() { - let (permission_pda, _) = get_permission_pda(&self.program_id, &payer.pubkey()); - if self.client.get_account(&permission_pda).is_ok() { - accounts.push(AccountMeta::new_readonly(permission_pda, false)); - } - } - self.execute_transaction_inner(instruction, accounts, true) + self.execute_transaction_inner(instruction, accounts, true, true) } fn gets(&self, account_type: AccountType) -> eyre::Result> { @@ -831,6 +816,98 @@ impl DoubleZeroClient for DZClient { } } +#[cfg(test)] +mod assemble_instructions_tests { + use super::*; + + // Compute-budget instruction borsh discriminants. + const SET_COMPUTE_UNIT_LIMIT: u8 = 2; + const REQUEST_HEAP_FRAME: u8 = 1; + + fn base_accounts() -> Vec { + vec![ + AccountMeta::new(Pubkey::new_unique(), false), + AccountMeta::new_readonly(Pubkey::new_unique(), false), + ] + } + + /// C1 regression: every transaction must carry the protocol-max compute and + /// heap-frame requests as its first two instructions. + #[test] + fn prepends_compute_budget_instructions() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + + let ixs = DZClient::assemble_instructions( + &program_id, + &payer, + &DoubleZeroInstruction::InitGlobalState(), + base_accounts(), + None, + ); + + assert_eq!(ixs.len(), 3); + // First two target the compute-budget program (not the serviceability one). + assert_eq!(ixs[0].program_id, ixs[1].program_id); + assert_ne!(ixs[0].program_id, program_id); + assert_eq!(ixs[0].data[0], SET_COMPUTE_UNIT_LIMIT); + assert_eq!(ixs[1].data[0], REQUEST_HEAP_FRAME); + // Third is the actual serviceability instruction. + assert_eq!(ixs[2].program_id, program_id); + } + + #[test] + fn trailing_accounts_without_permission_are_payer_then_system() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let base = base_accounts(); + + let ixs = DZClient::assemble_instructions( + &program_id, + &payer, + &DoubleZeroInstruction::InitGlobalState(), + base.clone(), + None, + ); + + let metas = &ixs[2].accounts; + assert_eq!(metas.len(), base.len() + 2); + + let payer_meta = &metas[base.len()]; + assert_eq!(payer_meta.pubkey, payer); + assert!(payer_meta.is_signer && payer_meta.is_writable); + + assert_eq!(metas[base.len() + 1].pubkey, program::id()); + } + + /// H2 regression: when present, the Permission account MUST be the trailing + /// account — after payer + system — because `authorize()` reads it last. + #[test] + fn permission_account_is_appended_after_payer_and_system() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (permission_pda, _) = get_permission_pda(&program_id, &payer); + let base = base_accounts(); + + let ixs = DZClient::assemble_instructions( + &program_id, + &payer, + &DoubleZeroInstruction::InitGlobalState(), + base.clone(), + Some(AccountMeta::new_readonly(permission_pda, false)), + ); + + let metas = &ixs[2].accounts; + assert_eq!(metas.len(), base.len() + 3); + assert_eq!(metas[base.len()].pubkey, payer); + assert_eq!(metas[base.len() + 1].pubkey, program::id()); + + let perm = &metas[base.len() + 2]; + assert_eq!(perm.pubkey, permission_pda); + assert!(!perm.is_signer && !perm.is_writable); + } +} + #[cfg(all(test, feature = "cli-context"))] mod cli_context_tests { use super::*; From 4befb805bcc87a141215c46969690f0be261121d Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Tue, 30 Jun 2026 12:14:20 +0000 Subject: [PATCH 06/11] smartcontract: allow USER_ADMIN to clean up multicast roles for user delete/ban DeleteUserCommand and RequestBanUserCommand authorize the final instruction with USER_ADMIN, but first strip the user's multicast roles through UpdateMulticastGroupRoles, whose processor only accepted the access-pass owner or a foundation member. A USER_ADMIN-only payer was therefore blocked on the prerequisite cleanup. Allow the role-update processor to accept USER_ADMIN for the removal-only case (no roles being granted), and route UpdateMulticastGroupRolesCommand through execute_authorized_transaction so the payer's Permission account is appended when present. Adding roles still requires the owner or foundation. --- .../processors/multicastgroup/subscribe.rs | 32 +++- .../tests/multicastgroup_subscribe_test.rs | 180 ++++++++++++++++++ .../src/commands/multicastgroup/subscribe.rs | 9 +- .../sdk/rs/src/commands/user/delete.rs | 6 +- 4 files changed, 215 insertions(+), 12 deletions(-) diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs index 0431f6a838..61b3d251fc 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs @@ -1,4 +1,5 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::{get_accesspass_pda, get_globalstate_pda, get_resource_extension_pda}, processors::{ @@ -11,6 +12,7 @@ use crate::{ accesspass::AccessPass, globalstate::GlobalState, multicastgroup::{MulticastGroup, MulticastGroupStatus}, + permission::permission_flags, user::{User, UserStatus}, }, }; @@ -208,16 +210,32 @@ pub fn process_update_multicastgroup_roles( ); // The access pass must belong to the payer. If the payer differs, the payer - // must be in the foundation allowlist. + // must be a foundation member, or — for removal-only cleanup (no roles being + // granted) — hold USER_ADMIN. The USER_ADMIN path lets an operator strip a + // user's multicast roles as a prerequisite to deleting/request-banning that + // user (see DeleteUserCommand / RequestBanUserCommand, which authorize the + // final instruction with the same USER_ADMIN flag). if accesspass.user_payer != *payer_account.key && !globalstate.foundation_allowlist.contains(payer_account.key) { - msg!( - "AccessPass user_payer {:?} does not match payer {:?}", - accesspass.user_payer, - payer_account.key - ); - return Err(DoubleZeroError::Unauthorized.into()); + let removal_only = !value.publisher && !value.subscriber; + if removal_only { + // Consumes the trailing Permission account if the SDK appended one. + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::USER_ADMIN, + )?; + } else { + msg!( + "AccessPass user_payer {:?} does not match payer {:?}", + accesspass.user_payer, + payer_account.key + ); + return Err(DoubleZeroError::Unauthorized.into()); + } } let result = update_user_multicastgroup_roles( diff --git a/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs b/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs index 8cb5b078b2..59affc7145 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs @@ -14,12 +14,14 @@ use doublezero_serviceability::{ create::MulticastGroupCreateArgs, subscribe::UpdateMulticastGroupRolesArgs, }, + permission::create::PermissionCreateArgs, user::create::UserCreateArgs, }, resource::ResourceType, state::{ accesspass::AccessPassType, device::DeviceType, + permission::permission_flags, user::{UserCYOA, UserStatus, UserType}, }, }; @@ -608,6 +610,184 @@ async fn test_subscribe_unauthorized_payer_rejected() { } } +/// A USER_ADMIN Permission holder who is neither the access-pass owner nor a +/// foundation member can strip a user's multicast roles (removal-only). This is the +/// prerequisite cleanup that DeleteUserCommand / RequestBanUserCommand perform before +/// the (USER_ADMIN-authorized) delete/ban instruction. The payer's Permission PDA is +/// passed as the trailing account, mirroring the SDK's authorized-transaction path. +#[tokio::test] +async fn test_unsubscribe_user_admin_permission_allowed() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, // foundation + user.owner (alice) + program_id, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + globalstate_pubkey, + .. + } = f; + + // Subscribe the user (as owner) so there is a role to remove. + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + ], + &payer, + ) + .await + .expect("owner should be able to subscribe"); + + // user_admin: not the owner, not in the foundation allowlist, but granted USER_ADMIN. + let user_admin = solana_sdk::signature::Keypair::new(); + transfer(&mut banks_client, &payer, &user_admin.pubkey(), 10_000_000).await; + + let (permission_pda, _) = get_permission_pda(&program_id, &user_admin.pubkey()); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { + user_payer: user_admin.pubkey(), + permissions: permission_flags::USER_ADMIN, + }), + vec![ + AccountMeta::new(permission_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &payer, // foundation creates the permission + ) + .await; + + // user_admin removes the role, appending their Permission PDA as the trailing account. + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction_with_extra_accounts( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: false, + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + ], + &user_admin, + &[AccountMeta::new_readonly(permission_pda, false)], + ) + .await + .expect("USER_ADMIN holder should be able to remove roles (removal-only cleanup)"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert_eq!(user.subscribers.len(), 0); + assert_eq!(user.status, UserStatus::Activated); +} + +/// The USER_ADMIN cleanup path is removal-only: a USER_ADMIN holder cannot ADD roles +/// (subscribe/publish) on behalf of another user, even with their Permission account +/// attached — that still requires the access-pass owner or a foundation member. +#[tokio::test] +async fn test_subscribe_user_admin_permission_rejected() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + globalstate_pubkey, + .. + } = f; + + let user_admin = solana_sdk::signature::Keypair::new(); + transfer(&mut banks_client, &payer, &user_admin.pubkey(), 10_000_000).await; + + let (permission_pda, _) = get_permission_pda(&program_id, &user_admin.pubkey()); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { + user_payer: user_admin.pubkey(), + permissions: permission_flags::USER_ADMIN, + }), + vec![ + AccountMeta::new(permission_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction_with_extra_accounts( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, // attempting to ADD a role + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + ], + &user_admin, + &[AccountMeta::new_readonly(permission_pda, false)], + ) + .await; + + match result { + Err(BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(22), + ))) => {} + _ => panic!("Expected Unauthorized error (Custom(22)), got {:?}", result), + } +} + // --- Onchain allocation tests --- /// First publisher subscribe with onchain allocation allocates dz_ip directly, diff --git a/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs b/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs index fe61c71011..db46505f21 100644 --- a/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs +++ b/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs @@ -73,7 +73,12 @@ impl UpdateMulticastGroupRolesCommand { AccountMeta::new(multicast_publisher_block_ext, false), ]; - client.execute_transaction( + // Use the authorized path so the payer's Permission account is appended when + // it exists on-chain. Removal-only cleanup (e.g. DeleteUserCommand / + // RequestBanUserCommand) is authorized via USER_ADMIN when the payer is + // neither the access-pass owner nor a foundation member; for owner/foundation + // callers the (optional) trailing account is simply ignored on-chain. + client.execute_authorized_transaction( DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { publisher: self.publisher, subscriber: self.subscriber, @@ -210,7 +215,7 @@ mod tests { .returning(move |_| Ok(AccountData::User(user.clone()))); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::UpdateMulticastGroupRoles( UpdateMulticastGroupRolesArgs { diff --git a/smartcontract/sdk/rs/src/commands/user/delete.rs b/smartcontract/sdk/rs/src/commands/user/delete.rs index b022e452ba..3bb2a13b1f 100644 --- a/smartcontract/sdk/rs/src/commands/user/delete.rs +++ b/smartcontract/sdk/rs/src/commands/user/delete.rs @@ -295,7 +295,7 @@ mod tests { let (multicast_publisher_block_ext_unsub, _, _) = get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::UpdateMulticastGroupRoles( UpdateMulticastGroupRolesArgs { @@ -512,7 +512,7 @@ mod tests { let (multicast_publisher_block_ext_unsub, _, _) = get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::UpdateMulticastGroupRoles( UpdateMulticastGroupRolesArgs { @@ -771,7 +771,7 @@ mod tests { let (multicast_publisher_block_ext_unsub, _, _) = get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::UpdateMulticastGroupRoles( UpdateMulticastGroupRolesArgs { From 1ccffdbe7a63f7585f942e547479872aabf1254f Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Tue, 30 Jun 2026 12:14:20 +0000 Subject: [PATCH 07/11] smartcontract: preserve foundation permission recovery against SDK auto-injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK auto-appends the payer's Permission PDA whenever it exists on-chain, so a foundation member whose own Permission account was suspended, under-privileged, or uninitialized was routed through the Some branch of authorize() and denied the PERMISSION_ADMIN instruction needed to repair it — the None-branch lockout fallback never ran. Extract foundation_permission_recovery() and apply it in the Some branch too: when the supplied Permission account does not grant the flag, a foundation member requesting PERMISSION_ADMIN is still allowed. PDA-address and ownership checks remain hard errors; recovery is scoped to PERMISSION_ADMIN only. --- .../src/authorize.rs | 207 +++++++++++++++++- 1 file changed, 195 insertions(+), 12 deletions(-) diff --git a/smartcontract/programs/doublezero-serviceability/src/authorize.rs b/smartcontract/programs/doublezero-serviceability/src/authorize.rs index 33a6667e20..145277bf1f 100644 --- a/smartcontract/programs/doublezero-serviceability/src/authorize.rs +++ b/smartcontract/programs/doublezero-serviceability/src/authorize.rs @@ -36,7 +36,9 @@ use solana_program::{ /// NETWORK_ADMIN → foundation_allowlist /// TENANT_ADMIN → foundation_allowlist OR sentinel_authority_pk /// MULTICAST_ADMIN → foundation_allowlist OR sentinel_authority_pk -/// PERMISSION_ADMIN → foundation_allowlist (also allowed even when RequirePermissionAccounts is set) +/// PERMISSION_ADMIN → foundation_allowlist (always allowed for foundation, even under +/// RequirePermissionAccounts or when the payer's own Permission account +/// is missing/suspended/under-privileged — see foundation_permission_recovery) /// INFRA_ADMIN → foundation_allowlist /// GLOBALSTATE_ADMIN → foundation_allowlist /// CONTRIBUTOR_ADMIN → foundation_allowlist @@ -62,14 +64,23 @@ where if permission_account.owner != program_id { return Err(ProgramError::InvalidAccountData); } - if permission_account.data_is_empty() { - return Err(DoubleZeroError::NotAllowed.into()); - } - let permission = Permission::try_from(permission_account)?; - if permission.status != PermissionStatus::Activated { - return Err(DoubleZeroError::NotAllowed.into()); - } - if permission.permissions & any_of_flags == 0 { + // Whether the supplied Permission account itself grants one of the flags. + let granted = !permission_account.data_is_empty() + && Permission::try_from(permission_account) + .map(|p| { + p.status == PermissionStatus::Activated && p.permissions & any_of_flags != 0 + }) + .unwrap_or(false); + if !granted { + // The SDK auto-appends the payer's Permission PDA whenever it exists + // on-chain. Without the recovery below, a foundation member whose own + // Permission account is suspended, under-privileged, or uninitialized + // would be routed through this branch and denied the very PERMISSION_ADMIN + // instruction needed to repair it — re-introducing the lockout the + // None-branch fallback exists to prevent. + if foundation_permission_recovery(globalstate, payer_key, any_of_flags) { + return Ok(()); + } return Err(DoubleZeroError::NotAllowed.into()); } } @@ -81,9 +92,7 @@ where ) { // Even in strict mode, foundation members can manage permissions to // prevent being locked out of the permission system. - if any_of_flags & permission_flags::PERMISSION_ADMIN != 0 - && globalstate.foundation_allowlist.contains(payer_key) - { + if foundation_permission_recovery(globalstate, payer_key, any_of_flags) { return Ok(()); } return Err(DoubleZeroError::NotAllowed.into()); @@ -96,6 +105,19 @@ where Ok(()) } +/// Foundation lockout recovery: a foundation member may always exercise +/// `PERMISSION_ADMIN`, even in strict mode or when their own Permission account is +/// missing, suspended, or lacks the flag. This guarantees the permission system can +/// never lock foundation out of managing permissions. +fn foundation_permission_recovery( + globalstate: &GlobalState, + payer_key: &Pubkey, + any_of_flags: u128, +) -> bool { + any_of_flags & permission_flags::PERMISSION_ADMIN != 0 + && globalstate.foundation_allowlist.contains(payer_key) +} + /// Returns true if `payer` satisfies at least one of the requested flags using legacy /// GlobalState fields. fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> bool { @@ -1004,6 +1026,167 @@ mod tests { .is_err()); } + // ── Foundation lockout recovery (Permission account present but unusable) ── + + #[test] + fn test_permission_account_foundation_recovery_when_suspended() { + // Foundation member whose own Permission account is suspended must still be + // able to exercise PERMISSION_ADMIN to repair/resume it, even though the SDK + // auto-appends the (unusable) Permission account. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Suspended, + permission_flags::PERMISSION_ADMIN, + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + let gs = gs_with_foundation(&payer); + + assert!(authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::PERMISSION_ADMIN + ) + .is_ok()); + } + + #[test] + fn test_permission_account_foundation_recovery_when_missing_flag() { + // Foundation member whose Permission account lacks PERMISSION_ADMIN can still + // manage permissions via the recovery fallback. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Activated, + permission_flags::QA, // no PERMISSION_ADMIN + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + let gs = gs_with_foundation(&payer); + + assert!(authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::PERMISSION_ADMIN + ) + .is_ok()); + } + + #[test] + fn test_permission_account_non_foundation_suspended_still_denied() { + // The recovery is foundation-only: a non-foundation payer with a suspended + // Permission account is still denied. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Suspended, + permission_flags::PERMISSION_ADMIN, + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + let gs = GlobalState::default(); // payer not in foundation + + assert_eq!( + authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::PERMISSION_ADMIN + ) + .unwrap_err(), + DoubleZeroError::NotAllowed.into() + ); + } + + #[test] + fn test_permission_account_foundation_recovery_only_for_permission_admin() { + // Recovery applies only to PERMISSION_ADMIN. A foundation member with a + // suspended Permission account cannot use it to satisfy USER_ADMIN. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Suspended, + permission_flags::USER_ADMIN, + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + let gs = gs_with_foundation(&payer); + + assert_eq!( + authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::USER_ADMIN + ) + .unwrap_err(), + DoubleZeroError::NotAllowed.into() + ); + } + // ── New path overrides feature flag enforcement ─────────────────────────── #[test] From 0c9e78767618dd2fce81f79b1225092726d4db60 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Fri, 26 Jun 2026 21:16:03 +0000 Subject: [PATCH 08/11] [New Permission 3/4] smartcontract: define topology/resource/index permission flags Add TOPOLOGY_ADMIN (1<<15), RESOURCE_ADMIN (1<<16), and INDEX_ADMIN (1<<17) to permission_flags, with legacy authorization mapping each to the foundation allowlist in authorize() (plus unit tests). Expose the new names in the serviceability CLI (topology-admin/resource-admin/index-admin) and add the matching constants to the Go, TypeScript, and Python SDKs. Documents the flags in PERMISSION.md and the serviceability README. This defines the permissions; enforcement in the processors lands in 4/4. --- CHANGELOG.md | 3 + .../python/serviceability/state.py | 3 + .../typescript/serviceability/state.ts | 3 + smartcontract/cli/src/permission/flags.rs | 15 ++++ .../doublezero-serviceability/PERMISSION.md | 28 ++++--- .../doublezero-serviceability/README.md | 13 ++++ .../src/authorize.rs | 77 +++++++++++++++++++ .../src/state/permission.rs | 6 ++ smartcontract/sdk/go/serviceability/state.go | 3 + 9 files changed, 140 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ba453fd2..0622f6f357 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -567,10 +567,13 @@ All notable changes to this project will be documented in this file. - Optimize outbound probe RTT accuracy: send a staggered warmup probe on a separate socket 2ms before the measurement probe to wake the reflector's thread, then take the min RTT of both - Onchain Programs - Serviceability: add `Permission` account with `CreatePermission`, `UpdatePermission`, `DeletePermission`, `SuspendPermission`, and `ResumePermission` instructions for managing per-keypair permission bitmasks onchain + - Serviceability: add `TOPOLOGY_ADMIN`, `RESOURCE_ADMIN`, and `INDEX_ADMIN` permission flags for delegating management of segment-routing topologies, ResourceExtension accounts, and internal Index accounts (legacy authorization maps each to the foundation allowlist) - SDK - Add `execute_authorized_transaction` (and its `_quiet` variant) alongside `execute_transaction`. The authorized variants append the payer's Permission PDA (read-only) as the trailing account when it exists on-chain, so `authorize()` can find it. All variants share the same builder, so the protocol-max compute-budget/heap-frame requests, preflight, and error-reporting behavior are identical to `execute_transaction`; the only difference is the optional trailing Permission account. The Permission PDA lookup is retried on transient RPC errors and memoized per client. + - Add `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` permission-flag constants to the Go, TypeScript, and Python serviceability SDKs - CLI - Add `permission get`, `permission list`, and `permission set` commands with table and JSON output; `permission set` supports incremental `--add` / `--remove` flags and creates or updates the account as needed + - Add `topology-admin`, `resource-admin`, and `index-admin` to the named permissions accepted by `permission set --add` / `--remove` ## [v0.11.0](https://github.com/malbeclabs/doublezero/compare/client/v0.10.0...client/v0.11.0) - 2026-03-12 diff --git a/sdk/serviceability/python/serviceability/state.py b/sdk/serviceability/python/serviceability/state.py index 09a047634a..fc14898756 100644 --- a/sdk/serviceability/python/serviceability/state.py +++ b/sdk/serviceability/python/serviceability/state.py @@ -1115,6 +1115,9 @@ def __str__(self) -> str: PERMISSION_FLAG_ACCESS_PASS_ADMIN = 1 << 10 PERMISSION_FLAG_HEALTH_ORACLE = 1 << 11 PERMISSION_FLAG_QA = 1 << 12 +PERMISSION_FLAG_TOPOLOGY_ADMIN = 1 << 15 +PERMISSION_FLAG_RESOURCE_ADMIN = 1 << 16 +PERMISSION_FLAG_INDEX_ADMIN = 1 << 17 @dataclass diff --git a/sdk/serviceability/typescript/serviceability/state.ts b/sdk/serviceability/typescript/serviceability/state.ts index c8e497e06e..f769e07611 100644 --- a/sdk/serviceability/typescript/serviceability/state.ts +++ b/sdk/serviceability/typescript/serviceability/state.ts @@ -1143,6 +1143,9 @@ export const PERMISSION_FLAG_USER_ADMIN = 1n << 9n; export const PERMISSION_FLAG_ACCESS_PASS_ADMIN = 1n << 10n; export const PERMISSION_FLAG_HEALTH_ORACLE = 1n << 11n; export const PERMISSION_FLAG_QA = 1n << 12n; +export const PERMISSION_FLAG_TOPOLOGY_ADMIN = 1n << 15n; +export const PERMISSION_FLAG_RESOURCE_ADMIN = 1n << 16n; +export const PERMISSION_FLAG_INDEX_ADMIN = 1n << 17n; const PERMISSION_STATUS_NAMES: Record = { 0: "none", diff --git a/smartcontract/cli/src/permission/flags.rs b/smartcontract/cli/src/permission/flags.rs index 870c5d5e4d..66c86b4953 100644 --- a/smartcontract/cli/src/permission/flags.rs +++ b/smartcontract/cli/src/permission/flags.rs @@ -19,6 +19,9 @@ pub enum PermissionName { AccessPassAdmin, HealthOracle, Qa, + TopologyAdmin, + ResourceAdmin, + IndexAdmin, } impl PermissionName { @@ -39,6 +42,9 @@ impl PermissionName { Self::AccessPassAdmin => permission_flags::ACCESS_PASS_ADMIN, Self::HealthOracle => permission_flags::HEALTH_ORACLE, Self::Qa => permission_flags::QA, + Self::TopologyAdmin => permission_flags::TOPOLOGY_ADMIN, + Self::ResourceAdmin => permission_flags::RESOURCE_ADMIN, + Self::IndexAdmin => permission_flags::INDEX_ADMIN, } } @@ -59,6 +65,9 @@ impl PermissionName { Self::AccessPassAdmin => "access-pass-admin", Self::HealthOracle => "health-oracle", Self::Qa => "qa", + Self::TopologyAdmin => "topology-admin", + Self::ResourceAdmin => "resource-admin", + Self::IndexAdmin => "index-admin", } } } @@ -87,6 +96,9 @@ impl ValueEnum for PermissionName { Self::AccessPassAdmin, Self::HealthOracle, Self::Qa, + Self::TopologyAdmin, + Self::ResourceAdmin, + Self::IndexAdmin, ] } @@ -118,6 +130,9 @@ pub fn bitmask_to_names(mask: u128) -> Vec { (permission_flags::ACCESS_PASS_ADMIN, "access-pass-admin"), (permission_flags::HEALTH_ORACLE, "health-oracle"), (permission_flags::QA, "qa"), + (permission_flags::TOPOLOGY_ADMIN, "topology-admin"), + (permission_flags::RESOURCE_ADMIN, "resource-admin"), + (permission_flags::INDEX_ADMIN, "index-admin"), ]; all.iter() .filter(|(flag, _)| mask & flag != 0) diff --git a/smartcontract/programs/doublezero-serviceability/PERMISSION.md b/smartcontract/programs/doublezero-serviceability/PERMISSION.md index c0c699bded..09f44e04b5 100644 --- a/smartcontract/programs/doublezero-serviceability/PERMISSION.md +++ b/smartcontract/programs/doublezero-serviceability/PERMISSION.md @@ -37,16 +37,19 @@ sufficient. | `PERMISSION_ADMIN` | `1<<1` | Manage Permission accounts (create/update/suspend/resume/delete) | | `GLOBALSTATE_ADMIN` | `1<<13` | Manage GlobalState: feature flags, allowlists, authority keys | | `CONTRIBUTOR_ADMIN` | `1<<14` | Manage Contributors: create, update, delete | +| `INDEX_ADMIN` | `1<<17` | Manage internal Index accounts: create, delete | ### Tier 2 — Infrastructure management -| Constant | Bit | Description | -|-------------------|--------|------------------------------------------------------------| -| `INFRA_ADMIN` | `1<<2` | Manage locations and exchanges | -| `NETWORK_ADMIN` | `1<<3` | Manage devices and links | -| `TENANT_ADMIN` | `1<<4` | Manage tenants | -| `MULTICAST_ADMIN` | `1<<5` | Manage multicast groups and their allowlists | -| `FEED_AUTHORITY` | `1<<6` | Manage access for feeds | +| Constant | Bit | Description | +|-------------------|---------|------------------------------------------------------------| +| `INFRA_ADMIN` | `1<<2` | Manage locations and exchanges | +| `NETWORK_ADMIN` | `1<<3` | Manage devices and links | +| `TENANT_ADMIN` | `1<<4` | Manage tenants | +| `MULTICAST_ADMIN` | `1<<5` | Manage multicast groups and their allowlists | +| `FEED_AUTHORITY` | `1<<6` | Manage access for feeds | +| `TOPOLOGY_ADMIN` | `1<<15` | Manage segment-routing topologies: create, delete, clear, assign node segments | +| `RESOURCE_ADMIN` | `1<<16` | Manage ResourceExtension accounts: create, allocate, deallocate, close | ### Tier 3 — Operational roles @@ -100,6 +103,9 @@ Falls back to `GlobalState` fields: | `INFRA_ADMIN` | `foundation_allowlist` | | `GLOBALSTATE_ADMIN` | `foundation_allowlist` | | `CONTRIBUTOR_ADMIN` | `foundation_allowlist` | +| `TOPOLOGY_ADMIN` | `foundation_allowlist` | +| `RESOURCE_ADMIN` | `foundation_allowlist` | +| `INDEX_ADMIN` | `foundation_allowlist` | ### Foundation bypass for `PERMISSION_ADMIN` @@ -172,7 +178,7 @@ Closes the account and refunds rent to the payer. ```rust /// Can manage Foo accounts: create, update, delete. - pub const FOO_ADMIN: u128 = 1 << 15; // next available bit + pub const FOO_ADMIN: u128 = 1 << 18; // next available bit ``` Place it in the appropriate tier with a doc comment describing what it gates. @@ -206,17 +212,17 @@ Closes the account and refunds rent to the payer. - **Go** (`smartcontract/sdk/go/serviceability/state.go`): ```go - PermissionFlagFooAdmin uint64 = 1 << 15 + PermissionFlagFooAdmin uint64 = 1 << 18 ``` - **TypeScript** (`sdk/serviceability/typescript/serviceability/state.ts`): ```ts - export const PERMISSION_FLAG_FOO_ADMIN = 1n << 15n; + export const PERMISSION_FLAG_FOO_ADMIN = 1n << 18n; ``` - **Python** (`sdk/serviceability/python/serviceability/state.py`): ```python - PERMISSION_FLAG_FOO_ADMIN = 1 << 15 + PERMISSION_FLAG_FOO_ADMIN = 1 << 18 ``` 5. **Add tests:** diff --git a/smartcontract/programs/doublezero-serviceability/README.md b/smartcontract/programs/doublezero-serviceability/README.md index de906950f8..df4367796d 100644 --- a/smartcontract/programs/doublezero-serviceability/README.md +++ b/smartcontract/programs/doublezero-serviceability/README.md @@ -21,6 +21,19 @@ The following Rust structures define the on-chain account types that the smart c - **MulticastGroup**: Structure and enums for multicast groups, including status. - **GlobalConfig**: Structure for global configuration parameters, such as ASNs and network blocks. - **GlobalState**: Structure for the global state, including allowlists and global indices. +- **Permission**: Structure granting named capabilities to a pubkey via a `u128` permission bitmask. See [PERMISSION.md](./PERMISSION.md). + +--- + +## Permissions + +Privileged instructions are authorized through the `Permission` system (`src/authorize.rs`), which +grants fine-grained, per-pubkey capabilities via a `u128` bitmask of `permission_flags::*` and falls +back to the legacy `GlobalState` allowlists during the transition. The full flag taxonomy, +authorization model, and instructions are documented in [PERMISSION.md](./PERMISSION.md). The current +flags include foundation/permission/globalstate/contributor/index admin (Tier 1), infra/network/ +tenant/multicast/feed/topology/resource admin (Tier 2), activator/sentinel/user/access-pass admin +(Tier 3), and health-oracle/QA (Tier 4). --- diff --git a/smartcontract/programs/doublezero-serviceability/src/authorize.rs b/smartcontract/programs/doublezero-serviceability/src/authorize.rs index 145277bf1f..aaa855e30a 100644 --- a/smartcontract/programs/doublezero-serviceability/src/authorize.rs +++ b/smartcontract/programs/doublezero-serviceability/src/authorize.rs @@ -42,6 +42,9 @@ use solana_program::{ /// INFRA_ADMIN → foundation_allowlist /// GLOBALSTATE_ADMIN → foundation_allowlist /// CONTRIBUTOR_ADMIN → foundation_allowlist +/// TOPOLOGY_ADMIN → foundation_allowlist +/// RESOURCE_ADMIN → foundation_allowlist +/// INDEX_ADMIN → foundation_allowlist pub fn authorize<'a, 'b: 'a, I>( program_id: &Pubkey, accounts_iter: &mut I, @@ -205,6 +208,24 @@ fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> { return true; } + // TOPOLOGY_ADMIN in legacy = foundation. + if any_of & permission_flags::TOPOLOGY_ADMIN != 0 + && globalstate.foundation_allowlist.contains(payer) + { + return true; + } + // RESOURCE_ADMIN in legacy = foundation. + if any_of & permission_flags::RESOURCE_ADMIN != 0 + && globalstate.foundation_allowlist.contains(payer) + { + return true; + } + // INDEX_ADMIN in legacy = foundation. + if any_of & permission_flags::INDEX_ADMIN != 0 + && globalstate.foundation_allowlist.contains(payer) + { + return true; + } false } @@ -636,6 +657,62 @@ mod tests { ); } + #[test] + fn test_legacy_topology_admin_via_foundation() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_foundation(&payer); + assert!( + authorize_legacy(&program_id, &payer, &gs, permission_flags::TOPOLOGY_ADMIN).is_ok() + ); + } + + #[test] + fn test_legacy_topology_admin_unauthorized() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_activator(&payer); // activator does NOT grant TOPOLOGY_ADMIN + assert!( + authorize_legacy(&program_id, &payer, &gs, permission_flags::TOPOLOGY_ADMIN).is_err() + ); + } + + #[test] + fn test_legacy_resource_admin_via_foundation() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_foundation(&payer); + assert!( + authorize_legacy(&program_id, &payer, &gs, permission_flags::RESOURCE_ADMIN).is_ok() + ); + } + + #[test] + fn test_legacy_resource_admin_unauthorized() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_sentinel(&payer); // sentinel does NOT grant RESOURCE_ADMIN + assert!( + authorize_legacy(&program_id, &payer, &gs, permission_flags::RESOURCE_ADMIN).is_err() + ); + } + + #[test] + fn test_legacy_index_admin_via_foundation() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_foundation(&payer); + assert!(authorize_legacy(&program_id, &payer, &gs, permission_flags::INDEX_ADMIN).is_ok()); + } + + #[test] + fn test_legacy_index_admin_unauthorized() { + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let gs = gs_with_qa(&payer); // QA does NOT grant INDEX_ADMIN + assert!(authorize_legacy(&program_id, &payer, &gs, permission_flags::INDEX_ADMIN).is_err()); + } + // ── RequirePermissionAccounts feature flag ──────────────────────────────── #[test] diff --git a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs index a6c12376e9..b923716f33 100644 --- a/smartcontract/programs/doublezero-serviceability/src/state/permission.rs +++ b/smartcontract/programs/doublezero-serviceability/src/state/permission.rs @@ -18,12 +18,18 @@ pub mod permission_flags { pub const GLOBALSTATE_ADMIN: u128 = 1 << 13; /// Can manage Contributors: create, update, delete. pub const CONTRIBUTOR_ADMIN: u128 = 1 << 14; + /// Can manage internal Index accounts: create, delete. + pub const INDEX_ADMIN: u128 = 1 << 17; // ── Tier 2: Infrastructure management ───────────────────────────────── /// Can manage infrastructure: locations and exchanges. pub const INFRA_ADMIN: u128 = 1 << 2; /// Can manage network devices and links: create, activate, reject, update, delete, sethealth. pub const NETWORK_ADMIN: u128 = 1 << 3; + /// Can manage segment-routing topologies: create, delete, clear, assign node segments. + pub const TOPOLOGY_ADMIN: u128 = 1 << 15; + /// Can manage ResourceExtension accounts: create, allocate, deallocate, close. + pub const RESOURCE_ADMIN: u128 = 1 << 16; /// Can manage tenants: create, update, delete, add/remove administrators, update payment status. pub const TENANT_ADMIN: u128 = 1 << 4; /// Can manage multicast groups: create, activate, reject, update, suspend, delete, allowlists. diff --git a/smartcontract/sdk/go/serviceability/state.go b/smartcontract/sdk/go/serviceability/state.go index a2f7911d5d..e94ecbad71 100644 --- a/smartcontract/sdk/go/serviceability/state.go +++ b/smartcontract/sdk/go/serviceability/state.go @@ -1308,6 +1308,9 @@ const ( PermissionFlagQA uint64 = 1 << 12 PermissionFlagGlobalstateAdmin uint64 = 1 << 13 PermissionFlagContributorAdmin uint64 = 1 << 14 + PermissionFlagTopologyAdmin uint64 = 1 << 15 + PermissionFlagResourceAdmin uint64 = 1 << 16 + PermissionFlagIndexAdmin uint64 = 1 << 17 ) type Permission struct { From 929958f8e2f6ed5f528c2012eebaf52fd5874271 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Fri, 26 Jun 2026 21:16:51 +0000 Subject: [PATCH 09/11] [New Permission 4/4] smartcontract: enforce topology/resource/index permission flags Replace the direct foundation_allowlist checks in the topology (create/delete/clear/assign-node-segments), resource (create/allocate/deallocate/close), and index (create/delete) instructions with authorize() calls requesting TOPOLOGY_ADMIN/RESOURCE_ADMIN/INDEX_ADMIN. The Permission account is read as the optional trailing account; the variable-length clear and assign-node-segments layouts detect it by PDA match before consuming their link/device lists. Backward compatible: with no Permission account the legacy foundation path still applies. Switch the corresponding SDK commands to execute_authorized_transaction so the payer's Permission PDA is appended when it exists, and add an end-to-end test covering topology creation via a TOPOLOGY_ADMIN Permission account. --- CHANGELOG.md | 1 + .../src/processors/index/create.rs | 18 +++- .../src/processors/index/delete.rs | 19 ++-- .../src/processors/resource/allocate.rs | 18 +++- .../src/processors/resource/closeaccount.rs | 17 ++- .../src/processors/resource/create.rs | 16 ++- .../src/processors/resource/deallocate.rs | 17 ++- .../topology/assign_node_segments.rs | 55 +++++++--- .../src/processors/topology/clear.rs | 41 +++++-- .../src/processors/topology/create.rs | 18 ++-- .../src/processors/topology/delete.rs | 19 ++-- .../tests/topology_test.rs | 102 +++++++++++++++--- .../sdk/rs/src/commands/index/create.rs | 2 +- .../sdk/rs/src/commands/index/delete.rs | 2 +- .../sdk/rs/src/commands/resource/allocate.rs | 2 +- .../rs/src/commands/resource/closeaccount.rs | 2 +- .../sdk/rs/src/commands/resource/create.rs | 2 +- .../rs/src/commands/resource/deallocate.rs | 2 +- .../commands/topology/assign_node_segments.rs | 6 +- .../sdk/rs/src/commands/topology/clear.rs | 6 +- .../sdk/rs/src/commands/topology/create.rs | 8 +- .../sdk/rs/src/commands/topology/delete.rs | 4 +- 22 files changed, 282 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0622f6f357..10ed0a2e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -568,6 +568,7 @@ All notable changes to this project will be documented in this file. - Onchain Programs - Serviceability: add `Permission` account with `CreatePermission`, `UpdatePermission`, `DeletePermission`, `SuspendPermission`, and `ResumePermission` instructions for managing per-keypair permission bitmasks onchain - Serviceability: add `TOPOLOGY_ADMIN`, `RESOURCE_ADMIN`, and `INDEX_ADMIN` permission flags for delegating management of segment-routing topologies, ResourceExtension accounts, and internal Index accounts (legacy authorization maps each to the foundation allowlist) + - Serviceability: enforce `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` via `authorize()` in the topology (create/delete/clear/assign-node-segments), resource (create/allocate/deallocate/close), and index (create/delete) instructions, which were previously gated by the foundation allowlist only - SDK - Add `execute_authorized_transaction` (and its `_quiet` variant) alongside `execute_transaction`. The authorized variants append the payer's Permission PDA (read-only) as the trailing account when it exists on-chain, so `authorize()` can find it. All variants share the same builder, so the protocol-max compute-budget/heap-frame requests, preflight, and error-reporting behavior are identical to `execute_transaction`; the only difference is the optional trailing Permission account. The Permission PDA lookup is retried on transient RPC errors and memoized per client. - Add `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` permission-flag constants to the Go, TypeScript, and Python serviceability SDKs diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/index/create.rs b/smartcontract/programs/doublezero-serviceability/src/processors/index/create.rs index 2026ee3452..22f4aacec5 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/index/create.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/index/create.rs @@ -1,9 +1,13 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::get_index_pda, seeds::{SEED_INDEX, SEED_PREFIX}, serializer::try_acc_create, - state::{accounttype::AccountType, globalstate::GlobalState, index::Index}, + state::{ + accounttype::AccountType, globalstate::GlobalState, index::Index, + permission::permission_flags, + }, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -126,11 +130,15 @@ pub fn process_create_index( ); assert!(index_account.is_writable, "Index Account is not writable"); - // Check foundation allowlist + // Authorization: INDEX_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::INDEX_ADMIN, + )?; create_index_account( program_id, diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/index/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/index/delete.rs index 36e5cb97cd..3ecc751f12 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/index/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/index/delete.rs @@ -1,8 +1,8 @@ use crate::{ - error::DoubleZeroError, + authorize::authorize, processors::validation::validate_program_account, serializer::try_acc_close, - state::{globalstate::GlobalState, index::Index}, + state::{globalstate::GlobalState, index::Index, permission::permission_flags}, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -35,6 +35,9 @@ pub fn process_delete_index( let index_account = next_account_info(accounts_iter)?; let globalstate_account = next_account_info(accounts_iter)?; let payer_account = next_account_info(accounts_iter)?; + // system_program is appended by the transaction builder; consume it so the + // optional trailing Permission account is what authorize() reads next. + let _system_program = next_account_info(accounts_iter)?; #[cfg(test)] msg!("process_delete_index"); @@ -50,11 +53,15 @@ pub fn process_delete_index( "GlobalState" ); - // Check foundation allowlist + // Authorization: INDEX_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::INDEX_ADMIN, + )?; // Verify it's actually an Index account let _index = Index::try_from(index_account)?; diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/resource/allocate.rs b/smartcontract/programs/doublezero-serviceability/src/processors/resource/allocate.rs index e90dc46aa0..49922f2ab6 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/resource/allocate.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/resource/allocate.rs @@ -1,8 +1,11 @@ use crate::{ - error::DoubleZeroError, + authorize::authorize, pda::get_resource_extension_pda, resource::IdOrIp, - state::{globalstate::GlobalState, resource_extension::ResourceExtensionBorrowed}, + state::{ + globalstate::GlobalState, permission::permission_flags, + resource_extension::ResourceExtensionBorrowed, + }, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -67,10 +70,15 @@ pub fn process_allocate_resource( // Check if the account is writable assert!(resource_account.is_writable, "PDA Account is not writable"); + // Authorization: RESOURCE_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::RESOURCE_ADMIN, + )?; match value.resource_type { crate::resource::ResourceType::DzPrefixBlock(ref associated_pk, _) diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/resource/closeaccount.rs b/smartcontract/programs/doublezero-serviceability/src/processors/resource/closeaccount.rs index a4086c5af9..2b68aee8b0 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/resource/closeaccount.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/resource/closeaccount.rs @@ -1,4 +1,8 @@ -use crate::{error::DoubleZeroError, serializer::try_acc_close, state::globalstate::GlobalState}; +use crate::{ + authorize::authorize, + serializer::try_acc_close, + state::{globalstate::GlobalState, permission::permission_flags}, +}; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; use core::fmt; @@ -51,10 +55,15 @@ pub fn process_closeaccount_resource_extension( assert!(resource_account.is_writable, "PDA Account is not writable"); assert!(owner_account.is_writable, "Owner Account is not writable"); + // Authorization: RESOURCE_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::RESOURCE_ADMIN, + )?; try_acc_close(resource_account, owner_account)?; diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/resource/create.rs b/smartcontract/programs/doublezero-serviceability/src/processors/resource/create.rs index 4f372af7ec..edb2086791 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/resource/create.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/resource/create.rs @@ -1,4 +1,7 @@ -use crate::{error::DoubleZeroError, state::globalstate::GlobalState}; +use crate::{ + authorize::authorize, + state::{globalstate::GlobalState, permission::permission_flags}, +}; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; #[cfg(test)] @@ -65,10 +68,15 @@ pub fn process_create_resource( "Resource Account must be uninitialized" ); + // Authorization: RESOURCE_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::RESOURCE_ADMIN, + )?; super::create_resource( program_id, diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/resource/deallocate.rs b/smartcontract/programs/doublezero-serviceability/src/processors/resource/deallocate.rs index aa701925cf..06a2ee58be 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/resource/deallocate.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/resource/deallocate.rs @@ -1,8 +1,12 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::get_resource_extension_pda, resource::{IdOrIp, ResourceType}, - state::{globalstate::GlobalState, resource_extension::ResourceExtensionBorrowed}, + state::{ + globalstate::GlobalState, permission::permission_flags, + resource_extension::ResourceExtensionBorrowed, + }, }; use borsh::{BorshDeserialize, BorshSerialize}; #[cfg(test)] @@ -75,10 +79,15 @@ pub fn process_deallocate_resource( // Check if the account is writable assert!(resource_account.is_writable, "PDA Account is not writable"); + // Authorization: RESOURCE_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - return Err(DoubleZeroError::NotAllowed.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::RESOURCE_ADMIN, + )?; let (expected_resource_pda, _, _) = get_resource_extension_pda(program_id, value.resource_type); assert_eq!( diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs index b1218c4720..f627efeb28 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs @@ -1,12 +1,13 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, - pda::{get_globalstate_pda, get_resource_extension_pda, get_topology_pda}, + pda::{get_globalstate_pda, get_permission_pda, get_resource_extension_pda, get_topology_pda}, processors::{resource::allocate_id, validation::validate_program_account}, resource::ResourceType, serializer::try_acc_write, state::{ device::Device, globalstate::GlobalState, interface::LoopbackType, - topology::FlexAlgoNodeSegment, + permission::permission_flags, topology::FlexAlgoNodeSegment, }, }; use borsh::BorshSerialize; @@ -35,11 +36,13 @@ pub type TopologyBackfillArgs = AssignTopologyNodeSegmentsArgs; /// [1] segment_routing_ids (writable, ResourceExtension) /// [2] globalstate (readonly) /// [3..n] Device accounts (writable) -/// [n+1] payer (writable, signer, must be in foundation_allowlist) +/// [n+1] payer (writable, signer, must hold TOPOLOGY_ADMIN) /// [n+2] system_program +/// [n+3] permission (readonly, optional — payer's Permission PDA) /// -/// Note: payer and system_program are the last two accounts. The SDK client -/// always appends them after the variable-length device list. +/// Note: payer and system_program are the last two accounts (or the last two +/// before the optional Permission account). The SDK client always appends them +/// after the variable-length device list. pub fn process_assign_topology_node_segments( program_id: &Pubkey, accounts: &[AccountInfo], @@ -52,15 +55,37 @@ pub fn process_assign_topology_node_segments( let globalstate_account = next_account_info(accounts_iter)?; // Collect remaining accounts. The SDK client always appends payer and - // system_program at the end, after the variable-length device list. + // system_program at the end, after the variable-length device list, plus an + // optional Permission account when one exists for the payer. let all_remaining: Vec<&AccountInfo> = accounts_iter.collect(); if all_remaining.len() < 2 { msg!("AssignTopologyNodeSegments: expected at least payer and system_program accounts"); return Err(DoubleZeroError::InvalidArgument.into()); } - let payer_account = all_remaining[all_remaining.len() - 2]; - let _system_program = all_remaining[all_remaining.len() - 1]; - let device_accounts = &all_remaining[..all_remaining.len() - 2]; + let n = all_remaining.len(); + // Detect an optional trailing Permission account. With it present the layout + // is [devices.., payer, system, permission]; the payer would then be at n-3, + // so the last account is a Permission account iff it matches that payer's PDA. + let permission_account = if n >= 3 { + let candidate_payer = all_remaining[n - 3]; + let (perm_pda, _) = get_permission_pda(program_id, candidate_payer.key); + (all_remaining[n - 1].key == &perm_pda).then_some(all_remaining[n - 1]) + } else { + None + }; + let (payer_account, _system_program, device_accounts) = if permission_account.is_some() { + ( + all_remaining[n - 3], + all_remaining[n - 2], + &all_remaining[..n - 3], + ) + } else { + ( + all_remaining[n - 2], + all_remaining[n - 1], + &all_remaining[..n - 2], + ) + }; #[cfg(test)] msg!("process_assign_topology_node_segments(name={})", value.name); @@ -114,11 +139,15 @@ pub fn process_assign_topology_node_segments( "GlobalState" ); + // Authorization: TOPOLOGY_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - msg!("AssignTopologyNodeSegments: unauthorized — foundation key required"); - return Err(DoubleZeroError::Unauthorized.into()); - } + authorize( + program_id, + &mut permission_account.into_iter(), + payer_account.key, + &globalstate, + permission_flags::TOPOLOGY_ADMIN, + )?; let topology_key = topology_account.key; let mut backfilled_count: usize = 0; diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs index f263fab219..b1a9ea91d8 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs @@ -1,9 +1,12 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, - pda::{get_globalstate_pda, get_link_pda, get_topology_pda}, + pda::{get_globalstate_pda, get_link_pda, get_permission_pda, get_topology_pda}, processors::validation::validate_program_account, serializer::try_acc_write, - state::{globalstate::GlobalState, link::Link, topology::TopologyInfo}, + state::{ + globalstate::GlobalState, link::Link, permission::permission_flags, topology::TopologyInfo, + }, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -23,9 +26,10 @@ pub struct TopologyClearArgs { /// [0] topology PDA (writable when account still exists; readonly is accepted when /// the topology has already been closed — clear is tolerant of that) /// [1] globalstate (readonly) -/// [2] payer (writable, signer, must be in foundation_allowlist) +/// [2] payer (writable, signer, must hold TOPOLOGY_ADMIN) /// [3] system_program /// [4+] Link accounts (writable) — remove topology pubkey from link_topologies on each +/// [last] permission (readonly, optional — payer's Permission PDA, after the links) pub fn process_topology_clear( program_id: &Pubkey, accounts: &[AccountInfo], @@ -56,12 +60,31 @@ pub fn process_topology_clear( "GlobalState" ); - // Authorization: foundation keys only + // The remaining accounts are the variable-length Link list, optionally + // followed by the payer's Permission account (appended last by the SDK). + // Peel it off so it is not mistaken for a Link in the loop below. + let remaining: Vec<&AccountInfo> = accounts_iter.collect(); + let (permission_account, link_accounts) = match remaining.last() { + Some(last) => { + let (perm_pda, _) = get_permission_pda(program_id, payer_account.key); + if last.key == &perm_pda { + (Some(*last), &remaining[..remaining.len() - 1]) + } else { + (None, &remaining[..]) + } + } + None => (None, &remaining[..]), + }; + + // Authorization: TOPOLOGY_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - msg!("TopologyClear: unauthorized — foundation key required"); - return Err(DoubleZeroError::Unauthorized.into()); - } + authorize( + program_id, + &mut permission_account.into_iter(), + payer_account.key, + &globalstate, + permission_flags::TOPOLOGY_ADMIN, + )?; // Validate topology PDA. Clear is tolerant of an already-closed topology, // so we cannot call validate_program_account! (it asserts non-empty). If @@ -83,7 +106,7 @@ pub fn process_topology_clear( let mut cleared_count: usize = 0; // Process remaining Link accounts: remove topology key from link_topologies - for link_account in accounts_iter { + for link_account in link_accounts.iter().copied() { validate_program_account!(link_account, program_id, writable = true, "Link"); let mut link = Link::try_from(link_account)?; assert_eq!( diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/create.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/create.rs index 382cffd4bd..0dbc7a980c 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/create.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/create.rs @@ -1,4 +1,5 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::{get_globalstate_pda, get_resource_extension_pda, get_topology_pda}, processors::{resource::allocate_id, validation::validate_program_account}, @@ -8,6 +9,7 @@ use crate::{ state::{ accounttype::AccountType, globalstate::GlobalState, + permission::permission_flags, topology::{validate_topology_name, TopologyConstraint, TopologyInfo}, }, }; @@ -31,8 +33,9 @@ pub struct TopologyCreateArgs { /// [0] topology PDA (writable, to be created) /// [1] admin_group_bits (writable, ResourceExtension) /// [2] globalstate (readonly) -/// [3] payer (writable, signer, must be in foundation_allowlist) +/// [3] payer (writable, signer, must hold TOPOLOGY_ADMIN) /// [4] system_program +/// [5] permission (readonly, optional — payer's Permission PDA) pub fn process_topology_create( program_id: &Pubkey, accounts: &[AccountInfo], @@ -56,12 +59,15 @@ pub fn process_topology_create( "GlobalState" ); - // Authorization: foundation keys only + // Authorization: TOPOLOGY_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(&globalstate_account.data.borrow()[..])?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - msg!("TopologyCreate: unauthorized — foundation key required"); - return Err(DoubleZeroError::Unauthorized.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::TOPOLOGY_ADMIN, + )?; // Normalize name to canonical uppercase form and validate format. let name = value.name.to_ascii_uppercase(); diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/delete.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/delete.rs index ec76eb837d..65b33750a6 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/delete.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/delete.rs @@ -1,9 +1,10 @@ use crate::{ + authorize::authorize, error::DoubleZeroError, pda::{get_globalstate_pda, get_topology_pda}, processors::validation::validate_program_account, serializer::try_acc_close, - state::{globalstate::GlobalState, topology::TopologyInfo}, + state::{globalstate::GlobalState, permission::permission_flags, topology::TopologyInfo}, }; use borsh::BorshSerialize; use borsh_incremental::BorshDeserializeIncremental; @@ -22,8 +23,9 @@ pub struct TopologyDeleteArgs { /// Accounts layout: /// [0] topology PDA (writable, to be closed) /// [1] globalstate (readonly) -/// [2] payer (writable, signer, must be in foundation_allowlist) +/// [2] payer (writable, signer, must hold TOPOLOGY_ADMIN) /// [3] system_program +/// [4] permission (readonly, optional — payer's Permission PDA) pub fn process_topology_delete( program_id: &Pubkey, accounts: &[AccountInfo], @@ -60,12 +62,15 @@ pub fn process_topology_delete( "GlobalState" ); - // Authorization: foundation keys only + // Authorization: TOPOLOGY_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; - if !globalstate.foundation_allowlist.contains(payer_account.key) { - msg!("TopologyDelete: unauthorized — foundation key required"); - return Err(DoubleZeroError::Unauthorized.into()); - } + authorize( + program_id, + accounts_iter, + payer_account.key, + &globalstate, + permission_flags::TOPOLOGY_ADMIN, + )?; // Guard: topology must have no remaining Link references. let topology = TopologyInfo::try_from(topology_account)?; diff --git a/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs b/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs index 81864e6c60..22a28c58be 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs @@ -4,7 +4,7 @@ use doublezero_serviceability::{ instructions::DoubleZeroInstruction, pda::{ get_contributor_pda, get_device_pda, get_exchange_pda, get_globalconfig_pda, get_link_pda, - get_location_pda, get_resource_extension_pda, get_topology_pda, + get_location_pda, get_permission_pda, get_resource_extension_pda, get_topology_pda, }, processors::{ contributor::create::ContributorCreateArgs, @@ -13,6 +13,7 @@ use doublezero_serviceability::{ globalstate::setfeatureflags::SetFeatureFlagsArgs, link::{create::LinkCreateArgs, update::LinkUpdateArgs}, location::create::LocationCreateArgs, + permission::create::PermissionCreateArgs, topology::{ assign_node_segments::AssignTopologyNodeSegmentsArgs, clear::TopologyClearArgs, create::TopologyCreateArgs, delete::TopologyDeleteArgs, @@ -25,6 +26,7 @@ use doublezero_serviceability::{ feature_flags::FeatureFlag, interface::{InterfaceCYOA, InterfaceDIA, LoopbackType, RoutingMode}, link::{Link, LinkDesiredStatus, LinkLinkType}, + permission::permission_flags, topology::{TopologyConstraint, TopologyInfo}, }, }; @@ -315,18 +317,89 @@ async fn test_topology_create_non_foundation_rejected() { ) .await; - // DoubleZeroError::Unauthorized = Custom(22) + // DoubleZeroError::NotAllowed = Custom(8) match result { Err(BanksClientError::TransactionError(TransactionError::InstructionError( 0, - InstructionError::Custom(22), + InstructionError::Custom(8), ))) => {} - _ => panic!("Expected Unauthorized error (Custom(22)), got {:?}", result), + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), } println!("[PASS] test_topology_create_non_foundation_rejected"); } +/// A non-foundation key holding a TOPOLOGY_ADMIN Permission account can create +/// a topology — exercises the new Permission-account authorization path end to end. +#[tokio::test] +async fn test_topology_create_with_permission_account_allowed() { + println!("[TEST] test_topology_create_with_permission_account_allowed"); + + let (mut banks_client, payer, program_id, globalstate_pubkey, _globalconfig_pubkey) = + setup_program_with_globalconfig().await; + + let (admin_group_bits_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::AdminGroupBits); + + // A keypair that is NOT in the foundation allowlist. + let topology_admin = Keypair::new(); + transfer( + &mut banks_client, + &payer, + &topology_admin.pubkey(), + 10_000_000, + ) + .await; + + // Foundation grants the key a Permission account with TOPOLOGY_ADMIN. + let (permission_pda, _) = get_permission_pda(&program_id, &topology_admin.pubkey()); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { + user_payer: topology_admin.pubkey(), + permissions: permission_flags::TOPOLOGY_ADMIN, + }), + vec![ + AccountMeta::new(permission_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + // The TOPOLOGY_ADMIN holder creates a topology, passing its Permission PDA + // as the optional trailing account that authorize() reads. + let (topology_pda, _) = get_topology_pda(&program_id, "permissioned-topology"); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let mut tx = create_transaction_with_extra_accounts( + program_id, + &DoubleZeroInstruction::CreateTopology(TopologyCreateArgs { + name: "permissioned-topology".to_string(), + constraint: TopologyConstraint::IncludeAny, + }), + &vec![ + AccountMeta::new(topology_pda, false), + AccountMeta::new(admin_group_bits_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &topology_admin, + &[AccountMeta::new_readonly(permission_pda, false)], + ); + tx.try_sign(&[&topology_admin], recent_blockhash).unwrap(); + banks_client + .process_transaction(tx) + .await + .expect("TOPOLOGY_ADMIN permission holder should be able to create a topology"); + + let topology = get_topology(&mut banks_client, topology_pda).await; + assert_eq!(topology.name, "PERMISSIONED-TOPOLOGY"); + + println!("[PASS] test_topology_create_with_permission_account_allowed"); +} + #[tokio::test] async fn test_topology_create_name_too_long_rejected() { println!("[TEST] test_topology_create_name_too_long_rejected"); @@ -952,12 +1025,13 @@ async fn test_topology_delete_fails_when_link_references_it() { assert!(link.link_topologies.contains(&topology_pda)); // Attempt to delete — should fail because the link still references it + // (the guard reads topology.reference_count; no link account is passed to delete). let result = delete_topology( &mut banks_client, program_id, globalstate_pubkey, "test-topology", - vec![AccountMeta::new_readonly(link_pubkey, false)], + vec![], &payer, ) .await; @@ -1373,13 +1447,13 @@ async fn test_topology_delete_non_foundation_rejected() { ) .await; - // DoubleZeroError::Unauthorized = Custom(22) + // DoubleZeroError::NotAllowed = Custom(8) match result { Err(BanksClientError::TransactionError(TransactionError::InstructionError( 0, - InstructionError::Custom(22), + InstructionError::Custom(8), ))) => {} - _ => panic!("Expected Unauthorized error (Custom(22)), got {:?}", result), + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), } println!("[PASS] test_topology_delete_non_foundation_rejected"); @@ -1437,13 +1511,13 @@ async fn test_topology_clear_non_foundation_rejected() { ) .await; - // DoubleZeroError::Unauthorized = Custom(22) + // DoubleZeroError::NotAllowed = Custom(8) match result { Err(BanksClientError::TransactionError(TransactionError::InstructionError( 0, - InstructionError::Custom(22), + InstructionError::Custom(8), ))) => {} - _ => panic!("Expected Unauthorized error (Custom(22)), got {:?}", result), + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), } println!("[PASS] test_topology_clear_non_foundation_rejected"); @@ -1737,13 +1811,13 @@ async fn test_topology_backfill_non_foundation_rejected() { ) .await; - // DoubleZeroError::Unauthorized = Custom(22) + // DoubleZeroError::NotAllowed = Custom(8) match result { Err(BanksClientError::TransactionError(TransactionError::InstructionError( 0, - InstructionError::Custom(22), + InstructionError::Custom(8), ))) => {} - _ => panic!("Expected Unauthorized error (Custom(22)), got {:?}", result), + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), } println!("[PASS] test_topology_backfill_non_foundation_rejected"); diff --git a/smartcontract/sdk/rs/src/commands/index/create.rs b/smartcontract/sdk/rs/src/commands/index/create.rs index 4f71204d07..0250a9cc2f 100644 --- a/smartcontract/sdk/rs/src/commands/index/create.rs +++ b/smartcontract/sdk/rs/src/commands/index/create.rs @@ -32,7 +32,7 @@ impl CreateIndexCommand { ]; client - .execute_transaction( + .execute_authorized_transaction( DoubleZeroInstruction::CreateIndex(IndexCreateArgs { entity_seed: self.entity_seed.clone(), key, diff --git a/smartcontract/sdk/rs/src/commands/index/delete.rs b/smartcontract/sdk/rs/src/commands/index/delete.rs index bea2160a0b..17e2952d5b 100644 --- a/smartcontract/sdk/rs/src/commands/index/delete.rs +++ b/smartcontract/sdk/rs/src/commands/index/delete.rs @@ -20,7 +20,7 @@ impl DeleteIndexCommand { AccountMeta::new_readonly(globalstate_pubkey, false), ]; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::DeleteIndex(IndexDeleteArgs {}), accounts, ) diff --git a/smartcontract/sdk/rs/src/commands/resource/allocate.rs b/smartcontract/sdk/rs/src/commands/resource/allocate.rs index c5654b0fc9..196cf7fc99 100644 --- a/smartcontract/sdk/rs/src/commands/resource/allocate.rs +++ b/smartcontract/sdk/rs/src/commands/resource/allocate.rs @@ -32,7 +32,7 @@ impl AllocateResourceCommand { _ => Pubkey::default(), }; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::AllocateResource(resource_allocate_args), vec![ AccountMeta::new(resource_pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/resource/closeaccount.rs b/smartcontract/sdk/rs/src/commands/resource/closeaccount.rs index 551df45ae9..7632b7990b 100644 --- a/smartcontract/sdk/rs/src/commands/resource/closeaccount.rs +++ b/smartcontract/sdk/rs/src/commands/resource/closeaccount.rs @@ -44,7 +44,7 @@ impl CloseResourceByPubkeyCommand { .execute(client) .map_err(|_err| eyre::eyre!("Globalstate not initialized"))?; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::CloseResource(ResourceExtensionCloseAccountArgs {}), vec![ AccountMeta::new(self.pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/resource/create.rs b/smartcontract/sdk/rs/src/commands/resource/create.rs index f636c4a316..229225a7ac 100644 --- a/smartcontract/sdk/rs/src/commands/resource/create.rs +++ b/smartcontract/sdk/rs/src/commands/resource/create.rs @@ -32,7 +32,7 @@ impl CreateResourceCommand { _ => Pubkey::default(), }; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::CreateResource(resource_create_args), vec![ AccountMeta::new(resource_pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/resource/deallocate.rs b/smartcontract/sdk/rs/src/commands/resource/deallocate.rs index 7a5b77b6a4..bf70d58766 100644 --- a/smartcontract/sdk/rs/src/commands/resource/deallocate.rs +++ b/smartcontract/sdk/rs/src/commands/resource/deallocate.rs @@ -32,7 +32,7 @@ impl DeallocateResourceCommand { _ => Pubkey::default(), }; - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::DeallocateResource(resource_deallocate_args), vec![ AccountMeta::new(resource_pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/topology/assign_node_segments.rs b/smartcontract/sdk/rs/src/commands/topology/assign_node_segments.rs index 41509817df..c5935611e2 100644 --- a/smartcontract/sdk/rs/src/commands/topology/assign_node_segments.rs +++ b/smartcontract/sdk/rs/src/commands/topology/assign_node_segments.rs @@ -41,7 +41,7 @@ impl AssignTopologyNodeSegmentsCommand { accounts.push(AccountMeta::new(*device_pk, false)); } - let sig = client.execute_transaction( + let sig = client.execute_authorized_transaction( DoubleZeroInstruction::AssignTopologyNodeSegments(AssignTopologyNodeSegmentsArgs { name: self.name.clone(), }), @@ -97,7 +97,7 @@ mod tests { let device2 = Pubkey::new_unique(); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::AssignTopologyNodeSegments( AssignTopologyNodeSegmentsArgs { @@ -152,7 +152,7 @@ mod tests { expected_accounts.push(AccountMeta::new(*device_pk, false)); } client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .times(1) .in_sequence(&mut seq) .with( diff --git a/smartcontract/sdk/rs/src/commands/topology/clear.rs b/smartcontract/sdk/rs/src/commands/topology/clear.rs index 264503f6ef..36db9510ef 100644 --- a/smartcontract/sdk/rs/src/commands/topology/clear.rs +++ b/smartcontract/sdk/rs/src/commands/topology/clear.rs @@ -39,7 +39,7 @@ impl ClearTopologyCommand { accounts.push(AccountMeta::new(*link_pk, false)); } - let sig = client.execute_transaction( + let sig = client.execute_authorized_transaction( DoubleZeroInstruction::ClearTopology(TopologyClearArgs { name: self.name.clone(), }), @@ -91,7 +91,7 @@ mod tests { let link2 = Pubkey::new_unique(); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::ClearTopology(TopologyClearArgs { name: "my-topology".to_string(), @@ -142,7 +142,7 @@ mod tests { expected_accounts.push(AccountMeta::new(*link_pk, false)); } client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .times(1) .in_sequence(&mut seq) .with( diff --git a/smartcontract/sdk/rs/src/commands/topology/create.rs b/smartcontract/sdk/rs/src/commands/topology/create.rs index e2083ed27f..693f61a3c2 100644 --- a/smartcontract/sdk/rs/src/commands/topology/create.rs +++ b/smartcontract/sdk/rs/src/commands/topology/create.rs @@ -46,7 +46,7 @@ impl CreateTopologyCommand { ) })?; - let signature = client.execute_transaction( + let signature = client.execute_authorized_transaction( DoubleZeroInstruction::CreateTopology(TopologyCreateArgs { name: self.name.clone(), constraint: self.constraint, @@ -131,7 +131,7 @@ mod tests { .returning(|_| Ok(Account::default())); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::CreateTopology(TopologyCreateArgs { name: "unicast-default".to_string(), @@ -199,7 +199,7 @@ mod tests { let mut seq = Sequence::new(); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .times(1) .in_sequence(&mut seq) .with( @@ -226,7 +226,7 @@ mod tests { }); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .times(1) .in_sequence(&mut seq) .with( diff --git a/smartcontract/sdk/rs/src/commands/topology/delete.rs b/smartcontract/sdk/rs/src/commands/topology/delete.rs index df5d0ec6e7..a30f552117 100644 --- a/smartcontract/sdk/rs/src/commands/topology/delete.rs +++ b/smartcontract/sdk/rs/src/commands/topology/delete.rs @@ -18,7 +18,7 @@ impl DeleteTopologyCommand { let (topology_pda, _) = get_topology_pda(&client.get_program_id(), &self.name); - client.execute_transaction( + client.execute_authorized_transaction( DoubleZeroInstruction::DeleteTopology(TopologyDeleteArgs { name: self.name.clone(), }), @@ -52,7 +52,7 @@ mod tests { let (topology_pda, _) = get_topology_pda(&client.get_program_id(), "unicast-default"); client - .expect_execute_transaction() + .expect_execute_authorized_transaction() .with( predicate::eq(DoubleZeroInstruction::DeleteTopology(TopologyDeleteArgs { name: "unicast-default".to_string(), From 77e86d4d17c6e2fd7ae112d41c2024061fb8563a Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Tue, 30 Jun 2026 13:53:55 +0000 Subject: [PATCH 10/11] smartcontract: fix ClearTopology account layout to match SDK trailing accounts The clear processor read payer/system_program at fixed front positions [2]/[3] while the SDK client appends them (plus the optional Permission account) after the variable-length link list, so doublezero topology clear reverted whenever links were passed. Parse payer/system/permission off the tail instead, mirroring assign_node_segments, and drop the redundant payer from the SDK fixed_accounts. Add a production-layout test helper and a TOPOLOGY_ADMIN permission clear test so the real on-wire layout is exercised (the previous helper placed payer/system before the extras, masking the mismatch). --- CHANGELOG.md | 1 + .../src/processors/topology/clear.rs | 69 +++++---- .../tests/test_helpers.rs | 36 +++++ .../tests/topology_test.rs | 139 +++++++++++++++++- .../sdk/rs/src/commands/topology/clear.rs | 14 +- 5 files changed, 221 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ed0a2e93..6d140b177e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -569,6 +569,7 @@ All notable changes to this project will be documented in this file. - Serviceability: add `Permission` account with `CreatePermission`, `UpdatePermission`, `DeletePermission`, `SuspendPermission`, and `ResumePermission` instructions for managing per-keypair permission bitmasks onchain - Serviceability: add `TOPOLOGY_ADMIN`, `RESOURCE_ADMIN`, and `INDEX_ADMIN` permission flags for delegating management of segment-routing topologies, ResourceExtension accounts, and internal Index accounts (legacy authorization maps each to the foundation allowlist) - Serviceability: enforce `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` via `authorize()` in the topology (create/delete/clear/assign-node-segments), resource (create/allocate/deallocate/close), and index (create/delete) instructions, which were previously gated by the foundation allowlist only + - Serviceability: fix `ClearTopology` account layout — the processor now parses `payer`/`system_program`/`permission` from the tail of the account list (matching what the SDK client appends after the variable-length link list) instead of reading them at fixed front positions, so `doublezero topology clear` no longer reverts when links are passed - SDK - Add `execute_authorized_transaction` (and its `_quiet` variant) alongside `execute_transaction`. The authorized variants append the payer's Permission PDA (read-only) as the trailing account when it exists on-chain, so `authorize()` can find it. All variants share the same builder, so the protocol-max compute-budget/heap-frame requests, preflight, and error-reporting behavior are identical to `execute_transaction`; the only difference is the optional trailing Permission account. The Permission PDA lookup is retried on transient RPC errors and memoized per client. - Add `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` permission-flag constants to the Go, TypeScript, and Python serviceability SDKs diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs index b1a9ea91d8..76874d113e 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs @@ -23,13 +23,17 @@ pub struct TopologyClearArgs { } /// Accounts layout: -/// [0] topology PDA (writable when account still exists; readonly is accepted when -/// the topology has already been closed — clear is tolerant of that) -/// [1] globalstate (readonly) -/// [2] payer (writable, signer, must hold TOPOLOGY_ADMIN) -/// [3] system_program -/// [4+] Link accounts (writable) — remove topology pubkey from link_topologies on each -/// [last] permission (readonly, optional — payer's Permission PDA, after the links) +/// [0] topology PDA (writable when account still exists; readonly is accepted when +/// the topology has already been closed — clear is tolerant of that) +/// [1] globalstate (readonly) +/// [2..n] Link accounts (writable) — remove topology pubkey from link_topologies on each +/// [n+1] payer (writable, signer, must hold TOPOLOGY_ADMIN) +/// [n+2] system_program +/// [n+3] permission (readonly, optional — payer's Permission PDA) +/// +/// Note: payer and system_program are the last two accounts (or the last two +/// before the optional Permission account). The SDK client always appends them +/// after the variable-length link list. pub fn process_topology_clear( program_id: &Pubkey, accounts: &[AccountInfo], @@ -39,12 +43,43 @@ pub fn process_topology_clear( let topology_account = next_account_info(accounts_iter)?; let globalstate_account = next_account_info(accounts_iter)?; - let payer_account = next_account_info(accounts_iter)?; - let _system_program = next_account_info(accounts_iter)?; #[cfg(test)] msg!("process_topology_clear(name={})", value.name); + // Collect remaining accounts. The SDK client always appends payer and + // system_program at the end, after the variable-length Link list, plus an + // optional Permission account when one exists for the payer. + let all_remaining: Vec<&AccountInfo> = accounts_iter.collect(); + if all_remaining.len() < 2 { + msg!("TopologyClear: expected at least payer and system_program accounts"); + return Err(DoubleZeroError::InvalidArgument.into()); + } + let n = all_remaining.len(); + // Detect an optional trailing Permission account. With it present the layout + // is [links.., payer, system, permission]; the payer would then be at n-3, + // so the last account is a Permission account iff it matches that payer's PDA. + let permission_account = if n >= 3 { + let candidate_payer = all_remaining[n - 3]; + let (perm_pda, _) = get_permission_pda(program_id, candidate_payer.key); + (all_remaining[n - 1].key == &perm_pda).then_some(all_remaining[n - 1]) + } else { + None + }; + let (payer_account, _system_program, link_accounts) = if permission_account.is_some() { + ( + all_remaining[n - 3], + all_remaining[n - 2], + &all_remaining[..n - 3], + ) + } else { + ( + all_remaining[n - 2], + all_remaining[n - 1], + &all_remaining[..n - 2], + ) + }; + // Payer must be a signer if !payer_account.is_signer { msg!("TopologyClear: payer must be a signer"); @@ -60,22 +95,6 @@ pub fn process_topology_clear( "GlobalState" ); - // The remaining accounts are the variable-length Link list, optionally - // followed by the payer's Permission account (appended last by the SDK). - // Peel it off so it is not mistaken for a Link in the loop below. - let remaining: Vec<&AccountInfo> = accounts_iter.collect(); - let (permission_account, link_accounts) = match remaining.last() { - Some(last) => { - let (perm_pda, _) = get_permission_pda(program_id, payer_account.key); - if last.key == &perm_pda { - (Some(*last), &remaining[..remaining.len() - 1]) - } else { - (None, &remaining[..]) - } - } - None => (None, &remaining[..]), - }; - // Authorization: TOPOLOGY_ADMIN (Permission account) or foundation (legacy). let globalstate = GlobalState::try_from(globalstate_account)?; authorize( diff --git a/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs b/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs index e8cdd52386..cd543dad2c 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs @@ -419,6 +419,42 @@ pub fn create_transaction_with_extra_accounts( ) } +/// Builds a transaction whose account list matches what the production SDK +/// client (`assemble_instructions`) emits: the caller's `accounts`, then the +/// payer and system_program, then an optional Permission account — all appended +/// *after* any variable-length accounts the caller put in `accounts`. +/// +/// Use this (rather than [`create_transaction_with_extra_accounts`], which +/// places payer/system *before* the extras) for the variable-length processors +/// that parse payer/system/permission off the tail of the account list, so the +/// test exercises the same on-wire layout the CLI produces. +#[allow(dead_code)] +pub fn create_authorized_transaction( + program_id: Pubkey, + instruction: &DoubleZeroInstruction, + accounts: &[AccountMeta], + payer: &Keypair, + permission: Option, +) -> Transaction { + let mut metas = accounts.to_vec(); + metas.push(AccountMeta::new(payer.pubkey(), true)); + metas.push(AccountMeta::new( + solana_system_interface::program::ID, + false, + )); + if let Some(permission) = permission { + metas.push(permission); + } + Transaction::new_with_payer( + &[Instruction::new_with_bytes( + program_id, + &to_vec(instruction).unwrap(), + metas, + )], + Some(&payer.pubkey()), + ) +} + #[allow(dead_code)] pub async fn get_resource_extension_data( banks_client: &mut BanksClient, diff --git a/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs b/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs index 22a28c58be..ff73af12a1 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/topology_test.rs @@ -580,6 +580,10 @@ async fn delete_topology( } /// Creates a clear topology instruction, passing the given link accounts as writable. +/// +/// Uses the production on-wire layout (`[topology, globalstate, links.., payer, +/// system, permission?]`) so the variable-length link list comes before the +/// payer/system_program that the SDK appends — exactly what the CLI emits. async fn clear_topology( banks_client: &mut BanksClient, program_id: Pubkey, @@ -587,21 +591,45 @@ async fn clear_topology( name: &str, link_accounts: Vec, payer: &Keypair, +) { + clear_topology_with_permission( + banks_client, + program_id, + globalstate_pubkey, + name, + link_accounts, + payer, + None, + ) + .await +} + +/// Like [`clear_topology`], but allows passing the payer's Permission PDA as the +/// optional trailing account that `authorize()` reads. +async fn clear_topology_with_permission( + banks_client: &mut BanksClient, + program_id: Pubkey, + globalstate_pubkey: Pubkey, + name: &str, + link_accounts: Vec, + payer: &Keypair, + permission: Option, ) { let (topology_pda, _) = get_topology_pda(&program_id, name); let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); - let base_accounts = vec![ + let mut accounts = vec![ AccountMeta::new(topology_pda, false), AccountMeta::new_readonly(globalstate_pubkey, false), ]; - let mut tx = create_transaction_with_extra_accounts( + accounts.extend(link_accounts); + let mut tx = create_authorized_transaction( program_id, &DoubleZeroInstruction::ClearTopology(TopologyClearArgs { name: name.to_string(), }), - &base_accounts, + &accounts, payer, - &link_accounts, + permission, ); tx.try_sign(&[&payer], recent_blockhash).unwrap(); banks_client.process_transaction(tx).await.unwrap(); @@ -1331,6 +1359,109 @@ async fn test_topology_clear_removes_from_links() { println!("[PASS] test_topology_clear_removes_from_links"); } +/// A non-foundation key holding a TOPOLOGY_ADMIN Permission account can clear a +/// topology from a link. Exercises the Permission-account authorization path for +/// `clear` through the production on-wire account layout (links before the +/// trailing payer/system/permission), which is what the CLI/SDK actually emits. +#[tokio::test] +async fn test_topology_clear_with_permission_account_allowed() { + println!("[TEST] test_topology_clear_with_permission_account_allowed"); + + let (mut banks_client, payer, program_id, globalstate_pubkey, _globalconfig_pubkey) = + setup_program_with_globalconfig().await; + + let (admin_group_bits_pda, _, _) = + get_resource_extension_pda(&program_id, ResourceType::AdminGroupBits); + + let topology_pda = create_topology( + &mut banks_client, + program_id, + globalstate_pubkey, + admin_group_bits_pda, + "test-topology", + TopologyConstraint::IncludeAny, + &payer, + ) + .await; + + // unicast-default topology is required for link activation. + create_topology( + &mut banks_client, + program_id, + globalstate_pubkey, + admin_group_bits_pda, + "unicast-default", + TopologyConstraint::IncludeAny, + &payer, + ) + .await; + + // Set up a WAN link and assign the topology to it (foundation payer). + let (link_pubkey, contributor_pubkey, _, _) = + setup_wan_link(&mut banks_client, program_id, globalstate_pubkey, &payer).await; + assign_link_topology( + &mut banks_client, + program_id, + globalstate_pubkey, + link_pubkey, + contributor_pubkey, + vec![topology_pda], + &payer, + ) + .await; + let link = get_link(&mut banks_client, link_pubkey).await; + assert!(link.link_topologies.contains(&topology_pda)); + + // A keypair that is NOT in the foundation allowlist, granted TOPOLOGY_ADMIN. + let topology_admin = Keypair::new(); + transfer( + &mut banks_client, + &payer, + &topology_admin.pubkey(), + 10_000_000, + ) + .await; + let (permission_pda, _) = get_permission_pda(&program_id, &topology_admin.pubkey()); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { + user_payer: topology_admin.pubkey(), + permissions: permission_flags::TOPOLOGY_ADMIN, + }), + vec![ + AccountMeta::new(permission_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + // The TOPOLOGY_ADMIN holder clears the topology from the link, passing its + // Permission PDA as the optional trailing account that authorize() reads. + clear_topology_with_permission( + &mut banks_client, + program_id, + globalstate_pubkey, + "test-topology", + vec![AccountMeta::new(link_pubkey, false)], + &topology_admin, + Some(AccountMeta::new_readonly(permission_pda, false)), + ) + .await; + + // Verify the link no longer references the topology. + let link = get_link(&mut banks_client, link_pubkey).await; + assert!( + link.link_topologies.is_empty(), + "link_topologies should be empty after clear" + ); + + println!("[PASS] test_topology_clear_with_permission_account_allowed"); +} + #[tokio::test] async fn test_topology_clear_is_idempotent() { println!("[TEST] test_topology_clear_is_idempotent"); diff --git a/smartcontract/sdk/rs/src/commands/topology/clear.rs b/smartcontract/sdk/rs/src/commands/topology/clear.rs index 36db9510ef..2bc03d4b6d 100644 --- a/smartcontract/sdk/rs/src/commands/topology/clear.rs +++ b/smartcontract/sdk/rs/src/commands/topology/clear.rs @@ -6,8 +6,9 @@ use doublezero_serviceability::{ use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Signature}; /// Max link accounts per clear transaction. Solana caps transactions at 32 -/// accounts; with 3 fixed accounts (topology PDA, globalstate, payer) we -/// stay well under that limit at 16 (same constant as backfill). +/// accounts; with 2 fixed accounts (topology PDA, globalstate) plus the payer +/// and system_program appended by the client, we stay well under that limit at +/// 16 (same constant as backfill). pub const CLEAR_BATCH_SIZE: usize = 16; #[derive(Debug, PartialEq, Clone)] @@ -24,12 +25,11 @@ impl ClearTopologyCommand { let (topology_pda, _) = get_topology_pda(&client.get_program_id(), &self.name); - let payer = client.get_payer(); - + // payer and system_program are appended by execute_authorized_transaction + // after the variable-length link list, so they are not listed here. let fixed_accounts = [ AccountMeta::new_readonly(topology_pda, false), AccountMeta::new_readonly(globalstate_pubkey, false), - AccountMeta::new(payer, true), ]; let mut signatures = Vec::new(); @@ -86,7 +86,6 @@ mod tests { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); let (topology_pda, _) = get_topology_pda(&client.get_program_id(), "my-topology"); - let payer = client.get_payer(); let link1 = Pubkey::new_unique(); let link2 = Pubkey::new_unique(); @@ -99,7 +98,6 @@ mod tests { predicate::eq(vec![ AccountMeta::new_readonly(topology_pda, false), AccountMeta::new_readonly(globalstate_pubkey, false), - AccountMeta::new(payer, true), AccountMeta::new(link1, false), AccountMeta::new(link2, false), ]), @@ -121,14 +119,12 @@ mod tests { let (globalstate_pubkey, _) = get_globalstate_pda(&client.get_program_id()); let (topology_pda, _) = get_topology_pda(&client.get_program_id(), "my-topology"); - let payer = client.get_payer(); let links: Vec = (0..33).map(|_| Pubkey::new_unique()).collect(); let fixed_accounts = vec![ AccountMeta::new_readonly(topology_pda, false), AccountMeta::new_readonly(globalstate_pubkey, false), - AccountMeta::new(payer, true), ]; let expected_args = DoubleZeroInstruction::ClearTopology(TopologyClearArgs { From 641791e34beba1e8aec0807e5d706aa3c5ee5f78 Mon Sep 17 00:00:00 2001 From: Juan Olveira Date: Tue, 30 Jun 2026 16:54:13 +0000 Subject: [PATCH 11/11] smartcontract: extract trailing-permission helper and broaden legacy auth fallback Extract the duplicated trailing-account parsing in topology clear and assign-node-segments into a shared authorize::split_trailing_permission helper, so the variable-length [leading.., payer, system, permission?] layout is parsed in one tested place. Also broaden authorize(): when a payer's auto-injected Permission account exists but does not grant the requested flag, fall back to the legacy allowlist check as long as RequirePermissionAccounts is off. This makes the Some/None branches symmetric until strict mode is enabled, so a foundation (or other legacy-authorized) key is not locked out of an instruction merely because it also holds an unrelated, under-privileged Permission account. --- CHANGELOG.md | 1 + .../src/authorize.rs | 175 +++++++++++++++++- .../topology/assign_node_segments.rs | 40 +--- .../src/processors/topology/clear.rs | 40 +--- 4 files changed, 185 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d140b177e..59ff3cffe6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -570,6 +570,7 @@ All notable changes to this project will be documented in this file. - Serviceability: add `TOPOLOGY_ADMIN`, `RESOURCE_ADMIN`, and `INDEX_ADMIN` permission flags for delegating management of segment-routing topologies, ResourceExtension accounts, and internal Index accounts (legacy authorization maps each to the foundation allowlist) - Serviceability: enforce `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` via `authorize()` in the topology (create/delete/clear/assign-node-segments), resource (create/allocate/deallocate/close), and index (create/delete) instructions, which were previously gated by the foundation allowlist only - Serviceability: fix `ClearTopology` account layout — the processor now parses `payer`/`system_program`/`permission` from the tail of the account list (matching what the SDK client appends after the variable-length link list) instead of reading them at fixed front positions, so `doublezero topology clear` no longer reverts when links are passed + - Serviceability: `authorize()` now falls back to the legacy allowlists when a payer's auto-injected Permission account exists but does not grant the requested flag, as long as `RequirePermissionAccounts` is off — so foundation (and other legacy-authorized) keys are not locked out of an instruction merely because they also hold an unrelated, under-privileged Permission account - SDK - Add `execute_authorized_transaction` (and its `_quiet` variant) alongside `execute_transaction`. The authorized variants append the payer's Permission PDA (read-only) as the trailing account when it exists on-chain, so `authorize()` can find it. All variants share the same builder, so the protocol-max compute-budget/heap-frame requests, preflight, and error-reporting behavior are identical to `execute_transaction`; the only difference is the optional trailing Permission account. The Permission PDA lookup is retried on transient RPC errors and memoized per client. - Add `TOPOLOGY_ADMIN`/`RESOURCE_ADMIN`/`INDEX_ADMIN` permission-flag constants to the Go, TypeScript, and Python serviceability SDKs diff --git a/smartcontract/programs/doublezero-serviceability/src/authorize.rs b/smartcontract/programs/doublezero-serviceability/src/authorize.rs index aaa855e30a..eca86d8de0 100644 --- a/smartcontract/programs/doublezero-serviceability/src/authorize.rs +++ b/smartcontract/programs/doublezero-serviceability/src/authorize.rs @@ -10,6 +10,7 @@ use crate::{ use solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, + msg, program_error::ProgramError, pubkey::Pubkey, }; @@ -23,8 +24,10 @@ use solana_program::{ /// `any_of_flags` uses OR semantics: the payer is authorized if their Permission account has /// at least one of the specified `permission_flags::*` bits set. /// -/// Legacy fallback mapping (used when no Permission account is provided and -/// `FeatureFlag::RequirePermissionAccounts` is not set): +/// Legacy fallback mapping (used whenever `FeatureFlag::RequirePermissionAccounts` +/// is not set — both when no Permission account is provided and when the provided +/// Permission account exists but does not grant the requested flag, so the SDK +/// auto-injecting the payer's Permission PDA can never lock out a legacy key): /// FOUNDATION → foundation_allowlist /// QA → qa_allowlist /// ACTIVATOR → activator_authority_pk @@ -84,6 +87,21 @@ where if foundation_permission_recovery(globalstate, payer_key, any_of_flags) { return Ok(()); } + // While strict mode is off, the legacy allowlists/authorities remain + // authoritative — exactly as in the None branch. Because the SDK + // auto-appends the payer's Permission PDA whenever one exists on-chain, + // a present-but-insufficient Permission account must not lock out a key + // that legacy authorization would still accept (e.g. a foundation member + // who also holds an unrelated, under-privileged Permission account). + // Once RequirePermissionAccounts is enabled, only the Permission account + // (or the foundation PERMISSION_ADMIN recovery above) authorizes. + if !is_feature_enabled( + globalstate.feature_flags, + FeatureFlag::RequirePermissionAccounts, + ) && check_legacy_any(payer_key, globalstate, any_of_flags) + { + return Ok(()); + } return Err(DoubleZeroError::NotAllowed.into()); } } @@ -229,6 +247,64 @@ fn check_legacy_any(payer: &Pubkey, globalstate: &GlobalState, any_of: u128) -> false } +/// Splits the trailing accounts of a variable-length instruction into its +/// `(payer, system_program, leading, permission)` parts. +/// +/// Variable-length instructions (e.g. topology clear / assign-node-segments) +/// place a caller-controlled list of accounts first; the SDK client then +/// appends `payer`, `system_program`, and — when one exists on-chain — the +/// payer's Permission PDA. `remaining` is everything left after the +/// instruction's own fixed leading accounts have been consumed. +/// +/// With a Permission account present the layout is `[leading.., payer, system, +/// permission]`, so the payer sits at `n - 3` and the last account is the +/// Permission account iff it matches that payer's PDA. The returned +/// `permission` is ready to hand to [`authorize`] (via a single-element +/// iterator); `leading` is the caller's variable-length list. +/// +/// Errors when the two mandatory `payer`/`system_program` accounts are absent. +#[allow(clippy::type_complexity)] +pub fn split_trailing_permission<'a, 'r, 'info>( + program_id: &Pubkey, + remaining: &'a [&'r AccountInfo<'info>], +) -> Result< + ( + &'r AccountInfo<'info>, + &'r AccountInfo<'info>, + &'a [&'r AccountInfo<'info>], + Option<&'r AccountInfo<'info>>, + ), + ProgramError, +> { + let n = remaining.len(); + if n < 2 { + msg!("expected at least payer and system_program accounts"); + return Err(DoubleZeroError::InvalidArgument.into()); + } + let permission = if n >= 3 { + let candidate_payer = remaining[n - 3]; + let (perm_pda, _) = get_permission_pda(program_id, candidate_payer.key); + (remaining[n - 1].key == &perm_pda).then_some(remaining[n - 1]) + } else { + None + }; + Ok(if permission.is_some() { + ( + remaining[n - 3], + remaining[n - 2], + &remaining[..n - 3], + permission, + ) + } else { + ( + remaining[n - 2], + remaining[n - 1], + &remaining[..n - 2], + None, + ) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -1225,8 +1301,11 @@ mod tests { #[test] fn test_permission_account_foundation_recovery_only_for_permission_admin() { - // Recovery applies only to PERMISSION_ADMIN. A foundation member with a - // suspended Permission account cannot use it to satisfy USER_ADMIN. + // Recovery applies only to PERMISSION_ADMIN. In strict mode a foundation + // member with a suspended Permission account cannot use it to satisfy + // USER_ADMIN. Strict mode is required to isolate the recovery semantics: + // while RequirePermissionAccounts is off the legacy fallback would accept + // the foundation member (see test_permission_account_insufficient_falls_back_to_legacy_when_flag_off). let program_id = Pubkey::new_unique(); let payer = Pubkey::new_unique(); let (pda, _, mut data) = make_permission_data( @@ -1249,7 +1328,8 @@ mod tests { ); let accounts = [account]; let mut iter = accounts.iter(); - let gs = gs_with_foundation(&payer); + let mut gs = gs_with_foundation(&payer); + gs.feature_flags = FeatureFlag::RequirePermissionAccounts.to_mask(); assert_eq!( authorize( @@ -1264,6 +1344,91 @@ mod tests { ); } + #[test] + fn test_permission_account_insufficient_falls_back_to_legacy_when_flag_off() { + // While RequirePermissionAccounts is off, a present-but-insufficient + // Permission account must not lock out a legacy-authorized key. The SDK + // auto-appends the payer's Permission PDA whenever one exists on-chain, so + // a foundation member who also holds an unrelated, under-privileged + // Permission account must still be authorized for legacy-mapped flags. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + // Permission account grants only QA, but the instruction needs TOPOLOGY_ADMIN. + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Activated, + permission_flags::QA, + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + // Payer is a foundation member; flag is off (default). + let gs = gs_with_foundation(&payer); + + assert!(authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::TOPOLOGY_ADMIN + ) + .is_ok()); + } + + #[test] + fn test_permission_account_insufficient_denied_when_flag_on() { + // In strict mode the legacy fallback is disabled: the same foundation + // member with a QA-only Permission account is denied TOPOLOGY_ADMIN. + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let (pda, _, mut data) = make_permission_data( + &program_id, + &payer, + PermissionStatus::Activated, + permission_flags::QA, + ); + + let mut lamports = 100_000u64; + let account = AccountInfo::new( + &pda, + false, + false, + &mut lamports, + &mut data, + &program_id, + false, + Epoch::default(), + ); + let accounts = [account]; + let mut iter = accounts.iter(); + let mut gs = gs_with_foundation(&payer); + gs.feature_flags = FeatureFlag::RequirePermissionAccounts.to_mask(); + + assert_eq!( + authorize( + &program_id, + &mut iter, + &payer, + &gs, + permission_flags::TOPOLOGY_ADMIN + ) + .unwrap_err(), + DoubleZeroError::NotAllowed.into() + ); + } + // ── New path overrides feature flag enforcement ─────────────────────────── #[test] diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs index f627efeb28..5e214f1014 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/assign_node_segments.rs @@ -1,7 +1,7 @@ use crate::{ - authorize::authorize, + authorize::{authorize, split_trailing_permission}, error::DoubleZeroError, - pda::{get_globalstate_pda, get_permission_pda, get_resource_extension_pda, get_topology_pda}, + pda::{get_globalstate_pda, get_resource_extension_pda, get_topology_pda}, processors::{resource::allocate_id, validation::validate_program_account}, resource::ResourceType, serializer::try_acc_write, @@ -54,38 +54,12 @@ pub fn process_assign_topology_node_segments( let segment_routing_ids_account = next_account_info(accounts_iter)?; let globalstate_account = next_account_info(accounts_iter)?; - // Collect remaining accounts. The SDK client always appends payer and - // system_program at the end, after the variable-length device list, plus an - // optional Permission account when one exists for the payer. + // The SDK client appends payer and system_program after the variable-length + // device list, plus an optional Permission account when one exists for the + // payer. split_trailing_permission peels those off the tail. let all_remaining: Vec<&AccountInfo> = accounts_iter.collect(); - if all_remaining.len() < 2 { - msg!("AssignTopologyNodeSegments: expected at least payer and system_program accounts"); - return Err(DoubleZeroError::InvalidArgument.into()); - } - let n = all_remaining.len(); - // Detect an optional trailing Permission account. With it present the layout - // is [devices.., payer, system, permission]; the payer would then be at n-3, - // so the last account is a Permission account iff it matches that payer's PDA. - let permission_account = if n >= 3 { - let candidate_payer = all_remaining[n - 3]; - let (perm_pda, _) = get_permission_pda(program_id, candidate_payer.key); - (all_remaining[n - 1].key == &perm_pda).then_some(all_remaining[n - 1]) - } else { - None - }; - let (payer_account, _system_program, device_accounts) = if permission_account.is_some() { - ( - all_remaining[n - 3], - all_remaining[n - 2], - &all_remaining[..n - 3], - ) - } else { - ( - all_remaining[n - 2], - all_remaining[n - 1], - &all_remaining[..n - 2], - ) - }; + let (payer_account, _system_program, device_accounts, permission_account) = + split_trailing_permission(program_id, &all_remaining)?; #[cfg(test)] msg!("process_assign_topology_node_segments(name={})", value.name); diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs index 76874d113e..adcda80413 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/topology/clear.rs @@ -1,7 +1,7 @@ use crate::{ - authorize::authorize, + authorize::{authorize, split_trailing_permission}, error::DoubleZeroError, - pda::{get_globalstate_pda, get_link_pda, get_permission_pda, get_topology_pda}, + pda::{get_globalstate_pda, get_link_pda, get_topology_pda}, processors::validation::validate_program_account, serializer::try_acc_write, state::{ @@ -47,38 +47,12 @@ pub fn process_topology_clear( #[cfg(test)] msg!("process_topology_clear(name={})", value.name); - // Collect remaining accounts. The SDK client always appends payer and - // system_program at the end, after the variable-length Link list, plus an - // optional Permission account when one exists for the payer. + // The SDK client appends payer and system_program after the variable-length + // Link list, plus an optional Permission account when one exists for the + // payer. split_trailing_permission peels those off the tail. let all_remaining: Vec<&AccountInfo> = accounts_iter.collect(); - if all_remaining.len() < 2 { - msg!("TopologyClear: expected at least payer and system_program accounts"); - return Err(DoubleZeroError::InvalidArgument.into()); - } - let n = all_remaining.len(); - // Detect an optional trailing Permission account. With it present the layout - // is [links.., payer, system, permission]; the payer would then be at n-3, - // so the last account is a Permission account iff it matches that payer's PDA. - let permission_account = if n >= 3 { - let candidate_payer = all_remaining[n - 3]; - let (perm_pda, _) = get_permission_pda(program_id, candidate_payer.key); - (all_remaining[n - 1].key == &perm_pda).then_some(all_remaining[n - 1]) - } else { - None - }; - let (payer_account, _system_program, link_accounts) = if permission_account.is_some() { - ( - all_remaining[n - 3], - all_remaining[n - 2], - &all_remaining[..n - 3], - ) - } else { - ( - all_remaining[n - 2], - all_remaining[n - 1], - &all_remaining[..n - 2], - ) - }; + let (payer_account, _system_program, link_accounts, permission_account) = + split_trailing_permission(program_id, &all_remaining)?; // Payer must be a signer if !payer_account.is_signer {