Chore/sdk alpha1 hardening#26
Merged
Merged
Conversation
Three stale claims fixed after the sentrix-proto extraction landed (PR #23 merged): 1. **grpc surface row** said "Pre-generated proto types committed; no protoc needed by consumers". src/grpc/pb.rs is gone — the grpc feature now depends on sentrix-proto from crates.io. Updated to reflect the new dep + the protoc build-time requirement. 2. **Status section** claimed "EVM (alloy) and gRPC (tonic) modules are doors-only stubs". Both are functional — SentrixGrpcClient has connect / get_latest_block / get_balance / get_validator_set / get_supply / get_mempool / subscribe_events all live in src/grpc/mod.rs. Reframed to "all six doors compile + have working client paths". 3. **Roadmap "Published to crates.io once feature surface stabilises"** was already published — checkbox was lying. Replaced with a forward-looking "next: surface stabilisation toward 1.0" line.
📝 WalkthroughWalkthroughThis PR prepares the sentrix-chain Rust SDK for v0.1.0-alpha.1 release. It increments the package version, adds CI automation for format, lint, and test validation across feature configurations, implements five runnable examples demonstrating native chain queries, EVM block queries, gRPC interactions, wallet signing, and websocket subscriptions. README content is refreshed to reflect alpha.1 status and updated surface descriptions, while inline documentation clarifies unit semantics (sentri for native amounts, display-SRX for supply values) and receiver patterns. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/rust.yml:
- Around line 39-58: Add CI steps to exercise common feature combinations and
per-feature tests: extend the existing cargo check/test matrix by adding runs
like cargo check --no-default-features --features "native,wallet" and cargo
check --no-default-features --features "evm,grpc" (and any other common pairs),
and add cargo test --no-default-features --features <feature> for features that
include tests (mirror the existing single-feature check steps but using cargo
test). Also add an optional job that runs the test matrix against the project's
MSRV (use the same cargo check/test commands under that Rust toolchain) so
combinations and feature-specific tests are validated against the supported
compiler version; update job names to reflect combination coverage (e.g., "cargo
check native,wallet", "cargo test evm,grpc") so the new steps are easy to locate
by name.
- Around line 19-22: Add a caching step after the "Install Rust" step (the step
using dtolnay/rust-toolchain@stable) that runs the Swatinem/rust-cache action to
cache the Cargo registry and build artifacts (e.g., ~/.cargo and target/) so
dependencies and compiled outputs are reused across CI runs; reference the
existing "Install Rust" step to place the new cache action immediately after it
and configure it to restore before builds and save after successful build steps.
- Around line 39-61: Add a new CI step immediately after the existing "cargo
check all features" step to verify the example programs compile by running cargo
check --examples --all-features (so the examples with required-features like
native/evm/grpc/bft/wallet are built); update the workflow to include this
"cargo check examples all features" step (mirroring the naming pattern used for
other steps) so examples are validated in CI.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b325708a-4ff7-453f-a528-6a7e65be7c9d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (12)
.github/workflows/rust.ymlCargo.tomlREADME.mdexamples/chain_info.rsexamples/evm_block_number.rsexamples/grpc_latest_block.rsexamples/sign_native_transfer.rsexamples/websocket_subscribe.rssrc/bft.rssrc/grpc/mod.rssrc/lib.rssrc/native.rs
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 @.github/workflows/rust.yml:
- Around line 78-79: The CI step running the cargo doc command should treat
documentation warnings as errors; update the step that runs "cargo doc
--all-features --no-deps" to include "-D warnings" (i.e. "cargo doc -D warnings
--all-features --no-deps") so doc warnings fail the build, matching the clippy
enforcement used elsewhere.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 590722e3-f50c-4f8f-a2fd-308537bf0e4b
📒 Files selected for processing (1)
.github/workflows/rust.yml
Comment on lines
+78
to
+79
| - name: cargo doc all features | ||
| run: cargo doc --all-features --no-deps |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider enforcing documentation warnings.
The cargo doc command doesn't include -D warnings, so documentation issues (broken links, invalid syntax, missing docs on public items) will not fail the CI build. For consistency with the clippy step (line 34) and to maintain doc quality, consider treating doc warnings as errors.
📚 Suggested enhancement
- name: cargo doc all features
- run: cargo doc --all-features --no-deps
+ run: cargo doc --all-features --no-deps -- -D warnings📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: cargo doc all features | |
| run: cargo doc --all-features --no-deps | |
| - name: cargo doc all features | |
| run: cargo doc --all-features --no-deps -- -D warnings |
🤖 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/rust.yml around lines 78 - 79, The CI step running the
cargo doc command should treat documentation warnings as errors; update the step
that runs "cargo doc --all-features --no-deps" to include "-D warnings" (i.e.
"cargo doc -D warnings --all-features --no-deps") so doc warnings fail the
build, matching the clippy enforcement used elsewhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed and why. 1-3 sentences.
Scope
Checks
forge buildcleanforge testgreen (including fuzz + invariant)forge fmt --checkcleanslither --no-fail-pedanticreviewed (no new high-severity findings)make storageand inspectdocs/storage/*.json)Linked issue
Closes #
Deploy impact
RELEASES.mdSummary by CodeRabbit
New Features
Documentation
Chores
Bug Fixes