Skip to content

chore: depend on sentrix-proto crate, drop vendored proto#17

Merged
satyakwok merged 4 commits into
mainfrom
chore/depend-on-sentrix-proto-crate
May 13, 2026
Merged

chore: depend on sentrix-proto crate, drop vendored proto#17
satyakwok merged 4 commits into
mainfrom
chore/depend-on-sentrix-proto-crate

Conversation

@satyakwok
Copy link
Copy Markdown
Member

@satyakwok satyakwok commented May 13, 2026

🚧 Draft — blocked on `cargo publish -p sentrix-proto` from sentrix-labs/sentrix. CI will fail with `no matching package named 'sentrix-proto' found` until publish lands. Mark ready-for-review once sentrix-proto v0.1.0 is on crates.io.

Why

The proto schema lives in four places (chain server, sdk-rs, this crate, sentrix-explorer-v2). Drift had already started — the chain server's proto was 8979 bytes while this crate's was 8897 bytes as of 2026-05-13.

Cosmos solved this with `ibc-proto` on crates.io. The sentrix-labs/sentrix repo now ships a sibling `sentrix-proto` crate (PR sentrix-labs/sentrix#667 merged) as the single source of truth.

What

  • Drop `proto/sentrix.proto` (vendored copy)
  • Drop `build.rs` (tonic-build invocation)
  • Drop `tonic-prost` + `prost` + `tonic-prost-build` from explicit deps (now transitive via `sentrix-proto`)
  • Add `sentrix-proto = "0.1"`
  • `src/lib.rs` keeps `pub mod pb { pub use sentrix_proto::*; }` for backwards compat

Net: -279 / +9 lines.

Test plan

  • Local: `cargo build` clean (with path-dep)
  • Local: `cargo test --doc` clean
  • Local: `cargo clippy --all-targets -- -D warnings` clean
  • CI: blocks on sentrix-proto publish

After publish

  1. CI re-runs and goes green
  2. Bump `sentrix-grpc-wasm` 0.1.0-alpha.0 → 0.2.0-alpha.0 (follow-up commit on this branch)
  3. Merge
  4. Re-publish `sentrix-grpc-wasm` 0.2.0-alpha.0 to crates.io

Summary by CodeRabbit

  • Chores
    • Switched to a published protocol-definitions crate for protobuf/gRPC types.
    • Removed compile-time code generation and the associated build orchestration.
    • Replaced in-repo proto sources with re-exported, pre-generated protocol types to simplify maintenance and streamline builds.

Replace the vendored proto/sentrix.proto + the build.rs tonic-build
invocation with a dependency on the standalone `sentrix-proto` crate
that the chain repo (sentrix-labs/sentrix) publishes to crates.io.

This is the Cosmos `ibc-proto` pattern: one repo owns the .proto, every
consumer pulls the same generated types from crates.io. Eliminates the
schema drift that had already started across the four consumers.

The `pb` module stays as a thin re-export of `sentrix_proto`, so
existing consumers that path-into `sentrix_grpc_wasm::pb::*` don't
churn:

    pub mod pb { pub use sentrix_proto::*; }

Drops `tonic-prost` + `prost` + `tonic-prost-build` from explicit deps
— they're transitive through `sentrix-proto` now. `tonic` stays for
the codegen macros + transport.

Tested locally with a path-dep against the chain repo's sentrix-proto
crate: `cargo build / test --doc / clippy --all-targets -- -D warnings`
all clean.

NOTE: CI will fail on this PR until `cargo publish -p sentrix-proto`
runs from the chain repo.
@satyakwok satyakwok marked this pull request as ready for review May 13, 2026 16:46
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

The PR removes local protobuf generation: deletes proto/sentrix.proto and the build.rs script, replaces in-repo prost/tonic type generation with a dependency on the published sentrix-proto crate (added to Cargo.toml with default-features = false), removes direct prost/tonic-prost and build-dependencies, and updates src/lib.rs to re-export sentrix_proto::* instead of using tonic::include_proto!.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Sentriscloud/sentrix-grpc-wasm#12: Changes protobuf/gRPC generation pipeline; adds local tonic_prost_build usage in build.rs whereas this PR removes local generation in favor of the sentrix-proto crate.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely summarizes the main change: switching from vendored proto files to a published sentrix-proto crate dependency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/depend-on-sentrix-proto-crate

Comment @coderabbitai help to get the list of available commands and usage tips.

v0.1.0 of sentrix-proto pulled `tonic/transport` unconditionally, which
propagates a tokio-net / mio dep that doesn't compile for the
wasm32-unknown-unknown target.

v0.1.1 (sentrix-labs/sentrix#670) makes `transport` an opt-in feature.
This crate's browser-only consumer drops it via `default-features = false`
and uses `tonic-web-wasm-client` for the actual HTTP transport instead
of tonic's hyper-based Channel.

Tested locally:
- `cargo build --target wasm32-unknown-unknown` clean
- `cargo build` (native) clean
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@Cargo.toml`:
- Line 24: The dependency entry for sentrix-proto in Cargo.toml currently uses
version = "0.1" which permits 0.1.0; update the sentrix-proto dependency to
require version = "0.1.1" (keeping default-features = false) so the Cargo
manifest enforces the safe minimum release that includes the wasm-compatible
transport change.
🪄 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: b0f38b6b-f26a-42e5-ab47-6cac95f2bb5c

📥 Commits

Reviewing files that changed from the base of the PR and between eadbc94 and 91ec4a5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !**/*.lock
📒 Files selected for processing (1)
  • Cargo.toml

Comment thread Cargo.toml Outdated
sentrix-proto 0.1.0 pulled tonic/transport unconditionally and broke
wasm32 builds. 0.1.1 added the `transport` feature flag — that's the
version we ACTUALLY rely on. Bare `version = "0.1"` would let cargo
resolve to 0.1.0 on a fresh lock-file regen and reintroduce the
breakage.

Followup to CodeRabbit comment on PR #17.
@satyakwok satyakwok merged commit 3b1d61d into main May 13, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant