Skip to content

Conversation

@bgm-malbeclabs
Copy link
Contributor

@bgm-malbeclabs bgm-malbeclabs commented Jan 30, 2026

Summary of Changes

This PR adds the ability from the CLI and the smart contract to add simultaneous tunnels: a single unicast and a single mulitcast tunnel (subscriber or publisher only). This also ensures that a second tunnel appears on a second device.

This PR is split out from #2728 so that we can roll this out more safely and in two discrete steps (cli changes then activator,controller changes)

Closes #2725

Testing Verification

  • E2E tests were extended to include creating multiple tunnels for a single user
  • The coexistence tests that were skipped are now enabled and pass
  • This also adds tests to verify that pubkey is always the last deserialized field in both go and rust
  • Claude helped extend the tests

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables simultaneous unicast (IBRL) and multicast tunnels from a single client by creating separate user accounts per tunnel type. Previously, only one tunnel was allowed per client IP.

Changes:

  • Added capability helper methods in the smart contract to determine user capabilities based on state rather than UserType alone
  • Modified CLI logic to create separate user accounts (PDAs) for different tunnel types on the same client IP
  • Enhanced device selection to exclude IPs already used by the client's existing tunnels
  • Extended E2E tests to verify concurrent tunnel scenarios

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
smartcontract/programs/doublezero-serviceability/src/state/user.rs Added capability helper methods to determine user state and comprehensive tests for tunnel state detection
smartcontract/programs/doublezero-serviceability/src/processors/accesspass/set.rs Added test to verify rent allocation constant covers concurrent publisher+subscriber configurations
e2e/multicast_test.go Removed test that enforced single tunnel restriction
e2e/internal/devnet/client.go Added constants for user types and method to wait for multiple tunnels
e2e/internal/arista/commands.go Enhanced documentation for PIM neighbor JSON structures
e2e/ibrl_test.go Removed test that enforced single tunnel restriction
e2e/ibrl_multicast_coexistence_test.go Re-enabled coexistence tests and added new single-client concurrent tunnel tests
client/doublezero/src/dzd_latency.rs Added IP exclusion parameter to device selection to ensure concurrent tunnels use different devices
client/doublezero/src/command/status.rs Updated latency test values to reflect new tolerance
client/doublezero/src/command/connect.rs Refactored user lookup by UserType, added logic to create separate user accounts for concurrent tunnels
CHANGELOG.md Added entry documenting removal of single tunnel constraint

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

require.NoError(t, err)
log.Info("--> Device added", "deviceID", device.ID)

// Add second device for multicast (separate device required for concurrent tunnels)
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "separate device required for concurrent tunnels" but this requirement isn't enforced in the code. The second device is added but the actual device selection happens through latency-based selection. Consider clarifying whether this is truly required or just preferred for the test setup.

Suggested change
// Add second device for multicast (separate device required for concurrent tunnels)
// Add a second device primarily for multicast; a separate device is preferred for concurrent tunnels, but actual device selection is latency-based.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code in connect.rs with best_latency does the device exclusion

Comment on lines +1026 to +1036
if snippet != lastConfigSnippet {
log.Info("Agent config does not yet include client",
"clientIP", client.CYOANetworkIP,
"deviceCode", device.Spec.Code,
"deviceID", device.ID,
"configLen", configLen,
"configSnippet", snippet)
lastConfigSnippet = snippet
} else {
log.Debug("Agent config does not yet include client", "clientIP", client.CYOANetworkIP)
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic to prevent duplicate logging could be simplified by moving the condition check earlier or extracting it into a helper function to reduce nesting and improve readability.

Suggested change
if snippet != lastConfigSnippet {
log.Info("Agent config does not yet include client",
"clientIP", client.CYOANetworkIP,
"deviceCode", device.Spec.Code,
"deviceID", device.ID,
"configLen", configLen,
"configSnippet", snippet)
lastConfigSnippet = snippet
} else {
log.Debug("Agent config does not yet include client", "clientIP", client.CYOANetworkIP)
}
if snippet == lastConfigSnippet {
log.Debug("Agent config does not yet include client", "clientIP", client.CYOANetworkIP)
return false
}
log.Info("Agent config does not yet include client",
"clientIP", client.CYOANetworkIP,
"deviceCode", device.Spec.Code,
"deviceID", device.ID,
"configLen", configLen,
"configSnippet", snippet)
lastConfigSnippet = snippet

Copilot uses AI. Check for mistakes.
}
let lowest_latency_device =
match best_latency(controller, &devices, true, None, current_device).await {
match best_latency(controller, &devices, true, None, current_device, &[]).await {
Copy link
Contributor

@packethog packethog Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a bug in here when reporting the status output. The onchain data says my IBRL tunnel is connected to dz2 but in the doublezero status output, the current device keeps flipping between dz1 and dz2 as well as the metro field. Smells like map keys somewhere that we're assuming are ordered. Also, the multicast tunnel isn't reporting a connected device (N/A):

root@client-41JaXfSxFmuggZSxJrrHyG7fQ75hKHJF2RFbpFqq27HJ:/# doublezero user list
 account                                      | user_type | groups | device | location    | cyoa_type  | client_ip    | dz_ip        | accesspass     | tunnel_id | tunnel_net     | status    | owner
 F9i9HWuaMG6fcRpwGE85sPYFtyATv9MeeaTj9cjBeASu | Multicast | S:demo | dz1    | Amsterdam   | GREOverDIA | 9.169.90.110 | 9.169.90.110 | Prepaid: (MAX) | 500       | 169.254.0.4/31 | activated | 41JaXfSxFmuggZSxJrrHyG7fQ75hKHJF2RFbpFqq27HJ
 CvshkNksx7PqNCeMxA3zn2zUL55BskYAszJvkoRhpVLY | IBRL      |        | dz2    | Los Angeles | GREOverDIA | 9.169.90.110 | 9.169.90.110 | Prepaid: (MAX) | 501       | 169.254.0.0/31 | activated | 41JaXfSxFmuggZSxJrrHyG7fQ75hKHJF2RFbpFqq27HJ
root@client-41JaXfSxFmuggZSxJrrHyG7fQ75hKHJF2RFbpFqq27HJ:/# doublezero status
 Tunnel Status  | Last Session Update     | Tunnel Name | Tunnel Src   | Tunnel Dst  | Doublezero IP | User Type | Current Device | Lowest Latency Device | Metro     | Network
 BGP Session Up | 2026-02-03 15:20:16 UTC | doublezero0 | 9.169.90.110 | 9.169.90.16 | 9.169.90.110  | IBRL      | dz1            | ✅ dz1                | Amsterdam | local
 BGP Session Up | 2026-02-03 15:23:22 UTC | doublezero1 | 9.169.90.110 | 9.169.90.8  |               | Multicast | N/A            | ✅ dz1                | N/A       | local
root@client-41JaXfSxFmuggZSxJrrHyG7fQ75hKHJF2RFbpFqq27HJ:/# doublezero status
 Tunnel Status  | Last Session Update     | Tunnel Name | Tunnel Src   | Tunnel Dst  | Doublezero IP | User Type | Current Device | Lowest Latency Device | Metro       | Network
 BGP Session Up | 2026-02-03 15:20:16 UTC | doublezero0 | 9.169.90.110 | 9.169.90.16 | 9.169.90.110  | IBRL      | dz2            | ✅ dz2                | Los Angeles | local
 BGP Session Up | 2026-02-03 15:23:22 UTC | doublezero1 | 9.169.90.110 | 9.169.90.8  |               | Multicast | N/A            | ✅ dz1                | N/A         | local

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great catch - indeed a bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove multiple tunnel restriction from the client

4 participants