fix(l1): keep global_asm! blocks from leaking their section state

[MegaRedHand]

Motivation

keccak1600-x86_64.s ends with a .section .note.gnu.property block carrying the CET
feature flags, and never switches back to .text. Blocks emitted by global_asm! share the
assembler's section state within a codegen unit, so whichever block rustc places after it keeps
emitting into that note section.

On Rust 1.99-beta that next block is the blake2f AVX2 implementation, so _blake2b_f is
defined inside a non-executable NOTE section. The link then fails:

rust-lld: error: ethrex_crypto-<hash>.ethrex_crypto.<hash>-cgu.0.rcgu.o:(.note.gnu.property+0x20): data is too short

(lld parses .note.gnu.property to compute the output's GNU property bits, and walks off the
end of what it thinks is a note once ~5 KiB of blake2f code is sitting in there.)

As @cuviper points out in #7246, this is not really a compiler regression: nothing guarantees
an ordering between global_asm! blocks, and the old ordering working was luck. Confirmed:
on stable rustc emits the blake2f block first, which is the only reason the stray section
state never reached it.

This affects release builds specifically. [profile.release] sets codegen-units = 1,
which is what puts both blocks in the same codegen unit; the default multi-CGU split used by
debug and cargo test builds happens to separate them, and those are unaffected on either
toolchain.

Description

• keccak1600-x86_64.s: wrap the trailing note block in .pushsection/.popsection so it
  restores the previous section instead of leaking it. This is the fix for Rust 1.99-beta regression due to global_asm! section change #7246, and is what
  the issue suggests.
• Make each global_asm! block declare .text up front rather than inheriting whatever
  section it happens to start in, so the pairing is robust in both directions:
  blake2f/x86_64.s, keccak1600-armv8-elf.s, keccak1600-armv8-macho.s. (The x86_64 keccak
  file already opened with .text.) These three are defensive, not required to close the issue.
• Record the change in each file's Modified: header, and update the keccak README.md, which
  claimed the x86 file was imported unmodified.

No functional change to any of the assembly itself.

Verification

Reproduced and fixed on an x86_64 Linux host (stable 1.92.0 vs beta 1.99.0-beta.3, linking
through the host cc with -fuse-ld=lld), building the real ethrex-crypto crate:

CARGO_PROFILE_DEV_CODEGEN_UNITS=1 CARGO_PROFILE_DEV_INCREMENTAL=false \
  cargo +beta test --no-run -p ethrex-crypto

rev	toolchain	_blake2b_f lands in	.note.gnu.property size	result
base	stable 1.92.0	.text	0x20	ok (by luck: blake2f block emitted first)
base	beta 1.99.0-beta.3	.note.gnu.property	0x13c0	link error
this PR	stable 1.92.0	.text	0x20	ok
this PR	beta 1.99.0-beta.3	.text	0x20	ok

The failure:

rust-lld: error: ethrex_crypto-<hash>.ethrex_crypto.<hash>-cgu.0.rcgu.o:(.note.gnu.property+0x20): data is too short
error: could not compile `ethrex-crypto` (lib test) due to 1 previous error

The section column above comes from unpacking the rlib and reading the symbol table, which is
worth doing on the base commit even where the link succeeds, since it shows the note section
ballooning from its correct 0x20 bytes to 0x13c0:

ar x "$CARGO_TARGET_DIR"/debug/deps/libethrex_crypto-*.rlib
readelf -sW *cgu.0.rcgu.o | grep _blake2b_f   # field 7 is the section index
readelf -SW *cgu.0.rcgu.o | grep -E 'note\.gnu\.property|\.text'

codegen-units = 1 is required to reproduce. Plain cargo test uses the test profile
(inheriting [profile.dev]), where rustc's default multi-CGU split places the keccak and
blake2f global_asm! blocks in different codegen units, so the leaked section state never
reaches blake2f. For the record, on the same host cargo +stable test --no-run and
cargo +beta test --no-run over the whole workspace both exit 0 on the pre-fix base as well as
on this PR — those commands alone do not exercise the bug.

