Skip to content

serviceability: split access pass removal into one instruction per pass type - #4267

Open
bgm-malbeclabs wants to merge 9 commits into
mainfrom
access-pass-program
Open

serviceability: split access pass removal into one instruction per pass type#4267
bgm-malbeclabs wants to merge 9 commits into
mainfrom
access-pass-program

Conversation

@bgm-malbeclabs

@bgm-malbeclabs bgm-malbeclabs commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Part of malbeclabs/infra#2470. Child of malbeclabs/infra#2385.

Serviceability has one CloseAccessPass (variant 69) and one DeleteUser (variant 42). Neither looks at AccessPassType. Both destroy state, so a caller meaning to remove a prepaid pass can remove an EdgeSeat pass and the program accepts it.

This adds one instruction per pass type. Each loads the pass and refuses unless the stored pass matches the instruction.

This PR is purely additive. CloseAccessPass and DeleteUser keep working exactly as they do today, so this is safe to merge and safe to deploy on its own. The follow-up, #4268, deprecates them and moves every caller in one commit range.

Summary of Changes

  • Ten new instructions. 119-123 close a pass: ClosePrepaidAccessPass, CloseSolanaValidatorAccessPass, CloseSolanaRPCAccessPass, CloseOthersAccessPass, CloseEdgeSeatAccessPass. 124-128 delete a user: DeletePrepaidUser, DeleteSolanaValidatorUser, DeleteSolanaRPCUser, DeleteOthersUser, DeleteEdgeSeatUser.
  • AccessPassKind, a payload-free tag for AccessPassType. It has no Borsh derive on purpose: issue telemetry: add pcap input/stdout support to flow-enricher #2470 requires that the pass type is never an instruction argument, so the kind is a Rust parameter the dispatch arm supplies and it never reaches the wire.
  • New error AccessPassTypeMismatch (119), returned when the instruction and the stored pass disagree.
  • Two handler bodies, not ten. process_close_access_pass and process_delete_user each gained one expected: AccessPassKind parameter. Ten copies of the 250-line delete body would be ten places to fix its next bug.
  • process_close_access_pass and process_delete_user take an Option<AccessPassKind>. None is the old general-purpose path and performs no kind check, which is how 69 and 42 keep their current behaviour. The follow-up removes the Option.
  • Fixes a pre-existing bug found on the way. close.rs wrapped its account-type check and its connection_count check in if let Ok(data) = accesspass_account.try_borrow_data(), with an else that logged a warning and then closed the pass anyway. A failed borrow skipped both checks. The read is now hoisted and a failed borrow returns an error.

Not breaking, on purpose

Nothing changes for existing callers in this PR. That is deliberate: an earlier version of this split deprecated 69 and 42 here, and CI showed the CLI on this very branch then failed with Deprecated on doublezero user delete. Deploying that alone would have broken user deletion until the follow-up landed.

The deprecation, and the warning that the oracle in doublezero-shreds must ship with it, live in #4268.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 3 +130 / -29 +101
Scaffolding 6 +186 / -46 +140
Tests 11 +709 / -16 +693
Docs 1 +235 / -0 +235
Total 21 +1260 / -91 +1169

Mostly tests and instruction registration. The behaviour change is about 100 lines across three files.

Key files (click to expand)
  • smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs — the AccessPassKind tag, its From<&AccessPassType> and Display. One arm per pass type in one place, so a future pass type costs one line.
  • smartcontract/programs/doublezero-serviceability/src/instructions.rs — the ten variants, each registered in the enum, the decoder, get_name, the args formatter and the round-trip test.
  • smartcontract/programs/doublezero-serviceability/src/entrypoint.rs — ten dispatch arms, each naming its kind. 69 and 42 join the existing deprecated arm.
  • smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs — the kind check, plus the try_borrow_data fall-through fix.
  • smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs — the kind check, placed before the ownership checks so a wrong-kind call fails for the reason it actually failed.
  • crates/doublezero-serviceability-instruction/src/{accesspass,user}.rs — the two builders keep compiling against the now payload-free 69 and 42. The follow-up PR replaces both functions.

