Skip to content

Comments

feat: support standard rpc getprogramaccounts for compression#2304

Open
sergeytimoshin wants to merge 1 commit intosergey/photon-combined-discriminator-datafrom
sergey/forester-programsaccs-errors
Open

feat: support standard rpc getprogramaccounts for compression#2304
sergeytimoshin wants to merge 1 commit intosergey/photon-combined-discriminator-datafrom
sergey/forester-programsaccs-errors

Conversation

@sergeytimoshin
Copy link
Contributor

@sergeytimoshin sergeytimoshin commented Feb 20, 2026

No description provided.

feat: add support for getProgramAccounts standard rpc calls for compression
feat: structured error logging
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • main
  • release/*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sergey/forester-programsaccs-errors

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ananas-block
Copy link
Contributor

Logic Review: get_or_create_state_processor / get_or_create_address_processor (automated)

Three issues found in the new per-tree init-lock design. The design itself is correct and an improvement over the old Entry API pattern — no data corruption or immediate deadlocks.


F-01 (Low) — Epoch consistency window: map updated before processor

In the epoch-transition branch, the DashMap is updated to the new epoch before update_epoch() is called on the processor:

// line ~2825: map now advertises epoch 6
self.state_processors.insert(tree_accounts.merkle_tree, (epoch_info.epoch, processor_clone.clone()));
// processor internal state still epoch 5 here
processor_clone.lock().await.update_epoch(epoch_info.epoch, epoch_info.phases.clone());

Any concurrent caller that holds the processor Arc (obtained from a prior get_or_create call) can call process() during this window with stale phase boundaries while the map already shows the new epoch. The init lock does not protect this: outside callers hold the processor lock, not the init lock.

Fix: swap the order — update the processor first, then insert into the map.


F-02 (Medium) — state_processors.remove() in error handler bypasses init lock

The constraint-error handler removes the processor from the map without acquiring the init lock:

self.state_processors.remove(&tree_accounts.merkle_tree);  // no init lock held

Concurrent scenario:

  1. Task A holds a processor Arc (obtained via get_or_create under init lock, already returned)
  2. Task B (error handler) calls remove() — map entry gone
  3. Task C calls get_or_create, finds nothing, creates a new processor and inserts it
  4. Task A and Task C now hold separate QueueProcessor instances for the same tree and both call process(), risking duplicate ZKP submissions for the same queue items

Either acquire the init lock in the error handler before removing, or add an explicit comment that this is intentional best-effort invalidation and document the bounded duplicate-submission risk.


F-03 (Medium) — Latent deadlock: processor lock held across prewarm_from_indexer + epoch transition

The pre-warm path acquires the processor lock and holds it across a long async operation:

let mut p = processor.lock().await;          // processor lock acquired
p.prewarm_from_indexer(cache, ...).await     // long async hold

Meanwhile, the epoch-transition branch inside get_or_create acquires the processor lock while already holding the init lock:

let _init_guard = init_lock.lock().await;    // init lock held
// ...
processor_clone.lock().await.update_epoch(...)  // tries to acquire processor lock

If any future code path causes get_or_create to be called for the same tree while the processor lock is held in pre-warm (e.g., via scheduling or recursive task dispatch during an epoch boundary), it deadlocks. No such recursion exists today but the pattern is one refactor away and the lock-hold window during prewarm_from_indexer is wide.

Suggested guard comment at the top of get_or_create_state_processor / get_or_create_address_processor:

// SAFETY: Callers must not hold this tree's processor Mutex when calling this function.
// The epoch-transition path acquires the processor lock internally. Holding it on entry
// will deadlock for same-tree epoch transitions.

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