Skip to content

smartcontract: address PR #3648 review#3688

Draft
elitegreg wants to merge 6 commits intomainfrom
gm/pr-3648-fixes
Draft

smartcontract: address PR #3648 review#3688
elitegreg wants to merge 6 commits intomainfrom
gm/pr-3648-fixes

Conversation

@elitegreg
Copy link
Copy Markdown
Contributor

Summary

Address the four unresolved Copilot review threads on #3648:

  • Reject duplicate topology PDAs in CreateDeviceInterface so a caller can't allocate two flex-algo SR IDs for the same topology — mirrors the existing dedupe in UpdateDeviceInterface.
  • Compute topology_count via u8::try_from(topology_names.len()) in the Rust SDK's CreateDeviceInterfaceCommand and UpdateDeviceInterfaceCommand so passing more than 255 topology names errors out instead of silently wrapping.
  • In UpdateDeviceInterfaceCommand, return an error when topology reconciliation is requested but the OnChainAllocation feature flag is off, instead of silently dropping the request.
  • Normalize CLI --topologies names with to_ascii_uppercase to match the canonical form used to derive topology PDAs.

Stacks on top of #3648 (base bc/rfc18-devnet-fixes).

Related RFC: rfcs/rfc-0018-flex-algo.md

Testing Verification

  • New test_create_device_interface_rejects_duplicate_topology_accounts in interface_onchain_allocation_test.rs exercises the dedupe path on CreateDeviceInterface.
  • cargo test -p doublezero-serviceability — full smartcontract suite passes, including the existing test_update_topologies_* cases.
  • cargo test -p doublezero_sdk device::interface and cargo test -p doublezero_cli device::interface cover the SDK / CLI changes.

ben-malbeclabs and others added 6 commits May 8, 2026 20:01
Community stamping only resolved colors for users with a tenant
assigned. Since most users (all of mainnet, most of testnet) have no
tenant, they were silently skipped. Add an else branch that calls
resolveTenantColors with nil includeTopologies, which falls through to
the default UNICAST-DEFAULT color.
When creating a VPNv4 loopback with onchain allocation, the instruction
now accepts optional topology PDA accounts and allocates a
FlexAlgoNodeSegment for each. This replaces the activator's post-
activation backfill that was lost during the activator removal.

The CLI automatically discovers existing topologies and passes them to
the instruction, so `doublezero device interface create ... --loopback-type
vpn-ipv4` assigns all node segments atomically — no manual steps.

Also rename BackfillTopology → AssignTopologyNodeSegments across the
codebase (instruction discriminant 110 unchanged).
- cli/device-interface-get: cache list_topology in a HashMap so we
  don't refetch per flex-algo segment
- create/update: validate topology accounts by first-byte
  AccountType::Topology in addition to program-owner
- update: allow contributor.owner to drive update_topologies, not just
  the foundation allowlist
- update: size kept Vec by max(existing, desired) so we don't realloc
  when the desired set grows
- tests: add test_update_topologies_allowed_for_contributor exercising
  the contributor-owner authorization branch
- changelog: add Unreleased entries covering this PR's smartcontract,
  controller, and CLI changes
- create processor: reject duplicate topology PDAs in CreateDeviceInterface
  with InvalidArgument before allocating an SR ID, matching the dedupe in
  UpdateDeviceInterface.
- sdk: compute topology_count once via u8::try_from(names.len()) on both
  create and update commands so > 255 topologies returns an error instead
  of silently wrapping.
- sdk: in UpdateDeviceInterfaceCommand, return an error when topology_names
  is provided but OnChainAllocation is disabled, instead of silently
  dropping the request.
- cli: normalize --topologies names with to_ascii_uppercase to match the
  canonical form used to derive topology PDAs.
Base automatically changed from bc/rfc18-devnet-fixes to main May 8, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants