publish.yaml: switch to crates.io trusted publishing (OIDC)#10
Conversation
📝 WalkthroughWalkthroughThe publish workflow is updated to authenticate with crates.io using a dedicated OIDC action instead of a static token. Job permissions are granted for OIDC token generation, and the publish job steps are refactored to use the auth action and pass its token to ChangesCrates.io OIDC authentication migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hyperfuel-client/tests/api_test.rs (1)
1-523:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEntire test suite is disabled.
All tests in this file are commented out, resulting in zero test coverage for the API. The commented code also references
hypersync_client/hypersync_formatmodules rather than the actualhyperfuel_*crate names, suggesting these tests were imported but never adapted to the current crate structure.Consider either:
- Restoring and fixing these tests to work with the new
polars-arrowbased API- Deleting the file entirely and tracking test restoration in a follow-up issue
Leaving 500+ lines of dead commented code reduces maintainability without providing value.
Would you like me to open an issue to track restoring test coverage for the API?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-client/tests/api_test.rs` around lines 1 - 523, The file contains an entire disabled test suite (all tests commented out, e.g., functions test_api_arrow_ipc, test_api_arrow_ipc_ordering, test_api_decode_logs, test_api_preset_query_blocks_and_transactions, etc.) and uses old crate names (hypersync_client, hypersync_format) so it provides no coverage; either restore and adapt these tests to the current crate or remove the dead file and track restoration in an issue. To fix: decide on one approach — (A) Re-enable and update tests: replace old crate references (hypersync_client, hypersync_format) with the current hyperfuel_* equivalents, update API usages around Client::new, preset_query::*, StreamConfig, get_arrow/collect_arrow/get_events/collect_parquet, and any type names (Transaction, FieldSelection, Query) so tests compile and run; or (B) delete the commented test file and create a concise issue referencing the need to reintroduce tests for the API (include examples like test_api_preset_query_blocks_and_transactions and test_api_arrow_ipc to guide future work). Ensure CI runs the restored tests or that the issue is linked to an implementation task.
🧹 Nitpick comments (5)
hyperfuel-client/src/rayon_async.rs (1)
3-7: ⚡ Quick winRemove unnecessary
T: Syncbound.The
spawnfunction transfers ownership ofTthrough the oneshot channel, so onlyT: Sendis required. TheT: Syncbound is unnecessarily restrictive and prevents using this helper with!Synctypes likeCell<T>orRc<T>that are otherwise safe to send across threads.♻️ Suggested fix
pub fn spawn<F, T>(func: F) -> oneshot::Receiver<T> where F: 'static + FnOnce() -> T + Send, - T: 'static + Send + Sync, + T: 'static + Send, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-client/src/rayon_async.rs` around lines 3 - 7, The spawn function unnecessarily requires T: Sync even though it only moves T across a oneshot channel; remove the Sync bound from the generic constraints so the signature is pub fn spawn<F, T>(func: F) -> oneshot::Receiver<T> where F: 'static + FnOnce() -> T + Send, T: 'static + Send { ... } — update the where clause in the spawn function (and any related type annotations/usages in rayon_async.rs) to drop T: Sync so !Sync types (e.g., Rc or Cell) can be used safely when sent.hyperfuel-format/src/types/transaction_type.rs (1)
10-24: 💤 Low valueConsider removing arithmetic derives for a type identifier.
derive_more::Addandderive_more::Subenable arithmetic operations onTransactionType, but transaction types are typically identifiers rather than numeric values you'd add or subtract. This could lead to confusing code likeTransactionType(1) + TransactionType(2).If arithmetic isn't needed, removing these derives makes the type's intent clearer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-format/src/types/transaction_type.rs` around lines 10 - 24, The TransactionType newtype currently derives derive_more::Add and derive_more::Sub which allow arithmetic on what should be an identifier; remove derive_more::Add and derive_more::Sub from the derive list on the TransactionType struct so it no longer supports addition/subtraction, leaving Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, derive_more::From, derive_more::Into, and derive_more::Deref intact; update any code or tests that relied on adding/subtracting TransactionType to use the underlying value (TransactionType.0) or an explicit conversion instead.hyperfuel-client/Cargo.toml (1)
27-33: ⚡ Quick win
test-utilfeature should be in dev-dependencies only.The
test-utilfeature for tokio is typically used for testing (e.g.,tokio::time::pause()). Including it in the main dependencies adds testing code to production builds.♻️ Suggested fix
tokio = { version = "1", default-features = false, features = [ "rt-multi-thread", "fs", - "test-util", "rt", "macros", ] }And add to dev-dependencies if needed:
[dev-dependencies] tokio = { version = "1", features = ["test-util"] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-client/Cargo.toml` around lines 27 - 33, Remove "test-util" from the main tokio dependency features list in Cargo.toml (the tokio = { version = "1", default-features = false, features = [...] } entry) so production builds don't include testing utilities, and add a dev-dependencies entry for tokio that enables "test-util" (i.e., declare tokio under [dev-dependencies] with features = ["test-util"]) so tests still have access to tokio::time::pause and other test helpers.hyperfuel-client/src/util.rs (1)
107-110: 💤 Low valueSchema metadata is discarded.
The new schema is created with
metadata: Default::default(), which drops any metadata from the original batch schema. If schema metadata is used for any purpose (e.g., key-value annotations), this will lose that information.♻️ Suggested fix to preserve metadata
let schema = Schema { fields, - metadata: Default::default(), + metadata: batch.schema.metadata.clone(), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-client/src/util.rs` around lines 107 - 110, The Schema is being rebuilt with metadata: Default::default(), which drops any original schema metadata; update the constructor used to build the new Schema so it preserves the original metadata (e.g., set metadata: original_schema.metadata.clone() or metadata: batch.schema().metadata().clone() instead of Default::default()), keeping the same fields variable and resulting schema variable name.hyperfuel-client/src/stream.rs (1)
100-106: 💤 Low valueTask panic will crash the stream.
The double
.unwrap()onset.join_next()results will propagate panics from spawned tasks. Consider handlingJoinErrorgracefully to surface errors through the channel instead:- let (generation, req_idx, resps) = res.unwrap(); + let (generation, req_idx, resps) = match res { + Ok(r) => r, + Err(e) => { + // Task panicked or was cancelled + log::error!("task failed: {:?}", e); + continue; + } + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfuel-client/src/stream.rs` around lines 100 - 106, The code currently uses double unwrap on set.join_next().await which will panic if a spawned task panicked (JoinError); change the join handling in the loop that uses set.join_next().await to match on the Result from join_next() and handle Err(join_error) instead of unwrap: detect Err from the JoinHandle (the outer await) and convert it into a channel-safe error value (e.g., send Err(...) via the same queue/channel or an error variant that includes the JoinError) so the stream surfaces the task failure instead of crashing; adjust consumers of queue.insert to accept a Result<(generation, req_idx, resps), JoinError> (or an enum) and update call sites around set.try_join_next(), set.join_next(), and queue.insert accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hyperfuel-client/src/column_mapping.rs`:
- Around line 87-89: Fix the typo in the error context string used when
mapping/casting a column: update the message passed to .context(...) in the
map_column(...) call so "apply cast to colum '{}'" reads "apply cast to column
'{}'" (reference symbols: map_column, .context, field.name).
- Around line 29-37: Update the misleading doc comments for the struct fields so
they describe the actual fields: change the comment above receipt to something
like "Mapping for receipt data" (or the correct domain term used elsewhere),
change the comment above input to "Mapping for input/trace data" if it
represents trace inputs, and change the comment above output to "Mapping for
decoded output data" (or the precise meaning used in your codebase); ensure the
comments directly reference the field names receipt, input, and output in
column_mapping.rs so they accurately document each BTreeMap<String, DataType>.
- Around line 111-132: The map_l1_fee_scalar function currently unwraps at
several points and can panic; change it to return Err on invalid input instead:
check downcast_ref::<BinaryArray<i32>>() and return an anyhow error if the
downcast fails (instead of unwrap), convert each BinaryArray entry by mapping
Option<&[u8]> -> Result<Option<f64>, anyhow::Error> where invalid UTF-8 or parse
errors produce Err with context (use std::str::from_utf8 and str::parse but
propagate errors), then collect the mapped results into a Float64Array (or build
a Float64Array from an iterator of Option<f64>) and pass that boxed array into
compute::cast::cast with CastOptions as before; ensure all error paths are
returned via Result and include context messages mentioning map_l1_fee_scalar,
BinaryArray<i32>, and target_data_type.
In `@hyperfuel-client/src/from_arrow.rs`:
- Around line 156-159: The code in from_arrow.rs currently calls
bincode::deserialize(...).unwrap() for input_asset_ids and input_contracts which
can panic on malformed data; change these to handle failures safely by replacing
unwrap with a non-panicking path (e.g. use .ok() to convert the Result to Option
and let input_asset_ids/input_contracts yield None on deserialization failure,
or propagate the error from the containing function if it returns Result).
Update the expressions that build input_asset_ids and input_contracts (the
closures using .and_then(|arr| arr.get(idx).map(|v|
bincode::deserialize(v).unwrap()))) to map the deserialize Result into Option
via .ok() or handle the error with ? so callers can handle it, ensuring no
unwrap remains.
- Around line 10-14: The FromArrow trait currently panics because from_arrow
returns Vec<Self> while implementations use unwrap/expect; revert the trait
signature to return a Result (e.g., fn from_arrow(batch: &ArrowBatch) ->
Result<Vec<Self>, ArrowConversionError>) and update all implementations to
remove unwraps/expect and instead propagate or map errors with ?/map_err so
malformed or missing columns return errors; add a simple ArrowConversionError
enum or use an existing error type and update callers to handle the Result (or,
if you intentionally accept panics for now, add a clear comment and open a
tracking issue), making sure to update references to FromArrow::from_arrow and
ArrowBatch handling accordingly.
In `@hyperfuel-client/src/lib.rs`:
- Around line 273-294: The retry loop in the get_chain_id call sleeps even after
the final failed attempt; update the loop that calls self.get_chain_id_impl() so
it only performs the backoff sleep and updates base when there are remaining
attempts. Concretely, use the loop index or compare the current attempt against
self.max_num_retries (e.g., for attempt in 0..=self.max_num_retries or check if
attempt < self.max_num_retries) and wrap the sleep/jitter and base = min(...)
logic inside that conditional; keep get_chain_id_impl(), retry_backoff_ms,
retry_ceiling_ms, and fastrange_64 logic but skip them on the last iteration so
the function returns Err immediately without the extra delay.
- Around line 262-264: The code currently collapses a missing
ArchiveHeight::height into 0 via height.height.unwrap_or(0); instead return an
error when height.height is None so malformed/partial responses don't appear
successful. Replace the unwrap_or(0) usage (the variable height of type
ArchiveHeight) with a check like height.height.ok_or_else(||
anyhow::anyhow!("missing height in ArchiveHeight response")).map(|h| h) (or use
anyhow::bail!/context to produce a descriptive error) and propagate that error
instead of returning 0.
- Around line 67-74: The code currently panics by calling .build().unwrap() when
constructing the reqwest client (see http_client / reqwest::Client::builder());
change this to propagate the error to the caller instead: update the enclosing
function to return a Result (or a compatible error type), replace
.build().unwrap() with .build()? (or map the error with .map_err(...) to your
crate error type) so TLS/socket configuration failures are returned rather than
causing a panic; keep the same builder settings (no_gzip, http1_only, timeout,
tcp_keepalive, connect_timeout) and ensure the error is propagated up the call
chain.
- Around line 371-373: Replace the synchronous tokio::task::block_in_place call
with tokio::task::spawn_blocking to avoid panics on current-thread runtimes:
spawn a blocking task that calls parse_query_response(&bytes), await the
JoinHandle, map/propagate both join errors and the parse_query_response error
while preserving the "parse query response" context; update the let res = ...
assignment in get_arrow to await the spawn_blocking handle, handle JoinError
(e.g., .await.map_err(|e| anyhow::anyhow!(e))?) and then apply .context("parse
query response") to the inner result so the original error context is preserved
for callers.
In `@hyperfuel-client/src/parquet_out.rs`:
- Around line 329-331: The closure passed to flat_map is calling
array_to_columns(...).unwrap(), which can panic on encoding errors; change the
closure to return a Result instead of unwrapping (use the ? operator when
calling array_to_columns) and propagate the error out of the iterator pipeline
so callers can handle it; update the surrounding iterator usage (replace
flat_map with a fallible mapping approach or collect into a Result<Vec<_>, _>
and then flatten) so functions that use this closure (the closure passed into
flat_map and its caller) accept and propagate Result errors rather than
panicking, and ensure references to array_to_columns, the closure, and
encoded_columns are updated accordingly.
In `@hyperfuel-client/src/parse_response.rs`:
- Around line 30-33: The reader options currently disable Cap'n Proto safeguards
by calling opts.nesting_limit(i32::MAX) and traversal_limit_in_words(None)
before read_message; change this to enforce reasonable limits instead (or remove
these overrides to use library defaults) so untrusted network payloads cannot
exhaust memory/CPU. Update the capnp::message::ReaderOptions setup in the same
block (the opts variable and the call to capnp::serialize_packed::read_message)
to set a modest nesting limit (e.g., ~64) and a traversal limit in words (e.g.,
Some(64 MiB converted to 8-byte words) or another safe u64), or simply omit the
explicit calls so defaults apply, then pass the modified opts into read_message.
In `@hyperfuel-format/src/types/quantity.rs`:
- Around line 27-30: The From<u64> implementation for Quantity currently stores
all 8 big-endian bytes (via to_be_bytes()), producing non-canonical values with
leading zeros; update the impl (impl From<u64> for Quantity) to produce the
canonical byte representation by converting value.to_be_bytes(), dropping all
leading 0x00 bytes (e.g. find first non-zero or use skip_while) and constructing
the Quantity from the remaining slice, and if the value is zero produce the
empty/zero representation expected by the existing From<Vec<u8>>/From<&[u8]>
invariant.
In `@hyperfuel-schema/src/lib.rs`:
- Around line 203-238: In concat_chunks, the current special-case only detects
DataType::Utf8 via the is_utf8 flag and later casts the concatenated binary back
to DataType::Utf8; update the logic to treat DataType::Utf8View the same as
DataType::Utf8 (e.g., set is_utf8 when col.data_type() == &DataType::Utf8 ||
col.data_type() == &DataType::Utf8View), and when performing the final cast with
compute::cast::binary_to_utf8 use the original column's DataType (switch between
DataType::Utf8 and DataType::Utf8View or pass the detected type) instead of
hardcoding DataType::Utf8 so view-backed Utf8View columns are concatenated and
cast back correctly (references: concat_chunks, is_utf8 flag,
DataType::Utf8View, Utf8Array<i32>, BinaryArray<i32>,
compute::cast::binary_to_utf8).
In `@hyperfuel-schema/src/util.rs`:
- Around line 5-15: project_schema currently ignores unknown field names when
building select_indices; change it to enforce strict projection by detecting any
requested column names that do not match schema.fields and returning an error
listing missing columns instead of silently dropping them. Update the function
signature from pub fn project_schema(schema: &Schema, field_selection:
&BTreeSet<String>) -> Schema to return a Result<Schema, YourErrorType> (or
Result<Schema, String>), collect missing names by comparing field_selection
against found names (use select_indices and a missing_vec), and if missing_vec
is non-empty return Err(...) with a clear message; otherwise build and return
Ok(projected_schema). Remember to update callers of project_schema to handle the
Result.
In `@README.md`:
- Line 15: Update the two incorrect H3 headings to H2 so the markdown heading
levels increment correctly: change the "### Dependencies" heading (symbol:
Dependencies) and the other heading at the referenced spot currently using "###"
to use "##" instead, ensuring both are H2 headings to satisfy markdownlint’s
heading increment rule.
---
Outside diff comments:
In `@hyperfuel-client/tests/api_test.rs`:
- Around line 1-523: The file contains an entire disabled test suite (all tests
commented out, e.g., functions test_api_arrow_ipc, test_api_arrow_ipc_ordering,
test_api_decode_logs, test_api_preset_query_blocks_and_transactions, etc.) and
uses old crate names (hypersync_client, hypersync_format) so it provides no
coverage; either restore and adapt these tests to the current crate or remove
the dead file and track restoration in an issue. To fix: decide on one approach
— (A) Re-enable and update tests: replace old crate references
(hypersync_client, hypersync_format) with the current hyperfuel_* equivalents,
update API usages around Client::new, preset_query::*, StreamConfig,
get_arrow/collect_arrow/get_events/collect_parquet, and any type names
(Transaction, FieldSelection, Query) so tests compile and run; or (B) delete the
commented test file and create a concise issue referencing the need to
reintroduce tests for the API (include examples like
test_api_preset_query_blocks_and_transactions and test_api_arrow_ipc to guide
future work). Ensure CI runs the restored tests or that the issue is linked to
an implementation task.
---
Nitpick comments:
In `@hyperfuel-client/Cargo.toml`:
- Around line 27-33: Remove "test-util" from the main tokio dependency features
list in Cargo.toml (the tokio = { version = "1", default-features = false,
features = [...] } entry) so production builds don't include testing utilities,
and add a dev-dependencies entry for tokio that enables "test-util" (i.e.,
declare tokio under [dev-dependencies] with features = ["test-util"]) so tests
still have access to tokio::time::pause and other test helpers.
In `@hyperfuel-client/src/rayon_async.rs`:
- Around line 3-7: The spawn function unnecessarily requires T: Sync even though
it only moves T across a oneshot channel; remove the Sync bound from the generic
constraints so the signature is pub fn spawn<F, T>(func: F) ->
oneshot::Receiver<T> where F: 'static + FnOnce() -> T + Send, T: 'static + Send
{ ... } — update the where clause in the spawn function (and any related type
annotations/usages in rayon_async.rs) to drop T: Sync so !Sync types (e.g., Rc
or Cell) can be used safely when sent.
In `@hyperfuel-client/src/stream.rs`:
- Around line 100-106: The code currently uses double unwrap on
set.join_next().await which will panic if a spawned task panicked (JoinError);
change the join handling in the loop that uses set.join_next().await to match on
the Result from join_next() and handle Err(join_error) instead of unwrap: detect
Err from the JoinHandle (the outer await) and convert it into a channel-safe
error value (e.g., send Err(...) via the same queue/channel or an error variant
that includes the JoinError) so the stream surfaces the task failure instead of
crashing; adjust consumers of queue.insert to accept a Result<(generation,
req_idx, resps), JoinError> (or an enum) and update call sites around
set.try_join_next(), set.join_next(), and queue.insert accordingly.
In `@hyperfuel-client/src/util.rs`:
- Around line 107-110: The Schema is being rebuilt with metadata:
Default::default(), which drops any original schema metadata; update the
constructor used to build the new Schema so it preserves the original metadata
(e.g., set metadata: original_schema.metadata.clone() or metadata:
batch.schema().metadata().clone() instead of Default::default()), keeping the
same fields variable and resulting schema variable name.
In `@hyperfuel-format/src/types/transaction_type.rs`:
- Around line 10-24: The TransactionType newtype currently derives
derive_more::Add and derive_more::Sub which allow arithmetic on what should be
an identifier; remove derive_more::Add and derive_more::Sub from the derive list
on the TransactionType struct so it no longer supports addition/subtraction,
leaving Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, derive_more::From,
derive_more::Into, and derive_more::Deref intact; update any code or tests that
relied on adding/subtracting TransactionType to use the underlying value
(TransactionType.0) or an explicit conversion instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88824f02-23ee-4d8f-b39a-7ea4c36ca7d4
📒 Files selected for processing (58)
.github/workflows/publish.yamlCargo.tomlLICENSEREADME.mdexamples/asset-id/Cargo.tomlexamples/asset-id/README.mdexamples/asset-id/src/main.rsexamples/predicate-root/Cargo.tomlexamples/predicate-root/README.mdexamples/predicate-root/src/main.rsexamples/simple-logs/Cargo.tomlexamples/simple-logs/README.mdexamples/simple-logs/src/main.rsexamples/stream-logs/Cargo.tomlexamples/stream-logs/README.mdexamples/stream-logs/src/main.rshyperfuel-client/Cargo.tomlhyperfuel-client/src/column_mapping.rshyperfuel-client/src/config.rshyperfuel-client/src/filter.rshyperfuel-client/src/from_arrow.rshyperfuel-client/src/lib.rshyperfuel-client/src/parquet_out.rshyperfuel-client/src/parse_response.rshyperfuel-client/src/rayon_async.rshyperfuel-client/src/stream.rshyperfuel-client/src/transport_format.rshyperfuel-client/src/types.rshyperfuel-client/src/util.rshyperfuel-client/test-data/ens_token_abi.jsonhyperfuel-client/test-data/erc20.abi.jsonhyperfuel-client/test-data/nameless.abi.jsonhyperfuel-client/tests/api_test.rshyperfuel-format/Cargo.tomlhyperfuel-format/src/error.rshyperfuel-format/src/lib.rshyperfuel-format/src/types/data.rshyperfuel-format/src/types/fixed_size_data.rshyperfuel-format/src/types/mod.rshyperfuel-format/src/types/quantity.rshyperfuel-format/src/types/transaction_status.rshyperfuel-format/src/types/transaction_type.rshyperfuel-format/src/types/uint.rshyperfuel-format/src/types/util.rshyperfuel-format/test-data/arbitrum_tx_receipt.jsonhyperfuel-format/test-data/arbitrum_tx_receipt2.jsonhyperfuel-format/test-data/base_tx_receipt.jsonhyperfuel-format/test-data/block_with_tx.jsonhyperfuel-format/test-data/block_without_tx.jsonhyperfuel-format/test-data/log.jsonhyperfuel-format/test-data/transaction.jsonhyperfuel-format/test-data/transaction_receipt.jsonhyperfuel-format/tests/deserialize_json.rshyperfuel-net-types/Cargo.tomlhyperfuel-net-types/src/lib.rshyperfuel-schema/Cargo.tomlhyperfuel-schema/src/lib.rshyperfuel-schema/src/util.rs
💤 Files with no reviewable changes (23)
- examples/asset-id/Cargo.toml
- examples/stream-logs/README.md
- examples/predicate-root/src/main.rs
- hyperfuel-format/test-data/log.json
- examples/stream-logs/Cargo.toml
- examples/predicate-root/Cargo.toml
- examples/predicate-root/README.md
- examples/simple-logs/src/main.rs
- hyperfuel-format/test-data/arbitrum_tx_receipt.json
- examples/simple-logs/Cargo.toml
- examples/asset-id/src/main.rs
- hyperfuel-format/test-data/transaction.json
- hyperfuel-client/src/transport_format.rs
- hyperfuel-format/test-data/base_tx_receipt.json
- hyperfuel-format/test-data/transaction_receipt.json
- hyperfuel-format/tests/deserialize_json.rs
- hyperfuel-format/test-data/arbitrum_tx_receipt2.json
- examples/asset-id/README.md
- hyperfuel-format/test-data/block_without_tx.json
- examples/stream-logs/src/main.rs
- Cargo.toml
- examples/simple-logs/README.md
- hyperfuel-client/src/filter.rs
All four crates (hyperfuel-format, hyperfuel-schema, hyperfuel-net-types, hyperfuel-client) now have this repo + publish.yaml configured as a Trusted Publisher on crates.io. This commit updates the workflow to: - Request `id-token: write` permission so GitHub Actions issues the OIDC token the auth action exchanges for a temporary crates.io API token. - Run rust-lang/crates-io-auth-action@v1 instead of relying on the long-lived CARGO_REGISTRY_TOKEN secret. - Drop the stray duplicate `actions/checkout@v2` and unused VERBOSE env var, bump checkout to v4. After this PR merges, the CARGO_REGISTRY_TOKEN repo/org secret can be deleted - trusted publishing supersedes it. No GitHub environment is required by the trusted publisher config; one can be added later by setting it in both the crates.io publisher settings and as `environment:` on this job, if reviewer approval on publishes becomes desired. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fd7dfcf to
f3b76f8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish.yaml (1)
27-37: ⚡ Quick winPin GitHub Actions to immutable SHAs in publish workflow.
Lines 27–28 use mutable tags (
@v4,@v2) in the publish pipeline. Use full commit SHAs (40-character hashes) instead to strengthen supply-chain security.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yaml around lines 27 - 37, Replace mutable GitHub Action tags with immutable full commit SHAs: for the checkout step (actions/checkout@v4) and the rust cache step (Swatinem/rust-cache@v2) -- and any other workflow uses of tags like rust-lang/crates-io-auth-action@v1 -- update their uses: entries should reference the corresponding 40-character commit SHA for the exact release you want to lock to. Locate the occurrences of actions/checkout, Swatinem/rust-cache, and rust-lang/crates-io-auth-action in the publish workflow and substitute the @<tag> suffix with @<full-commit-sha> for each action to pin them immutably.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/publish.yaml:
- Around line 27-37: Replace mutable GitHub Action tags with immutable full
commit SHAs: for the checkout step (actions/checkout@v4) and the rust cache step
(Swatinem/rust-cache@v2) -- and any other workflow uses of tags like
rust-lang/crates-io-auth-action@v1 -- update their uses: entries should
reference the corresponding 40-character commit SHA for the exact release you
want to lock to. Locate the occurrences of actions/checkout,
Swatinem/rust-cache, and rust-lang/crates-io-auth-action in the publish workflow
and substitute the @<tag> suffix with @<full-commit-sha> for each action to pin
them immutably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b94c2f2-5af5-4050-8a86-8edbfacf7b87
📒 Files selected for processing (1)
.github/workflows/publish.yaml
What
Switches the manual `cargo publish` workflow from the long-lived `CARGO_REGISTRY_TOKEN` repo secret to crates.io's trusted publishing (OIDC).
All four crates - `hyperfuel-format`, `hyperfuel-schema`, `hyperfuel-net-types`, `hyperfuel-client` - have already been configured on crates.io to trust this repo + `publish.yaml` as a publisher (no GitHub environment requirement, matching the current lack of env-gating on the secret).
Changes
The `capnproto libcapnp-dev` install step is kept because `hyperfuel-net-types` still has `build.rs` running `capnpc`. Dmitri's earlier proposal to drop `build.rs` (originally PR #8) can be reapplied on top of the new baseline as a separate follow-up; the apt-install step can disappear then.
After this merges
Optional security upgrade (not in this PR)
If you ever want reviewer approval gating on publishes: create a `release` GitHub environment with required reviewers, add `environment: release` on the `publish_crates` job, and edit each crate's trusted publisher on crates.io to set the same environment name. Both sides must agree or publishes will fail.
Depends on
#9 (migration baseline). This PR's workflow file change is independent of that PR's content, but conceptually the trusted-publishing handoff makes most sense applied on top of the reconciled v2 baseline.
Summary by CodeRabbit