Skip to content

serviceability-instruction: golden fixtures + CI guard (RFC-26 RF) - #4059

Merged
juan-malbeclabs merged 3 commits into
mainfrom
feat/rfc26-rf-fixtures
Jul 25, 2026
Merged

serviceability-instruction: golden fixtures + CI guard (RFC-26 RF)#4059
juan-malbeclabs merged 3 commits into
mainfrom
feat/rfc26-rf-fixtures

Conversation

@juan-malbeclabs

Copy link
Copy Markdown
Contributor

Summary

RFC-26 RF: golden fixtures for the instruction builders + a CI drift guard.

  • Extend the serviceability fixture generator to depend on doublezero-serviceability-instruction and emit, per instruction, ix_<name>.bin (wire bytes = tag + borsh) and ix_<name>.json ({ variant, data_hex, accounts: [{pubkey, is_signer, is_writable}] }) from fixed, deterministic inputs.
  • Representative set covering the trickiest layouts: create_device (variable dz_prefix), delete_device (atomic close), create_link, create_subscribe_user (optional feed), create_user (length-detected), clear_topology / assign_topology_node_segments (batched), set_global_config (config PDA + all 8 pools).
  • make generate-fixtures-check (regenerate + fail on drift, scoped to the emitted .bin/.json) and a fixtures-check job in the rust workflow.

These fixtures capture byte-for-byte the current SDK trailing convention (they will drive Go/Python/TS parity in a later phase).

Testing Verification

  • make generate-fixtures-check passes (regenerated output is byte-identical to the committed fixtures); a hand-edited fixture makes it fail as expected.

Closes #4026. Part of RFC-26 (rfcs/rfc26-rust-instruction-builder-library.md).


PR stack (RFC-26 builder library)

Stacked PRs, merge in order (each based on the previous one's branch):

  1. serviceability-instruction: scaffold pure instruction-builder crate (RFC-26 R0) #4049 — R0 scaffold + exemplars
  2. serviceability-instruction: device domain builders (RFC-26 R1) #4050 — R1 device
  3. serviceability-instruction: link domain builders (RFC-26 R2) #4051 — R2 link
  4. serviceability-instruction: user domain builders (RFC-26 R3) #4052 — R3 user
  5. serviceability-instruction: location/exchange/contributor builders (RFC-26 R4) #4053 — R4 location/exchange/contributor
  6. serviceability-instruction: multicastgroup + allowlist builders (RFC-26 R5) #4054 — R5 multicastgroup + allowlists
  7. serviceability-instruction: tenant + permission builders (RFC-26 R6) #4055 — R6 tenant + permission
  8. serviceability-instruction: topology + feed builders (RFC-26 R7) #4056 — R7 topology + feed
  9. serviceability-instruction: accesspass + resource builders (RFC-26 R8) #4057 — R8 accesspass + resource
  10. serviceability-instruction: globalstate/globalconfig/allowlist/index/migrate builders (RFC-26 R9) #4058 — R9 globalstate/config/allowlist/index/migrate
  11. this PR — RF fixtures + CI guard

R10 (commands/* migration + program-test) is the final PR and will carry the global changelog entry.

@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-r9-admin-builders branch from e0235a0 to 9240816 Compare July 21, 2026 17:02
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-rf-fixtures branch from 70541a5 to 57fe622 Compare July 21, 2026 17:02
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-r9-admin-builders branch from 9240816 to 9e23736 Compare July 21, 2026 17:19
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-rf-fixtures branch from 57fe622 to 7ab5da2 Compare July 21, 2026 17:19
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-r9-admin-builders branch 4 times, most recently from 155cbc7 to d934652 Compare July 23, 2026 23:10
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-rf-fixtures branch from 7ab5da2 to 92e417d Compare July 23, 2026 23:10
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-r9-admin-builders branch from d934652 to bde6207 Compare July 24, 2026 13:49
Base automatically changed from feat/rfc26-r9-admin-builders to main July 24, 2026 20:06
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-rf-fixtures branch from 92e417d to b0a7627 Compare July 24, 2026 20:31
@juan-malbeclabs
juan-malbeclabs requested a review from ben-dz July 24, 2026 20:38

@ben-dz ben-dz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things worth changing before merge; nothing blocking. (1) The new fixtures-check job in rust.yml is a strict duplicate of the pre-existing sdk-fixture-drift job in sdk.yml — same make check-fixtures, both triggered on every PR/push with no path filters — so the drift check runs twice per PR for zero added coverage, and the new copy runs on an oversized ubuntu-24.04-16c-64gb runner with no timeout-minutes. Delete one, or move the check and delete the other. (2) The set_global_config fixture is built from SetGlobalConfigArgs::default(), emitting an all-zero body that pins only the length — it can't catch field-order/endianness/signedness/Option-encoding regressions, which is the whole point of a wire-format golden; use distinct nonzero values. Low: the fixture JSON records neither program_id nor the input args, so a future parity harness must read the Rust source to reproduce inputs. Fixtures are deterministic (regenerated locally, byte-identical) and otherwise correctly capture the builder output for the representative set.

Comment thread .github/workflows/rust.yml Outdated
Comment thread sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs Outdated
Comment thread sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs
…+ CI guard (RFC-26 RF)

Extend the serviceability fixture generator to depend on the builder crate and
emit, per instruction, ix_<name>.bin (tag + borsh wire bytes) and ix_<name>.json
({ variant, data_hex, accounts:[{pubkey,is_signer,is_writable}] }) with fixed,
deterministic inputs. Covers a representative set exercising the trickiest
layouts: create_device (variable dz_prefix), delete_device (atomic close),
create_link, create_subscribe_user (optional feed), create_user (length-detected),
clear_topology / assign_topology_node_segments (batched), set_global_config.

Add `make generate-fixtures-check` (regenerate + fail on drift) and a
`fixtures-check` job in the rust CI workflow.

Refs #4026, RFC-26.
…nerator feed borrow

The clear_topology builder now marks the topology account writable (the
processor decrements its reference_count), so regenerate ix_clear_topology.json.
Also fix the generator to pass feed as Option<&Pubkey> (Some(&d)), matching the
current create_subscribe_user signature.
…iscriminating global_config fixture, self-describing goldens

- Delete duplicate fixtures-check job from rust.yml; sdk-fixture-drift in
  sdk.yml already runs make check-fixtures (on ubuntu-latest with a timeout).
- Build set_global_config args from distinct nonzero values instead of
  ::default(), so the golden pins field order, endianness, both u32 ASNs, and
  the Some-encoding of next_bgp_community rather than only the body length.
- Record instruction.program_id in the ix_*.json goldens so a future parity
  harness need not read the generator to reproduce inputs.
@juan-malbeclabs
juan-malbeclabs force-pushed the feat/rfc26-rf-fixtures branch from b0a7627 to ebe5e54 Compare July 25, 2026 15:05
@juan-malbeclabs
juan-malbeclabs requested a review from ben-dz July 25, 2026 15:08

@ben-dz ben-dz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All three findings from the prior review are addressed in ebe5e54 and verified in the current code: the duplicate fixtures-check CI job is deleted from rust.yml (sdk.yml's better-provisioned sdk-fixture-drift retained); set_global_config now builds SetGlobalConfigArgs with distinct nonzero values so the golden pins field order, both u32 ASNs, and the optional community's Some-encoding instead of an all-zero body; and IxFixtureMeta records program_id in every ix_*.json. The passing sdk-fixture-drift job confirms the goldens regenerate byte-identically. lgtm.

@juan-malbeclabs
juan-malbeclabs merged commit 9c2ceb7 into main Jul 25, 2026
36 checks passed
@juan-malbeclabs
juan-malbeclabs deleted the feat/rfc26-rf-fixtures branch July 25, 2026 17:58
juan-malbeclabs added a commit that referenced this pull request Jul 27, 2026
…R10) (#4060)

## Summary of Changes
- Delegate every serviceability `commands/*` `execute()` to the pure
`doublezero-serviceability-instruction` builders (RFC-26), so account
layout has a single source of truth in the builder crate rather than
being hand-assembled per command.
- Replace the four `DoubleZeroClient::execute_*(instruction, accounts)`
trait methods and the client-side account assembly with a single
`send_transaction(Instruction)` that just prepends the compute-budget
prelude, signs, and sends. Command `execute()` signatures/returns are
unchanged, so CLI/sentinel/daemon consumers are unaffected. The `_quiet`
variant is not carried forward — its only caller was the `activator/`
crate, deleted in #3647.
- **Fixes #4078:** `doublezero topology clear` reverted with `Topology
Account is not writable` on a live topology — the SDK passed the
topology PDA read-only while the processor asserts it writable whenever
it actually clears a link reference (it decrements `reference_count`).
The command therefore only worked on the already-closed-topology path,
blocking `topology delete`, which requires `reference_count == 0`. The
`clear_topology` builder passes the account writable; writable is a
strict superset privilege, so no other path changes.
- **Breaking (see CHANGELOG `### Breaking`):** the Rust SDK no longer
attaches the payer's Permission PDA to serviceability transactions —
`authorize()` uses the legacy GlobalState allowlist path until the
builder-side Permission append is re-enabled per instruction (must be
sequenced with the permission-model rollout). A payer whose only grant
is an activated Permission account would lose access to gated commands.
- **No cluster is exposed — verified against live chain state.**
`RequirePermissionAccounts` is off everywhere, so the legacy fallback is
live. mainnet-beta's 8 Permission accounts all grant `FOUNDATION` to
keys already in `foundation_allowlist`. The one grant on testnet/devnet
that no legacy authority covers (`ACCESS_PASS_ADMIN` to
`EWFXDTBWhNxsmj4mbahZL7HH1XpmC6Ao3LZUhJGCyyPh`) belongs to the shred
oracle, which never uses `DoubleZeroClient` — it assembles its
serviceability instructions itself and appends its own Permission
account after its own RPC existence check (`doublezero-shreds`,
`crates/shred-oracle/src/dz_ledger.rs`, `permission_account_meta`, 9
call sites). It is unaffected by this send path. No deploy blocker.
- **The next grant is now self-documenting.** `permission set` emits a
`tracing::warn!` whenever the resulting bitmask carries an
`AUTHORIZE_GATED_FLAGS` bit, and `permission audit` renders a matching
paragraph (plus a `sdk_attaches_permission_accounts` field in `--json`),
stating that the CLI/Rust SDK do not attach Permission accounts yet so
the key still needs a legacy allowlist or authority entry. The
chain-state check above is point-in-time by nature; this covers grants
made after it. Both are removed when the append is re-enabled.
- Add a `solana-program-test` safety net that runs the
highest-cardinality builders (`create_device`, `create_link`,
`create_subscribe_user`, atomic `delete_device`, `clear_topology`)
against the real program to catch account-order drift.
- Remove the now-unreachable permission-resolve/cache/stale-retry
machinery from `client.rs` (the append, when re-enabled, lands in the
builder — derived offline — so the RPC-based path is not needed).

This is the final PR of the RFC-26 stack; it carries the global
changelog entry. See
[`rfcs/rfc26-rust-instruction-builder-library.md`](https://github.com/malbeclabs/doublezero/blob/main/rfcs/rfc26-rust-instruction-builder-library.md).

> **Stack:** R0 → R1 → … → R9 → RF (#4059, merged) → **R10 (this PR)**.
Now rebased onto `main`, so the diff below is R10 only.

## Diff Breakdown
| Category     | Files | Lines (+/-)   | Net   |
|--------------|-------|---------------|-------|
| Core logic   |    13 | +868 / -1441  | -573  |
| Scaffolding  |    78 | +2396 / -3298 | -902  |
| Tests        |     1 | +763 / -0     | +763  |
| Config       |     3 | +6 / -0       | +6    |
| Docs         |     2 | +10 / -3      | +7    |
| Generated    |     1 | +2 / -0       | +2    |

Net **−697 lines**: a mostly-mechanical migration (77 command
delegations) on top of a genuine send-path/trait rewrite and one new
program-test suite. This exceeds doublezero's ~500-line PR norm, but the
migration + its program-test safety net are one logical unit and the net
change is a reduction; splitting would land the safety net separately
from the code it guards.

<details>
<summary>Key files (click to expand)</summary>

-
`smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs`
(+763) — new program-test safety net; builds via the RFC-26 builders and
runs them against the real program.
- `smartcontract/sdk/rs/src/client.rs` (+70/-560) — rework
`execute_transaction_inner` into `send_transaction_inner(ix)`; drop
`assemble_instructions`, the permission-resolve/cache and stale-retry
loop, and their tests.
- `smartcontract/sdk/rs/src/commands/user/delete.rs` (+153/-246) —
composed unsubscribe + `delete_user` builder; keeps cascade
orchestration.
- `smartcontract/sdk/rs/src/commands/exchange/setdevice.rs` (+117/-131)
— two-send command migrated to `set_device_exchange` builder.
- `smartcontract/sdk/rs/src/commands/tenant/delete.rs` (+108/-116) —
`delete_tenant` builder; keeps cascade + reference-count poll.
- `smartcontract/sdk/rs/src/commands/link/update.rs` (+77/-81) —
`update_link` builder with `LinkUpdateAuthority`; builder recomputes
`use_onchain_allocation`.
- `smartcontract/sdk/rs/src/commands/device/delete.rs` (+59/-87) —
legacy/atomic paths via `delete_device` + `DeviceDeleteResources`.
- `smartcontract/sdk/rs/src/commands/topology/clear.rs` (+57/-85) —
batched sends via `clear_topology`; carries the #4078 writable-PDA fix.
- `smartcontract/sdk/rs/src/doublezeroclient.rs` (+7/-35) — trait: four
`execute_*` methods → a single `send_transaction`.
- `smartcontract/cli/src/permission/{set,audit}.rs` (+92/-1) —
operator-facing warning that Permission grants are inert until the
builder-side append returns.

</details>

## Testing Verification
- New `solana-program-test` suite (`rfc26_builders_test.rs`): 5 tests
exercising the highest-cardinality builders against the real program —
`create_device`, `create_link` + `clear_topology`,
`create_subscribe_user` (atomic subscriber), and atomic `delete_device`
(owners read on-chain in processor order). All pass.
- `clear_topology` covers both processor branches: `cleared_count == 0`
(untagged topology, no write) and `cleared_count > 0` (the topology the
link is tagged into), the latter exercising the `is_writable` assert and
`reference_count` decrement that #4078 is about. Verified it is a real
guard — flipping the builder to `new_readonly` fails with `Topology
Account is not writable`.
- `cargo test -p doublezero_sdk`: 167 passed. Rewritten command tests
assert `predicate::eq(builder(...))` on the exact instruction each
command sends. The composed cascades in `user/delete` and
`tenant/delete` pin every step — the resolved access-pass PDA,
`dz_prefix_count`, the optional `tenant` account, the `[current_tenant,
new_tenant]` reset pair, and the trailing `owner` (which differs from
the payer on the foundation-key path).
- Assertion strength across `sdk/rs/src/commands/` went up, not down:
`predicate::always()` appears in 0 files here, down from 8 on `main`
(where it relaxed the *accounts* argument of the two-arg `execute_*`
expectation). The four `multicastgroup/allowlist/*` tests now pin the
built instruction end to end — on `main` they asserted the
`DoubleZeroInstruction` but not the accounts — with `user_payer`
deliberately distinct from the group pubkey so an account-meta swap
fails. Confirmed discriminating: perturbing `client_ip` in the expected
instruction fails with `No matching expectation found`.
- `permission set`'s new warning is gated on `AUTHORIZE_GATED_FLAGS`,
unit-tested to fire for every gated flag and stay quiet for `SENTINEL` /
`QA` / `FEED_AUTHORITY` (which no processor hands to `authorize()`), so
it cannot become background noise.
- The `u8` bounds on `resource_count` / `topology_count` stay errors
rather than builder panics: `device/create`, `device/interface/create`
and `device/interface/update` guard before building, with tests
asserting the exact messages for both interface guards.
- Confirmed the batched topology and multi-send commands
(`exchange/setdevice`, `topology/clear`,
`topology/assign_node_segments`) preserve their per-instruction
sequencing.
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.

serviceability: golden fixtures for instruction builders + CI diff guard (Rust)

2 participants