feat(registrar): hinted nitro registration on pre-dynamic nitro-host - #4610
Draft
mw2000 wants to merge 12 commits into
Draft
feat(registrar): hinted nitro registration on pre-dynamic nitro-host#4610mw2000 wants to merge 12 commits into
mw2000 wants to merge 12 commits into
Conversation
Co-authored-by: Codex <codex-noreply@coinbase.com> (cherry picked from commit 573f769)
Co-authored-by: Codex <codex-noreply@coinbase.com> (cherry picked from commit ac1c815)
* fix(registrar): prevent processing unhealthy instances Co-authored-by: Codex <codex-noreply@coinbase.com> * fix(registrar): treat unhealthy targets as absent Co-authored-by: Codex <codex-noreply@coinbase.com> * fix(registrar): remove unreachable cache branch Co-authored-by: Codex <codex-noreply@coinbase.com> * fix(registrar): preserve unhealthy signer cache Co-authored-by: Codex <codex-noreply@coinbase.com> * fix(registrar): preserve cold-start unhealthy signer grace Co-authored-by: Codex <codex-noreply@coinbase.com> * docs(registrar): clarify unhealthy signer grace Co-authored-by: Codex <codex-noreply@coinbase.com> --------- Co-authored-by: Codex <codex-noreply@coinbase.com> (cherry picked from commit 1345ec1)
…4164) * fix(registrar): probe readyz to break registration health deadlock ALB /healthz is registration-gated on nitro-host, so the registrar now overlays discovery health with a direct readyz probe before registering. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(registrar): streamline readiness probing Avoid an intermediate readiness snapshot and stop pending probes promptly during shutdown. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(registrar): simplify readiness probing Remove redundant readiness state handling and polling noise while retaining wire-level coverage for the host readiness endpoint. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): pipeline readiness probes with resolution Allow healthy prover instances to resolve while another readiness request waits for its timeout. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(registrar): simplify readiness probe test state Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 7ba1984)
* feat(registrar): add Rust Nitro attestation planner (CHAIN-4835) Build CertManager-oriented registration plans from Nitro attestations via AttestationReport, without wiring into the Boundless path yet. Co-authored-by: Cursor <cursoragent@cursor.com> Co-Authored-By: Claude <noreply@anthropic.com> * fix(registrar): address planner review feedback (CHAIN-4835) Parse each cert once for cache/revocation keys, preserve typed Parse/Certificate error sources, and cover full plan construction against the Nitro fixture. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): align attestation planner with NitroValidator (CHAIN-4835) Parse COSE with raw protected/payload TLVs, enforce CertManager DER/v3 rules, and pin Base fixture plan outputs so hinted registration cannot diverge from onchain acceptance. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): tighten Nitro COSE envelope checks (CHAIN-4835) Require an empty unprotected header, reject indefinite COSE arrays with a clear error, and avoid silent u64-to-usize length truncation. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): address Nitro COSE planner review feedback (CHAIN-4835) Add CBOR/COSE rejection unit tests, keep PCR/cabundle validators internal, and document intentional dual validation plus exact-fit DER TLV preconditions. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): reject duplicate payload keys and oversized CBOR containers (CHAIN-4835) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): pin COSE/cert parsing to nitro-validator main (CHAIN-4835) Agora's Go planner still encodes older contract rules (contiguous PCRs, empty unprotected maps). Match current NitroValidator/CertManager: compact 0xD2/0x84 envelopes, skip any unprotected map, sparse PCR bank, indefinite pcrs/cabundle, recognized-key duplicates only, and exact version-wrapper consume. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): use b256! for pinned root and document serial hashing (CHAIN-4835) Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 2de80c2)
* feat(registrar): add hinted contract clients (CHAIN-4837) Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * fix(registrar): remove legacy CRL compatibility Co-authored-by: OpenCode <opencode-noreply@coinbase.com> --------- Co-authored-by: OpenCode <opencode-noreply@coinbase.com> (cherry picked from commit 054a3bd)
#4376) * feat(registrar): port Agora P-384 inverse-hint generation (CHAIN-4830) Add an offchain transcript collector that walks the onchain P-384 ECDSA affine schedule and emits packed 48-byte BE inverses for CA/leaf cert signatures and the attestation COSE sig. Exposed via P384Hints and AttestationPlanner::prepare_hinted_registration_plan, but left unused by the Boundless registrar path until hinted orchestration lands. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): address P-384 hints review feedback (CHAIN-4830) Share one parse path for registration and hinted plans, fold the inverse collector into P384Hints, and cover prepare_hinted_registration_plan against for_registration_plan on the Base Nitro fixture. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): clarify P-384 pubkey width and reuse s⁻¹ (CHAIN-4830) Separate the affine pubkey length constant from the signature encoding and record the scalar inverse once while still appending it twice for transcript parity with the onchain verifier. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): match onchain P-384 infinity and drop reserialized TBS Treat x==0 as infinity in affine helpers and generate attestation hints only via raw plan TBS. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): silence needless_return in P-384 add_affine Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): keep twice3 going through intermediate x==0 Solidity only treats x==0 as infinity on twice3 entry; reusing twice_affine aborted early. Co-authored-by: Cursor <cursoragent@cursor.com> * style(registrar): rustfmt twice3 intermediate-x-zero test Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 9260231)
* feat(registrar): cut over to hinted registration (CHAIN-4834) Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * fix(registrar): address hinted registration review Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * fix(registrar): distinguish empty contract reverts Co-authored-by: OpenCode <opencode-noreply@coinbase.com> --------- Co-authored-by: OpenCode <opencode-noreply@coinbase.com> (cherry picked from commit 09104a6)
* refactor(registrar): remove Boundless deployment requirements Co-authored-by: OpenCode <opencode-noreply@coinbase.com> * style(registrar): format metrics feature list Co-authored-by: OpenCode <opencode-noreply@coinbase.com> --------- Co-authored-by: OpenCode <opencode-noreply@coinbase.com> (cherry picked from commit 771af22)
Co-authored-by: OpenCode <opencode-noreply@coinbase.com> (cherry picked from commit 1469893)
* feat(registrar): add hinted registration metrics (CHAIN-4841) Emit bounded-label metrics for hint generation, hint sizes, certificate-cache lookups/transactions, recovery, and final registration so operators can see the hinted path without Boundless-era proof stages. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): record hinted final-registration outcomes only on that path Keep stale and observed-registered counts on the final registration attempt so cache/plan freshness checks and recovery rereads do not skew the metric. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): close hinted metric gaps after submit Record a failed cache-tx outcome when the receipt succeeds but the certificate is still unusable, and a cancelled final-registration outcome when shutdown lands after send. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): drop overlapping hinted registration metrics Keep hint sizes and cert-cache series; reuse registration_stage_total for hint generation and final registration instead of parallel counters. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): close remaining cert-cache tx metric exits Record cancelled when the post-send cache reread is cancelled, and failed when that reread returns a permanent cert error, so submitted stays reconcilable. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(registrar): derive cache-tx and stale metrics from outcomes, not exits Recording metrics as side effects at each control-flow exit is why the submitted/terminal counts kept drifting: `ensure_cert_cached` had eight returns that each had to remember a terminal, and `proof_stale` lived in the shared freshness check, so it also fired once per revoked certificate on the CRL path. One cache attempt now returns a CacheTxAttempt that the loop records once, and proof_stale is classified at the registration-attempt boundary. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit ebb0946)
The CHAIN-4834 error.rs tests need serde_json; this older nitro-host branch never added that contracts dev-dependency. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| *keccak256(input) | ||
| } | ||
|
|
||
| fn cert_lock(&self, cert_hash: B256) -> Arc<AsyncMutex<()>> { | ||
| let mut locks = self.cert_locks.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); |
Contributor
There was a problem hiding this comment.
unwrap_or_else(|poisoned| poisoned.into_inner()) silently swallows mutex poisoning throughout (cert_lock, and cert_locks lock). If the previous holder panicked, the HashMap invariants might be violated (e.g. a half-inserted entry). This is a defensive choice that keeps the system running, but consider at minimum logging a warning when the lock is poisoned so operators get visibility into the root-cause panic.
| @@ -188,15 +347,14 @@ impl ProofTaskSet { | |||
| RegistrarMetrics::PROOF_TASK_OUTCOME_JOIN_ERROR, | |||
| ); | |||
| let id = join_err.id(); | |||
| let removed = self.pending.extract_if(|_, p| p.task_id == id).next(); | |||
| let signer = removed.as_ref().map(|(signer, _)| *signer); | |||
| let removed = self.pending.extract_if(|_, pending| pending.task_id == id).next(); | |||
Contributor
There was a problem hiding this comment.
extract_if is a nightly-only API (#![feature(hash_extract_if)]). Depending on the project's MSRV and toolchain policy, this may break on stable Rust. If the project pins nightly, disregard — otherwise this should use .retain() plus a separate lookup, or iterate/collect/remove manually.
| if revoked.is_empty() { | ||
| return Ok(true); | ||
| } | ||
| RegistrarMetrics::crl_revocations_detected().increment(revoked.len() as u64); |
Contributor
There was a problem hiding this comment.
The count of revoked items is cast via as u64. revoked is a Vec<&CertCrlInfo> bounded by cert_infos, which itself is bounded by plan.certs (at most ~32 certificates). So the cast is safe here, but if this pattern appears in hotter paths consider using .try_into().unwrap_or(u64::MAX) for consistency with how other counters are handled.
| } | ||
| } else { | ||
| // Each array item is at least one byte. | ||
| let remaining = bytes.len().saturating_sub(end) as u64; |
Contributor
There was a problem hiding this comment.
Minor: remaining is computed as bytes.len().saturating_sub(end) as u64. If bytes.len() is equal to end (empty remaining), this correctly yields 0. However, the as u64 cast on usize is fine on 64-bit but on a hypothetical 128-bit platform would truncate. Low concern for this codebase, just noting for completeness.
| } | ||
| unreachable!("bounded revocation retry loop must return") |
Contributor
There was a problem hiding this comment.
The unreachable! at the end of submit_revocation's retry loop is correct when max_tx_retries >= 0 (which it always is as a u32), since the loop runs 0..=max_tx_retries and the final iteration either returns on non-retryable or when retry == self.max_tx_retries. Same logic applies to lines 784 and 1032.
However, consider whether a debug assertion or a more descriptive panic message (e.g., including the retry count) would help if this invariant were ever violated due to a future refactor.
Contributor
Review SummaryLarge PR introducing the FindingsMinor / Informational:
Note: I posted an inline comment about Architecture
No blocking issues found. |
Contributor
❌ base-std fork tests: 250 failed, 596 passedbase/base diverges from the base-std spec.
Failing tests
|
mw2000
marked this pull request as draft
August 21, 2026 18:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
feat/pre-dynamic-upgrade-nitro-host-newso Zeronet/pre-dynamic nitro-host can register without Boundless/RISC Zero.#4373/ CHAIN-4835), contract clients (#4406), P-384 inverse hints (#4376/ CHAIN-4830), hinted cutover (#4425/ CHAIN-4834), Boundless removal (#4449), legacy attestation-prover removal (#4477), and hinted metrics (#4480/ CHAIN-4841).#4042,#4067,#4141,#4164) plus aserde_jsoncontracts test dep the cutover tests require here.Test plan
cargo check -p base-proof-tee-registrar -p base-proof-tee-registrar-bin -p base-proof-contracts --all-targetscargo test -p base-proof-tee-registrar -p base-proof-tee-registrar-bin -p base-proof-contracts --libMade with Cursor