Repository navigation
feat(beacon): log a Glamsterdam banner after the first gloas block - #666
MegaRedHand wants to merge 3 commits into
Conversation
Marks the fork in the node's own log at the moment it goes live, as ethrex does for amsterdam (lambdaclass/ethrex#7341). The banner follows the import of the chain's first gloas block: the first block gloas applies to while it did not apply to its parent, so missed slots at the start of the fork epoch still leave one. The decision settles on the first gloas block this process imports, banner or not. Blocks import parent-first, so a node that sees the chain's first gloas block sees it before any other, and a node started after the fork would otherwise pay a whole-block parent decode on every import for nothing. The art is a placeholder: a polar bear in sunglasses over the wordmark, plain ASCII so it needs no locale fallback.
🤖 Kimi Code ReviewI'll review this PR for the Glamsterdam banner feature, focusing on correctness, security, performance, and Rust best practices. Overall AssessmentThis is a well-crafted, low-risk feature. The code is correct, safe, and thoughtfully optimized. The lazy evaluation of Detailed Review
|
| Priority | Location | Suggestion |
|---|---|---|
| Low | glamsterdam.rs:82 |
Document why has_payload_envelopes() is the proxy for Gloas detection |
| Low | lib.rs:572 |
Rephrase doc comment to match type (GlamsterdamBanner, not bool) |
| Very Low | glamsterdam.rs:88 |
Consider tracing::info!(target: "glamsterdam", ...) for filterable logs |
Conclusion
Approve with minor nits. The code is correct, safe, performant, and well-tested. The lazy evaluation pattern and state machine design are exemplary. Fix the doc comment wording if convenient; otherwise merge as-is.
Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt
🤖 Claude Code ReviewReview of PR 666: Glamsterdam bannerOverall this is clean and small, and it is well covered by tests. The "settle on the first gloas block" design keeps the per-import cost to a single bool check after the first gloas block. I found no correctness or security problems, and the change doesn't touch consensus logic. I did not build or run the tests. Minor points, none blocking:
LGTM. Automated review by Claude (Anthropic) · sonnet · custom prompt |
🤖 Codex Code Review
Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Replaces the placeholder art with a polar bear in the startup logo's style: block characters over a wordmark plate like the logo's own. The banner is no longer plain ASCII, so a terminal that cannot decode UTF-8 shows it garbled, as it would the logo without its ASCII fallback.
The polar bear now stands over both plates, ethlambda's above Glamsterdam's, so the banner says which client crossed the fork.
…3-636-638-gloas-live One conflict, in `BlockChainServer::for_replay`'s struct literal: tmp moved `store` last because `ChainEvents::new` borrows it first. Kept both, with `glamsterdam_banner` before `store`.
Summary
Logs a Glamsterdam banner when the fork actually goes live, following lambdaclass/ethrex#7341:
Block imported successfullyline.crates/blockchain/assets/glamsterdam_banner.txt. Like the logo it uses block characters, but it has no ASCII fallback: a terminal that cannot decode UTF-8 shows it garbled. Each line is logged separately, so every line carries the log prefix.It is followed by a structured line:
Glamsterdam is live: imported the first gloas block slot=… epoch=… block_root=….Verification
glamsterdam::tests(7 tests: crossing the fork, missed slots, pre-fork blocks neither log nor settle, once per process, node started after the fork, parent block not on record, no gloas scheduled) all pass.cargo test -p ethlambda-blockchain --profile release-fast --lib --bins: 243 passed.make lintis clean.