Skip to content

smartcontract/serviceability: collapse OnChainAllocation flag branches#3649

Open
elitegreg wants to merge 12 commits intomainfrom
gm/3613-collapse-onchainallocation-flag-branches
Open

smartcontract/serviceability: collapse OnChainAllocation flag branches#3649
elitegreg wants to merge 12 commits intomainfrom
gm/3613-collapse-onchainallocation-flag-branches

Conversation

@elitegreg
Copy link
Copy Markdown
Contributor

@elitegreg elitegreg commented May 2, 2026 •
edited
Loading

Summary

  • Collapse every is_feature_enabled(FeatureFlag::OnChainAllocation) branch in
    the serviceability program; rename the variant to OnChainAllocationDeprecated
    with bit 0 documented as reserved-and-never-reused.
  • For the boolean-flagged instructions (multicastgroup create/delete/update/
    subscribe, link create/delete/accept/update, device interface
    create/delete), require use_onchain_(de)allocation: true and accept the
    resource extension accounts as positional, non-optional args. Calls that flip
    the flag false now return InvalidArgument. link::update and
    multicastgroup::update only enforce this when tunnel / multicast-IP fields
    are actually being changed, so trivial field-only updates don't need extras.
  • Relax ActivateDeviceInterface and UnlinkDeviceInterface status guards to
    accept the target status as a no-op, since CreateDeviceInterface now
    produces Loopback-Activated / Physical-Unlinked atomically.
  • Propagate the rename into the Rust SDK.

Phase 4, PR 4.1 of the activator removal effort. Closes #3613. Tracker: #3607.

Testing Verification

  • cargo test -p doublezero-serviceability --features no-entrypoint — 453
    passed, 0 failed.
  • cargo clippy --features no-entrypoint --all-targets -- -Dclippy::all -Dwarnings — clean across the workspace.
  • cargo test -p doublezero_sdk — 183 passed, 0 failed (rename-only changes in
    the SDK).
  • Test handling: deleted *_legacy_* / *_backward_compat_* cases that
    exercised the now-rejected paths and tests that asserted intermediate Pending
    status (Create now activates atomically); migrated the remaining tests onto
    the new account layouts. Net coverage is preserved by the onchain-pair tests
    that already covered the same instruction logic.
  • New init_globalstate_and_config helper in tests/test_helpers.rs so tests
    don't have to set up every ResourceExtension by hand.

elitegreg
elitegreg commented May 3, 2026
View reviewed changes
Comment thread smartcontract/programs/doublezero-serviceability/src/processors/device/interface/activate.rs
Copy link
Copy Markdown
Contributor Author

@elitegreg elitegreg May 3, 2026

Choose a reason for hiding this comment

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

This change is unnecessary

Comment thread smartcontract/programs/doublezero-serviceability/src/processors/device/interface/unlink.rs
Copy link
Copy Markdown
Contributor Author

@elitegreg elitegreg May 3, 2026

Choose a reason for hiding this comment

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

This change is unnecessary

