From 356433df04313965e7c68198c84120478860d2fa Mon Sep 17 00:00:00 2001 From: Greg Mitchell Date: Wed, 20 May 2026 19:59:12 +0000 Subject: [PATCH 1/3] smartcontract/cli: fix resource verify for flex-algo segments and duplicate usages verify_segment_routing_ids now walks Interface.flex_algo_node_segments in addition to the base node_segment_idx, so per-topology segment IDs allocated by interface-create and assign_node_segments are recognized as in-use instead of appearing under "Allocated but not used." A value with multiple owners is now emitted as a single DuplicateUsage that lists every owner (collapsing N-way collisions into one entry) and is suppressed from the "Used but not allocated" section, where it was previously double-reported. --- smartcontract/cli/src/resource/verify.rs | 500 +++++++++++++++++------ smartcontract/programs/CLAUDE.md | 4 + 2 files changed, 386 insertions(+), 118 deletions(-) diff --git a/smartcontract/cli/src/resource/verify.rs b/smartcontract/cli/src/resource/verify.rs index efa3235b6e..1317ce0ce8 100644 --- a/smartcontract/cli/src/resource/verify.rs +++ b/smartcontract/cli/src/resource/verify.rs @@ -20,7 +20,7 @@ use doublezero_serviceability::{ }; use solana_sdk::pubkey::Pubkey; use std::{ - collections::{hash_map::Entry, HashMap, HashSet}, + collections::{HashMap, HashSet}, io::{BufRead, Write}, }; @@ -41,14 +41,12 @@ pub enum ResourceDiscrepancy { }, /// Resource extension account not found ExtensionNotFound { resource_type: ResourceType }, - /// Resource is used by multiple accounts (duplicate usage) + /// Resource is used by multiple accounts (duplicate usage). `accounts` + /// lists every owner in insertion order; `accounts.len() >= 2`. DuplicateUsage { resource_type: ResourceType, value: IdOrIp, - first_account_pubkey: Pubkey, - first_account_type: String, - second_account_pubkey: Pubkey, - second_account_type: String, + accounts: Vec<(Pubkey, String)>, }, /// ResourceExtension account exists onchain but does not correspond to any /// currently-expected PDA (global singleton or per-device extension for a @@ -277,21 +275,21 @@ impl VerifyResourceCliCommand { if let ResourceDiscrepancy::DuplicateUsage { resource_type, value, - first_account_pubkey, - first_account_type, - second_account_pubkey, - second_account_type, + accounts, } = d { + let owners = accounts + .iter() + .map(|(pk, ty)| format!("{} {}", ty, pk)) + .collect::>() + .join(", "); writeln!( out, - " {} = {} (used by {} {} AND {} {})", + " {} = {} (used by {} owners: {})", resource_type, value, - first_account_type, - first_account_pubkey, - second_account_type, - second_account_pubkey + accounts.len(), + owners )?; } } @@ -369,21 +367,21 @@ impl VerifyResourceCliCommand { if let ResourceDiscrepancy::DuplicateUsage { resource_type, value, - first_account_pubkey, - first_account_type, - second_account_pubkey, - second_account_type, + accounts, } = d { + let owners = accounts + .iter() + .map(|(pk, ty)| format!("{} {}", ty, pk)) + .collect::>() + .join(", "); writeln!( out, - " {} = {} (used by {} {} AND {} {})", + " {} = {} (used by {} owners: {})", resource_type, value, - first_account_type, - first_account_pubkey, - second_account_type, - second_account_pubkey + accounts.len(), + owners )?; duplicate_values.push((*resource_type, value.clone())); } @@ -729,8 +727,7 @@ fn verify_user_tunnel_block( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); // Build set of in-use IPs from users - let resource_type = ResourceType::UserTunnelBlock; - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); for (user_pk, user) in users { let tunnel_ip = user.tunnel_net.ip(); if !tunnel_ip.is_unspecified() && user.tunnel_net.prefix() > 0 { @@ -739,14 +736,7 @@ fn verify_user_tunnel_block( if let Some(ip) = user.tunnel_net.nth(i) { let ip_net = NetworkV4::new(ip, 32).unwrap(); let id_or_ip = IdOrIp::Ip(ip_net); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *user_pk, - "User".to_string(), - result, - ); + insert_usage(&mut in_use, id_or_ip, *user_pk, "User".to_string()); } } result.user_tunnel_block_checked += 1; @@ -789,19 +779,12 @@ fn verify_tunnel_ids( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); if let Some(device_users) = users_by_device.get(device_pk) { for (user_pk, user) in device_users { if user.tunnel_id != 0 { let id_or_ip = IdOrIp::Id(user.tunnel_id); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *user_pk, - "User".to_string(), - result, - ); + insert_usage(&mut in_use, id_or_ip, *user_pk, "User".to_string()); result.tunnel_ids_checked += 1; } } @@ -834,18 +817,16 @@ fn verify_dz_prefix_block( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); // Find users whose dz_ip falls within this prefix - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); // First IP is reserved for the device itself (Loopback100) let first_ip = prefix.ip(); let first_ip_net = NetworkV4::new(first_ip, 32).unwrap(); insert_usage( &mut in_use, - resource_type, IdOrIp::Ip(first_ip_net), *device_pk, "Device (reserved)".to_string(), - result, ); for (user_pk, user) in users { @@ -863,14 +844,7 @@ fn verify_dz_prefix_block( if prefix.contains(dz_ip) { let ip_net = NetworkV4::new(dz_ip, 32).unwrap(); let id_or_ip = IdOrIp::Ip(ip_net); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *user_pk, - "User".to_string(), - result, - ); + insert_usage(&mut in_use, id_or_ip, *user_pk, "User".to_string()); result.dz_prefix_block_checked += 1; } } @@ -899,7 +873,7 @@ fn verify_device_tunnel_block( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); // Check device loopback interfaces (only vpnv4/ipv4 loopback types) for (device_pk, device) in devices { @@ -917,11 +891,9 @@ fn verify_device_tunnel_block( let id_or_ip = IdOrIp::Ip(ip_net); insert_usage( &mut in_use, - resource_type, id_or_ip, *device_pk, format!("Device interface {}", iface.name), - result, ); } } @@ -940,14 +912,7 @@ fn verify_device_tunnel_block( if let Some(ip) = link.tunnel_net.nth(i) { let ip_net = NetworkV4::new(ip, 32).unwrap(); let id_or_ip = IdOrIp::Ip(ip_net); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *link_pk, - "Link".to_string(), - result, - ); + insert_usage(&mut in_use, id_or_ip, *link_pk, "Link".to_string()); } } result.device_tunnel_block_checked += 1; @@ -975,26 +940,43 @@ fn verify_segment_routing_ids( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); for (device_pk, device) in devices { for iface in &device.interfaces { - // Only check vpnv4/ipv4 loopbacks, and node_segment_idx == 0 means not allocated + // Only check vpnv4/ipv4 loopbacks; node_segment_idx == 0 means + // unallocated. flex_algo_node_segments is only populated on Vpnv4 + // loopbacks (one entry per topology) and pulls from the same + // SegmentRoutingIds allocator, so it must be counted here too — + // otherwise every flex-algo segment ID looks allocated-but-unused. if iface.interface_type == InterfaceType::Loopback && (iface.loopback_type == LoopbackType::Vpnv4 || iface.loopback_type == LoopbackType::Ipv4) - && iface.node_segment_idx != 0 { - let id_or_ip = IdOrIp::Id(iface.node_segment_idx); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *device_pk, - format!("Device interface {}", iface.name), - result, - ); - result.segment_routing_ids_checked += 1; + if iface.node_segment_idx != 0 { + insert_usage( + &mut in_use, + IdOrIp::Id(iface.node_segment_idx), + *device_pk, + format!("Device interface {}", iface.name), + ); + result.segment_routing_ids_checked += 1; + } + for segment in &iface.flex_algo_node_segments { + if segment.node_segment_idx == 0 { + continue; + } + insert_usage( + &mut in_use, + IdOrIp::Id(segment.node_segment_idx), + *device_pk, + format!( + "Device interface {} flex-algo segment (topology {})", + iface.name, segment.topology + ), + ); + result.segment_routing_ids_checked += 1; + } } } } @@ -1020,18 +1002,11 @@ fn verify_link_ids( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); for (link_pk, link) in links { let id_or_ip = IdOrIp::Id(link.tunnel_id); - insert_usage( - &mut in_use, - resource_type, - id_or_ip, - *link_pk, - "Link".to_string(), - result, - ); + insert_usage(&mut in_use, id_or_ip, *link_pk, "Link".to_string()); result.link_ids_checked += 1; } @@ -1056,7 +1031,7 @@ fn verify_multicast_group_block( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); for (group_pk, group) in multicast_groups { let ip = group.multicast_ip; @@ -1065,11 +1040,9 @@ fn verify_multicast_group_block( let id_or_ip = IdOrIp::Ip(ip_net); insert_usage( &mut in_use, - resource_type, id_or_ip, *group_pk, "MulticastGroup".to_string(), - result, ); result.multicast_group_block_checked += 1; } @@ -1103,7 +1076,7 @@ fn verify_multicast_publisher_block( let allocated: HashSet = extension.iter_allocated().into_iter().collect(); - let mut in_use: HashMap = HashMap::new(); + let mut in_use: HashMap> = HashMap::new(); for (user_pk, user) in users { if user.user_type != UserType::Multicast || user.publishers.is_empty() { continue; @@ -1121,11 +1094,9 @@ fn verify_multicast_publisher_block( let ip_net = NetworkV4::new(dz_ip, 32).unwrap(); insert_usage( &mut in_use, - resource_type, IdOrIp::Ip(ip_net), *user_pk, "User".to_string(), - result, ); result.multicast_publisher_block_checked += 1; } @@ -1133,39 +1104,24 @@ fn verify_multicast_publisher_block( check_discrepancies(resource_type, &allocated, &in_use, result); } -/// Insert a resource usage into the in_use map, detecting duplicates +/// Append a resource usage to the in_use map. Duplicates are detected later in +/// `check_discrepancies` by inspecting `accounts.len() >= 2`. fn insert_usage( - in_use: &mut HashMap, - resource_type: ResourceType, + in_use: &mut HashMap>, value: IdOrIp, account_pubkey: Pubkey, account_type: String, - result: &mut VerifyResourceResult, ) { - match in_use.entry(value) { - Entry::Occupied(entry) => { - let (first_pk, first_type) = entry.get(); - result - .discrepancies - .push(ResourceDiscrepancy::DuplicateUsage { - resource_type, - value: entry.key().clone(), - first_account_pubkey: *first_pk, - first_account_type: first_type.clone(), - second_account_pubkey: account_pubkey, - second_account_type: account_type, - }); - } - Entry::Vacant(entry) => { - entry.insert((account_pubkey, account_type)); - } - } + in_use + .entry(value) + .or_default() + .push((account_pubkey, account_type)); } fn check_discrepancies( resource_type: ResourceType, allocated: &HashSet, - in_use: &HashMap, + in_use: &HashMap>, result: &mut VerifyResourceResult, ) { // Find allocated but not used @@ -1180,9 +1136,22 @@ fn check_discrepancies( } } - // Find used but not allocated - for (id_or_ip, (account_pk, account_type)) in in_use { + // Emit one DuplicateUsage per value with multiple owners; otherwise check + // used-but-not-allocated. Suppressing the second report avoids the same + // value showing up under both sections. + for (id_or_ip, owners) in in_use { + if owners.len() >= 2 { + result + .discrepancies + .push(ResourceDiscrepancy::DuplicateUsage { + resource_type, + value: id_or_ip.clone(), + accounts: owners.clone(), + }); + continue; + } if !allocated.contains(id_or_ip) { + let (account_pk, account_type) = &owners[0]; result .discrepancies .push(ResourceDiscrepancy::UsedButNotAllocated { @@ -2111,4 +2080,299 @@ mod tests { assert!(output_str.contains(&dead_device_pk.to_string())); assert!(output_str.contains("Hint: use --fix to close orphaned resource extensions.")); } + + fn make_segment_routing_ext( + program_id: &Pubkey, + allocated_ids: &[u16], + ) -> (Pubkey, ResourceExtensionOwned) { + let (pda, mut ext) = create_resource_extension_id( + program_id, + ResourceType::SegmentRoutingIds, + (0, 100), + vec![0; 13], + ); + if let Allocator::Id(ref mut a) = ext.allocator { + for id in allocated_ids { + a.allocate_specific(&mut ext.storage, *id).unwrap(); + } + } else { + panic!("expected Id allocator"); + } + (pda, ext) + } + + fn make_vpnv4_loopback( + name: &str, + node_segment_idx: u16, + flex_algo_segments: Vec, + ) -> doublezero_serviceability::state::interface::Interface { + use doublezero_serviceability::state::interface::{ + Interface, InterfaceStatus, InterfaceType, LoopbackType, + }; + Interface { + status: InterfaceStatus::Activated, + name: name.to_string(), + interface_type: InterfaceType::Loopback, + loopback_type: LoopbackType::Vpnv4, + node_segment_idx, + flex_algo_node_segments: flex_algo_segments, + ..Interface::default() + } + } + + fn segment_routing_discrepancies(result: &VerifyResourceResult) -> Vec<&ResourceDiscrepancy> { + result + .discrepancies + .iter() + .filter(|d| match d { + ResourceDiscrepancy::AllocatedButNotUsed { resource_type, .. } + | ResourceDiscrepancy::UsedButNotAllocated { resource_type, .. } + | ResourceDiscrepancy::DuplicateUsage { resource_type, .. } => { + matches!(resource_type, ResourceType::SegmentRoutingIds) + } + _ => false, + }) + .collect() + } + + #[test] + fn test_verify_segment_routing_ids_counts_flex_algo_segments() { + use doublezero_serviceability::state::topology::FlexAlgoNodeSegment; + + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + // Override the default SegmentRoutingIds extension to mark IDs 7 and 8 + // as allocated. 7 is the loopback's base segment, 8 is the flex-algo + // segment for one topology. + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + let sr = make_segment_routing_ext(&program_id, &[7, 8]); + accounts.insert( + Box::new(sr.0), + Box::new(AccountData::ResourceExtension(sr.1)), + ); + + let device_pk = Pubkey::new_unique(); + let topology_pk = Pubkey::new_unique(); + let device = Device { + interfaces: vec![make_vpnv4_loopback( + "Loopback0", + 7, + vec![FlexAlgoNodeSegment { + topology: topology_pk, + node_segment_idx: 8, + }], + )], + ..Device::default() + }; + accounts.insert(Box::new(device_pk), Box::new(AccountData::Device(device))); + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let result = verify_resources(&mock_client).unwrap(); + let discrepancies = segment_routing_discrepancies(&result); + assert!( + discrepancies.is_empty(), + "expected no SegmentRoutingIds discrepancies, got {:?}", + discrepancies + ); + // Both the base segment and the flex-algo segment should be counted. + assert_eq!(result.segment_routing_ids_checked, 2); + } + + #[test] + fn test_verify_segment_routing_ids_flex_algo_used_but_not_allocated() { + use doublezero_serviceability::state::topology::FlexAlgoNodeSegment; + + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + // Allocator has only the base segment (7), not the flex-algo one (8). + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + let sr = make_segment_routing_ext(&program_id, &[7]); + accounts.insert( + Box::new(sr.0), + Box::new(AccountData::ResourceExtension(sr.1)), + ); + + let device_pk = Pubkey::new_unique(); + let topology_pk = Pubkey::new_unique(); + let device = Device { + interfaces: vec![make_vpnv4_loopback( + "Loopback0", + 7, + vec![FlexAlgoNodeSegment { + topology: topology_pk, + node_segment_idx: 8, + }], + )], + ..Device::default() + }; + accounts.insert(Box::new(device_pk), Box::new(AccountData::Device(device))); + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let result = verify_resources(&mock_client).unwrap(); + let discrepancies = segment_routing_discrepancies(&result); + assert_eq!(discrepancies.len(), 1, "got {:?}", discrepancies); + match discrepancies[0] { + ResourceDiscrepancy::UsedButNotAllocated { + value, + account_type, + .. + } => { + assert_eq!(*value, IdOrIp::Id(8)); + assert!( + account_type.contains("flex-algo"), + "account_type should mention flex-algo: {}", + account_type + ); + assert!(account_type.contains(&topology_pk.to_string())); + } + other => panic!("unexpected discrepancy: {:?}", other), + } + } + + #[test] + fn test_duplicate_usage_not_double_reported_as_used_but_not_allocated() { + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + // Allocator has nothing — the shared ID is used by two devices but + // not allocated. We want exactly one DuplicateUsage and zero + // UsedButNotAllocated for that value. + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + let sr = make_segment_routing_ext(&program_id, &[]); + accounts.insert( + Box::new(sr.0), + Box::new(AccountData::ResourceExtension(sr.1)), + ); + + let dev_a = Pubkey::new_unique(); + let dev_b = Pubkey::new_unique(); + for pk in [dev_a, dev_b] { + let device = Device { + interfaces: vec![make_vpnv4_loopback("Loopback0", 42, vec![])], + ..Device::default() + }; + accounts.insert(Box::new(pk), Box::new(AccountData::Device(device))); + } + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let result = verify_resources(&mock_client).unwrap(); + let discrepancies = segment_routing_discrepancies(&result); + + let dup_count = discrepancies + .iter() + .filter(|d| { + matches!( + d, + ResourceDiscrepancy::DuplicateUsage { + value: IdOrIp::Id(42), + .. + } + ) + }) + .count(); + let used_not_alloc_count = discrepancies + .iter() + .filter(|d| { + matches!( + d, + ResourceDiscrepancy::UsedButNotAllocated { + value: IdOrIp::Id(42), + .. + } + ) + }) + .count(); + assert_eq!(dup_count, 1, "discrepancies: {:?}", discrepancies); + assert_eq!( + used_not_alloc_count, 0, + "discrepancies: {:?}", + discrepancies + ); + + // And the single DuplicateUsage should list both owners. + let dup = discrepancies + .iter() + .find_map(|d| match d { + ResourceDiscrepancy::DuplicateUsage { accounts, .. } => Some(accounts), + _ => None, + }) + .expect("DuplicateUsage present"); + assert_eq!(dup.len(), 2); + } + + #[test] + fn test_duplicate_usage_with_three_owners_emits_single_entry() { + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + // Allocator has the ID, so the only expected discrepancy is the + // duplicate report. + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + let sr = make_segment_routing_ext(&program_id, &[42]); + accounts.insert( + Box::new(sr.0), + Box::new(AccountData::ResourceExtension(sr.1)), + ); + + for _ in 0..3 { + let device_pk = Pubkey::new_unique(); + let device = Device { + interfaces: vec![make_vpnv4_loopback("Loopback0", 42, vec![])], + ..Device::default() + }; + accounts.insert(Box::new(device_pk), Box::new(AccountData::Device(device))); + } + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let result = verify_resources(&mock_client).unwrap(); + let dup_entries: Vec<&Vec<(Pubkey, String)>> = result + .discrepancies + .iter() + .filter_map(|d| match d { + ResourceDiscrepancy::DuplicateUsage { + resource_type: ResourceType::SegmentRoutingIds, + value: IdOrIp::Id(42), + accounts, + } => Some(accounts), + _ => None, + }) + .collect(); + assert_eq!( + dup_entries.len(), + 1, + "want one DuplicateUsage, got {:?}", + dup_entries + ); + assert_eq!(dup_entries[0].len(), 3); + } } diff --git a/smartcontract/programs/CLAUDE.md b/smartcontract/programs/CLAUDE.md index 4a0bc54a52..cce863d13f 100644 --- a/smartcontract/programs/CLAUDE.md +++ b/smartcontract/programs/CLAUDE.md @@ -46,5 +46,9 @@ 1. **Use standard interfaces**: Use `solana-loader-v3-interface` to parse `UpgradeableLoaderState` rather than implementing your own parser. The interface crate provides well-tested, maintained implementations. +### Resource Allocation + +1. **Keep `doublezero resource verify` in sync**: Anytime a processor allocates from or deallocates against a `ResourceExtension` (any field that pulls from `SegmentRoutingIds`, `TunnelIds`, `LinkIds`, `UserTunnelBlock`, `DeviceTunnelBlock`, `DzPrefixBlock`, `MulticastGroupBlock`, `MulticastPublisherBlock`, etc.), update the corresponding `verify_*` function in `smartcontract/cli/src/resource/verify.rs` so that field is counted as "in use." Otherwise the verifier will report the allocation as `AllocatedButNotUsed` (or miss a leak when a deallocation is dropped). Add coverage in the verify test module for the new field. + ### Pull Requests - Make sure `make rust-lint` and `make rust-test` both pass before posting pull requests \ No newline at end of file From 3e42a8839e5c4522d9dba95afac0076a0cf64c29 Mon Sep 17 00:00:00 2001 From: Greg Mitchell Date: Wed, 20 May 2026 21:22:36 +0000 Subject: [PATCH 2/3] smartcontract/cli: skip out-of-range loopback/link IPs in DeviceTunnelBlock verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interface and link processors honor a caller-supplied ip_net/tunnel_net and skip the DeviceTunnelBlock allocator on that path — most commonly for user-tunnel-endpoint Vpnv4 loopbacks that land on a globally routable IP outside the block. Those IPs are not in the allocator's bitmap, so verify_device_tunnel_block was falsely flagging them as UsedButNotAllocated. Filter loopback and link IPs through the block's base_net before counting them as in-use, mirroring how verify_multicast_publisher_block handles legacy out-of-range dz_ips. --- smartcontract/cli/src/resource/verify.rs | 77 ++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/smartcontract/cli/src/resource/verify.rs b/smartcontract/cli/src/resource/verify.rs index 1317ce0ce8..35189abe92 100644 --- a/smartcontract/cli/src/resource/verify.rs +++ b/smartcontract/cli/src/resource/verify.rs @@ -871,6 +871,19 @@ fn verify_device_tunnel_block( return; }; + // Pull the base network so we can ignore loopback/link IPs that fall + // outside the block's range. Both interface and link processors honor a + // caller-supplied ip_net/tunnel_net and skip the allocator in that path + // (see `processors/device/interface/create.rs` and + // `processors/link/resource_onchain_helpers.rs`) — most commonly for + // user-tunnel-endpoint loopbacks that land on a globally routable IP. + // Those IPs aren't in the DeviceTunnelBlock bitmap and would otherwise + // be falsely reported as `UsedButNotAllocated`. + let base_net = match &extension.allocator { + Allocator::Ip(ip_alloc) => ip_alloc.base_net, + Allocator::Id(_) => return, + }; + let allocated: HashSet = extension.iter_allocated().into_iter().collect(); let mut in_use: HashMap> = HashMap::new(); @@ -887,6 +900,9 @@ fn verify_device_tunnel_block( // Iterate over all IPs in the network for i in 0..iface.ip_net.size() { if let Some(ip) = iface.ip_net.nth(i) { + if !base_net.contains(ip) { + continue; + } let ip_net = NetworkV4::new(ip, 32).unwrap(); let id_or_ip = IdOrIp::Ip(ip_net); insert_usage( @@ -910,6 +926,9 @@ fn verify_device_tunnel_block( // Iterate over all IPs in the network (e.g., /31 has 2 IPs) for i in 0..link.tunnel_net.size() { if let Some(ip) = link.tunnel_net.nth(i) { + if !base_net.contains(ip) { + continue; + } let ip_net = NetworkV4::new(ip, 32).unwrap(); let id_or_ip = IdOrIp::Ip(ip_net); insert_usage(&mut in_use, id_or_ip, *link_pk, "Link".to_string()); @@ -2120,6 +2139,15 @@ mod tests { } } + fn make_vpnv4_loopback_with_ip( + name: &str, + ip_net: &str, + ) -> doublezero_serviceability::state::interface::Interface { + let mut iface = make_vpnv4_loopback(name, 0, vec![]); + iface.ip_net = ip_net.parse().unwrap(); + iface + } + fn segment_routing_discrepancies(result: &VerifyResourceResult) -> Vec<&ResourceDiscrepancy> { result .discrepancies @@ -2375,4 +2403,53 @@ mod tests { ); assert_eq!(dup_entries[0].len(), 3); } + + #[test] + fn test_verify_device_tunnel_block_ignores_loopback_ip_outside_base_net() { + // User-tunnel-endpoint Vpnv4 loopbacks land on a caller-supplied + // globally routable IP and skip the DeviceTunnelBlock allocator + // (see processors/device/interface/create.rs). The verifier must + // not report those IPs as `UsedButNotAllocated`. + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + + // The DeviceTunnelBlock from insert_all_globals is 172.16.0.0/24. + // Give the loopback a globally routable IP outside that block. + let device_pk = Pubkey::new_unique(); + let device = Device { + interfaces: vec![make_vpnv4_loopback_with_ip("Loopback0", "203.0.113.5/32")], + ..Device::default() + }; + accounts.insert(Box::new(device_pk), Box::new(AccountData::Device(device))); + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let result = verify_resources(&mock_client).unwrap(); + let dtb_used_not_alloc: Vec<_> = result + .discrepancies + .iter() + .filter(|d| { + matches!( + d, + ResourceDiscrepancy::UsedButNotAllocated { + resource_type: ResourceType::DeviceTunnelBlock, + .. + } + ) + }) + .collect(); + assert!( + dtb_used_not_alloc.is_empty(), + "loopback IP outside the DeviceTunnelBlock base_net should not produce a UsedButNotAllocated entry, got {:?}", + dtb_used_not_alloc + ); + } } From 8555f74f219d8f9c0566ef9601209d1dab5875e7 Mon Sep 17 00:00:00 2001 From: Greg Mitchell Date: Wed, 20 May 2026 21:29:52 +0000 Subject: [PATCH 3/3] smartcontract/cli: annotate pubkeys in resource verify output with device/link codes Build a pubkey -> code map from devices and links during verify_resources, expose it on VerifyResourceResult, and prefix every pubkey emitted in the 'Used but not allocated', 'Duplicate usage', and 'Orphaned resource extensions' sections with its code. Unknown pubkeys (e.g. Users, or extensions associated with a deleted device) print as before. --- smartcontract/cli/src/resource/verify.rs | 94 +++++++++++++++++++++++- 1 file changed, 90 insertions(+), 4 deletions(-) diff --git a/smartcontract/cli/src/resource/verify.rs b/smartcontract/cli/src/resource/verify.rs index 35189abe92..898939d37d 100644 --- a/smartcontract/cli/src/resource/verify.rs +++ b/smartcontract/cli/src/resource/verify.rs @@ -72,6 +72,9 @@ pub struct VerifyResourceResult { pub link_ids_checked: usize, pub multicast_group_block_checked: usize, pub multicast_publisher_block_checked: usize, + /// Pubkey → human-readable code, populated for devices and links so the + /// display layer can print `code (pubkey)` instead of raw pubkeys. + pub pubkey_labels: HashMap, } impl VerifyResourceResult { @@ -222,7 +225,10 @@ impl VerifyResourceCliCommand { writeln!( out, " {} = {} (used by {} {})", - resource_type, value, account_type, account_pubkey + resource_type, + value, + account_type, + format_pubkey(account_pubkey, &result.pubkey_labels) )?; } } @@ -249,7 +255,9 @@ impl VerifyResourceCliCommand { writeln!( out, " {} (allocator={}, associated_with={})", - pubkey, allocator_kind, associated_with + pubkey, + allocator_kind, + format_pubkey(associated_with, &result.pubkey_labels) )?; } } @@ -280,7 +288,9 @@ impl VerifyResourceCliCommand { { let owners = accounts .iter() - .map(|(pk, ty)| format!("{} {}", ty, pk)) + .map(|(pk, ty)| { + format!("{} {}", ty, format_pubkey(pk, &result.pubkey_labels)) + }) .collect::>() .join(", "); writeln!( @@ -372,7 +382,9 @@ impl VerifyResourceCliCommand { { let owners = accounts .iter() - .map(|(pk, ty)| format!("{} {}", ty, pk)) + .map(|(pk, ty)| { + format!("{} {}", ty, format_pubkey(pk, &result.pubkey_labels)) + }) .collect::>() .join(", "); writeln!( @@ -592,6 +604,19 @@ fn verify_resources(client: &C) -> eyre::Result) -> String { + match labels.get(pk) { + Some(code) => format!("{} ({})", code, pk), + None => pk.to_string(), + } +} + /// Append a resource usage to the in_use map. Duplicates are detected later in /// `check_discrepancies` by inspecting `accounts.len() >= 2`. fn insert_usage( @@ -2404,6 +2437,59 @@ mod tests { assert_eq!(dup_entries[0].len(), 3); } + #[test] + fn test_output_includes_device_and_link_codes() { + // A duplicate-usage on a SegmentRoutingId across two devices with + // codes "dz1" and "dz2" should show both codes alongside their + // pubkeys in the rendered "Duplicate usage" section. + let mut mock_client = MockCliCommand::new(); + let program_id = Pubkey::new_unique(); + + let mut accounts: HashMap, Box> = HashMap::new(); + insert_all_globals(&mut accounts, &program_id); + let sr = make_segment_routing_ext(&program_id, &[42]); + accounts.insert( + Box::new(sr.0), + Box::new(AccountData::ResourceExtension(sr.1)), + ); + + let dev_a_pk = Pubkey::new_unique(); + let dev_b_pk = Pubkey::new_unique(); + for (pk, code) in [(dev_a_pk, "dz1"), (dev_b_pk, "dz2")] { + let device = Device { + code: code.to_string(), + interfaces: vec![make_vpnv4_loopback("Loopback0", 42, vec![])], + ..Device::default() + }; + accounts.insert(Box::new(pk), Box::new(AccountData::Device(device))); + } + + mock_client + .expect_get_program_id() + .returning(move || program_id); + mock_client + .expect_get_all() + .returning(move || Ok(accounts.clone())); + + let cmd = VerifyResourceCliCommand { fix: false }; + let mut output = Cursor::new(Vec::new()); + cmd.execute(&mock_client, &mut output).unwrap(); + let output_str = String::from_utf8(output.into_inner()).unwrap(); + + assert!( + output_str.contains(&format!("dz1 ({})", dev_a_pk)), + "expected `dz1 ({})` in output, got:\n{}", + dev_a_pk, + output_str + ); + assert!( + output_str.contains(&format!("dz2 ({})", dev_b_pk)), + "expected `dz2 ({})` in output, got:\n{}", + dev_b_pk, + output_str + ); + } + #[test] fn test_verify_device_tunnel_block_ignores_loopback_ip_outside_base_net() { // User-tunnel-endpoint Vpnv4 loopbacks land on a caller-supplied