serviceability: prefer the accesspass that clears the epoch check - #4327
ayushsingh82 wants to merge 1 commit into
Conversation
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.
|
@elitegreg @juan-malbeclabs could you take a look when you have a chance? |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ 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, |
There was a problem hiding this comment.
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)
Reviewed by Cursor Bugbot for commit 40e905e. Configure here.


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 ibrlresolves to the stale dynamic pass, thelast_access_epoch >= epochcheck fails, and it printsUnable to find a valid AccessPass— even thoughprocessors/user/create_core.rswould 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 itslast_access_epochcurrent.Closes #4244.
Fix
New
GetAccessPassCommand::execute_usable(client, user_type)evaluates both the dynamic and exact-IP candidates againstepoch_allows_connection— the same check the on-chain program enforces — and prefers whichever one clears it, falling back to the dynamic pass (matchingexecute'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.executeitself, and its existing test coverage, are untouched — every other caller ofGetAccessPassCommandis unaffected.CreateUserCommandandCreateSubscribeUserCommand(the SDK commands that actually build thecreate_user/create_subscribe_usertransaction) switch toexecute_usable. So doesdoublezero 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_accesspassgained auser_typeparameter, threaded throughcheck_accesspass/require_accesspassincrates/doublezero-daemon-cli/src/connect.rs, so the baredoublezero connect's own epoch re-check (which reads the sameAccessPassrequire_accesspassreturns) sees the same pass the IBRL leg's transaction will use.Testing Verification
smartcontract/sdk/rs/src/commands/accesspass/get.rs: 4 new tests forexecute_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 unmockedget_epoch()otherwise).cargo test -p doublezero_sdk— 216/216 passing, including all pre-existingexecute()coverage unmodified.cargo test -p doublezero-daemon-cli— 223/223 passing (coversconnect.rs's dispatch, bare-connect, and preflight paths through the updatedLedgerClienttrait).cargo test -p doublezero-serviceability-cli— 437/437 passing (covers theCliCommand::get_accesspass_usableaddition).cargo clippy --all-targets -- -Dclippy::all -Dwarningsclean on every touched crate (doublezero_sdk,doublezero-daemon-cli,doublezero-serviceability-cli,doublezero).