aarch64 cannot hit this at all: neither ARMv8 file contains a .note.gnu.property block, so
the .text added to them is purely defensive. Checked regardless on aarch64-apple-darwin —
full-workspace cargo test --no-run on both stable and beta, plus the keccak suite
(crypto::keccak_tests, 11 tests) — all green.

Checklist

• Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

  Not applicable: assembly and documentation only, no Store changes.

Closes #7246

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.

---

The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Three
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.

---

ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rs → unsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.

---

Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
   reviewers, and add environment: to the two jobs that sign.
2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

[github-actions]

🤖 Kimi Code Review

I'll review this PR which addresses issue #7246 regarding global_asm! section inheritance in Rust.

Summary

This PR fixes a subtle but serious codegen bug where global_asm! blocks can emit functions into incorrect sections due to shared assembler state within codegen units. The fix adds explicit .text directives and uses .pushsection/.popsection for note sections.

Detailed Review

1. Core Fix Correctness — crates/common/crypto/blake2f/x86_64.s (line 38)

    .text
    .global _blake2b_f

Good: Adding .text before the function entry point ensures this global_asm! block emits code to the correct section regardless of what previous blocks left as current.

Question: Should this also have a trailing .text to be symmetric with the documentation's stated requirement? The README says "each global_asm! block must both start in .text and leave .text current when it ends." However, since this file ends with a function and no section changes, .text remains current. This is acceptable.

2. ARM Files — keccak1600-armv8-elf.s and keccak1600-armv8-macho.s (lines 16-17)

+.text
+
 .align	8

Good: Both ARM variants get the explicit .text directive. The placement before .align is correct since .text doesn't affect alignment state.

Minor: The blank line after .text is inconsistent with the x86_64 file style but harmless.

3. x86_64 Keccak — keccak1600-x86_64.s (lines 1-8, 537-544)

The .pushsection/.popsection change is the most critical fix:

-.section	.note.gnu.property,"a",@note
+.pushsection	.note.gnu.property,"a",@note
 	.long	4,2f-1f,5
 	.byte	0x47,0x4E,0x55,0
 1:	.long	0xc0000002,4,3
 .align	8
 2:
+.popsection

Critical fix confirmed: Without .popsection, any subsequent global_asm! in the same codegen unit would emit its functions into .note.gnu.property, causing:

• Linker errors or silent corruption
• Functions marked as notes/properties instead of executable code
• Potential runtime crashes or security issues (non-executable sections)

4. Documentation — README.md (lines 40-46)

The explanation is accurate and well-written. However, I suggest one clarification:

"each global_asm! block must both start in .text and leave .text current when it ends"

Nit: This is slightly overstated. The requirement is that each block starts in whatever section it intends to use and restores any changed state. For data blocks, .text wouldn't be appropriate. The current phrasing works for this codebase since all these blocks emit functions.

Potential Issues

5. Missing .text in x86_64 Keccak start?

The x86_64 keccak file already had .text at line 3 (now line 9). No change needed there — it was already correct at entry, only the exit state was broken.

6. Mach-O vs ELF Divergence

The ARM files have separate ELF and Mach-O variants with identical .text additions. This is correct — the section directive syntax is the same across both object formats.

Security Assessment

Aspect	Status
Code injection via wrong section	Mitigated — .pushsection/.popsection prevents leakage
W^X violations	Prevented — functions won't land in non-executable note sections
Supply chain (asm provenance)	Documented — modifications clearly tracked

Recommendations

1. Consider a macro/wrapper (future PR): For new assembly additions, consider a Rust-side wrapper that enforces .text + .popsection patterns, or add a CI check that validates global_asm! files for section hygiene.

2. Verify no other asm files affected: Search for other .section directives in global_asm! usage:

   grep -r "\.section" crates/common/crypto/ --include="*.s" | grep -v pushsection | grep -v popsection

Verdict

Approve. This is a correct, minimal fix for a subtle codegen bug. The .pushsection/.popsection pattern is the standard solution for this problem, and the explicit .text entries make the code robust against ordering changes in codegen units.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

