chore(l1,l2): make workspace crates publishable to crates.io#6896
Conversation
Prepare the 16-crate transitive closure of ethrex-common, ethrex-l2-common, ethrex-rpc, ethrex-l2-rpc, and ethrex-sdk for publication to crates.io, and add a release workflow that publishes them in dependency order. - Add a description to every publish-set crate's [package] (crates.io requires it) and a shared repository via [workspace.package]. - Add version = "17.0.0" to the publish-set [workspace.dependencies] entries and to the three inline path deps (ethrex-vm -> ethrex-levm, ethrex-blockchain -> ethrex-metrics, ethrex-sdk -> ethrex-sdk-contract-utils in both [dependencies] and [build-dependencies]). The path is kept for local builds; the version is used on publish. - Remove the orphaned optional ethrex-guest-program / ethrex-prover dependencies from ethrex-blockchain. No feature wired them and no source used them; they were the only link from the publish set to the git-backed prover crates. - Vendor the L1 genesis test fixture into ethrex-rpc and repoint the include_str! calls in test_utils.rs, so the published tarball is self-contained (test_utils is a public module compiled into the library). - Add .github/workflows/publish.yml: publishes the 16 crates with cargo publish in dependency order on release, idempotent across re-runs, with a workflow_dispatch dry-run.
🤖 Kimi Code ReviewReview Summary The PR prepares the workspace for crates.io publishing by adding versions, metadata, and a release workflow. The changes are structurally sound but require one fix to ensure test fixtures are included in the package. Critical Issues1. Missing Fix: Add to include = ["src/**/*", "fixtures/**/*", "Cargo.toml", "README.md"]Or move the file to Suggestions2. Fragile error detection in publish workflow (.github/workflows/publish.yml:49) 3. Verify unpublished workspace members Correctness & Security
Code Quality
Nits
Action Required: Address Item 1 (fixture inclusion) before merging to ensure the publish workflow succeeds. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
🤖 Claude Code ReviewNow I have a complete view of the diff. Let me write up the review. PR Review:
|
🤖 Codex Code Review
No consensus, EVM, gas-accounting, trie, or RLP correctness issues stood out here; this PR is mostly packaging/release plumbing. I wasn’t able to run a full Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
There was a problem hiding this comment.
Pull request overview
Prepares the ethrex Rust workspace to be publishable on crates.io by adding required crate metadata, making internal dependencies publish-compatible, removing an unpublishable dependency edge, vendoring an RPC fixture into the crate, and introducing an automated publish workflow that releases crates in dependency order.
Changes:
- Add crates.io-required metadata (
description, workspace-inheritedrepository) across the publish set. - Add versions alongside
pathfor internal dependencies socargo publishaccepts them; remove unused optional prover-related deps fromethrex-blockchain. - Add a GitHub Actions workflow to publish the crate set in topological (dependency) order; vendor the L1 genesis JSON fixture into
ethrex-rpc.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/vm/levm/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/vm/Cargo.toml | Adds crates.io metadata and makes ethrex-levm dependency publishable by adding a version. |
| crates/storage/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/networking/rpc/test_utils.rs | Repoints include_str! to a vendored in-crate fixture path for publishable packaging. |
| crates/networking/rpc/fixtures/l1_genesis.json | Adds the vendored L1 genesis fixture to ship within the ethrex-rpc crate tarball. |
| crates/networking/rpc/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/networking/p2p/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/l2/storage/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/l2/sdk/contract_utils/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/l2/sdk/Cargo.toml | Adds crates.io metadata and makes ethrex-sdk-contract-utils dependency publishable by adding a version (deps and build-deps). |
| crates/l2/networking/rpc/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/l2/common/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/common/trie/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/common/rlp/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/common/crypto/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/common/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/blockchain/metrics/Cargo.toml | Adds crates.io metadata (description, repository.workspace). |
| crates/blockchain/Cargo.toml | Adds crates.io metadata, adds version to ethrex-metrics path dep, and removes unused optional unpublishable deps. |
| Cargo.toml | Adds workspace.package.repository and adds versions to workspace internal dependencies for publishability. |
| Cargo.lock | Removes the dropped prover-related dependencies from the resolved graph. |
| .github/workflows/publish.yml | Adds a release-triggered workflow to publish the crate set to crates.io in dependency order (supports dry-run on manual dispatch). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let genesis: &str = include_str!("fixtures/l1_genesis.json"); | ||
| let genesis: Genesis = serde_json::from_str(genesis).expect("Fatal: test config is invalid"); |
Greptile SummaryThis PR prepares 16 ethrex crates for publication to crates.io by adding required
Confidence Score: 4/5Safe to merge — the Cargo metadata changes are mechanical and correct, the publish workflow is well-structured with idempotency handling, and the test_utils.rs change only re-points include_str! paths to the new vendored copy. The changes are largely additive (metadata, versioned deps, a new workflow file, a vendored fixture). The one ongoing maintenance risk is the duplicated genesis fixture, which has no automated sync check and could silently diverge in future PRs — but it does not break anything today. The workflow logging issues (swallowed success output, unclosed group on hard failure) are cosmetic and do not affect correctness. .github/workflows/publish.yml and crates/networking/rpc/fixtures/l1_genesis.json (paired with fixtures/genesis/l1.json) are the two areas worth a second look before the first real publish run.
|
| Filename | Overview |
|---|---|
| .github/workflows/publish.yml | New workflow publishing 16 crates in topological order; publish success output is captured but silently discarded, and ::endgroup:: is not emitted before exit 1. |
| crates/networking/rpc/test_utils.rs | Two include_str! calls updated to reference the vendored local fixture; change is correct but creates a second copy of the genesis file that can silently drift from the workspace-root original. |
| crates/networking/rpc/fixtures/l1_genesis.json | New vendored copy of fixtures/genesis/l1.json; content appears correct and byte-identical to the original. |
| Cargo.toml | Adds repository to [workspace.package] and adds version = "17.0.0" to all 16 publish-set workspace dependencies alongside existing path entries; correct pattern for Cargo path+version dual-mode. |
| crates/blockchain/Cargo.toml | Adds description, repository, and inline version for ethrex-metrics; removes orphaned optional ethrex-guest-program and ethrex-prover entries that blocked publishing. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Trigger: release/published or workflow_dispatch] --> B{DRY_RUN?}
B -- true --> C[cargo package --no-verify --list]
B -- false --> D[Loop over 16 crates in topo order]
D --> E[cargo publish -p crate]
E -- success --> F[sleep 20s]
F --> G{More crates?}
G -- yes --> D
G -- no --> H[Done]
E -- failure --> I{output contains 'already exists'?}
I -- yes --> J[Skip, continue]
J --> G
I -- no --> K[exit 1]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Trigger: release/published or workflow_dispatch] --> B{DRY_RUN?}
B -- true --> C[cargo package --no-verify --list]
B -- false --> D[Loop over 16 crates in topo order]
D --> E[cargo publish -p crate]
E -- success --> F[sleep 20s]
F --> G{More crates?}
G -- yes --> D
G -- no --> H[Done]
E -- failure --> I{output contains 'already exists'?}
I -- yes --> J[Skip, continue]
J --> G
I -- no --> K[exit 1]
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
.github/workflows/publish.yml:56-67
The `cargo publish` output is captured into `OUTPUT` and only echoed on failure. When publishing succeeds the log is silent, making it impossible to audit what version was uploaded or to diagnose slow runs without re-running. Printing it unconditionally before the sleep preserves the audit trail.
```suggestion
OUTPUT=$(cargo publish -p "$c" 2>&1) || {
echo "$OUTPUT"
if echo "$OUTPUT" | grep -q "already exists"; then
echo "$c already published, skipping"
echo "::endgroup::"
continue
fi
exit 1
}
echo "$OUTPUT"
# cargo >=1.66 blocks until the version is in the index;
# a short settle covers any registry propagation lag.
sleep 20
```
### Issue 2 of 3
.github/workflows/publish.yml:63
**`::endgroup::` not emitted before `exit 1`**
When `cargo publish` fails with an error that is not "already exists", the script hits `exit 1` without first calling `echo "::endgroup::"`. In the GitHub Actions UI this leaves the log group unclosed, causing subsequent workflow output to appear nested under the failed crate's group. Adding `echo "::endgroup::"` immediately before `exit 1` keeps the log structure clean.
### Issue 3 of 3
crates/networking/rpc/test_utils.rs:50
**Vendored genesis fixture can silently drift from the workspace original**
`crates/networking/rpc/fixtures/l1_genesis.json` is a byte-identical copy of `fixtures/genesis/l1.json`. If the workspace-root file is updated (e.g. a new hardfork time or a contract address change) without also updating the vendored copy, `test_utils.rs` will compile against the stale fixture and RPC tests will run against a different genesis than the rest of the suite — with no compile error or CI failure to surface the mismatch. Consider adding a CI step (e.g. `diff -q`) that asserts the two files are identical.
Reviews (1): Last reviewed commit: "chore(l1,l2): make workspace crates publ..." | Re-trigger Greptile
| OUTPUT=$(cargo publish -p "$c" 2>&1) || { | ||
| echo "$OUTPUT" | ||
| if echo "$OUTPUT" | grep -q "already exists"; then | ||
| echo "$c already published, skipping" | ||
| echo "::endgroup::" | ||
| continue | ||
| fi | ||
| exit 1 | ||
| } | ||
| # cargo >=1.66 blocks until the version is in the index; | ||
| # a short settle covers any registry propagation lag. | ||
| sleep 20 |
There was a problem hiding this comment.
The
cargo publish output is captured into OUTPUT and only echoed on failure. When publishing succeeds the log is silent, making it impossible to audit what version was uploaded or to diagnose slow runs without re-running. Printing it unconditionally before the sleep preserves the audit trail.
| OUTPUT=$(cargo publish -p "$c" 2>&1) || { | |
| echo "$OUTPUT" | |
| if echo "$OUTPUT" | grep -q "already exists"; then | |
| echo "$c already published, skipping" | |
| echo "::endgroup::" | |
| continue | |
| fi | |
| exit 1 | |
| } | |
| # cargo >=1.66 blocks until the version is in the index; | |
| # a short settle covers any registry propagation lag. | |
| sleep 20 | |
| OUTPUT=$(cargo publish -p "$c" 2>&1) || { | |
| echo "$OUTPUT" | |
| if echo "$OUTPUT" | grep -q "already exists"; then | |
| echo "$c already published, skipping" | |
| echo "::endgroup::" | |
| continue | |
| fi | |
| exit 1 | |
| } | |
| echo "$OUTPUT" | |
| # cargo >=1.66 blocks until the version is in the index; | |
| # a short settle covers any registry propagation lag. | |
| sleep 20 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish.yml
Line: 56-67
Comment:
The `cargo publish` output is captured into `OUTPUT` and only echoed on failure. When publishing succeeds the log is silent, making it impossible to audit what version was uploaded or to diagnose slow runs without re-running. Printing it unconditionally before the sleep preserves the audit trail.
```suggestion
OUTPUT=$(cargo publish -p "$c" 2>&1) || {
echo "$OUTPUT"
if echo "$OUTPUT" | grep -q "already exists"; then
echo "$c already published, skipping"
echo "::endgroup::"
continue
fi
exit 1
}
echo "$OUTPUT"
# cargo >=1.66 blocks until the version is in the index;
# a short settle covers any registry propagation lag.
sleep 20
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| echo "::endgroup::" | ||
| continue | ||
| fi | ||
| exit 1 |
There was a problem hiding this comment.
::endgroup:: not emitted before exit 1
When cargo publish fails with an error that is not "already exists", the script hits exit 1 without first calling echo "::endgroup::". In the GitHub Actions UI this leaves the log group unclosed, causing subsequent workflow output to appear nested under the failed crate's group. Adding echo "::endgroup::" immediately before exit 1 keeps the log structure clean.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish.yml
Line: 63
Comment:
**`::endgroup::` not emitted before `exit 1`**
When `cargo publish` fails with an error that is not "already exists", the script hits `exit 1` without first calling `echo "::endgroup::"`. In the GitHub Actions UI this leaves the log group unclosed, causing subsequent workflow output to appear nested under the failed crate's group. Adding `echo "::endgroup::"` immediately before `exit 1` keeps the log structure clean.
How can I resolve this? If you propose a fix, please make it concise.| // Base price for each test transaction. | ||
| pub const BASE_PRICE_IN_WEI: u64 = 10_u64.pow(9); | ||
| pub const TEST_GENESIS: &str = include_str!("../../../fixtures/genesis/l1.json"); | ||
| pub const TEST_GENESIS: &str = include_str!("fixtures/l1_genesis.json"); |
There was a problem hiding this comment.
Vendored genesis fixture can silently drift from the workspace original
crates/networking/rpc/fixtures/l1_genesis.json is a byte-identical copy of fixtures/genesis/l1.json. If the workspace-root file is updated (e.g. a new hardfork time or a contract address change) without also updating the vendored copy, test_utils.rs will compile against the stale fixture and RPC tests will run against a different genesis than the rest of the suite — with no compile error or CI failure to surface the mismatch. Consider adding a CI step (e.g. diff -q) that asserts the two files are identical.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/test_utils.rs
Line: 50
Comment:
**Vendored genesis fixture can silently drift from the workspace original**
`crates/networking/rpc/fixtures/l1_genesis.json` is a byte-identical copy of `fixtures/genesis/l1.json`. If the workspace-root file is updated (e.g. a new hardfork time or a contract address change) without also updating the vendored copy, `test_utils.rs` will compile against the stale fixture and RPC tests will run against a different genesis than the rest of the suite — with no compile error or CI failure to surface the mismatch. Consider adding a CI step (e.g. `diff -q`) that asserts the two files are identical.
How can I resolve this? If you propose a fix, please make it concise.Add a workflow-level `permissions: contents: read` block so the publish workflow does not run with the default broad GITHUB_TOKEN scopes. Publishing uses CARGO_REGISTRY_TOKEN; the GITHUB_TOKEN only needs read access for checkout. Resolves the CodeQL "workflow does not contain permissions" finding.
…vendoring a fixture
Replace the vendored crates/networking/rpc/fixtures/l1_genesis.json copy with a
test-utils Cargo feature (off by default) that gates `pub mod test_utils`.
The published default build of ethrex-rpc no longer compiles test_utils, so its
cross-crate include_str! of fixtures/genesis/l1.json is never evaluated and the
crate publishes without a vendored copy. In-workspace test consumers
(ethrex-l2-rpc, ethrex-test) enable the feature via a dev-dependency on
ethrex-rpc, so the fixture is read from the workspace root as before; the
dev-dependency is stripped from the published manifests.
- Gate `pub mod test_utils` with cfg(any(test, feature = "test-utils")).
- Add the empty test-utils feature to ethrex-rpc.
- Restore the original include_str!("../../../fixtures/genesis/l1.json") path.
- Remove the vendored crates/networking/rpc/fixtures/l1_genesis.json.
- Enable ethrex-rpc/test-utils via [dev-dependencies] in ethrex-l2-rpc and ethrex-test.
Lines of code reportTotal lines added: Detailed view |
ef_tests-engine's engine_ctx.rs imports ethrex_rpc::test_utils in library code, so after gating test_utils behind the test-utils feature this crate must enable it. ef_tests-engine is not published, so enabling the feature on the normal ethrex-rpc dependency is fine. Lives in the separate tooling/ workspace, which is why the root-workspace check did not catch the break.
…io publishing Trigger publish.yml on `release: released` instead of `published`. The release process creates a `vX.Y.Z-rc.W` pre-release first, tests it, then promotes it to a full release. `published` fires on pre-release creation, which would publish the crates to crates.io at version X.Y.Z before testing -- and crates.io versions are immutable, so a failed RC would burn the version. `released` fires only when a pre-release is promoted to a full release (never on pre-release creation), so only tested, final releases publish. Document the crates.io publishing step in docs/developers/release-process.md: how it triggers, that pre-releases are skipped, the one-time CRATES_IO_TOKEN / crates-release-prod / crate-name-ownership setup, the dry-run path, and a troubleshooting entry.
Motivation
Make the ethrex library crates consumable from crates.io. The immediate goal is to publish
ethrex-common,ethrex-l2-common,ethrex-rpc,ethrex-l2-rpc, andethrex-sdk. crates.io requires every non-dev dependency in a crate's closure to already be published, so this prepares the full 16-crate transitive closure of those five and adds a workflow that publishes them in dependency order.The crates were not publishable as-is:
[package].description(crates.io rejects a publish without one).paths with no version requirement (cargo publishrejects path-only deps).ethrex-blockchaincarried orphaned optional dependencies onethrex-guest-program/ethrex-prover, which can never reach crates.io (git deps).ethrex-rpcembedded a workspace-root file viainclude_str!, which a published tarball cannot contain.Description
descriptionto all 16 publish-set crates; add a sharedrepositoryvia[workspace.package](inherited withrepository.workspace = true).version = "17.0.0"to the publish-set[workspace.dependencies]entries and to the three inline path deps (ethrex-vm → ethrex-levm,ethrex-blockchain → ethrex-metrics, andethrex-sdk → ethrex-sdk-contract-utilsin both[dependencies]and[build-dependencies]). Thepathis kept for local builds; the version is used when publishing.ethrex-guest-program/ethrex-proverdependencies fromethrex-blockchain— no[features]entry wired them and no source used them; they were the only link from the publish set to the git-backed prover crates.Cargo.lockdrops exactly those two edges.ethrex-rpc'spub mod test_utilsdid a cross-crateinclude_str!of the workspace-root genesis fixture, which a published tarball can't contain. Gate it behind a newtest-utilsfeature (off by default) viacfg(any(test, feature = "test-utils")), so the default published build excludestest_utilsand theinclude_str!is never compiled. In-workspace test consumers (ethrex-l2-rpc,ethrex-test) enableethrex-rpc/test-utilsthrough a[dev-dependencies]entry (stripped on publish), reading the fixture from the workspace root as before. No fixture is duplicated..github/workflows/publish.yml: publishes the 16 crates withcargo publish -p <crate>in dependency order onrelease: released(final releases only —vX.Y.Z-rc.Wpre-releases are skipped, since crates.io versions are immutable and an RC must not claim the version before testing), idempotent on "already exists", with aworkflow_dispatchdry-run path and a least-privilegepermissions: contents: readblock.docs/developers/release-process.md(trigger, pre-release exclusion, one-timeCRATES_IO_TOKEN/crates-release-prod/name-ownership setup, dry-run, troubleshooting).Publish order:
rlp, crypto, sdk-contract-utils, trie, common, metrics, levm, l2-common, storage, vm, storage-rollup, blockchain, p2p, rpc, l2-rpc, sdk.Before the first publish (org setup, not in this PR): a
CRATES_IO_TOKENrepo secret, acrates-release-prodGitHub environment, and claiming the 16 crate names on crates.io (all currently unregistered).Validation
cargo package --no-verify --listpasses for all 16 (no missing-description warnings, no unversioned-path-dep errors). Confirmedethrex-crypto's assembly.sfiles ship in its tarball, andethrex-rpcpackages cleanly withtest_utilsexcluded from the default build (no fixture needed).cargo checkpasses forethrex-rpc(default build,test_utilsexcluded),ethrex-rpc --tests,ethrex-l2-rpc --tests, andethrex-test --tests(the feature-gatedtest_utilsresolves for all test consumers). Publish set + binary also compile.cargo fmt --all --checkandactionlintare clean.Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PR includes breaking changes to theStorerequiring a re-sync. — N/A, noStorechanges.