Skip to content

fix: address PR #21823 review comments (promiseWithResolvers, listener cleanup)#21898

Closed
AztecBot wants to merge 1 commit into
nextfrom
claudebox/fix-pr-21823-review-comments
Closed

fix: address PR #21823 review comments (promiseWithResolvers, listener cleanup)#21898
AztecBot wants to merge 1 commit into
nextfrom
claudebox/fix-pr-21823-review-comments

Conversation

@AztecBot

Copy link
Copy Markdown
Collaborator

Summary

Addresses post-merge review comments from #21823:

  • Use promiseWithResolvers from foundation instead of manual Promise constructor in BatchChonkVerifier.enqueueProof (alexghr)
  • Clean up stream event listeners in FifoFrameReader.stop() to prevent memory leaks (alexghr)
  • Added TODO for ring buffer optimization in FifoFrameReader (alexghr said "fine for now")

Other comments were already addressed (concurrency default=6) or no longer applicable (pinned hash removed).

Full analysis: https://gist.github.com/AztecBot/b3c5afd7ae842758e3039ee5a8c342b6

Test plan

  • Verify yarn build compiles
  • Existing batch chonk verifier tests pass
  • FifoFrameReader tests pass

ClaudeBox log: https://claudebox.work/s/820ad2fe44664e4c?run=2

@AztecBot AztecBot added ci-draft Run CI on draft PRs. claudebox Owned by claudebox. it can push to this PR. labels Mar 23, 2026
@ludamad ludamad self-assigned this Mar 24, 2026
@AztecBot

Copy link
Copy Markdown
Collaborator Author

Automatically closing this stale claudebox draft PR (no updates for 5+ days). Re-open if still needed.

@AztecBot AztecBot closed this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on draft PRs. claudebox Owned by claudebox. it can push to this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants