feat: pbft to single sequencer & sequencer rotate#30
Conversation
…de height 1. Add continue FOR_LOOP after successful handleTheLastTMBlock to prevent fall-through to type assertion secondSync.(*types.Block) which panics when secondSync is *types.BlockV2. 2. Populate BlockID in nilCommit to prevent 'commit cannot be for nil block' panic on node restart when LoadSeenCommit validates the commit. 3. Use RedoRequest instead of PopRequest on error for proper retry handling.
… mode After upgrade, the SeenCommit for the last TM block has no valid signatures. Skip reconstructLastCommit and allow nil LastCommit in updateToState when already in sequencer mode, since consensus won't run.
…eMsg Reject V1 BlockResponse at/after upgrade height and V2 BlockResponseV2 before upgrade height. This prevents incorrect block types from entering the pool and causing type assertion panics in poolRoutine.
| seqR, ok := bcR.Switch.Reactor("SEQUENCER").(sequencerReactor) | ||
| if ok { | ||
| if err := seqR.StartSequencerRoutines(); err != nil { | ||
| bcR.Logger.Error("Failed to start sequencer mode", "err", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
The general fix is to ensure that we do not log error values that might contain sensitive data (like passwords) in clear text. Instead, we should log a high-level, non‑sensitive message and, if necessary, a sanitized or redacted representation of the error. In this case the taint reaches blocksync/reactor.go where err from seqR.StartSequencerRoutines() is logged directly. The minimal, behavior‑preserving fix is to change this log call so it does not output the raw err value, while still indicating that a failure occurred. Since we cannot see or change the inner implementations that might be embedding the password into the error string, the safest step in this snippet is to stop including the error object in the log at this point.
Concretely, in blocksync/reactor.go, around line 473–475, change:
if err := seqR.StartSequencerRoutines(); err != nil {
bcR.Logger.Error("Failed to start sequencer mode", "err", err)
}to a form that does not log err at all, for example:
if err := seqR.StartSequencerRoutines(); err != nil {
bcR.Logger.Error("Failed to start sequencer mode")
}This keeps existing control flow and semantics (we still detect and branch on the error), but prevents any tainted data that might be embedded in err.Error() from reaching the logs. No new imports or helpers are required, and the change is limited to the provided blocksync/reactor.go snippet as requested.
| @@ -471,7 +471,8 @@ | ||
| seqR, ok := bcR.Switch.Reactor("SEQUENCER").(sequencerReactor) | ||
| if ok { | ||
| if err := seqR.StartSequencerRoutines(); err != nil { | ||
| bcR.Logger.Error("Failed to start sequencer mode", "err", err) | ||
| // Do not log the raw error to avoid leaking potentially sensitive data. | ||
| bcR.Logger.Error("Failed to start sequencer mode") | ||
| } | ||
| } | ||
| } else { |
| if seqR, ok := bcR.(*bc.Reactor); ok { | ||
| if bbR, ok := seqR.Switch.Reactor("SEQUENCER").(interface{ StartSequencerRoutines() error }); ok { | ||
| if err := bbR.StartSequencerRoutines(); err != nil { | ||
| ssR.Logger.Error("Failed to start sequencer routines", "err", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
To fix the problem, we should stop logging potentially sensitive error messages that may include credentials, or sanitize the logged data before it is emitted. In this specific case, the problematic sink is the logging of err from bbR.StartSequencerRoutines() in node/node.go. The simplest, least invasive fix that preserves general behavior is to adjust that log statement so it does not include the error value, but still logs that starting sequencer routines failed. This avoids clear‑text leakage while keeping a useful operational log.
Concretely, in node/node.go, within the startStateSync helper (the snippet around lines 756–763), we will modify the inner block handling bbR.StartSequencerRoutines() so that on error it logs a generic message without the err field. We will keep returning nil from the goroutine as before, so control flow and external behavior are unchanged; only the log content is reduced. No new imports or helper functions are required: we will just remove the "err", err key/value pair from that particular Logger.Error call.
| @@ -759,7 +759,8 @@ | ||
| if seqR, ok := bcR.(*bc.Reactor); ok { | ||
| if bbR, ok := seqR.Switch.Reactor("SEQUENCER").(interface{ StartSequencerRoutines() error }); ok { | ||
| if err := bbR.StartSequencerRoutines(); err != nil { | ||
| ssR.Logger.Error("Failed to start sequencer routines", "err", err) | ||
| // Do not log the error value directly to avoid leaking sensitive information. | ||
| ssR.Logger.Error("Failed to start sequencer routines") | ||
| } | ||
| } | ||
| } |
PR checklist
CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed