add location support to device update functionality

update device location instruction for reference counter management

Author: juanolv

CHANGELOG.md

@@ -6,6 +6,8 @@ All notable changes to this project will be documented in this file.
 
 ### Breaking
 
+- Updated the device update command to allow modifying a device’s location.
+
 ### Changes
 - RFCs
   - RFC9 Link Draining
@@ -82,6 +84,7 @@ All notable changes to this project will be documented in this file.
   - serviceability: prevent device interface name duplication
   - Update serviceability and telemetry program instruction args to use the `BorshDeserializeIncremental` derive macro incremental, backward-compatible, deserialization of structs.
   - Add explicit signer checks for payer accounts across various processors to improve security and ensure correct transaction authorization.
+    - Add the ability to update a Device’s location, managing the reference counters accordingly.
 - CLI
   - Removed `--bgp-community` option from `doublezero exchange create` since these values are now assigned automatically
   - Add `--next-bgp-community` option to `doublezero global-config set` so authorized users can control which bgp_community will be assigned next

smartcontract/cli/src/device/update.rs

@@ -35,6 +35,9 @@ pub struct UpdateDeviceCliCommand {
     /// Contributor Pubkey (optional)
     #[arg(long, value_parser = validate_pubkey)]
     pub contributor: Option<String>,
+    /// Location Pubkey (optional)
+    #[arg(long, value_parser = validate_pubkey)]
+    pub location: Option<String>,
     /// Management VRF name (optional)
     #[arg(long)]
     pub mgmt_vrf: Option<String>,
@@ -113,6 +116,10 @@ impl UpdateDeviceCliCommand {
             dz_prefixes: self.dz_prefixes,
             metrics_publisher,
             contributor_pk: contributor,
+            location_pk: match &self.location {
+                Some(location) => Some(Pubkey::from_str(location)?),
+                None => None,
+            },
             mgmt_vrf: self.mgmt_vrf,
             interfaces: None,
             max_users: self.max_users,
@@ -258,6 +265,9 @@ mod tests {
                 contributor_pk: Some(Pubkey::from_str_const(
                     "HQ2UUt18uJqKaQFJhgV9zaTdQxUZjNrsKFgoEDquBkcx",
                 )),
+                location_pk: Some(Pubkey::from_str_const(
+                    "HQ2UUt18uJqKaQFJhgV9zaTdQxUZjNrsKFgoEDquBkcx",
+                )),
                 mgmt_vrf: Some("default".to_string()),
                 interfaces: None,
                 max_users: Some(1025),
@@ -275,6 +285,7 @@ mod tests {
             dz_prefixes: Some("1.2.3.4/32".parse().unwrap()),
             metrics_publisher: Some("HQ2UUt18uJqKaQFJhgV9zaTdQxUZjNrsKFgoEDquBkcx".to_string()),
             contributor: Some("HQ2UUt18uJqKaQFJhgV9zaTdQxUZjNrsKFgoEDquBkcx".to_string()),
+            location: Some("HQ2UUt18uJqKaQFJhgV9zaTdQxUZjNrsKFgoEDquBkcx".to_string()),
             mgmt_vrf: Some("default".to_string()),
             max_users: Some(1025),
             users_count: Some(0),
@@ -364,6 +375,7 @@ mod tests {
             public_ip: None,
             dz_prefixes: None,
             metrics_publisher: None,
+            location: None,
             contributor: None,
             mgmt_vrf: None,
             max_users: Some(255),
@@ -453,6 +465,7 @@ mod tests {
             public_ip: Some([10, 20, 30, 40].into()),
             dz_prefixes: None,
             metrics_publisher: None,
+            location: None,
             contributor: None,
             mgmt_vrf: None,
             max_users: None,

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

@@ -121,6 +121,8 @@ pub enum DoubleZeroError {
     InvalidInterfaceType, // variant 57
     #[error("Invalid Loopback Type")]
     InvalidLoopbackType, // variant 58
+    #[error("Invalid Actual Location")]
+    InvalidActualLocation, // variant 59
 }
 
 impl From<DoubleZeroError> for ProgramError {
@@ -185,6 +187,7 @@ impl From<DoubleZeroError> for ProgramError {
             DoubleZeroError::InterfaceAlreadyExists => ProgramError::Custom(56),
             DoubleZeroError::InvalidInterfaceType => ProgramError::Custom(57),
             DoubleZeroError::InvalidLoopbackType => ProgramError::Custom(58),
+            DoubleZeroError::InvalidActualLocation => ProgramError::Custom(59),
         }
     }
 }
@@ -250,6 +253,7 @@ impl From<u32> for DoubleZeroError {
             56 => DoubleZeroError::InterfaceAlreadyExists,
             57 => DoubleZeroError::InvalidInterfaceType,
             58 => DoubleZeroError::InvalidLoopbackType,
+            59 => DoubleZeroError::InvalidActualLocation,
             _ => DoubleZeroError::Custom(e),
         }
     }
@@ -335,6 +339,7 @@ mod tests {
             InterfaceAlreadyExists,
             InvalidInterfaceType,
             InvalidLoopbackType,
+            InvalidActualLocation,
         ];
         for err in variants {
             let pe: ProgramError = err.clone().into();

smartcontract/programs/doublezero-serviceability/src/processors/device/update.rs

@@ -2,17 +2,16 @@ use crate::{
     error::DoubleZeroError,
     globalstate::globalstate_get,
     helper::*,
-    state::{accounttype::AccountType, contributor::Contributor, device::*},
+    state::{accounttype::AccountType, contributor::Contributor, device::*, location::Location},
 };
 use borsh::BorshSerialize;
 use borsh_incremental::BorshDeserializeIncremental;
 use core::fmt;
 use doublezero_program_common::{types::NetworkV4List, validate_account_code};
-#[cfg(test)]
-use solana_program::msg;
 use solana_program::{
     account_info::{next_account_info, AccountInfo},
     entrypoint::ProgramResult,
+    msg,
     pubkey::Pubkey,
 };
 
@@ -71,6 +70,17 @@ pub fn process_update_device(
 
     let device_account = next_account_info(accounts_iter)?;
     let contributor_account = next_account_info(accounts_iter)?;
+    // Update location accounts (old and new)
+
+    let (location_old_account, location_new_account) = if accounts.len() == 7 {
+        (
+            Some(next_account_info(accounts_iter)?),
+            Some(next_account_info(accounts_iter)?),
+        )
+    } else {
+        (None, None)
+    };
+
     let globalstate_account = next_account_info(accounts_iter)?;
     let payer_account = next_account_info(accounts_iter)?;
     let system_program = next_account_info(accounts_iter)?;
@@ -148,6 +158,43 @@ pub fn process_update_device(
         device.max_users = max_users;
     }
 
+    // Handle location update if both old and new location accounts are provided
+    if let (Some(location_old_account), Some(location_new_account)) =
+        (location_old_account, location_new_account)
+    {
+        if location_old_account.key != location_new_account.key {
+            let mut location_old = Location::try_from(location_old_account)?;
+            let mut location_new = Location::try_from(location_new_account)?;
+            if device.location_pk != *location_old_account.key {
+                msg!(
+                    "Invalid location account. Device location_pk: {}, location_old_account: {}",
+                    device.location_pk,
+                    location_old_account.key
+                );
+                return Err(DoubleZeroError::InvalidActualLocation.into());
+            }
+
+            location_old.reference_count = location_old.reference_count.saturating_sub(1);
+            location_new.reference_count = location_new.reference_count.saturating_add(1);
+
+            // Set new location pk in device
+            device.location_pk = *location_new_account.key;
+
+            account_write(
+                location_old_account,
+                &location_old,
+                payer_account,
+                system_program,
+            )?;
+            account_write(
+                location_new_account,
+                &location_new,
+                payer_account,
+                system_program,
+            )?;
+        }
+    }
+
     account_write(device_account, &device, payer_account, system_program)?;
 
     #[cfg(test)]

smartcontract/programs/doublezero-serviceability/src/processors/link/update.rs

@@ -2,17 +2,17 @@ use crate::{
     error::DoubleZeroError,
     globalstate::globalstate_get,
     helper::*,
-    state::{contributor::Contributor, link::*},
+    state::{contributor::Contributor, device::Device, link::*},
 };
 use borsh::BorshSerialize;
 use borsh_incremental::BorshDeserializeIncremental;
 use core::fmt;
 use doublezero_program_common::validate_account_code;
-#[cfg(test)]
-use solana_program::msg;
+
 use solana_program::{
     account_info::{next_account_info, AccountInfo},
     entrypoint::ProgramResult,
+    msg,
     pubkey::Pubkey,
 };
 #[derive(BorshSerialize, BorshDeserializeIncremental, PartialEq, Clone, Default)]
@@ -30,11 +30,35 @@ pub struct LinkUpdateArgs {
 
 impl fmt::Debug for LinkUpdateArgs {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "code: {:?}, tunnel_type: {:?}, bandwidth: {:?}, mtu: {:?}, delay_ns: {:?}, jitter_ns: {:?}, delay_override_ns: {:?}",
-            self.code, self.tunnel_type, self.bandwidth, self.mtu, self.delay_ns, self.jitter_ns, self.delay_override_ns
-        )
+        let mut parts = Vec::new();
+        if let Some(ref code) = self.code {
+            parts.push(format!("code: {:?}", code));
+        }
+        if let Some(ref contributor_pk) = self.contributor_pk {
+            parts.push(format!("contributor_pk: {:?}", contributor_pk));
+        }
+        if let Some(ref tunnel_type) = self.tunnel_type {
+            parts.push(format!("tunnel_type: {:?}", tunnel_type));
+        }
+        if let Some(bandwidth) = self.bandwidth {
+            parts.push(format!("bandwidth: {:?}", bandwidth));
+        }
+        if let Some(mtu) = self.mtu {
+            parts.push(format!("mtu: {:?}", mtu));
+        }
+        if let Some(delay_ns) = self.delay_ns {
+            parts.push(format!("delay_ns: {:?}", delay_ns));
+        }
+        if let Some(jitter_ns) = self.jitter_ns {
+            parts.push(format!("jitter_ns: {:?}", jitter_ns));
+        }
+        if let Some(ref status) = self.status {
+            parts.push(format!("status: {:?}", status));
+        }
+        if let Some(delay_override_ns) = self.delay_override_ns {
+            parts.push(format!("delay_override_ns: {:?}", delay_override_ns));
+        }
+        write!(f, "{}", parts.join(", "))
     }
 }
 
@@ -47,6 +71,11 @@ pub fn process_update_link(
 
     let link_account = next_account_info(accounts_iter)?;
     let contributor_account = next_account_info(accounts_iter)?;
+    let side_z_account: Option<&AccountInfo> = if accounts.len() > 5 {
+        Some(next_account_info(accounts_iter)?)
+    } else {
+        None
+    };
     let globalstate_account = next_account_info(accounts_iter)?;
     let payer_account = next_account_info(accounts_iter)?;
     let system_program = next_account_info(accounts_iter)?;
@@ -55,7 +84,11 @@ pub fn process_update_link(
     msg!("process_update_link({:?})", value);
 
     // Check if the payer is a signer
-    assert!(payer_account.is_signer, "Payer must be a signer");
+    assert!(
+        payer_account.is_signer,
+        "Payer must be a signer {:?}",
+        payer_account
+    );
 
     // Check the owner of the accounts
     assert_eq!(link_account.owner, program_id, "Invalid PDA Account Owner");
@@ -77,38 +110,77 @@ pub fn process_update_link(
     if contributor.owner != *payer_account.key
         && !globalstate.foundation_allowlist.contains(payer_account.key)
     {
+        msg!("contributor owner: {:?}", contributor.owner);
         return Err(DoubleZeroError::NotAllowed.into());
     }
+    if let Some(side_z_account) = side_z_account {
+        if side_z_account.owner != program_id {
+            return Err(DoubleZeroError::InvalidAccountOwner.into());
+        }
+    }
+
+    // Deserialize the optional side_z device account
+    let side_z: Option<Device> = if let Some(side_z_account) = side_z_account {
+        Some(Device::try_from(side_z_account)?)
+    } else {
+        None
+    };
 
+    // Deserialize the link account
     let mut link: Link = Link::try_from(link_account)?;
 
-    if let Some(ref code) = value.code {
-        link.code = validate_account_code(code).map_err(|_| DoubleZeroError::InvalidAccountCode)?;
-    }
-    if let Some(contributor_pk) = value.contributor_pk {
-        link.contributor_pk = contributor_pk;
-    }
-    if let Some(tunnel_type) = value.tunnel_type {
-        link.link_type = tunnel_type;
-    }
-    if let Some(bandwidth) = value.bandwidth {
-        link.bandwidth = bandwidth;
-    }
-    if let Some(mtu) = value.mtu {
-        link.mtu = mtu;
-    }
-    if let Some(delay_ns) = value.delay_ns {
-        link.delay_ns = delay_ns;
+    if side_z.is_none() {
+        // Link should be owned by the contributor A
+        if link.contributor_pk != *contributor_account.key {
+            msg!("link contributor_pk: {:?}", link.contributor_pk);
+            return Err(DoubleZeroError::NotAllowed.into());
+        }
+    } else if let Some(side_z) = side_z {
+        // Link should be owned by the side_z device's contributor B
+        if link.side_z_pk != *side_z_account.unwrap().key {
+            return Err(DoubleZeroError::InvalidAccountOwner.into());
+        }
+        if side_z.contributor_pk != *contributor_account.key {
+            msg!("side_z contributor_pk: {:?}", side_z.contributor_pk);
+            return Err(DoubleZeroError::NotAllowed.into());
+        }
     }
-    if let Some(jitter_ns) = value.jitter_ns {
-        link.jitter_ns = jitter_ns;
+
+    // can be updated by either contributor A or B
+    if link.contributor_pk == *contributor_account.key {
+        if let Some(ref code) = value.code {
+            link.code =
+                validate_account_code(code).map_err(|_| DoubleZeroError::InvalidAccountCode)?;
+        }
+        if let Some(tunnel_type) = value.tunnel_type {
+            link.link_type = tunnel_type;
+        }
+        if let Some(bandwidth) = value.bandwidth {
+            link.bandwidth = bandwidth;
+        }
+        if let Some(mtu) = value.mtu {
+            link.mtu = mtu;
+        }
+        if let Some(delay_ns) = value.delay_ns {
+            link.delay_ns = delay_ns;
+        }
+        if let Some(jitter_ns) = value.jitter_ns {
+            link.jitter_ns = jitter_ns;
+        }
     }
+    // Can be updated by both contributors A and B
     if let Some(delay_override_ns) = value.delay_override_ns {
         link.delay_override_ns = delay_override_ns;
     }
+
+    // For now only allow foundation to update status
     if let Some(status) = value.status {
         // Only allow to update the status if the payer is in the foundation allowlist
         if !globalstate.foundation_allowlist.contains(payer_account.key) {
+            msg!(
+                "Payer is not in the foundation allowlist: {:?}",
+                payer_account.key
+            );
             return Err(DoubleZeroError::NotAllowed.into());
         }
         link.status = status;