Testing Verification

  • Two new integration suites, close_access_pass_kind_test.rs and delete_user_kind_test.rs, each covering all five kinds: the matching instruction removes the account, and an instruction for any other kind is refused with AccessPassTypeMismatch while the account survives. Assertions are on the specific error code, not is_err().
  • The EdgeSeat delete case seeds a real FeedSeat and a matching feed_pks entry on the user, then asserts the seat was released after the delete. A feedless EdgeSeat pass makes release_feed_seats a silent no-op, so without the seat the test would prove nothing.
  • deprecated_removal_instructions_test.rs asserts variants 69 and 42 both return Deprecated, so the wire discriminants stay decodable rather than failing to parse.
  • Every pre-existing DeleteUser and CloseAccessPass call site in the program's tests was traced back to the pass type its own setup creates before being moved to the matching variant.
  • cd smartcontract && make test-programs: 78 binaries, zero failures. make rust-lint clean.

Copilot AI 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.

🔵 Needs a closer look

It changes onchain instruction semantics and introduces a deploy-time breaking change that needs coordinated updates across external callers.

Pull request overview

This pull request hardens the DoubleZero serviceability onchain program by preventing a caller from closing or deleting state for the wrong access pass type. It introduces per-pass-type removal instructions, deprecates the old generic variants while keeping their wire discriminants decodable, and adds program tests that validate the new mismatch guard.

Changes:

  • Add AccessPassKind and DoubleZeroError::AccessPassTypeMismatch to enforce that close and delete instructions match the stored AccessPassType.
  • Introduce 10 new instruction variants (5 close, 5 delete) and dispatch them in entrypoint.rs, while making variants 42 and 69 payload-free and deprecated.
  • Update processors and tests to use the new variants, plus add new integration suites and a design spec doc.
File summaries
File Description
smartcontract/programs/doublezero-serviceability/src/state/accesspass.rs Add AccessPassKind and conversions for variant-only matching.
smartcontract/programs/doublezero-serviceability/src/error.rs Add AccessPassTypeMismatch error and wire-code mapping.
smartcontract/programs/doublezero-serviceability/src/instructions.rs Add new close/delete variants; make 42 and 69 payload-free deprecated; update decode/name/args tests.
smartcontract/programs/doublezero-serviceability/src/entrypoint.rs Route new variants to shared handlers with an expected kind; return Deprecated for 42 and 69.
smartcontract/programs/doublezero-serviceability/src/processors/accesspass/close.rs Enforce kind match and fix the prior try_borrow_data fall-through close behavior.
smartcontract/programs/doublezero-serviceability/src/processors/user/delete.rs Enforce kind match before deeper access pass validation and state mutation.
smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs Update comment text to reflect new Delete<Kind>User instruction naming.
smartcontract/programs/doublezero-serviceability/tests/accesspass_test.rs Migrate close tests to the correct per-kind close variants.
smartcontract/programs/doublezero-serviceability/tests/accesspass_allow_multiple_ip.rs Migrate user delete call to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/user_tests.rs Migrate delete calls to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/user_old_test.rs Migrate delete call to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs Migrate delete calls to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/delete_user_dynamic_accesspass.rs Migrate delete calls to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs Migrate delete call to DeletePrepaidUser.
smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs Add a shared helper to assert ProgramError::Custom matches a DoubleZeroError.
smartcontract/programs/doublezero-serviceability/tests/deprecated_removal_instructions_test.rs Add coverage that variants 42 and 69 return Deprecated.
smartcontract/programs/doublezero-serviceability/tests/close_access_pass_kind_test.rs Add integration coverage for close mismatch guard across all pass kinds.
smartcontract/programs/doublezero-serviceability/tests/delete_user_kind_test.rs Add integration coverage for delete mismatch guard across all pass kinds, including an EdgeSeat seat-release case.
crates/doublezero-serviceability-instruction/src/accesspass.rs Keep the deprecated close_access_pass builder compiling by emitting payload-free variant 69.
crates/doublezero-serviceability-instruction/src/user.rs Keep the deprecated delete_user builder compiling by emitting payload-free variant 42 and updating the builder test.
docs/superpowers/specs/2026-09-01-per-pass-type-removal-guard-design.md Add a design spec documenting the per-kind instruction strategy and rollout constraints.
Review details
  • Files reviewed: 21/21 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 155 to 156