[github-actions]

🤖 Claude Code Review

This is a small, well-targeted assembly fix. Verified the diffs against the actual files.

Review

Correctness of the fix

• The root cause analysis is sound: global_asm! blocks in the same codegen unit share assembler section state, so a block that ends in a non-.text section (the .note.gnu.property note in keccak1600-x86_64.s) leaks that state into whatever block rustc emits next.
• .pushsection .note.gnu.property,"a",@note / .popsection (keccak1600-x86_64.s:8,17) is the right primitive — it restores whatever section was active before entering the note block (in this case .text, since the file already opens with .text at line 7), rather than assuming .text is what should be restored. This is more robust than replacing it with .section .text at the end, since it doesn't hardcode an assumption about what preceded it.
• Adding an explicit leading .text to blake2f/x86_64.s, keccak1600-armv8-elf.s, and keccak1600-armv8-macho.s is a reasonable defensive complement: it makes each block's start state also independent of emission order, closing the same class of bug from the other direction.
• The two fixes are complementary and correctly scoped — pushsection/popsection fixes the leak at the end of the keccak file, .text guards fix assumptions at the start of the other files.

Verification

The PR description shows a concrete, reproduced before/after (.note.gnu.property size dropping from 0x1400-ish back to 0x20, _blake2b_f landing back in .text), and correctly identifies why plain cargo test doesn't catch this (multi-CGU dev/test profile vs. codegen-units = 1 in release). That's good diligence for a bug that's inherently non-deterministic/toolchain-dependent.

Minor notes

• keccak1600-x86_64.s:7 — pre-existing trailing whitespace after .text (.text\t), unrelated to this PR, not worth touching here.
• The Modified: comment blocks and README update are a nice touch for future maintainability, given this code is machine-generated/vendored and easy to blindly re-import without preserving these fixes.
• No functional/semantic change to the assembly logic itself — this is purely section-directive bookkeeping, consistent with the stated intent.

No correctness, security, or gas-accounting concerns — this only affects linking of the crypto backend, not EVM semantics. Nothing here needs to change before merge.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

• No findings.

Static review looks good. The change is narrowly scoped and appears to fix the actual failure mode: crates/common/crypto/keccak/keccak1600-x86_64.s now restores the previous section after emitting .note.gnu.property, and crates/common/crypto/blake2f/x86_64.s, keccak1600-armv8-elf.s, and keccak1600-armv8-macho.s now explicitly re-enter .text, which is the right defense against global_asm! section-state leakage within a codegen unit.

I did not find correctness, security, gas-accounting, or maintainability concerns from this patch itself. I could not run cargo check in this environment because the pinned Rust toolchain tries to write under read-only /home/runner/.rustup, so the review is based on static inspection only.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[Copilot]

Pull request overview

Fixes a build/link failure caused by global_asm! blocks sharing the assembler’s “current section” within a single codegen unit, which could cause subsequent assembly blocks to accidentally emit code into .note.gnu.property instead of .text.

Changes:

• Prevent keccak1600-x86_64.s from leaking .note.gnu.property section state by using .pushsection/.popsection.
• Make embedded assembly blocks more robust by explicitly switching to .text in additional assembly sources.
• Update keccak documentation to reflect that both x86 and ARM assembly sources require local modifications for global_asm! inclusion.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/common/crypto/keccak/README.md	Documents the required .text / section-state discipline for global_asm!-included assembly.
crates/common/crypto/keccak/keccak1600-x86_64.s	Wraps the trailing .note.gnu.property block in .pushsection/.popsection to avoid leaking section state.
crates/common/crypto/keccak/keccak1600-armv8-macho.s	Adds an explicit .text directive so the block doesn’t inherit a prior section.
crates/common/crypto/keccak/keccak1600-armv8-elf.s	Adds an explicit .text directive so the block doesn’t inherit a prior section.
crates/common/crypto/blake2f/x86_64.s	Adds an explicit .text directive to ensure the function is emitted into the text section.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.