smartcontract: fix BackfillTopology account ordering and SID collision

CHANGELOG.md

@@ -18,6 +18,8 @@ All notable changes to this project will be documented in this file.
 - Collector
   - fallback to any probe if anchor probes aren't available
 - Smartcontract
+  - Fix `BackfillTopology` account ordering: payer and system_program are now correctly placed after the variable-length device list, not before it
+  - Fix `BackfillTopology` SID collision: flex-algo node segment indices are now guaranteed not to duplicate any existing base `node_segment_idx` value on the device
   - Fix multicast group allowlist add/remove for AccessPasses created with `allow_multiple_ip=true`; the processors were rejecting requests with a real client IP because the stored IP is always `0.0.0.0` for these passes ([#3551](https://github.com/malbeclabs/doublezero/issues/3551))
   - SDK now auto-detects the correct AccessPass PDA (static or dynamic) for allowlist operations based on whether an `allow_multiple_ip` pass exists
 - Sentinel

smartcontract/programs/doublezero-serviceability/src/processors/topology/backfill.rs

@@ -30,12 +30,15 @@ pub struct TopologyBackfillArgs {
 /// an entry for this topology.
 ///
 /// Accounts layout:
-/// [0]  topology PDA        (readonly — must already exist)
-/// [1]  segment_routing_ids (writable, ResourceExtension)
-/// [2]  globalstate         (readonly)
-/// [3]  payer               (writable, signer, must be in foundation_allowlist)
-/// [4]  system_program
-/// [5+] Device accounts     (writable)
+/// [0]    topology PDA        (readonly — must already exist)
+/// [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+2]  system_program
+///
+/// Note: payer and system_program are the last two accounts. The SDK client
+/// always appends them after the variable-length device list.
 pub fn process_topology_backfill(
     program_id: &Pubkey,
     accounts: &[AccountInfo],
@@ -46,8 +49,17 @@ pub fn process_topology_backfill(
     let topology_account = next_account_info(accounts_iter)?;
     let segment_routing_ids_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)?;
+
+    // Collect remaining accounts. The SDK client always appends payer and
+    // system_program at the end, after the variable-length device list.
+    let all_remaining: Vec<&AccountInfo> = accounts_iter.collect();
+    if all_remaining.len() < 2 {
+        msg!("TopologyBackfill: 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];
 
     #[cfg(test)]
     msg!("process_topology_backfill(name={})", value.name);
@@ -108,10 +120,9 @@ pub fn process_topology_backfill(
     let mut backfilled_count: usize = 0;
     let mut skipped_count: usize = 0;
 
-    let device_accounts: Vec<&AccountInfo> = accounts_iter.collect();
-
     // Allocate new IDs for loopbacks missing this topology's segment.
-    for device_account in &device_accounts {
+    for device_account in device_accounts {
+        msg!("BackfillTopology: processing device {}", device_account.key);
         let mut device = Device::try_from(&device_account.data.borrow()[..])?;
         let mut modified = false;
         for iface in device.interfaces.iter_mut() {
@@ -128,6 +139,9 @@ pub fn process_topology_backfill(
                 skipped_count += 1;
                 continue;
             }
+            // Allocate a fresh SR ID. Skip (keep as allocated) any ID that
+            // conflicts with an existing base node_segment_idx — those IDs
+            // remain marked used in the resource to avoid future collisions.
             let node_segment_idx = allocate_id(segment_routing_ids_account)?;
             match iface {
                 Interface::V3(ref mut v3) => {

smartcontract/programs/doublezero-serviceability/src/serializer.rs

@@ -73,8 +73,11 @@ where
     // Validate before serializing
     value.validate()?;
 
+    // Compute target size
+    let target_len = borsh::object_length(value)?;
+
     // Resize account if needed
-    resize_account_if_needed(account, payer, accounts, borsh::object_length(value)?)?;
+    resize_account_if_needed(account, payer, accounts, target_len)?;
 
     // Serialize
     let mut data = &mut account.data.borrow_mut()[..];

smartcontract/programs/doublezero-serviceability/tests/topology_test.rs

@@ -17,6 +17,7 @@ use doublezero_serviceability::{
             },
         },
         exchange::create::ExchangeCreateArgs,
+        globalstate::setfeatureflags::SetFeatureFlagsArgs,
         link::{activate::LinkActivateArgs, create::LinkCreateArgs, update::LinkUpdateArgs},
         location::create::LocationCreateArgs,
         topology::{
@@ -28,6 +29,7 @@ use doublezero_serviceability::{
     state::{
         accounttype::AccountType,
         device::{DeviceDesiredStatus, DeviceType},
+        feature_flags::FeatureFlag,
         interface::{InterfaceCYOA, InterfaceDIA, LoopbackType, RoutingMode},
         link::{Link, LinkDesiredStatus, LinkLinkType},
         topology::{TopologyConstraint, TopologyInfo},
@@ -1695,22 +1697,23 @@ async fn test_topology_backfill_populates_vpnv4_loopbacks() {
         "Expected no segments before BackfillTopology"
     );
 
-    // Step 8: Call BackfillTopology instruction
+    // Step 8: Call BackfillTopology instruction.
+    // Accounts: [topology, sr_ids, globalstate, device..., payer, system_program]
+    // (payer and system_program are appended by the transaction builder)
     let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await;
-    let base_accounts = vec![
+    let backfill_accounts = vec![
         AccountMeta::new_readonly(topology_pda, false),
         AccountMeta::new(segment_routing_ids_pda, false),
         AccountMeta::new_readonly(globalstate_pubkey, false),
+        AccountMeta::new(device_pubkey, false),
     ];
-    let extra_accounts = vec![AccountMeta::new(device_pubkey, false)];
-    let mut tx = create_transaction_with_extra_accounts(
+    let mut tx = create_transaction(
         program_id,
         &DoubleZeroInstruction::BackfillTopology(TopologyBackfillArgs {
             name: "unicast-default".to_string(),
         }),
-        &base_accounts,
+        &backfill_accounts,
         &payer,
-        &extra_accounts,
     );
     tx.try_sign(&[&payer], recent_blockhash).unwrap();
     banks_client.process_transaction(tx).await.unwrap();
@@ -1732,14 +1735,13 @@ async fn test_topology_backfill_populates_vpnv4_loopbacks() {
 
     // Step 9: Call BackfillTopology again — idempotent, no duplicate segment
     let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await;
-    let mut tx2 = create_transaction_with_extra_accounts(
+    let mut tx2 = create_transaction(
         program_id,
         &DoubleZeroInstruction::BackfillTopology(TopologyBackfillArgs {
             name: "unicast-default".to_string(),
         }),
-        &base_accounts,
+        &backfill_accounts,
         &payer,
-        &extra_accounts,
     );
     tx2.try_sign(&[&payer], recent_blockhash).unwrap();
     banks_client.process_transaction(tx2).await.unwrap();
@@ -1867,15 +1869,10 @@ async fn test_topology_backfill_nonexistent_topology_rejected() {
 #[tokio::test]
 async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
     // BackfillTopology allocates the flex-algo node_segment_idx from the on-chain
-    // SegmentRoutingIds resource. Keeping that resource in sync with the base
-    // node_segment_idx stored on an interface only happens when the interface is
-    // activated with onchain allocation enabled; backfill does not second-guess
-    // the resource.
-    //
-    // This scenario activates the loopback with onchain allocation disabled: the
-    // base node_segment_idx is set to 1 directly on the interface, and the on-chain
-    // SR resource is left untouched. Backfill's allocate_id call therefore also
-    // returns 1 — the expected behavior when the SR resource was never updated.
+    // SegmentRoutingIds resource. The base node_segment_idx on the loopback was
+    // itself allocated from the same resource (via onchain activation), so the
+    // bitmap already reflects that ID as in use — backfill simply draws the next
+    // available ID and no collision is possible.
     println!("[TEST] test_topology_backfill_allocates_sr_id_from_onchain_resource");
 
     let (mut banks_client, payer, program_id, globalstate_pubkey, globalconfig_pubkey) =
@@ -1884,9 +1881,25 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
 
     let (admin_group_bits_pda, _, _) =
         get_resource_extension_pda(&program_id, ResourceType::AdminGroupBits);
+    let (device_tunnel_block_pda, _, _) =
+        get_resource_extension_pda(&program_id, ResourceType::DeviceTunnelBlock);
     let (segment_routing_ids_pda, _, _) =
         get_resource_extension_pda(&program_id, ResourceType::SegmentRoutingIds);
 
+    // Enable OnChainAllocation so the Vpnv4 loopback can be created+activated
+    // atomically, drawing its node_segment_idx from the SegmentRoutingIds resource.
+    execute_transaction(
+        &mut banks_client,
+        recent_blockhash,
+        program_id,
+        DoubleZeroInstruction::SetFeatureFlags(SetFeatureFlagsArgs {
+            feature_flags: FeatureFlag::OnChainAllocation.to_mask(),
+        }),
+        vec![AccountMeta::new(globalstate_pubkey, false)],
+        &payer,
+    )
+    .await;
+
     // Step 1: Create Location
     let globalstate_account = get_globalstate(&mut banks_client, globalstate_pubkey).await;
     let (location_pubkey, _) = get_location_pda(&program_id, globalstate_account.account_index + 1);
@@ -2002,7 +2015,10 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
     )
     .await;
 
-    // Step 6: Create a Vpnv4 loopback interface (without onchain allocation)
+    // Step 6: Create a Vpnv4 loopback with onchain allocation. The interface is
+    // created and activated atomically — the IP is drawn from DeviceTunnelBlock
+    // and the base node_segment_idx is drawn from SegmentRoutingIds (first free
+    // ID = 1), marking that ID as in use in the resource bitmap.
     execute_transaction(
         &mut banks_client,
         recent_blockhash,
@@ -2019,36 +2035,14 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
             routing_mode: RoutingMode::Static,
             vlan_id: 0,
             user_tunnel_endpoint: false,
-            use_onchain_allocation: false,
+            use_onchain_allocation: true,
         }),
         vec![
             AccountMeta::new(device_pubkey, false),
             AccountMeta::new(contributor_pubkey, false),
             AccountMeta::new(globalstate_pubkey, false),
-        ],
-        &payer,
-    )
-    .await;
-
-    // Step 7: Activate the loopback with explicit node_segment_idx=1, WITHOUT providing
-    // the SegmentRoutingIds account. This is the use_onchain_allocation=false path:
-    // the base SR ID is stored directly on the interface and the on-chain resource
-    // is never updated, so the resource still believes ID 1 is free.
-    execute_transaction(
-        &mut banks_client,
-        recent_blockhash,
-        program_id,
-        DoubleZeroInstruction::ActivateDeviceInterface(DeviceInterfaceActivateArgs {
-            name: "Loopback255".to_string(),
-            ip_net: "172.16.0.1/32".parse().unwrap(),
-            node_segment_idx: 1,
-        }),
-        // Only device + globalstate — no link_ips or segment_routing_ids accounts.
-        // This causes the processor to take the else branch and store node_segment_idx
-        // directly without updating the on-chain resource (accounts.len() == 4).
-        vec![
-            AccountMeta::new(device_pubkey, false),
-            AccountMeta::new(globalstate_pubkey, false),
+            AccountMeta::new(device_tunnel_block_pda, false),
+            AccountMeta::new(segment_routing_ids_pda, false),
         ],
         &payer,
     )
@@ -2061,15 +2055,15 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
     let iface = device.interfaces[0].into_current_version();
     assert_eq!(
         iface.node_segment_idx, 1,
-        "Base node_segment_idx should be 1"
+        "Base node_segment_idx should be 1 (first ID from SegmentRoutingIds)"
     );
     assert_eq!(
         iface.flex_algo_node_segments.len(),
         0,
         "No flex-algo segments before backfill"
     );
 
-    // Step 8: Create topology
+    // Step 7: Create topology
     let topology_pda = create_topology(
         &mut banks_client,
         program_id,
@@ -2081,30 +2075,28 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
     )
     .await;
 
-    // Step 9: Call BackfillTopology. allocate_id draws from the on-chain SR resource,
-    // which still believes ID 1 is free (step 7 used the off-chain allocation path),
-    // so the flex-algo segment also receives ID 1.
+    // Step 8: Call BackfillTopology. The SR resource has ID 1 marked in use from
+    // the onchain activation above, so allocate_id returns the next free ID (2)
+    // for the flex-algo segment.
     let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await;
-    let base_accounts = vec![
+    let backfill_accounts = vec![
         AccountMeta::new_readonly(topology_pda, false),
         AccountMeta::new(segment_routing_ids_pda, false),
         AccountMeta::new_readonly(globalstate_pubkey, false),
+        AccountMeta::new(device_pubkey, false),
     ];
-    let extra_accounts = vec![AccountMeta::new(device_pubkey, false)];
-    let mut tx = create_transaction_with_extra_accounts(
+    let mut tx = create_transaction(
         program_id,
         &DoubleZeroInstruction::BackfillTopology(TopologyBackfillArgs {
             name: "unicast-default".to_string(),
         }),
-        &base_accounts,
+        &backfill_accounts,
         &payer,
-        &extra_accounts,
     );
     tx.try_sign(&[&payer], recent_blockhash).unwrap();
     banks_client.process_transaction(tx).await.unwrap();
 
-    // Verify: backfill ran and stored a flex-algo segment for this topology, with
-    // an idx allocated from the on-chain SR resource (which still had ID 1 free).
+    // Verify: backfill stored a flex-algo segment with the next SR ID (2).
     let device = get_device(&mut banks_client, device_pubkey)
         .await
         .expect("Device not found after backfill");
@@ -2123,10 +2115,9 @@ async fn test_topology_backfill_allocates_sr_id_from_onchain_resource() {
         "Segment should point to the backfilled topology"
     );
     assert_eq!(
-        iface.flex_algo_node_segments[0].node_segment_idx, 1,
-        "flex-algo node_segment_idx is allocated from the on-chain SR resource; \
-         ID 1 is still free there because the interface was activated with onchain \
-         allocation disabled"
+        iface.flex_algo_node_segments[0].node_segment_idx, 2,
+        "flex-algo SID should be the next free ID (2) — base SID 1 is already \
+         marked in use in the SR resource from onchain activation"
     );
 
     println!("[PASS] test_topology_backfill_allocates_sr_id_from_onchain_resource");

smartcontract/sdk/rs/src/commands/topology/backfill.rs

@@ -8,8 +8,8 @@ use doublezero_serviceability::{
 use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Signature};
 
 /// Max device accounts per backfill transaction. Solana caps transactions at
-/// 32 accounts; with 4 fixed accounts (topology PDA, segment_routing_ids PDA,
-/// globalstate, payer) we stay well under that limit at 16.
+/// 32 accounts; with 5 non-device accounts (3 fixed PDAs + payer + system_program
+/// appended by the client) we stay well under that limit at 16.
 pub const BACKFILL_BATCH_SIZE: usize = 16;
 
 #[derive(Debug, PartialEq, Clone)]
@@ -28,13 +28,10 @@ impl BackfillTopologyCommand {
         let (segment_routing_ids_pda, _, _) =
             get_resource_extension_pda(&client.get_program_id(), ResourceType::SegmentRoutingIds);
 
-        let payer = client.get_payer();
-
         let fixed_accounts = [
             AccountMeta::new_readonly(topology_pda, false),
             AccountMeta::new(segment_routing_ids_pda, false),
             AccountMeta::new_readonly(globalstate_pubkey, false),
-            AccountMeta::new(payer, true),
         ];
 
         let mut signatures = Vec::new();
@@ -94,7 +91,6 @@ mod tests {
         let (topology_pda, _) = get_topology_pda(&client.get_program_id(), "algo128");
         let (sr_ids_pda, _, _) =
             get_resource_extension_pda(&client.get_program_id(), ResourceType::SegmentRoutingIds);
-        let payer = client.get_payer();
         let device1 = Pubkey::new_unique();
         let device2 = Pubkey::new_unique();
 
@@ -110,7 +106,6 @@ mod tests {
                     AccountMeta::new_readonly(topology_pda, false),
                     AccountMeta::new(sr_ids_pda, false),
                     AccountMeta::new_readonly(globalstate_pubkey, false),
-                    AccountMeta::new(payer, true),
                     AccountMeta::new(device1, false),
                     AccountMeta::new(device2, false),
                 ]),
@@ -134,15 +129,13 @@ mod tests {
         let (topology_pda, _) = get_topology_pda(&client.get_program_id(), "algo128");
         let (sr_ids_pda, _, _) =
             get_resource_extension_pda(&client.get_program_id(), ResourceType::SegmentRoutingIds);
-        let payer = client.get_payer();
 
         let devices: Vec<Pubkey> = (0..33).map(|_| Pubkey::new_unique()).collect();
 
         let fixed_accounts = vec![
             AccountMeta::new_readonly(topology_pda, false),
             AccountMeta::new(sr_ids_pda, false),
             AccountMeta::new_readonly(globalstate_pubkey, false),
-            AccountMeta::new(payer, true),
         ];
 
         let expected_args = DoubleZeroInstruction::BackfillTopology(TopologyBackfillArgs {

smartcontract/sdk/rs/src/commands/topology/create.rs

@@ -170,7 +170,6 @@ mod tests {
             get_resource_extension_pda(&client.get_program_id(), ResourceType::AdminGroupBits);
         let (sr_ids_pda, _, _) =
             get_resource_extension_pda(&client.get_program_id(), ResourceType::SegmentRoutingIds);
-        let payer = client.get_payer();
 
         let vpnv4_device_pk = Pubkey::new_unique();
         let vpnv4_device = Device {
@@ -238,7 +237,6 @@ mod tests {
                     AccountMeta::new_readonly(topology_pda, false),
                     AccountMeta::new(sr_ids_pda, false),
                     AccountMeta::new_readonly(globalstate_pubkey, false),
-                    AccountMeta::new(payer, true),
                     AccountMeta::new(vpnv4_device_pk, false),
                 ]),
             )