Skip to content

fix(attestor): say 'not registered' instead of 'already registered' when status is None - #1356

Open
zhenek73 wants to merge 2 commits into
gluwa:usc-devfrom
zhenek73:fix/attestor-not-registered-log
Open

zhenek73 wants to merge 2 commits into
gluwa:usc-devfrom
zhenek73:fix/attestor-not-registered-log

Conversation

@zhenek73

Copy link
Copy Markdown

What's wrong

register_bls collapses two different situations into one message:

let status = cc3.get_attestor_status(chain_key).await?;
if status != Some(AttestorStatus::Idle) {
    tracing::info!(?status, %account_id, "ℹ️ skipping attest() — already registered");
    return Ok(());
}
  • Some(Waiting) / Some(Active) — the attestor really is already registered, and the message is right.
  • None — the attestor is not in the pool at all, and the message says exactly the opposite of the truth.

The behaviour is correct either wayattest() is only callable from Idle, and an account reaches Idle only through register_attestor, which a stash must call; an attestor cannot enrol itself. So skipping is right. Only the diagnosis is wrong.

Why it is worth fixing

Bringing an attestor up against a new source chain produces a log that looks entirely healthy:

🔍 balance ok account_id=5FHneW46… balance=1000000000000000000000000
ℹ️ skipping attest() — already registered status=None account_id=5FHneW46…
⏲️ waiting on election...
⏲️ waiting on election...

Balance fine, "already registered", waiting for an election — nothing suggests a problem. In reality the attestor is not registered, no election will ever seat it, and it will wait forever. status=None is printed, but it reads as a detail rather than a contradiction of the sentence next to it.

This cost us the better part of a day while integrating a new source chain, and it is the kind of thing that costs every operator the same day once.

What this changes

Replaces the equality check with a match that names the None case honestly, warns instead of informing, and says what to do about it:

⚠️ not registered as an attestor for this chain — skipping attest(). A stash account
   must call register_attestor(chain_key, attestor_id) first, using an account other
   than the attestor itself.

No behaviour change: every branch still returns Ok(()) exactly as before, and the Idle path is untouched. One file, logging only.

Context

Found while bringing up an attestor for exSat's EVM layer (chain id 7200) as part of a BUIDL CTC 2026 Fall project. A separate, more serious finding from the same work — the attestor cannot follow any chain that does not maintain a receipts trie, and reports that as a reorg — is filed as #1355.

@cursor

cursor Bot commented Sep 11, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Logging-only change in attestor startup with no change to registration or attestation behavior.

Overview
Fixes misleading startup logs when an attestor account is not in the chain’s attestor pool at all.

register_bls used to treat any non-Idle status (including None) as “already registered” and log at info level. The diff splits that into a match: None now emits a warning that the account is not registered for that chain, that attest() is skipped for that reason, and that a stash must call register_attestor first; non-Idle statuses like Waiting/Active still log the existing “already registered” info message. Control flow is unchanged—every skip path still returns Ok(()), and the Idle path that submits attest() is untouched.

Reviewed by Cursor Bugbot for commit 93d6375. Bugbot is set up for automated code reviews on this repo. Configure here.

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