Skip to content

serviceability: prefer the accesspass that clears the epoch check - #4327

Open
ayushsingh82 wants to merge 1 commit into
malbeclabs:mainfrom
ayushsingh82:fix/4244-accesspass-usable-pass
Open

ayushsingh82 wants to merge 1 commit into
malbeclabs:mainfrom
ayushsingh82:fix/4244-accesspass-usable-pass

Conversation

@ayushsingh82

Copy link
Copy Markdown
Contributor

Problem

GetAccessPassCommand::execute (get.rs:26-38) returns the dynamic (0.0.0.0) access pass whenever that account exists, and falls back to the exact-IP PDA only when it is absent — it never checks whether the pass it returns is usable.

A payer holding an epoch-stale dynamic pass and a valid prepaid pass at their exact IP gets turned away: doublezero connect ibrl resolves to the stale dynamic pass, the last_access_epoch >= epoch check fails, and it prints Unable to find a valid AccessPass — even though processors/user/create_core.rs would have accepted either PDA and taken the exact-IP one. An EdgeSeat pass is a realistic source of the stale dynamic half, since multicast is never epoch-gated (UserType::is_epoch_gated), so nothing keeps its last_access_epoch current.

Closes #4244.

Fix

New GetAccessPassCommand::execute_usable(client, user_type) evaluates both the dynamic and exact-IP candidates against epoch_allows_connection — the same check the on-chain program enforces — and prefers whichever one clears it, falling back to the dynamic pass (matching execute's default) only when neither does. It only reads the current epoch when both candidates exist, so it's a no-op cost change for the common single-candidate case. execute itself, and its existing test coverage, are untouched — every other caller of GetAccessPassCommand is unaffected.

CreateUserCommand and CreateSubscribeUserCommand (the SDK commands that actually build the create_user/create_subscribe_user transaction) switch to execute_usable. So does doublezero connect's own preflight: per the issue, "whatever resolution a pre-flight trusts has to be the resolution that picks the PDA the transaction sends" — LedgerClient::get_accesspass gained a user_type parameter, threaded through check_accesspass/require_accesspass in crates/doublezero-daemon-cli/src/connect.rs, so the bare doublezero connect's own epoch re-check (which reads the same AccessPass require_accesspass returns) sees the same pass the IBRL leg's transaction will use.

Testing Verification

  • smartcontract/sdk/rs/src/commands/accesspass/get.rs: 4 new tests for execute_usable — prefers the exact-IP pass when the dynamic one is epoch-stale (the reported bug), prefers dynamic when both are usable, falls back to dynamic when neither clears the epoch check, and confirms no epoch read happens when only one candidate exists (would panic on the unmocked get_epoch() otherwise). cargo test -p doublezero_sdk — 216/216 passing, including all pre-existing execute() coverage unmodified.
  • cargo test -p doublezero-daemon-cli — 223/223 passing (covers connect.rs's dispatch, bare-connect, and preflight paths through the updated LedgerClient trait).
  • cargo test -p doublezero-serviceability-cli — 437/437 passing (covers the CliCommand::get_accesspass_usable addition).
  • cargo clippy --all-targets -- -Dclippy::all -Dwarnings clean on every touched crate (doublezero_sdk, doublezero-daemon-cli, doublezero-serviceability-cli, doublezero).

GetAccessPassCommand::execute always returns the dynamic (0.0.0.0)
access pass over the exact-IP one when both exist, without checking
whether the dynamic one is actually usable. A dynamic pass left over
from a never-epoch-gated EdgeSeat multicast subscription can go stale
while a valid exact-IP prepaid pass sits unused at the same address, so
`doublezero connect ibrl` picks the stale pass and fails with "Unable
to find a valid AccessPass" even though create_core.rs would have
accepted the exact-IP one.

New execute_usable(client, user_type) evaluates both candidates against
the same epoch_allows_connection check the program enforces, and only
reads the epoch when both candidates exist (existing execute() callers
and its test coverage are unchanged). CreateUserCommand and
CreateSubscribeUserCommand switch to it, and so does doublezero
connect's preflight (LedgerClient::get_accesspass gains a user_type
parameter) so the check and the transaction it gates agree on the same
pass.

Closes malbeclabs#4244.
@ayushsingh82
ayushsingh82 requested a review from a team September 13, 2026 10:05
@ayushsingh82

Copy link
Copy Markdown
Contributor Author

@elitegreg @juan-malbeclabs could you take a look when you have a chance?

@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 1 potential issue.

Fix All in Cursor

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

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

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

Reviewed by Cursor Bugbot for commit 40e905e. Configure here.

ledger,
daemon,
user_type,
ibrl_user_type,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bare connect gates multicast on IBRL pass

Medium Severity

Bare doublezero connect resolves the preflight pass with the IBRL user_type, then uses that same account for the multicast EdgeSeat seat-cap skip. The multicast leg independently resolves with UserType::Multicast, which still treats a stale dynamic pass as usable. When the two PDAs differ, multicast can be skipped even though a usable pass exists, or attempted against a pass whose seats were never checked.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 40e905e. Configure here.

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.

sdk: GetAccessPassCommand should prefer a usable pass, not just the dynamic one

1 participant