Skip to content

refactor(registrar): simplify registration manager dedupe - #3502

Merged
jackchuma merged 2 commits into
mainfrom
refactor/registration-manager-dedupe
Jun 12, 2026
Merged

refactor(registrar): simplify registration manager dedupe#3502
jackchuma merged 2 commits into
mainfrom
refactor/registration-manager-dedupe

Conversation

@jackchuma

@jackchuma jackchuma commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What changed? Why?

RegistrationManager no longer owns a process-local in-flight signer set; it now focuses on the single-signer registration path: checking whether the signer still needs registration, generating or recovering the proof, and handing the result to ProofHandler.

This also removes the old in-flight guard export and updates the registrar driver glue that constructed RegistrationManager.

Notes to reviewers

Task-level deduplication remains owned by the registrar driver pending task tracking.

How has it been tested?

  • cargo fmt
  • cargo test -p base-proof-tee-registrar

Co-authored-by: Codex <codex-noreply@coinbase.com>
@cb-heimdall

cb-heimdall commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@jackchuma
jackchuma marked this pull request as ready for review June 12, 2026 18:48
@jackchuma
jackchuma enabled auto-merge June 12, 2026 18:48
Co-authored-by: Codex <codex-noreply@coinbase.com>
@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

No issues found. This is a clean refactor that correctly removes redundant deduplication from RegistrationManager.

What was removed:

  • in_flight_registrations: Arc<Mutex<HashSet<Address>>> from RegistrationDriver and the corresponding &Arc<Mutex<HashSet<Address>>> borrow in RegistrationManager
  • InFlightRegistrationGuard RAII type and its try_acquire/Drop impl
  • prepare_registration_attempt (replaced by signer_needs_registration which returns bool instead of Option<Guard>)
  • invoke_proof_handler (inlined at the single call site)
  • Two tests that specifically exercised the mutex-based deduplication

Why it's safe: The removed mutex was defence-in-depth. The driver's reconcile_proof_tasks already provides single-threaded deduplication via its local in_flight: HashSet<Address> built each cycle, and apply_join_outcome's task-id guard prevents stale joins from evicting fresh pending entries. Since reconcile_proof_tasks is the sole spawn path and runs synchronously within the run loop, no concurrent duplicate can slip through.

The updated doc comments accurately describe the new deduplication invariants.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

✅ base-std fork tests: all 616 passed

base/base is fully in sync with the base-std spec.

Dependency Ref Commit
base-std main e25553dc
base-anvil 0092692587d8d064dd2c6923ce26a682c58f3694 00926925

@mw2000 mw2000 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.

🧹 clean

@jackchuma
jackchuma added this pull request to the merge queue Jun 12, 2026
Merged via the queue into main with commit 0e85138 Jun 12, 2026
24 checks passed
@jackchuma
jackchuma deleted the refactor/registration-manager-dedupe branch June 12, 2026 19:38
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.

3 participants