Comment thread smartcontract/programs/doublezero-serviceability/src/processors/device/create.rs
Comment thread smartcontract/programs/doublezero-serviceability/src/processors/link/create.rs Outdated
if side_a_iface.status != InterfaceStatus::Unlinked {
// Atomic create+allocate+activate. DZX links stay in Requested until accepted by side Z;
// all other links activate immediately.
if link.status == LinkStatus::Pending {
Copy link
Copy Markdown
Contributor Author

@elitegreg elitegreg May 3, 2026

Choose a reason for hiding this comment

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

status != Requested

Comment thread smartcontract/sdk/rs/src/commands/device/interface/create.rs
Comment thread smartcontract/sdk/rs/src/commands/user/create.rs
Comment thread smartcontract/sdk/rs/src/commands/user/create_subscribe.rs
Comment thread smartcontract/sdk/rs/src/commands/user/delete.rs
Comment thread smartcontract/sdk/rs/src/commands/user/requestban.rs
Comment thread smartcontract/sdk/rs/src/commands/user/update.rs
elitegreg added a commit that referenced this pull request May 3, 2026
@elitegreg
smartcontract: address PR #3649 review feedback
7e76f28 
- Drop is_feature_enabled checks from Rust SDK commands; resource accounts
  are now always passed and the `use_*onchain_*` boolean flags are derived
  from whether the field is being set rather than the global feature flag.
- Enforce `resource_count >= 2` and `DeviceStatus::Activated` in
  process_create_device.
- Tighten link/create.rs status check to `!= LinkStatus::Requested`.
- Update integration tests to drop the legacy Pending → Activated flow
  for devices (CreateDevice now activates atomically).
- Update activator to use renamed FeatureFlag::OnChainAllocationDeprecated.
elitegreg added 10 commits May 5, 2026 15:57
@elitegreg
smartcontract/serviceability: collapse OnChainAllocation flag branches
77d2acf 
Treat onchain allocation as always-on by removing every is_feature_enabled(OnChainAllocation)
branch from the serviceability program processors. Rename the FeatureFlag::OnChainAllocation
variant to OnChainAllocationDeprecated so bit 0 is reserved and never reused — the variant
itself is retained so the flag discriminant doesn't shift.

Tests covering the now-impossible feature-flag-disabled case are deleted; their onchain pairs
remain. Tests that exercised legacy update paths gain the resource extension account they now
require. Coverage stays net-equivalent.

Refs #3613
@elitegreg
smartcontract/serviceability: enforce onchain-only paths in boolean-f…
08f508f 
…lagged instructions

Tighten the multicastgroup/link/device-interface processors that carry a
`use_onchain_(de)allocation` boolean: validate the field is true and require the
ResourceExtension accounts as non-optional positional args (no more legacy
`Option<>` branches). Calls that flip the flag false now return InvalidArgument.

In tests, delete `*_legacy_*` / `*_backward_compat_*` cases that exercised the
removed paths, migrate the remaining tests onto the always-on layout, and add a
`init_globalstate_and_config` test helper so tests don't need to bootstrap each
ResourceExtension manually.

Also relax the ActivateDeviceInterface and UnlinkDeviceInterface status guards
to accept already-Activated / already-Unlinked interfaces idempotently, since
CreateDeviceInterface now activates atomically.

Refs #3613
@elitegreg
smartcontract: address PR #3649 review feedback
31d1a51 
- Drop is_feature_enabled checks from Rust SDK commands; resource accounts
  are now always passed and the `use_*onchain_*` boolean flags are derived
  from whether the field is being set rather than the global feature flag.
- Enforce `resource_count >= 2` and `DeviceStatus::Activated` in
  process_create_device.
- Tighten link/create.rs status check to `!= LinkStatus::Requested`.
- Update integration tests to drop the legacy Pending → Activated flow
  for devices (CreateDevice now activates atomically).
- Update activator to use renamed FeatureFlag::OnChainAllocationDeprecated.
@elitegreg
smartcontract,e2e: fix dangling onchain-allocation feature flag refer…
6d7ad86 
…ences

The previous rename to OnChainAllocationDeprecated changed the CLI flag
string from "onchain-allocation" to "onchain-allocation-deprecated".
Update CLI tests to use the new string and remove the e2e devnet step that
enabled the flag — the flag is now always-on/no-op so the call adds no
value and would fail with the new name.
@elitegreg
smartcontract: fix link/create SDK account order and update telemetry…
9c2d887 
… test helper

- The SDK's CreateLink command was passing accounts in the legacy
  [globalstate, device_tunnel_block, link_ids, unicast_default] order, but
  commit 2b6cf3d moved unicast_default before the resource extension
  accounts. Reorder the SDK to match the program's positional layout.
- Update the doublezero-telemetry test helper to create devices with the
  required resource_count >= 2 and the corresponding TunnelIds and
  DzPrefixBlock accounts, since CreateDevice now activates atomically.
@elitegreg
smartcontract,e2e: fix telemetry test helpers and compat test for alw…
2da36fe 
…ays-on flow

- Update doublezero-telemetry test_helpers to:
  * Pass the required globalconfig + tunnel_ids + dz_prefix accounts in
    CreateDevice (resource_count >= 2).
  * Flip use_onchain_allocation to true in CreateLink, ActivateLink, and
    CreateDeviceInterface, and pass the corresponding resource extension
    accounts the program now expects.
  * Drop the now-redundant ActivateLink call after CreateLink (links
    activate atomically) and the UnlinkDeviceInterface call after
    CreateDeviceInterface (interfaces start in the right state).
  * Restore the legacy "create then activate" semantics by demoting the
    device back to Pending in create_device and re-activating in
    set_device_ready_for_users (so existing tests that check Pending →
    Activated transitions still work).
- Mark CLIs ≤ v0.21.0 as known-incompatible for link_create_wan /
  link_create_dzx in compatibility_test.go: the OnChainAllocation flag
  deprecation requires use_onchain_allocation=true, which older CLIs
  don't set.
@elitegreg
smartcontract/telemetry: fix rustfmt in test_helpers
936066d
@elitegreg
smartcontract,e2e: extend backward-compat known incompatibilities
7777620 
- Accept the legacy "onchain-allocation" string in FeatureFlag::from_str
  so older callers (e.g. doublezero-shreds e2e setup) that flip the
  now-no-op flag continue to parse. Bit 0 is reserved either way.
- Mark link_accept_dzx as known-incompatible for CLIs ≤ v0.21.0 in the
  backward-compat e2e — LinkAccept also requires use_onchain_allocation=true
  after the flag deprecation.
@elitegreg
e2e: cascade-skip link operations when link_create_wan/dzx is known-i…
09c9eea 
…ncompatible

When older CLIs (≤ v0.21.0) can't create links because they don't set
use_onchain_allocation=true, downstream link_update / link_set_health
steps run against a non-existent link and fail with "Link with code X not
found". Mark link_create_wan / link_create_dzx with cascadeKnownFail so
the wait_and_configure phase is skipped when the create phase couldn't
build the links.
@elitegreg
pr fixes
02f33c3
@elitegreg elitegreg force-pushed the gm/3613-collapse-onchainallocation-flag-branches branch from 04ec2cd to 02f33c3 Compare May 5, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

skip-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

activator removal phase 4.1: serviceability program — collapse OnChainAllocation flag branches

1 participant

@elitegreg