if !accesspass_account.data_is_empty() {
// Read Access Pass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in cbd95b6. The guard is dead as you say, and it matters more than tidiness here: the new access pass kind check sits inside that block, so a reader could not see it was unconditional without proving the earlier AccessPassNotFound return 45 lines above, and a future edit to that return could silently make the kind check skippable. Removed the wrapper and unindented the body; every statement and its order are unchanged. The guard predates this branch (it is on main too), so this is a cleanup of code the change already touches.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b164d63. Configure here.

executable: false,
rent_epoch: 0,
},
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Close tests skip real pass creation

Low Severity

Issue
The close-kind suite writes a raw access pass into the test bank and funds the payer by hand. It never runs the set-pass instruction, so the kind check never runs against a pass the program created.

Context
An operator closes a pass that SetAccessPass wrote. This suite instead serializes AccessPass bytes, inserts that account before start(), and funds test_payer() so it can sign. The delete-kind suite in this same change already creates the pass through SetAccessPass after InitGlobalState.

Proposed Fix
Create each pass with SetAccessPass after InitGlobalState, and drop the hand-written account and the hand-funded payer.

Fix in Cursor Fix in Web

Triggered by project rule: Comment shape

Reviewed by Cursor Bugbot for commit b164d63. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in cbd95b6. The suite now runs InitGlobalState and creates each pass with SetAccessPass, matching the delete-kind suite, and the hand-written account and hand-funded payer are gone. SetAccessPass covers all five kinds, with AccessPassType::EdgeSeat(vec![]) for the EdgeSeat case, which is all the close path needs since closing does not read feed seats. The assertions are unchanged: per kind, a close for a different kind is refused with AccessPassTypeMismatch and the pass survives, then the matching close removes it.

.set_account(&user_pubkey, &AccountSharedData::from(user_account));

feed_key
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Delete test rewrites pass by hand

Low Severity

Issue
The EdgeSeat delete case writes pass and user bytes after create. It never runs subscribe, so the seat-release path never sees a seat the program granted.

Context
A matching DeleteEdgeSeatUser must release a feed seat that subscribe recorded. seed_feed_seat overwrites the pass type and the user's feed list in the bank after SetAccessPass and CreateUser. A feedless EdgeSeat pass makes seat release a no-op, which is why this case exists.

Proposed Fix
Provision the seat with SetAccessPassFeeds and SubscribeFeed after the user exists.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by project rule: Comment shape

Reviewed by Cursor Bugbot for commit b164d63. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Half right, and the half you found is the more useful one: the comment was false. It claimed seeding by hand was "the only way" to put a real seat in front of release_feed_seats, and it is not — SetAccessPassFeeds is the real path and three suites already use it. That is corrected in cbd95b6.

I kept the direct seeding, for a reason worth stating. SetAccessPassFeeds puts a seat on the pass but never ticks current_users and never touches a user. Recording the feed on a user, ticked, is done by CreateSubscribeUser or SubscribeFeed, and both need a real MulticastGroup with its own create instruction and ResourceExtension accounts, which this suite does not otherwise stand up. Standing that up to seed one seat would roughly double the file without changing what the delete path is tested against: the assertion is still full-struct equality showing the seat's current_users going 1 to 0 after DeleteEdgeSeatUser, so the release path is exercised either way.

The comment now names SetAccessPassFeeds, points at set_access_pass_feeds_test.rs, and says why this suite seeds directly, so the next reader can weigh it rather than take my word for it.

Remove a stale #[repr(u8)] from the AccessPassKind design-doc snippet (the
enum never has one, deliberately). Delete a dead data_is_empty() wrapper in
process_delete_user that made the new AccessPassTypeMismatch check look
conditional on an early return 45 lines above. Rewrite
close_access_pass_kind_test.rs to build its passes with SetAccessPass instead
of hand-inserting AccessPass accounts, matching its delete_user_kind_test.rs
sibling. Correct delete_user_kind_test.rs's seed_feed_seat comment, which
claimed direct seeding was the only way to put a real seat on a user:
SetAccessPassFeeds is the real provisioning path, but ticking a user's seat
for real needs CreateSubscribeUser/SubscribeFeed and a live MulticastGroup,
which this suite does not otherwise stand up.
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.

2 participants