Pre-publish hardening and minor version bumps#11
Conversation
Addresses CodeRabbit findings on the v2-import PR (#9) that were worth fixing before the next publish; deliberately skips theoretical/cosmetic ones and defers the larger FromArrow trait Result-typing refactor. Crate version bumps (all minor): - hyperfuel-net-types 4.0.3 -> 4.1.0 - hyperfuel-format 4.0.1 -> 4.1.0 - hyperfuel-schema 4.0.0 -> 4.1.0 - hyperfuel-client 3.0.1 -> 3.1.0 Security / correctness fixes: - hyperfuel-client/src/parse_response.rs: restore bounded capnp limits (nesting_limit(64), traversal_limit_in_words(Some(64M)) ~= 512 MiB). Previous nesting_limit(i32::MAX) + traversal_limit_in_words(None) removed the DoS safeguards capnp provides specifically for untrusted network input. - hyperfuel-format/src/types/quantity.rs: From<u64> now strips leading zeros to preserve the no-leading-zero invariant enforced by the Vec<u8>/&[u8] constructors. Previously Quantity::from(5u64) stored [0,0,0,0,0,0,0,5] which broke Eq/Hash/round-trip equality against the canonical form. New test_from_u64_canonical covers this. - hyperfuel-client/src/lib.rs get_arrow_impl: swap block_in_place for spawn_blocking. block_in_place panics on tokio current-thread runtimes (#[tokio::test], flavor = "current_thread"), making the library hostile to common configurations. - hyperfuel-client/src/lib.rs get_height_impl: propagate missing height instead of unwrap_or(0). Previously a malformed response collapsed into a successful "genesis height" result, hiding bad data from retry / health-check loops. Schema API improvement (additive, minor-safe): - hyperfuel-schema/src/util.rs: add try_project_schema returning Result<Schema> that errors when requested columns are absent. The existing project_schema is unchanged (still silently drops unknown fields) for backward compatibility. New code should prefer try_project_schema to catch typos in field selections. Cleanup: - hyperfuel-client/Cargo.toml: remove tokio's test-util feature from main deps (was unused; bringing testing-only timer-mocking machinery into production builds). - hyperfuel-client/src/column_mapping.rs: fix doc comments on receipt / input / output (were labeled "log data" / "trace data" / "decoded log data" from the EVM-client copy-paste). Fix "colum" typo in error message. - hyperfuel-client/tests/api_test.rs: deleted. 523 lines of commented-out tests that referenced hypersync_client / hypersync_format (wrong crate names from the v2 import). Track restoring API test coverage in a follow-up issue if wanted. Examples: - Restore examples/simple-logs (workspace member), ported to the current ClientConfig API. Used as a live smoke test against https://fuel.hypersync.xyz: archive_height returned correctly, full HTTP -> capnp -> spawn_blocking -> arrow path exercised. Verified: cargo build --workspace --all-targets, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace (34 tests pass), cargo run -p simple-logs against the live server. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR canonicalizes Quantity encoding with tests, adds try_project_schema for validated schema projection, tightens Cap'n Proto parsing limits and improves client async/error handling, corrects small client docs/typos, adds a simple-logs example, and updates several crate versions. ChangesQuantity canonicalization and library updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-format/src/types/quantity.rs`:
- Around line 217-223: In test_from_u64_canonical, rustfmt requires the long
assert_eq! to be split across multiple lines; update the final
assert_eq!(Quantity::from(u64::MAX), Quantity::from(hex!("ffffffffffffffff")));
in the test to a multi-line assert_eq! with the first argument on its own line,
the second argument on the next line, and the closing parenthesis on its own
line so the test function (test_from_u64_canonical) passes cargo fmt --check.
🪄 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: a7947a7a-f0dc-48d0-b2c4-ff73e44c085a
📒 Files selected for processing (14)
Cargo.tomlexamples/simple-logs/Cargo.tomlexamples/simple-logs/src/main.rshyperfuel-client/Cargo.tomlhyperfuel-client/src/column_mapping.rshyperfuel-client/src/lib.rshyperfuel-client/src/parse_response.rshyperfuel-client/tests/api_test.rshyperfuel-format/Cargo.tomlhyperfuel-format/src/types/quantity.rshyperfuel-net-types/Cargo.tomlhyperfuel-schema/Cargo.tomlhyperfuel-schema/src/lib.rshyperfuel-schema/src/util.rs
💤 Files with no reviewable changes (1)
- hyperfuel-client/tests/api_test.rs
Splits the long assert_eq! in test_from_u64_canonical across multiple lines to satisfy cargo fmt --check. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cleans up the CodeRabbit findings on PR #9 that were worth fixing before the next publish, plus minor version bumps so the four crates can be released.
Version bumps
Security / correctness
parse_response.rscapnp limits restored. Previouslynesting_limit(i32::MAX)+traversal_limit_in_words(None)removed the DoS safeguards capnp provides for untrusted network input. Now bounded at64nesting /64M words(~512 MiB), enough for large paginated arrow responses but no longer effectively unbounded.Quantity::From<u64>canonical. Previously stored 8 bytes with leading zeros, breaking the no-leading-zero invariant enforced by theVec<u8>/&[u8]constructors.Quantity::from(5u64) != Quantity::from(vec![5])was the bug; newtest_from_u64_canonicalcovers the fix.get_arrow_implusesspawn_blocking.tokio::task::block_in_placepanics on current-thread runtimes (#[tokio::test],flavor = "current_thread"). Library code shouldn't make that assumption.get_height_implpropagates missing height. Previouslyunwrap_or(0)turned malformed / partial responses into a successful "genesis height" result, hiding bad data from retry and health-check loops.Additive schema API (minor-safe)
hyperfuel_schema::try_project_schemaadded, returningResult<Schema>with an error listing any requested columns not present in the source schema. The existingproject_schemais unchanged for backward compatibility; new code should prefer thetry_variant to catch typos infield_selection.Cleanup
tokio'stest-utilfeature fromhyperfuel-client's main deps (was unused; pulled timer-mocking machinery into production builds).ColumnMapping.{receipt,input,output}(had been labeled "log data" / "trace data" / "decoded log data" from the EVM-client copy-paste).hyperfuel-client/tests/api_test.rs(523 lines of commented-out tests still referencinghypersync_client/hypersync_formatfrom the v2 import). Restoring API test coverage can be tracked in a follow-up issue.Examples restored
examples/simple-logsadded back as a workspace member, ported from v1 to the currentClientConfig+preset_query_get_logsAPI. Used as a live smoke test in this PR:cargo run -p simple-logsreturnedarchive_height=Some(53810724)againsthttps://fuel.hypersync.xyz, exercising the full HTTP → capnp →spawn_blocking→ arrow path against the production server.Deliberately skipped (defer or won't-fix)
FromArrowtrait silently panic-instead-of-Result (from_arrow.rs:14/159) andparquet_out.rs:331unwrap onarray_to_columns: real tech debt acknowledged in source comments. Restoring proper Result-typing touches multiple files and is a larger refactor; defer to a tracking issue.map_l1_fee_scalartriple unwrap: same family as above (needs caller updates).Utf8Viewnot handled inconcat_chunksworkaround: worth verifying with a test against the real schema before fixing; not exercised by any current test.rayon_async::spawnT: Syncbound,TransactionTypeAdd/Subderives,util.rsschema metadata discard,stream.rsdouble-unwrap onJoinError,README.mdheading level,lib.rs:74reqwest::Client::builder().build().unwrap(),lib.rs:294extra retry sleep before final return.Test plan
cargo build --workspace --all-targetscargo clippy --workspace --all-targets -- -D warningscargo test --workspace(34 tests pass, including newtest_from_u64_canonical)cargo run -p simple-logsagainst the livefuel.hypersync.xyzserverSummary by CodeRabbit
New Features
Bug Fixes
Improvements
Chores