chore: upgrade tonic 0.9 → 0.10, prost 0.11 → 0.12, force BTreeMap on proto maps#280
Open
prasanna-anchorage wants to merge 2 commits into
Open
chore: upgrade tonic 0.9 → 0.10, prost 0.11 → 0.12, force BTreeMap on proto maps#280prasanna-anchorage wants to merge 2 commits into
prasanna-anchorage wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Upgrades the gRPC/protobuf codegen stack (tonic/prost) to enable btree_map generation for proto map<…, …> fields, enforcing the project’s determinism invariant by moving proto maps (and downstream callers) from HashMap to BTreeMap.
Changes:
- Bump
tonic/tonic-build/tonic-reflectionto0.10andprost/prost-typesto0.12. - Update codegen to emit
BTreeMapfor all protomapfields viatonic_build::Builder::btree_map(["."]), regeneratingsrc/generated/.... - Mechanical updates across crates/tests to use
BTreeMap/BTreeSet, plus small tonic 0.10 API migrations (enum conversions, generated server stubs).
Reviewed changes
Copilot reviewed 51 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/parser/grpc-server/src/main.rs | Update tonic status code conversion for tonic 0.10. |
| src/parser/cli/src/solana.rs | Switch IDL mapping construction from HashMap to BTreeMap. |
| src/parser/cli/src/mapping_parser.rs | Return BTreeMap from mapping loader and update docs accordingly. |
| src/parser/cli/src/ethereum.rs | Switch ABI mapping construction from HashMap to BTreeMap. |
| src/parser/cli/src/chains.rs | Use BTreeMap for deterministic chain string mapping. |
| src/parser/app/src/routes/parse.rs | Migrate proto enum conversion to TryFrom<i32> and update tests to BTreeMap. |
| src/integration/tests/signature_metadata_e2e.rs | Update ABI mappings to BTreeMap to match proto type changes. |
| src/integration/Cargo.toml | Bump tonic and prost dependency versions. |
| src/host_primitives/Cargo.toml | Bump tonic and prost dependency versions. |
| src/generated/src/generated/parser.rs | Regenerated code: proto map fields now use BTreeMap; tonic server stub generation updated. |
| src/generated/src/generated/health.rs | Regenerated code for tonic 0.10 server stubs. |
| src/generated/src/generated/grpc.health.v1.rs | Regenerated code: stream trait path updates for tonic 0.10. |
| src/generated/src/generated/google.rpc.rs | Regenerated docs/escaping and add @generated header. |
| src/generated/src/generated/_include.rs | Add @generated header. |
| src/generated/Cargo.toml | Bump generated crate dependencies (prost*, tonic*). |
| src/codegen/src/main.rs | Configure codegen to emit BTreeMap for all proto map fields via .btree_map(["."]). |
| src/codegen/Cargo.toml | Bump tonic-build to 0.10. |
| src/chain_parsers/visualsign-sui/src/utils/test_helpers.rs | Switch fixture/test data maps to BTreeMap for deterministic iteration. |
| src/chain_parsers/visualsign-sui/src/core/mod.rs | Update integration config data to BTreeMap-based structure. |
| src/chain_parsers/visualsign-sui/src/core/chain_config.rs | Build config maps using BTreeMap in macro expansion. |
| src/chain_parsers/visualsign-solana/tests/real_idl_validation.rs | Replace HashMap/HashSet with BTreeMap/BTreeSet in tests. |
| src/chain_parsers/visualsign-solana/tests/common/mod.rs | Switch IDL mappings in test helpers to BTreeMap. |
| src/chain_parsers/visualsign-solana/src/utils/mod.rs | Make token lookup table a BTreeMap for deterministic iteration. |
| src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs | Return deterministic BTreeMap of named accounts instead of iterating upstream HashMap. |
| src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/token_2022/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/system/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/swig_wallet/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/stakepool/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/onre_app/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/neutral_trade/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/kamino_vault/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/kamino_farms/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/compute_budget/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/presets/associated_token_account/config.rs | Use BTreeMap for integration config storage. |
| src/chain_parsers/visualsign-solana/src/idl/mod.rs | Store IDL registry maps as BTreeMap (configs/names/idl_names). |
| src/chain_parsers/visualsign-solana/src/core/visualsign.rs | Extract proto IDL mappings into a BTreeMap (aligning with generated types). |
| src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs | Build upstream HashMap argument without naming HashMap to avoid disallowed-types lint leakage. |
| src/chain_parsers/visualsign-solana/src/core/mod.rs | Update integration config data model to BTreeMap-based structure. |
| src/chain_parsers/visualsign-ethereum/tests/lib_test.rs | Switch ABI mappings in tests to BTreeMap. |
| src/chain_parsers/visualsign-ethereum/src/visualizer.rs | Use BTreeMap for registry storage of visualizers. |
| src/chain_parsers/visualsign-ethereum/src/token_metadata.rs | Use BTreeMap for assets map in token metadata. |
| src/chain_parsers/visualsign-ethereum/src/registry.rs | Convert multiple registries to BTreeMap and derive Ord for WellKnownAddress for key usage. |
| src/chain_parsers/visualsign-ethereum/src/networks.rs | Replace HashSet with BTreeSet in uniqueness tests. |
| src/chain_parsers/visualsign-ethereum/src/abi_registry.rs | Switch ABI registry internal maps to BTreeMap behind Arc. |
| src/chain_parsers/visualsign-ethereum/src/abi_metadata.rs | Update helper to return BTreeMap for ABI mappings in tests. |
| src/Cargo.lock | Lockfile update reflecting dependency bumps and transitive changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
98
to
100
| /// Verify that `metadata_digest` is deterministic for identical metadata, | ||
| /// including non-empty `abi_mappings` (exercises `HashMap` key ordering through borsh). | ||
| #[test] |
| @@ -90,7 +90,7 @@ impl SolanaTransactionWrapper { | |||
| /// Extract IDL mappings from VisualSignOptions metadata | |||
| /// | |||
| /// Returns a HashMap of program_id (base58 string) -> (IDL JSON string, program name) | |||
prasanna-anchorage
added a commit
that referenced
this pull request
May 5, 2026
… doc comments Two comment-only fixes: - `extract_idl_mappings` doc said "Returns a HashMap"; the function now returns `BTreeMap`. Updated to match the signature and added a brief note on why `BTreeMap` is required (downstream iteration order would otherwise leak into rendered SignablePayload output). - `metadata_digest_is_deterministic` test doc said "exercises `HashMap` key ordering through borsh"; the field is now a `BTreeMap` after the `tonic_build::Builder::btree_map(["."])` config. Updated to describe the actual contract under test. No code changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… proto maps
Bumps the gRPC + protobuf codegen stack to the smallest hop that exposes
`tonic_build::Builder::btree_map`. With that available we configure
`.btree_map(["."])` so every proto `map<…, …>` field generates as a
`BTreeMap` instead of a `HashMap`, aligning with the project's
"BTreeMap for proto maps" determinism invariant (CLAUDE.md /
visualsign-solana clippy.toml).
Version bumps:
- tonic 0.9.2 → 0.10.2 (4 Cargo.toml files)
- tonic-build 0.9.2 → 0.10.2
- tonic-reflection 0.9.2 → 0.10.2
- prost 0.11.9 → 0.12.6 (4 Cargo.toml files)
- prost-types 0.11.9 → 0.12.6
Generated code changes:
- `parser.rs`: `abi_mappings` and `idl_mappings` now use BTreeMap.
- All other proto map fields likewise.
Knock-on changes (mostly mechanical):
- Workspace-wide `HashMap` → `BTreeMap` in chain_parsers, parser_cli,
integration, examples — needed because callers that build
`EthereumMetadata { abi_mappings: ... }` /
`SolanaMetadata { idl_mappings: ... }` must now pass `BTreeMap`.
- `WellKnownAddress` enum (visualsign-ethereum/src/registry.rs) now
derives `PartialOrd, Ord` so it can be a `BTreeMap` key.
- `presets/unknown_program/mod.rs::try_parse_with_idl` now returns a
`(SolanaParsedInstructionData, BTreeMap<String, String>)` tuple
instead of mutating the upstream `parsed.named_accounts` field
(which is a `HashMap`); callers iterate the local `BTreeMap` for
deterministic field order. Same shape as the `meteora_damm_v2`
visualizer's existing pattern.
- `core/txtypes/v0.rs::decode_v0_transfers` builds the `custom_idls`
argument via `std::iter::once(...).collect()` so the concrete
`HashMap` type (required by upstream `solana_parser::parse_transaction`)
is inferred at the call site — no `HashMap` name leaks into our
source. Iteration order doesn't escape the function.
- `proto_chain` parsing in `parser_app/routes/parse.rs` switched from
the deprecated `Chain::from_i32` to `TryFrom<i32>` (tonic 0.10
removed `from_i32` in favor of `TryFrom`).
- `tonic::Code::from_i32` → `tonic::Code::from` in
`parser_grpc_server/main.rs` (same deprecation).
Verification:
- `cargo build --workspace` succeeds.
- `cargo clippy --workspace --all-targets -- -D warnings` clean.
- `cargo test --workspace --lib` passes (all 436 library tests).
- `cargo fmt --check` clean.
Unblocks the follow-up `clippy::disallowed_types` PR
(visualsign-solana clippy.toml banning HashMap/HashSet) — proto-gen
is now compatible with that rule out of the box.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… doc comments Two comment-only fixes: - `extract_idl_mappings` doc said "Returns a HashMap"; the function now returns `BTreeMap`. Updated to match the signature and added a brief note on why `BTreeMap` is required (downstream iteration order would otherwise leak into rendered SignablePayload output). - `metadata_digest_is_deterministic` test doc said "exercises `HashMap` key ordering through borsh"; the field is now a `BTreeMap` after the `tonic_build::Builder::btree_map(["."])` config. Updated to describe the actual contract under test. No code changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
prasanna-anchorage
force-pushed
the
prasanna/tonic-prost-upgrade
branch
from
501ce2d to
fb4df66
Compare
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
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.
Why
Step 1 of 2 to land the
clippy::disallowed_typesenforcement forHashMap/HashSetinvisualsign-solana. The proto-generated structs ingenerated/declare map fields asHashMap, which (a) violates the project's "BTreeMap for proto maps" determinism invariant (CLAUDE.md), and (b) blocks the lint because the proto types leak into all callers.tonic-build 0.9doesn't exposeprost-build'sbtree_mapconfig — that was added in0.10. So the smallest-hop fix is bumping the gRPC + protobuf codegen stack to versions that expose it.What changed
Version bumps (4 Cargo.toml files):
tonic0.9.2 → 0.10.2tonic-build0.9.2 → 0.10.2tonic-reflection0.9.2 → 0.10.2prost0.11.9 → 0.12.6prost-types0.11.9 → 0.12.6Codegen change:
codegen/src/main.rsnow calls.btree_map(["."])on thetonic_build::Builder— every protomap<…, …>field generates asBTreeMapinstead ofHashMap. Verified ingenerated/src/generated/parser.rs:```rust
pub abi_mappings: ::prost::alloc::collections::BTreeMap<...>
pub idl_mappings: ::prost::alloc::collections::BTreeMap<...>
```
Knock-on changes (mostly mechanical sed-converted, some hand fixes):
HashMap→BTreeMapinchain_parsers,parser_cli,integration,examples— necessary because callers buildingEthereumMetadata { abi_mappings: ... }orSolanaMetadata { idl_mappings: ... }must now passBTreeMap.WellKnownAddressenum (visualsign-ethereum/src/registry.rs) now derivesPartialOrd, Ordso it can be aBTreeMapkey.presets/unknown_program/mod.rs::try_parse_with_idlnow returns a(SolanaParsedInstructionData, BTreeMap<String, String>)tuple instead of mutating the upstreamparsed.named_accountsfield (which isHashMapfromsolana_parser); callers iterate the localBTreeMapfor deterministic field order. Same shape as the existingmeteora_damm_v2visualizer pattern.core/txtypes/v0.rs::decode_v0_transfersbuilds thecustom_idlsargument viastd::iter::once(...).collect()so the concreteHashMaptype (required by upstreamsolana_parser::parse_transaction) is inferred at the call site — noHashMapname leaks into our source. Iteration order doesn't escape the function.Chain::from_i32→TryFrom<i32>inparser_app/routes/parse.rs(tonic 0.10 deprecatedfrom_i32).tonic::Code::from_i32→tonic::Code::frominparser_grpc_server/main.rs(same deprecation).Test plan
cargo build --workspacesucceedscargo clippy --workspace --all-targets -- -D warningscleancargo test --workspace --libpasses (all 436 library tests)cargo fmt --checkcleanFollow-up
Step 2: a separate PR adds
clippy.tomltovisualsign-solanawithdisallowed-types = [HashMap, HashSet], banning their reintroduction crate-wide. Now that proto-gen emitsBTreeMap, the lint will land cleanly.🤖 Generated with Claude Code