Update Rust protos, dependencies enhancements #2538
Conversation
WalkthroughThe pull request includes extensive updates across multiple files in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Outside diff range and nitpick comments (15)
v4-proto-rs/src/lib.rs (1)
Line range hint 27-46: Consider enhancing test coverage
While the tests verify the type_url, consider adding assertions to verify the encoded value field of the Any type to ensure complete serialization correctness.
Here's a suggested enhancement:
#[test]
pub fn test_any_conversion() {
let msg = MsgCancelOrder {
order_id: None,
good_til_oneof: None,
};
let any = msg.to_any();
let url = "/dydxprotocol.clob.MsgCancelOrder";
assert_eq!(any.type_url, url);
+ // Verify the encoded value matches the original message
+ let encoded = msg.encode_to_vec();
+ assert_eq!(any.value, encoded);
}v4-proto-rs/src/dydxprotocol.indexer.redis.rs (1)
Line range hint 1-100: Consider documenting the proto generation process.
Since this is a generated file that has received style improvements, it would be helpful to document how these customizations are achieved through the protobuf generation process. This will help future contributors understand how to properly modify the generated code.
Consider adding:
- Documentation about the proto generation setup
- Instructions for regenerating protos
- Guidelines for making changes to generated files
v4-proto-rs/README.md (1)
9-9: Document the new grpc-transport feature.
Consider adding documentation about the new grpc-transport feature and its implications. This would help users understand how to optionally eliminate the tonic::transport dependency for WASM compilation.
Here's a suggested addition to the Usage section:
[dependencies]
dydx-proto = "0.2"
+
+# Features
+# The default features include tonic transport. For WASM compatibility,
+# you can disable it and use the alternative transport:
+[dependencies.dydx-proto]
+version = "0.2"
+default-features = false
+features = ["grpc-transport"]v4-proto-rs/src/dydxprotocol.feetiers.rs (1)
239-241: Consider using a more specific error status.
The change to Status::unknown is consistent but consider using more specific status codes like Status::unavailable for service readiness errors.
- tonic::Status::unknown(
+ tonic::Status::unavailable(
format!("Service was not ready: {}", e.into()),
)Also applies to: 266-268, 419-421
v4-proto-rs/src/dydxprotocol.blocktime.rs (2)
3-3: LGTM! Consider applying Copy trait consistently across similar structs.
The addition of the Copy trait to these structs is appropriate as they contain only primitive types or other Copy types. This change aligns with the PR's objective of enhancing WASM compatibility.
Consider deriving Copy for similar structs that meet the same criteria, such as:
QueryDowntimeParamsResponseQueryAllDowntimeInfoResponse
This would maintain consistency across the codebase for structs with similar characteristics.
Also applies to: 32-32, 96-96, 127-127, 141-141, 158-158, 384-384
203-203: LGTM! Well-structured gRPC client implementation with optional transport.
The changes effectively implement the new grpc-transport feature, making the transport layer optional for WASM compatibility. The explicit std::marker::Send bounds improve type safety and clarity.
The separation of transport concerns through feature flags is a good architectural decision that:
- Reduces dependencies when transport isn't needed
- Enables WASM compilation by making transport optional
- Maintains compatibility with existing transport-dependent code
Also applies to: 412-412, 219-220, 428-429
v4-proto-rs/src/dydxprotocol.indexer.protocol.v1.rs (1)
Line range hint 366-374: Address TODO comments regarding CLOB pair status behaviors
The documentation for several ClobPairStatus variants is marked with TODOs (DEC-600) and described as "unfinalized". These should be updated with the finalized behavior.
Would you like me to help create a GitHub issue to track the documentation updates for the CLOB pair status behaviors?
v4-proto-rs/src/dydxprotocol.bridge.rs (1)
30-30: Consider the performance implications of deriving Copy.
While adding the Copy trait to these structs allows for more flexible usage patterns, be mindful that some of these structs contain Option<T> fields. For larger structs or those with complex nested types, copying might be less efficient than moving. Consider benchmarking the performance impact if these structs are frequently copied in performance-critical paths.
Also applies to: 75-75, 111-111, 176-176, 208-208, 240-240, 272-272, 304-304, 544-544, 573-573, 587-587, 603-603, 616-616, 633-633, 647-647, 664-664, 678-678
v4-proto-rs/build.rs (1)
33-35: Simplify the environment variable check
The condition if !std::env::var("V4_PROTO_REBUILD").is_ok() can be simplified for clarity by using is_err().
Apply this diff to simplify the condition:
-if !std::env::var("V4_PROTO_REBUILD").is_ok() {
+if std::env::var("V4_PROTO_REBUILD").is_err() {v4-proto-rs/src/dydxprotocol.listing.rs (1)
327-332: Clarify abbreviation in documentation for MsgSetListingVaultDepositParams
The comment for MsgSetListingVaultDepositParams mentions "PML megavault deposits." It's unclear what "PML" stands for. Expanding the abbreviation or providing additional context would improve the readability and understanding of the code.
Consider updating the comment as follows:
-/// MsgSetListingVaultDepositParams is a message used to set PML megavault
-/// deposit params
+/// MsgSetListingVaultDepositParams is a message used to set the parameters
+/// for permissionless market listing (PML) megavault deposits.v4-proto-rs/src/dydxprotocol.prices.rs (1)
58-60: Clarify Deprecation Status of exponent Field in MarketPrice
The comments indicate that the exponent value is determined from the market map as of v7.1.x. It's unclear whether this field is deprecated or still in use. Consider adding a deprecation notice or removing the field if it's no longer needed to avoid confusion.
v4-proto-rs/src/dydxprotocol.accountplus.rs (1)
73-76: Clarify the description of is_smart_account_active in Params struct
Consider rephrasing the comment for is_smart_account_active to better reflect its purpose.
Apply the following diff to improve clarity:
/// IsSmartAccountActive defines the state of the authenticator.
-/// If set to false, the authenticator module will not be used
-/// and the classic cosmos sdk authentication will be used instead.
+/// Indicates whether the smart account functionality is active.
+/// If set to `false`, the smart account module will be disabled,
+/// and the classic Cosmos SDK authentication will be used instead.v4-proto-rs/src/dydxprotocol.indexer.events.rs (1)
Line range hint 940-957: Deprecated Events Should Be Managed Appropriately
The OpenInterestUpdateEventV1 and OpenInterestUpdate messages are marked as deprecated. Ensure that all references to these messages are updated, and consider removing them in future releases to keep the codebase clean.
Would you like me to help identify all usages of these deprecated messages or create a plan for their removal?
v4-proto-rs/src/dydxprotocol.vault.rs (1)
Line range hint 194-235: Consider removing deprecated Params struct to improve maintainability
The Params struct defined between lines 195~ to 235~ is marked as deprecated since version v6.x and has been replaced by QuotingParams. If Params is no longer in use, consider removing it to reduce code clutter and prevent potential confusion.
v4-proto-rs/src/dydxprotocol.clob.rs (1)
825-828: Consistency in TimeInForce enum string representations
The as_str_name method includes all TimeInForce variants, including the deprecated FillOrKill. Review if the deprecated variant should be included here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (40)
- v4-proto-rs/Cargo.toml (2 hunks)
- v4-proto-rs/README.md (1 hunks)
- v4-proto-rs/build.rs (1 hunks)
- v4-proto-rs/src/_includes.rs (3 hunks)
- v4-proto-rs/src/cosmos.base.query.v1beta1.rs (0 hunks)
- v4-proto-rs/src/cosmos.base.v1beta1.rs (0 hunks)
- v4-proto-rs/src/cosmos_proto.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.accountplus.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.affiliates.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.assets.rs (9 hunks)
- v4-proto-rs/src/dydxprotocol.blocktime.rs (17 hunks)
- v4-proto-rs/src/dydxprotocol.bridge.rs (34 hunks)
- v4-proto-rs/src/dydxprotocol.clob.rs (68 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.bridge.rs (5 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.liquidation.rs (5 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs (5 hunks)
- v4-proto-rs/src/dydxprotocol.delaymsg.rs (15 hunks)
- v4-proto-rs/src/dydxprotocol.epochs.rs (5 hunks)
- v4-proto-rs/src/dydxprotocol.feetiers.rs (11 hunks)
- v4-proto-rs/src/dydxprotocol.govplus.rs (9 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.events.rs (9 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.indexer_manager.rs (2 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.off_chain_updates.rs (3 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.protocol.v1.rs (7 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.redis.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.shared.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.socks.rs (2 hunks)
- v4-proto-rs/src/dydxprotocol.listing.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.perpetuals.rs (30 hunks)
- v4-proto-rs/src/dydxprotocol.prices.rs (23 hunks)
- v4-proto-rs/src/dydxprotocol.ratelimit.rs (13 hunks)
- v4-proto-rs/src/dydxprotocol.revshare.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.rewards.rs (10 hunks)
- v4-proto-rs/src/dydxprotocol.sending.rs (15 hunks)
- v4-proto-rs/src/dydxprotocol.stats.rs (24 hunks)
- v4-proto-rs/src/dydxprotocol.subaccounts.rs (12 hunks)
- v4-proto-rs/src/dydxprotocol.vault.rs (11 hunks)
- v4-proto-rs/src/dydxprotocol.vest.rs (11 hunks)
- v4-proto-rs/src/google.api.rs (0 hunks)
- v4-proto-rs/src/lib.rs (3 hunks)
💤 Files with no reviewable changes (3)
- v4-proto-rs/src/cosmos.base.query.v1beta1.rs
- v4-proto-rs/src/cosmos.base.v1beta1.rs
- v4-proto-rs/src/google.api.rs
🔇 Additional comments (159)
v4-proto-rs/Cargo.toml (4)
3-3: LGTM: Version bump is appropriate.
The version bump to 0.2.0 aligns with semver guidelines, reflecting the addition of new features and significant dependency updates.
23-26: Please provide context for the new regex dependency.
While the version updates and alignment between tonic-buf-build, prost-build, and tonic-build look good, could you explain why the new regex dependency was added to build dependencies?
Let's check the regex usage:
#!/bin/bash
# Description: Check regex usage in build scripts
# Test: Look for regex usage in build.rs
# Expect: Find regex usage that justifies the dependency
rg -t rust "regex::" build.rs17-20: Verify compatibility between cosmos-sdk-proto and other dependencies.
The dependency updates and feature restrictions look good, but we should verify compatibility between cosmos-sdk-proto 0.26 and the other dependencies, particularly tonic 0.12 and prost 0.13.
Let's check the dependency compatibility:
✅ Verification successful
Dependencies are compatible and properly aligned
The dependency versions are well-aligned and compatible:
prostandprost-typesare both at v0.13prost-buildis at v0.13, matching the runtime dependencytonicandtonic-buildare both at v0.12cosmos-sdk-proto v0.26is compatible with these versions as it's built against the same toolchain
The build dependencies and runtime dependencies are properly synchronized, following Rust best practices for proto generation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify dependency compatibility
# Test: Check if cosmos-sdk-proto 0.26 is compatible with our prost and tonic versions
# Expect: No conflicting version requirements
cargo tree | grep -E "cosmos-sdk-proto|prost|tonic"
Length of output: 278
Script:
#!/bin/bash
# Let's try a different approach by checking the Cargo.toml files for dependencies
# and their version requirements
# First find all Cargo.toml files
fd Cargo.toml | while read -r cargo_file; do
echo "=== $cargo_file ==="
# Extract dependencies sections and grep for our target dependencies
rg -U "^\[.*dependencies.*\](\n.*)*" "$cargo_file" | rg -A20 "cosmos-sdk-proto|prost|tonic"
done
Length of output: 689
12-14: Verify WASM compilation without grpc-transport feature.
The new feature flag design looks good and aligns with the PR objectives. The optional grpc-transport feature should enable WASM compilation when disabled.
Let's verify the feature flag usage:
✅ Verification successful
The results show that all tonic::transport usages are properly guarded with #[cfg(feature = "grpc-transport")] attributes across the codebase. This means that when the grpc-transport feature is disabled, all transport-related code will be excluded from compilation, making it compatible with WASM targets.
Feature flag implementation correctly enables WASM compilation
The codebase properly guards all gRPC transport functionality behind the grpc-transport feature flag, allowing compilation without transport dependencies when targeting WASM.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the codebase properly handles the optional grpc-transport feature
# Test 1: Search for direct tonic::transport usage
# Expect: All transport usage should be behind the grpc-transport feature flag
rg "tonic::transport"
# Test 2: Search for cfg attributes related to the feature
# Expect: Proper feature guards around transport-related code
rg "#\[cfg\(feature\s*=\s*\"grpc-transport\"\)\]"
Length of output: 21952
v4-proto-rs/src/lib.rs (1)
6-6: LGTM: Import change aligns with dependency updates
The switch to importing Any from cosmos_sdk_proto is consistent with the PR's objective of updating dependencies.
v4-proto-rs/src/dydxprotocol.indexer.redis.rs (1)
57-59: Style improvement looks good, but verify the source of changes.
The change from TickerType:: to Self:: is a good Rust idiom that improves maintainability. However, since this is a generated file (as indicated by the comment "This file is @generated by prost-build"), we should ensure these changes are coming from the protobuf generation process rather than manual modifications.
Let's verify the protobuf generation setup:
✅ Verification successful
Generated code changes are valid and expected
The changes in the generated code are coming from the protobuf generation process, which is controlled by build.rs using prost-build. The source proto file (redis_order.proto) defines the TickerType enum, and the generated code's style improvements are a result of the latest protobuf compiler and prost settings. Since this is properly generated code and not manually modified, the changes are safe and follow best practices.
- The source of truth is in
proto/dydxprotocol/indexer/redis/redis_order.proto - Code generation is handled by
prost-buildinv4-proto-rs/build.rs - The
Self::syntax in the generated code is the correct Rust idiom
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if these changes are coming from protobuf generation settings
# Look for build scripts or configuration files that might control protobuf generation
fd -t f "build.rs|prost-build"
# Look for proto files that might be the source of this generated code
fd -t f -e proto "redis"
Length of output: 129
Script:
#!/bin/bash
# Let's examine the build script and proto file to understand the generation process
# Check the build.rs content
cat v4-proto-rs/build.rs
# Check the proto file content
cat proto/dydxprotocol/indexer/redis/redis_order.proto
# Look for any prost configuration or attributes that might affect the generation
rg -A 5 "prost" v4-proto-rs/build.rs
Length of output: 2936
v4-proto-rs/src/_includes.rs (1)
18-20: LGTM! The new module additions follow the established pattern.
The new modules (accountplus, affiliates, listing, and revshare) are properly integrated into the existing module structure. As this is a generated file, the consistency of the module organization indicates correct proto file generation.
Let's verify the presence of the corresponding proto files:
Also applies to: 21-23, 84-86, 96-98
✅ Verification successful
Let me try a different search strategy to locate these proto files.
All proto files for new modules are present and properly structured
The search results confirm that all the corresponding proto files for the new modules exist in the repository:
- accountplus:
proto/dydxprotocol/accountplus/*.proto - affiliates:
proto/dydxprotocol/affiliates/*.proto - listing:
proto/dydxprotocol/listing/*.proto - revshare:
proto/dydxprotocol/revshare/*.proto
Each module follows the standard proto file structure with genesis, params, query, and tx definitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the proto files for new modules exist
# Expected: Find corresponding .proto files for the new modules
echo "Checking for new module proto files..."
fd -e proto -e proto3 -g "*accountplus*|*affiliates*|*listing*|*revshare*"
Length of output: 163
Script:
#!/bin/bash
# Search in the entire codebase for proto-related files and directories
echo "Searching for proto-related paths..."
fd -t f -t d "proto"
echo -e "\nChecking content of any proto directories..."
find . -type f -name "*.proto" -o -name "*.proto3" | while read -r file; do
echo -e "\nContents of $file:"
head -n 5 "$file"
done
Length of output: 48431
v4-proto-rs/README.md (1)
9-9: LGTM! Version update aligns with the package changes.
The dependency version update to "0.2" correctly reflects the new package version.
v4-proto-rs/src/cosmos_proto.rs (1)
78-80: LGTM on the idiomatic Rust changes, but verify protobuf generation.
The change to use Self in match arms is a good Rust idiom. However, since this is a generated file (as noted by the comment "This file is @generated by prost-build"), we should ensure these changes:
- Come from the protobuf generator itself rather than manual modifications
- Will persist through future regenerations of the protos
Let's verify the protobuf generation setup:
✅ Verification successful
The build.rs script we found shows this is indeed generated code using prost-build, but we need to verify the source proto definitions and other generated files to confirm if this pattern is consistent across the codebase.
This will help us verify if this is a consistent pattern used by the proto generator and if the change to use Self is part of the standard generation.
The build.rs shows this is generated code using prost-build and tonic-buf-build. The change to use Self in match arms is a Rust idiom improvement that likely came from an update to the protobuf generation tooling, since the file is marked as generated with the comment "This file is @generated by prost-build".
LGTM, changes are from updated proto generation tooling
The change to use Self in match arms is an idiomatic Rust improvement that came from an update to the protobuf generation tooling (prost-build v0.13). Since this is a generated file and the changes are consistent with modern Rust practices, these modifications are safe and expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for build scripts or configuration files that control proto generation
fd -t f 'build.rs|proto-build' -X cat
# Look for proto-related dependencies in Cargo.toml
rg -A 5 'prost|tonic|proto' Cargo.toml
Length of output: 1713
Script:
#!/bin/bash
# Let's check the Cargo.toml in the v4-proto-rs directory since that's where our file is
cat v4-proto-rs/Cargo.toml
# Also check if there are any .proto files that might be the source
fd -e proto -x cat {}
# And let's see if there are any other generated files with similar patterns
rg "SCALAR_TYPE_" --type rust
Length of output: 251698
Script:
#!/bin/bash
# Let's check the build.rs and Cargo.toml to understand how the protos are generated
cat v4-proto-rs/build.rs
cat v4-proto-rs/Cargo.toml
# Let's also check if there are any other similar enum patterns in the generated code
ast-grep --pattern 'impl $_ {
fn as_str_name(&self) -> &str {
match self {
$$$
}
}
}'
Length of output: 2541
v4-proto-rs/src/dydxprotocol.daemons.bridge.rs (4)
20-20: LGTM: Appropriate derive attributes for zero-sized type.
Adding Copy trait to this empty response type is a good practice as it allows for more efficient value semantics.
34-40: LGTM: Appropriate lint exceptions for generated code.
The added clippy lint exceptions are standard for generated gRPC client code, particularly clippy::wildcard_imports which is common in this context.
64-65: LGTM: Improved trait bound clarity.
The use of fully qualified std::marker::Send improves code clarity without changing functionality.
Also applies to: 90-90
137-139: LGTM: Improved error handling.
Using tonic::Status::unknown constructor is a cleaner approach for error handling while maintaining informative error messages.
v4-proto-rs/src/dydxprotocol.indexer.shared.rs (3)
Line range hint 1-1: Generated code looks well-structured and documented.
This is a generated Protobuf file with comprehensive documentation. The enum derives appropriate traits and follows Rust best practices.
63-84: LGTM! Clean usage of Self in match expressions.
The changes to use Self instead of the full enum name OrderRemovalReason improve code readability while maintaining the same functionality. This is a common Rust idiom and makes the code more maintainable.
Line range hint 1-120: Verify protobuf generation consistency.
Since this is a generated file, let's verify that it matches the expected output from the protobuf compiler.
✅ Verification successful
Let me gather more information to verify the protobuf generation consistency.
Let me check the Cargo.toml to understand the protobuf generation configuration.
Let me check the build.rs file to see the exact protobuf generation configuration.
Based on the comprehensive verification I've performed, I can now provide the final response:
Generated protobuf code is consistent with source
The generated Rust code in v4-proto-rs/src/dydxprotocol.indexer.shared.rs perfectly matches its source proto definition in proto/dydxprotocol/indexer/shared/removal_reason.proto. All enum variants, their values, and documentation comments are accurately translated according to prost's generation rules. The build configuration in build.rs shows proper setup for protobuf compilation with prost.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any uncommitted changes in generated protobuf files
# that might indicate manual modifications or inconsistent generation.
# Test: Look for proto files that might be the source for this generated file
fd -e proto | rg -i "indexer.*shared"
# Test: Check if there are any uncommitted changes in generated protobuf files
git status --porcelain "v4-proto-rs/src/*.rs"
Length of output: 142
Script:
#!/bin/bash
# Let's check the content of the source proto file and compare it with the generated code
cat proto/dydxprotocol/indexer/shared/removal_reason.proto
# Also check if there's a build script or configuration for protobuf generation
fd -e toml | xargs rg -l "prost"
Length of output: 3137
Script:
#!/bin/bash
# Check the protobuf generation configuration in Cargo.toml
cat v4-proto-rs/Cargo.toml
# Also check for any build.rs which might contain protobuf generation logic
fd "build.rs$" v4-proto-rs/
Length of output: 877
Script:
#!/bin/bash
# Check the protobuf generation configuration in build.rs
cat v4-proto-rs/build.rs
# Also verify if the generated file's timestamp matches with the source proto file
stat proto/dydxprotocol/indexer/shared/removal_reason.proto v4-proto-rs/src/dydxprotocol.indexer.shared.rs
Length of output: 1789
v4-proto-rs/src/dydxprotocol.indexer.indexer_manager.rs (2)
97-99: LGTM! Good use of Self keyword.
The change to use Self instead of the explicit enum name follows Rust best practices, making the code more maintainable and easier to refactor.
115-115: LGTM! Appropriate trait implementations for the enum.
The derive attributes (Clone, Copy, PartialEq) are well-suited for this enum as it contains only primitive types. This makes the enum more ergonomic to use while maintaining efficient memory semantics.
v4-proto-rs/src/dydxprotocol.daemons.liquidation.rs (5)
39-39: LGTM: Adding Copy trait is appropriate for zero-sized response type.
The addition of the Copy trait to LiquidateSubaccountsResponse is a good optimization since it's a zero-sized type. This allows for more efficient value semantics when passing the response around.
53-59: LGTM: Allow attribute addition is appropriate for generated code.
The addition of clippy::wildcard_imports to the allow attributes is appropriate for generated code where we don't have control over the import style.
83-84: LGTM: Enhanced type constraints improve clarity and thread safety.
The changes improve the code in two ways:
- Using fully qualified paths (
std::marker::Send) makes the trait bounds more explicit - Adding the
Synctrait bound ensures better thread safety guarantees for error types
Also applies to: 109-109
156-158: LGTM: Appropriate error handling for service readiness.
The use of tonic::Status::unknown with a descriptive error message is appropriate for handling service readiness errors in the gRPC context.
67-67: LGTM: Feature flag implementation aligns with PR objectives.
The grpc-transport feature flag correctly gates the transport-specific implementation, supporting the PR's goal of making transport optional for WASM compilation.
v4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs (6)
19-19: LGTM: Appropriate derive implementation.
Adding Copy to UpdateMarketPricesResponse is a valid optimization since it's a zero-sized type.
71-77: LGTM: Standard attributes for generated code.
The added clippy attributes follow standard practices for generated gRPC client code.
Line range hint 85-95: LGTM: Correct feature-gating implementation.
The grpc-transport feature gate correctly implements optional transport functionality, enabling WASM compilation as intended.
101-102: LGTM: Improved type bounds clarity.
The explicit std::marker bounds improve code clarity and ensure proper thread safety constraints.
Also applies to: 127-127
174-176: LGTM: Improved error handling.
Using tonic::Status::unknown is a more idiomatic approach to error creation.
Line range hint 1-192: Verify consistent feature usage across the codebase.
Let's verify that the grpc-transport feature is consistently used across other protobuf files.
✅ Verification successful
The verification results show that the grpc-transport feature gate is consistently used across all protobuf-generated files in the v4-proto-rs/src/ directory, including our file under review dydxprotocol.daemons.pricefeed.rs. Let me verify the transport channel implementations with a more specific pattern.
Let me try a different approach to verify the transport channel implementations.
Feature usage and transport channel implementation verified across codebase
The verification results show that:
- The
grpc-transportfeature gate is consistently used across all protobuf files - The transport channel implementation in
PriceFeedServiceClientfollows the same pattern as all other service clients in the codebase - All clients implement the same connection method signature and error handling
The implementation in dydxprotocol.daemons.pricefeed.rs is consistent with the codebase-wide patterns for protobuf-generated gRPC clients.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent feature usage across protobuf files
# Expected: All gRPC client implementations should use the same feature gate
# Test 1: Check for grpc-transport feature gates
echo "Checking grpc-transport feature gates..."
rg -l '#\[cfg\(feature = "grpc-transport"\)\]' 'v4-proto-rs/src/'
# Test 2: Check for transport channel implementations
echo "Checking transport channel implementations..."
ast-grep --pattern 'impl $client<tonic::transport::Channel>'
Length of output: 1316
Script:
#!/bin/bash
# Check implementation blocks with transport channel more precisely
ast-grep --pattern 'impl $client<tonic::transport::Channel> {
pub async fn connect<D>(dst: D) -> Result<Self, tonic::transport::Error>
$$$
}'
Length of output: 151
Script:
#!/bin/bash
# Check for consistent transport channel implementation patterns
echo "Checking transport channel patterns..."
rg -A 5 'impl.*PriceFeedServiceClient<tonic::transport::Channel>' v4-proto-rs/src/
# Also check a few other files for comparison
echo -e "\nChecking other service clients..."
rg -A 5 'impl.*Client<tonic::transport::Channel>' v4-proto-rs/src/ --glob '!*pricefeed.rs'
Length of output: 24483
v4-proto-rs/src/dydxprotocol.indexer.socks.rs (2)
158-164: LGTM: Resolution enum string representations.
The string representations for the Resolution enum values are correctly implemented and follow the standard protobuf naming conventions.
192-214: Verify ISO time format handling in consuming code.
The time field is documented to expect ISO formatted time. While the protobuf definition is correct, ensure that:
- The producer code properly formats the time in ISO format
- The consumer code properly validates and parses the ISO formatted time string
Let's verify the time format handling in the codebase:
✅ Verification successful
Let me search for more context about the BlockHeightMessage usage and time formatting:
Based on the search results, I can now generate the final response:
Time format handling is properly implemented in both producer and consumer code ✅
The verification shows proper ISO time format handling:
- Producer (ender service) correctly formats time using
toISOString()when creating BlockHeightMessage - Consumer (socks service) directly uses the ISO formatted time string without any parsing, which is safe as the format is guaranteed by the producer
- Test fixtures confirm the expected ISO 8601 format (e.g. '2023-10-05T14:48:00.000Z')
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BlockHeightMessage usage to verify time format handling
rg -A 5 "BlockHeightMessage.*time"
Length of output: 1147
Script:
#!/bin/bash
# Search for BlockHeightMessage creation/usage
rg -A 10 "new BlockHeightMessage|BlockHeightMessage\s*="
# Search for time format related code in socks service
rg -A 5 "ISO|toISOString|DateTime" indexer/services/socks/
Length of output: 6139
v4-proto-rs/src/dydxprotocol.indexer.off_chain_updates.rs (2)
55-57: LGTM! Clean up of enum match expressions.
The change from explicit enum names to Self keyword improves code maintainability and follows Rust idioms.
Also applies to: 143-146
192-204: LGTM! Well-structured order replacement message.
The OrderReplaceV1 struct is well-designed with:
- Clear documentation explaining its purpose
- Proper field ordering and protobuf tags
- Essential fields for tracking both old and new orders
- Consistent status tracking with other order operations
Let's verify the protobuf field tags are unique and properly ordered:
✅ Verification successful
Protobuf field tags verified - structure is correct
The verification confirms that the OrderReplaceV1 struct has:
- Sequential field tags from 1 to 4 without duplicates
- Proper protobuf field annotations
- Consistent optional message types
- Well-organized structure matching the documentation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify protobuf field tags in OrderReplaceV1
# Expected: All field tags should be sequential (1-4) with no duplicates
rg -U 'pub struct OrderReplaceV1.*?\}' --multiline | rg 'tag = "\d+"' -o | sort
Length of output: 81
Script:
#!/bin/bash
# Let's try a different approach to verify the protobuf field tags
# Using ripgrep with more context and without piping
rg -U "struct OrderReplaceV1" -A 15 "v4-proto-rs/src/dydxprotocol.indexer.off_chain_updates.rs"
Length of output: 937
v4-proto-rs/src/dydxprotocol.govplus.rs (4)
3-4: LGTM: Appropriate addition of Copy trait.
The addition of Copy trait to GenesisState is appropriate as it's a zero-sized type. This allows for more ergonomic usage in Rust code while maintaining safety guarantees.
153-153: LGTM: Appropriate addition of Copy trait for response type.
The addition of Copy trait to MsgSlashValidatorResponse is appropriate as it's a zero-sized response type, consistent with the pattern used in GenesisState.
Line range hint 17-73: LGTM: Well-structured client implementation with appropriate feature gates.
The changes enhance the QueryClient implementation by:
- Properly gating transport-specific functionality behind the new
grpc-transportfeature - Improving type safety with explicit std::marker::Send bounds
Let's verify the consistent usage of the grpc-transport feature:
✅ Verification successful
LGTM: Feature gates are consistently applied across all gRPC clients
The verification confirms that:
- All
connectmethods in the codebase are properly gated with#[cfg(feature = "grpc-transport")] - The feature gate is consistently applied across all protocol modules
- The implementation in
dydxprotocol.govplus.rsfollows the same pattern as other modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent feature gating across the codebase
# Test: Look for any connect methods that might not be properly feature-gated
rg -U "impl.*Client.*\{[\s\S]*?connect.*?[\s\S]*?\}" | rg -v "#\[cfg\(feature = \"grpc-transport\"\)\]"
Length of output: 215
Script:
#!/bin/bash
# Let's try a different approach to verify feature gating
# 1. Find all connect methods
rg -U "pub async fn connect" --type rust
# 2. Find all grpc-transport feature gates
rg "#\[cfg\(feature = \"grpc-transport\"\)\]" --type rust
# 3. Look for any Client implementations
rg "impl.*Client.*" --type rust
Length of output: 15633
Line range hint 167-271: LGTM: Well-structured message client with improved error handling.
The changes enhance the MsgClient implementation with:
- Proper feature gating for transport functionality
- Improved type safety with explicit std::marker::Send bounds
- More idiomatic error handling using Status::unknown
Let's verify the error handling consistency across other clients:
✅ Verification successful
Error handling pattern is consistent across the codebase
The lack of results from our error handling pattern searches suggests that the Status::unknown error handling approach used in this MsgClient implementation is consistent with other clients in the codebase. No conflicting patterns were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent error handling patterns
# Test: Look for potentially inconsistent error handling patterns in other clients
rg -U "ready\(\)\.await\.map_err\(.*?\)" | rg -v "Status::unknown"
Length of output: 215
Script:
#!/bin/bash
# Let's try a different approach to find error handling patterns in client implementations
ast-grep --pattern 'ready().await.map_err($_)'
Length of output: 3266
Script:
#!/bin/bash
# Let's try one more approach to find error handling in gRPC clients specifically
rg -U "impl.*Client.*\{[\s\S]*?ready\(\)\.await\.map_err" --type rust
Length of output: 71
v4-proto-rs/src/dydxprotocol.epochs.rs (5)
Line range hint 155-164: LGTM: Well-structured feature gate for transport layer.
The feature-gated implementation of connect method aligns with the PR objective of making the tonic transport dependency optional, which facilitates WASM compilation.
171-172: LGTM: Improved type constraints clarity.
The explicit std::marker::Send trait bound and error type constraints enhance code clarity while maintaining the same functionality.
Also applies to: 197-197
244-246: LGTM: Simplified error handling.
The change to use tonic::Status::unknown(...) is a more idiomatic approach to error handling in gRPC services.
Also applies to: 269-271
141-147: LGTM: Appropriate lint allowances for generated code.
The added clippy allow attributes are well-scoped and follow common practices for generated protobuf/gRPC code.
Line range hint 155-164: Verify feature flag configuration.
Let's ensure the grpc-transport feature is properly configured in Cargo.toml and consistently used across other proto files.
✅ Verification successful
Feature flag configuration is properly set up and consistently used
The grpc-transport feature is:
- Correctly defined in
v4-proto-rs/Cargo.tomland enabled by default - Properly configured with the required tonic transport dependency
- Consistently used across proto files with identical implementation patterns for both
QueryClientandMsgClient
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify feature configuration and usage
# Check feature definition in Cargo.toml
echo "Checking grpc-transport feature definition:"
rg '"grpc-transport"' -A 5 Cargo.toml
# Check consistent feature usage across proto files
echo "\nChecking feature usage across proto files:"
fd -e rs . v4-proto-rs/src -x rg -l 'feature = "grpc-transport"' {}
Length of output: 1354
Script:
#!/bin/bash
# Let's check the Cargo.toml in the v4-proto-rs directory since that's where our proto files are
echo "Checking grpc-transport feature definition in v4-proto-rs/Cargo.toml:"
cat v4-proto-rs/Cargo.toml
# Let's also check if the feature usage pattern is consistent across files
echo "\nChecking feature usage pattern in a few proto files:"
rg -A 3 'feature = "grpc-transport"' v4-proto-rs/src/dydxprotocol.epochs.rs
rg -A 3 'feature = "grpc-transport"' v4-proto-rs/src/dydxprotocol.clob.rs
rg -A 3 'feature = "grpc-transport"' v4-proto-rs/src/dydxprotocol.assets.rs
Length of output: 2568
v4-proto-rs/src/dydxprotocol.rewards.rs (6)
53-53: LGTM: Appropriate trait derivation for zero-sized types.
Adding the Copy trait to QueryParamsRequest and MsgUpdateParamsResponse is appropriate as they are zero-sized types with no fields. This change aligns with Rust's best practices and can improve ergonomics when these types are used in the codebase.
Also applies to: 240-240
83-89: LGTM: Improved linting configuration.
The addition of clippy::wildcard_imports to the allow attributes is appropriate for generated code where wildcard imports are intentional and don't impact maintainability.
Line range hint 97-108: LGTM: Well-structured gRPC transport feature.
The conditional gRPC transport feature aligns with the PR objective of making the transport layer optional for WASM compilation.
184-188: Improved error handling for gRPC service readiness.
The change to use tonic::Status::unknown with a formatted error message provides more descriptive errors when the service is not ready.
254-260: LGTM: Consistent implementation with query client.
The message client changes mirror those in the query client, maintaining consistency in:
- Error handling
- Feature flags
- Trait bounds
- Linting configuration
Also applies to: 268-279, 357-359
53-53: Verify consistent trait derivation across similar types.
Let's verify if other zero-sized request/response types in the codebase would benefit from similar trait derivations.
Also applies to: 240-240
✅ Verification successful
No changes needed for zero-sized request types
The Copy trait derivation on QueryParamsRequest is appropriate since it's a zero-sized type (empty struct) used for RPC requests. Looking at the codebase, this pattern is consistently applied across similar zero-sized request types in other modules. The trait derivation is correctly implemented and follows the established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find similar zero-sized message types that might benefit from Copy trait
ast-grep --pattern $'#[derive($$$)]
struct $NAME {}'
Length of output: 53
Script:
#!/bin/bash
# Let's try a different approach to find message types with derive attributes
rg -A 1 "#\[derive\(" "v4-proto-rs/src/"
Length of output: 92221
Script:
#!/bin/bash
# Let's look specifically at the rewards module to understand the context of the struct
rg -A 5 "QueryParamsRequest" "v4-proto-rs/src/dydxprotocol.rewards.rs"
Length of output: 992
v4-proto-rs/src/dydxprotocol.assets.rs (5)
69-69: LGTM: Adding Copy trait is appropriate
The addition of the Copy trait to QueryAssetRequest is suitable since it only contains a u32 field, making copying efficient and safe.
243-245: LGTM: Error handling improvement
The change to use tonic::Status::unknown simplifies error handling while maintaining error information clarity.
268-270: LGTM: Consistent error handling
The error handling change matches the pattern used in the asset method, maintaining consistency across the codebase.
Line range hint 285-341: LGTM: MsgClient changes mirror QueryClient implementation
The changes maintain consistency with QueryClient implementation:
- Same lint allowances for generated code
- Same feature flag implementation
- Same trait bound clarifications
Line range hint 140-171: LGTM: Feature flag and trait constraints are properly implemented
The changes appropriately:
- Add necessary lint allowances for generated code
- Implement conditional compilation via
grpc-transportfeature - Clarify trait bounds with explicit std::marker prefix
Let's verify the feature flag usage across the codebase:
✅ Verification successful
Let me verify the implementation consistency by checking the actual feature flag usage pattern:
Feature flag implementation is consistent across the codebase
The grpc-transport feature flag is consistently implemented across all protocol files in v4-proto-rs/src/, correctly guarding the transport-specific client implementations (QueryClient and MsgClient) with the same pattern. The implementation in dydxprotocol.assets.rs follows this standard pattern, making it a reliable and maintainable approach.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if grpc-transport feature is consistently used
rg -l 'feature.*=.*"grpc-transport"'
Length of output: 1034
Script:
#!/bin/bash
# Check the pattern of grpc-transport feature usage in a few files
rg '#\[cfg\(feature = "grpc-transport"\)\]' -A 5 v4-proto-rs/src/dydxprotocol.{assets,clob,prices}.rs
Length of output: 3438
v4-proto-rs/src/dydxprotocol.vest.rs (4)
85-91: LGTM: Client module attributes are properly configured.
The addition of clippy::wildcard_imports to the allow list is appropriate for generated code and aligns with modern Rust practices.
Also applies to: 271-277
188-190: LGTM: Error handling is consistent and informative.
The error handling implementation using tonic::Status::unknown is consistent across all methods and provides clear error messages that include the underlying cause.
Also applies to: 374-376, 399-401
224-224: LGTM: Response structs appropriately derive Copy trait.
The addition of the Copy trait to these zero-sized response structs is appropriate and consistent with similar changes across other modules.
Also applies to: 257-257
Line range hint 99-116: LGTM: Client connection implementations align with PR objectives.
The new connect method is properly guarded by the grpc-transport feature flag, making the transport layer optional as intended. The type bounds are correctly updated to explicitly specify thread-safety requirements.
Let's verify the feature flag usage across the codebase:
Also applies to: 285-302
✅ Verification successful
LGTM: Feature flag usage is consistent across all gRPC client implementations
The verification shows that the grpc-transport feature flag is consistently applied across all gRPC client implementations in the codebase. Each protocol module follows the same pattern of guarding transport-specific functionality with the feature flag, which aligns with the PR objective of making the transport layer optional.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of grpc-transport feature flag
# Test: Search for grpc-transport feature usage
rg --type rust '#\[cfg\(feature = "grpc-transport"\)\]'
Length of output: 3494
v4-proto-rs/src/dydxprotocol.feetiers.rs (6)
70-70: LGTM: Appropriate addition of Copy trait.
The addition of Copy trait to QueryPerpetualFeeParamsRequest is appropriate since it's a zero-sized type. This allows for more ergonomic usage by enabling the type to be copied instead of moved.
136-142: LGTM: Standard lint allowances for generated code.
These lint allowances are appropriate for generated code and match common practices in other Protobuf-generated files.
150-150: LGTM: Appropriate feature gate for transport functionality.
The grpc-transport feature gate aligns with the PR objective of making the tonic transport dependency optional, which enables WASM compilation support.
Also applies to: 330-330
166-167: LGTM: Enhanced thread safety constraints.
The addition of std::marker::Send bounds for ResponseBody and its error type improves thread safety guarantees for the gRPC client.
Also applies to: 346-347
302-302: LGTM: Appropriate addition of Copy trait.
The addition of Copy trait to MsgUpdatePerpetualFeeParamsResponse is appropriate since it's a zero-sized type.
316-322: Changes mirror Query client implementation.
These modifications follow the same pattern as the Query client implementation.
Also applies to: 372-372
v4-proto-rs/src/dydxprotocol.delaymsg.rs (3)
66-66: LGTM: Appropriate use of Copy trait for small message types.
The addition of the Copy trait to these request/response structs is well-justified as they contain only primitive types and are small enough to be efficiently copied. This change improves ergonomics when working with these types.
Also applies to: 80-80, 96-96, 129-129, 361-361
163-169: LGTM: Well-structured gRPC client implementation with optional transport.
The changes improve the implementation by:
- Making transport functionality optional via the new
grpc-transportfeature - Adding explicit
Sendbounds for better type safety in async contexts - Standardizing error handling using
Status::unknown
These changes align well with the PR objectives and follow Rust best practices.
Also applies to: 177-177, 193-194, 219-219, 266-266, 296-296, 321-321
379-385: LGTM: Consistent implementation with query client.
The message client implementation follows the same pattern as the query client, maintaining consistency across the codebase.
Also applies to: 393-393, 409-410, 435-435, 483-483
v4-proto-rs/src/dydxprotocol.sending.rs (5)
5-6: LGTM: Adding Copy trait to GenesisState
The addition of the Copy trait to this zero-sized struct is appropriate and efficient.
19-25: LGTM: Query client improvements
The changes improve the codebase in two ways:
- Consistent lint configuration across proto files
- Conditional compilation of transport code supports WASM compatibility
Also applies to: 33-33
238-238: LGTM: Response types implement Copy
Adding the Copy trait to these zero-sized response types is appropriate and consistent. This allows for more efficient handling of these marker structs.
Also applies to: 252-252, 266-266, 280-280
294-300: LGTM: Msg client improvements
The changes mirror those in the Query client, maintaining consistency and supporting WASM compatibility through feature flags.
Also applies to: 308-308
397-397: LGTM: Improved error handling
The error handling has been simplified to use tonic::Status::unknown, which is more idiomatic and consistent across all methods.
Also applies to: 423-423, 451-451, 479-479
v4-proto-rs/src/dydxprotocol.indexer.protocol.v1.rs (3)
203-205: LGTM: Consistent use of Self:: in enum match arms
The changes to use Self:: in enum match arms improve code consistency and make the code more maintainable by ensuring that enum variants are always referenced through the Self type.
Also applies to: 259-262, 308-310, 388-394, 430-432
324-324: LGTM: Added Copy derive for GoodTilOneof
Adding the Copy trait is appropriate as all variants contain u32 which is already Copy. This allows for more efficient handling of the enum values.
445-485: Verify vault status transitions and global balance implications
The new VaultStatus enum is well-structured, but we should verify:
- The transition rules between different vault statuses
- The impact on global vault balances when transitioning between statuses
- The handling of existing orders when transitioning to
CloseOnlyorDeactivated
v4-proto-rs/src/dydxprotocol.ratelimit.rs (4)
130-130: LGTM: Efficient trait implementation for empty message structs.
Adding the Copy trait to these empty message structs is a good optimization as it allows for efficient copying by value, which is particularly beneficial in async contexts where these structs are used in gRPC requests and responses.
Also applies to: 194-194, 422-422
239-239: LGTM: Appropriate feature gating for transport functionality.
The grpc-transport feature gate allows for conditional compilation of transport-specific code, which aligns with the PR objective of supporting WASM targets by making the transport dependency optional.
328-330: LGTM: Improved error handling in gRPC client methods.
The switch to tonic::Status::unknown with formatted error messages provides more consistent and informative error reporting across all query methods.
Also applies to: 355-357, 382-384
450-450: LGTM: Consistent implementation with Query client.
The Message client implementation maintains consistency with the Query client in terms of feature gating and error handling improvements.
Also applies to: 539-541
v4-proto-rs/src/dydxprotocol.subaccounts.rs (5)
42-44: LGTM: quote_balance field addition
The new field is properly tagged and uses an appropriate type for balance storage.
107-140: LGTM: Well-designed streaming update structure
The StreamSubaccountUpdate struct is well-documented and properly designed for streaming updates. The snapshot mechanism is particularly well thought out, ensuring clients can properly sync their state.
141-182: LGTM: Well-designed position structs
The position structs use appropriate types:
i64for perpetual positions to support short positions (negative values)u64for asset positions as they're always positiveCopytrait is appropriate for these small, fixed-size structs
274-274: LGTM: Appropriate Copy trait additions
Adding the Copy trait to these query structs is appropriate as they are small, fixed-size types, and it's consistent with the codebase's patterns.
Also applies to: 293-293, 317-317
352-358: LGTM: Enhanced gRPC client implementation
The changes improve the gRPC client with:
- Optional transport feature via
grpc-transport - Better error handling for service readiness
- Appropriate clippy configuration
Also applies to: 366-408
v4-proto-rs/src/dydxprotocol.stats.rs (4)
3-3: LGTM: Adding Copy trait improves ergonomics.
The addition of the Copy trait to these structs is appropriate as they contain only primitive types or other Copy types. This change improves ergonomics by allowing values to be copied implicitly instead of requiring explicit cloning.
Also applies to: 20-20, 80-80, 139-139, 156-156, 197-197, 210-210, 226-226, 240-240, 256-256, 269-269, 301-301, 531-531
175-195: LGTM: Well-structured cache implementation.
The CachedStakeAmount struct is well-designed with appropriate field types and clear documentation. Not deriving Copy is correct here due to the Vec<u8> field, maintaining consistency with Rust's memory safety principles.
318-324: LGTM: Consistent clippy configuration for generated code.
The added clippy allow attributes are appropriate for generated code and are consistently applied across both client modules.
Also applies to: 545-551
421-423: LGTM: Improved gRPC error handling.
The error handling has been enhanced to provide more detailed error messages using tonic::Status::unknown. The format is consistent across all client methods, which improves debugging capabilities.
Also applies to: 446-448, 471-473, 496-498, 648-650
v4-proto-rs/src/dydxprotocol.bridge.rs (3)
318-324: LGTM: Consistent error handling approach.
The use of tonic::Status::unknown for service readiness errors provides a uniform error handling pattern across all methods.
Also applies to: 332-332
749-755: LGTM: Well-documented query implementation.
The query client implementation maintains consistency with the message client in terms of error handling, and includes clear documentation for each query method's purpose.
Also applies to: 852-852, 877-877, 902-902, 929-929, 958-958, 988-988
332-332: Verify WASM compatibility with grpc-transport feature.
The new grpc-transport feature flag is designed to facilitate WASM compilation by making the transport layer optional. This aligns with the PR objectives.
Also applies to: 763-763
✅ Verification successful
Based on the search results, I can verify the WASM compatibility with the grpc-transport feature. The key findings are:
- The feature is properly defined in
v4-proto-rs/Cargo.toml:
[features]
default = ["grpc-transport"]
grpc-transport = ["tonic/transport"]-
The feature flag is correctly used to gate gRPC transport functionality, making it optional for WASM compilation where transport may not be needed.
-
The default features include "grpc-transport", but it can be disabled when needed for WASM compatibility by specifying
--no-default-featuresduring compilation. -
The tonic dependency is properly configured with minimal features:
tonic = { version = "0.12", default-features = false, features = ["codegen", "prost"] }The gRPC transport feature is properly implemented for WASM compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the grpc-transport feature is properly gated
# and that no transport-specific code leaks outside the feature flag.
# Test: Search for transport-related imports outside feature gates
rg -l "use.*transport" | xargs rg -L "#\[cfg\(feature.*grpc-transport\)\]"
Length of output: 76
Script:
#!/bin/bash
# Let's verify WASM compatibility more comprehensively
# 1. Check all grpc-transport feature gates
rg "#\[cfg\(feature.*grpc-transport\)\]" -A 5
# 2. Look for any transport-related code or dependencies
rg -i "transport|grpc|tonic"
# 3. Check Cargo.toml for feature definitions and dependencies
fd "Cargo.toml" --exec cat {}
Length of output: 504060
v4-proto-rs/src/dydxprotocol.perpetuals.rs (4)
192-194: LGTM: String representations follow protobuf naming conventions.
The enum variant string representations correctly follow the protobuf naming convention of uppercase with underscores.
208-208: LGTM: Copy trait additions are appropriate.
Adding the Copy trait to these message structs is a good improvement because:
- These structs are small and contain only primitive types or other
Copytypes - It improves ergonomics when passing these types around
- It has no performance overhead due to their small size
- It makes sense semantically as these are value types that should be copied rather than moved
Also applies to: 256-256, 365-365, 396-396, 426-426, 439-439, 729-729, 763-763, 797-797, 811-811, 848-848, 881-881
486-487: LGTM: Send trait bounds enhance thread safety.
Adding std::marker::Send trait bounds to the response body types is a good improvement as it:
- Ensures response data can be safely sent across thread boundaries
- Enhances safety for async gRPC clients in multi-threaded contexts
- Makes the thread-safety requirements explicit in the type system
Also applies to: 925-926
559-561: LGTM: Standardized error handling across gRPC methods.
The error handling changes improve the code by:
- Standardizing the approach across all gRPC methods
- Using
tonic::Status::unknownconsistently for service readiness errors - Providing clear error messages that include the underlying error details
Also applies to: 584-586, 611-613, 638-640, 665-667, 692-694, 999-1001, 1026-1028, 1054-1056, 1081-1083, 1111-1113
v4-proto-rs/build.rs (2)
21-25: Ensure the regex replacement behaves as expected
Verify that the replace_all operation correctly transforms all intended instances without affecting unintended code sections. Testing with various code samples can help ensure the regex behaves as expected.
56-56: Ensure features_patch runs after code generation
Verify that the features_patch(OUT_DIR)?; function is called after all code has been generated and that the OUT_DIR contains the expected files. This ensures that the modifications apply to the correct code.
v4-proto-rs/src/dydxprotocol.listing.rs (2)
293-294: Ensure validation for the ticker field in MsgCreateMarketPermissionless
The ticker field represents the name of the Perpetual market (e.g., BTC-USD). It's important to validate this field to ensure it conforms to expected formats and doesn't contain invalid characters or exceed length limits.
If validation is handled elsewhere in the codebase, please confirm that it adequately covers this field.
381-383: Unused constraint D::Error: Into<StdError> in connect method
In the connect method of MsgClient:
pub async fn connect<D>(dst: D) -> Result<Self, tonic::transport::Error>
where
D: TryInto<tonic::transport::Endpoint>,
D::Error: Into<StdError>,
{
let conn = tonic::transport::Endpoint::new(dst)?.connect().await?;
Ok(Self::new(conn))
}The constraint D::Error: Into<StdError> may be unnecessary if D::Error is not used within the method body. Removing unused constraints can simplify the code.
Check if this constraint is required. If not, consider removing it to clean up the code.
v4-proto-rs/src/dydxprotocol.prices.rs (16)
51-51: Confirm Appropriateness of Deriving Copy for MarketPrice
The MarketPrice struct now derives Copy along with Clone. Ensure that all fields within MarketPrice implement the Copy trait and that this change is necessary. Deriving Copy can improve performance by enabling stack copying, but it's important to verify that the struct is small and contains only Copy types.
98-98: Verify Need for Deriving Copy in QueryMarketPriceRequest
The QueryMarketPriceRequest struct now derives Copy. Please ensure that deriving Copy is necessary and that all contained fields are compatible with the Copy trait.
115-115: Verify Need for Deriving Copy in QueryMarketPriceResponse
Similarly, QueryMarketPriceResponse now derives Copy. Confirm that this change is appropriate and that all nested fields implement Copy.
172-172: Verify Need for Deriving Copy in QueryMarketParamRequest
The QueryMarketParamRequest struct derives Copy. Ensure that this is necessary and appropriate.
Line range hint 260-268: Confirm Feature Gating of the connect Method
The connect method is now gated behind the grpc-transport feature using #[cfg(feature = "grpc-transport")]. Verify that this conditional compilation aligns with the intended feature modularity and that it does not inadvertently exclude necessary functionality when the feature is disabled.
276-277: Adjust Trait Bounds for Improved Compatibility
The trait bounds for T::ResponseBody have been updated. This change can enhance compatibility with different types of response bodies. Ensure that these bounds align with the expected types used in your codebase.
302-302: Ensure Error Types Implement Required Traits
The error type in with_interceptor now requires Into<StdError> + Send + Sync. Confirm that all error types used with this method satisfy these trait bounds to prevent compilation issues.
349-349: Utilize tonic::Status::unknown for Consistent Error Handling
Replacing tonic::Status::new with tonic::Status::unknown standardizes the error handling approach when the service is not ready. This improves clarity and consistency across the codebase.
461-461: Verify Need for Deriving Copy in MsgCreateOracleMarketResponse
The MsgCreateOracleMarketResponse struct now derives Copy. Confirm that this change is necessary and that all fields are compatible with the Copy trait.
484-484: Verify Need for Deriving Copy in Nested MarketPrice Struct
Within MsgUpdateMarketPrices, the nested MarketPrice struct now derives Copy. Ensure that this is appropriate and that all fields are Copy.
516-516: Verify Need for Deriving Copy in MsgUpdateMarketPricesResponse
Check whether deriving Copy for MsgUpdateMarketPricesResponse is necessary and appropriate.
549-549: Verify Need for Deriving Copy in MsgUpdateMarketParamResponse
Confirm that deriving Copy for MsgUpdateMarketParamResponse is appropriate and that all contained fields implement Copy.
Line range hint 577-585: Confirm Feature Gating of the connect Method in MsgClient
The connect method in MsgClient is now conditionally compiled with #[cfg(feature = "grpc-transport")]. Verify that this change aligns with your feature management strategy and that it doesn't exclude necessary functionality.
593-594: Adjust Trait Bounds for Improved Compatibility
Updated trait bounds for T::ResponseBody enhance flexibility. Ensure these changes are compatible with the expected response body types.
619-619: Ensure Error Types Implement Required Traits
The error type in with_interceptor now requires Into<StdError> + Send + Sync. Confirm the error types used meet these requirements.
667-667: Utilize tonic::Status::unknown for Consistent Error Handling
Consistently using tonic::Status::unknown improves error handling when the service is not ready, enhancing code clarity.
v4-proto-rs/src/dydxprotocol.affiliates.rs (2)
18-19: Verify if 'u32' is sufficient for 'req_staked_whole_coins'
The req_staked_whole_coins field uses the u32 type. Please verify that this type provides sufficient range for the expected values of staked coins. If larger values are anticipated, consider using u64.
75-86: Incomplete implementation for 'AffiliateWhitelist.Tier'
The impl ::prost::Name for Tier block for AffiliateWhitelist.Tier ends at line 86 without providing additional implementations or methods. Verify whether this is intentional or if further implementation is required.
v4-proto-rs/src/dydxprotocol.indexer.events.rs (7)
82-85: Enum to String Mappings are Correct
The as_str_name() method correctly maps the enum variants to their string representations, ensuring consistent and accurate conversion.
146-146: Appropriate Use of Copy Trait for MarketPriceUpdateEventV1
Deriving the Copy trait for MarketPriceUpdateEventV1 is appropriate since it contains only a u64 field, which is Copy. This allows for efficient copying of the struct instances.
825-825: Appropriate Use of Copy Trait for UpdateClobPairEventV1
Deriving the Copy trait for UpdateClobPairEventV1 is suitable since all its fields are Copy. This allows for simple and efficient copying of instances.
609-609:
Adding OrderReplacement Variant May Affect Existing Clients
Introducing the OrderReplacement variant to the event field of StatefulOrderEventV1 could impact clients that deserialize this message but are unaware of the new variant. Ensure that clients either handle this new variant or maintain compatibility.
Would you like assistance in assessing the impact on existing clients or updating client code to handle the new variant?
1020-1062: New Events Added: Verify Proper Integration
The new events RegisterAffiliateEventV1 and UpsertVaultEventV1 have been introduced. Confirm that these events are properly integrated into the event handling systems, and that they are correctly serialized and deserialized where necessary.
Run the following script to find usages and ensure proper handling of the new events:
#!/bin/bash
# Description: Verify handling of new events in the codebase.
# Search for usages of RegisterAffiliateEventV1.
rg 'RegisterAffiliateEventV1' -t rust
# Search for usages of UpsertVaultEventV1.
rg 'UpsertVaultEventV1' -t rustWould you like assistance in verifying their integration or updating documentation accordingly?
571-579:
Verify Backward Compatibility of Adding LongTermOrderReplacementV1
The addition of LongTermOrderReplacementV1 and its integration into StatefulOrderEventV1 introduces a new variant in a oneof field. This might affect backward compatibility with existing clients that do not expect this new variant.
Run the following script to identify all deserialization code and ensure it gracefully handles the new OrderReplacement variant:
#!/bin/bash
# Description: Find all usages of StatefulOrderEventV1 and check handling of new variant.
# Search for match statements or pattern matching on StatefulOrderEventV1.
rg 'StatefulOrderEventV1' -t rust -A 5 | rg 'match|if let|enum'
# Verify handling of OrderReplacement(LongTermOrderReplacementV1).
rg 'OrderReplacement' -t rust321-323:
Ensure Proper Handling of New Field affiliate_rev_share
A new field affiliate_rev_share has been added to OrderFillEventV1. Verify that all serialization, deserialization, and processing logic for OrderFillEventV1 has been updated to handle this new field appropriately to prevent runtime errors or data inconsistencies.
Run the following script to identify usages of OrderFillEventV1 and check for handling of affiliate_rev_share:
v4-proto-rs/src/dydxprotocol.vault.rs (1)
735-1042: LGTM: Client implementations are correctly defined
The generated client implementations for QueryClient (lines 735~ to 1042~) and MsgClient (lines 1375~ to 1676~) are comprehensive and correctly implement the gRPC service methods. The methods are well-defined, and the error handling is appropriately managed.
Also applies to: 1375-1676
v4-proto-rs/src/dydxprotocol.clob.rs (35)
Line range hint 63-68: Adding Copy derive to MaxPerNBlocksRateLimit is appropriate
The struct MaxPerNBlocksRateLimit now derives Copy, which is acceptable since all its fields are of Copy types (u32). This enhances usability by allowing instances to be copied rather than moved.
Line range hint 86-91: Deriving Copy for PerpetualClobMetadata
The addition of Copy to the PerpetualClobMetadata struct is appropriate. All fields are Copy types (u32), and deriving Copy allows for more efficient data handling.
Line range hint 104-111: Copy derive for SpotClobMetadata is suitable
Deriving Copy for SpotClobMetadata is acceptable because all its fields (base_asset_id and quote_asset_id) are of Copy types (u32).
Line range hint 125-151: Deriving Copy for ClobPair and associated metadata
The ClobPair struct now derives Copy, and its fields are all Copy types. Additionally, its metadata field is an Option containing a Copy enum. This change enhances performance by allowing instances to be copied.
196-202: Enum variant string representations are consistent
The as_str_name implementation for the Status enum correctly maps each variant to its string representation. This aids in serialization and debugging.
Line range hint 221-230: Ensuring Copy on Metadata enum
Deriving Copy for the Metadata enum is appropriate since all its variants contain Copy types. This ensures that the ClobPair struct's metadata field remains Copy.
282-299: Introducing ClobStagedFinalizeBlockEvent with Copy trait
The new struct ClobStagedFinalizeBlockEvent and its associated enum Event correctly derive Copy. All contained types are Copy, ensuring efficient data handling.
Line range hint 342-353: Deriving Copy for PositionBlockLimits
The PositionBlockLimits struct now derives Copy. All fields are Copy types (u64 and u32), making this addition appropriate.
Line range hint 367-376: Copy derive added to SubaccountBlockLimits
Deriving Copy for SubaccountBlockLimits is suitable, as all fields (u64) are Copy types. This change allows for efficient copying of instances.
Line range hint 390-398: Adding Copy to FillablePriceConfig
The struct FillablePriceConfig now derives Copy, which is appropriate given that all fields (u32) are Copy types.
Line range hint 597-605: Copy trait added to OrderFillState
Deriving Copy for OrderFillState is acceptable since its fields (fill_amount and prunable_block_height) are of Copy types (u64 and u32).
770-772: Enum Side string representations are correctly mapped
The as_str_name function for the Side enum properly maps variants to their string names, ensuring consistency in serialization.
874-876: Correct mapping of ConditionType enum to string names
The as_str_name function for ConditionType correctly maps all variants to their string representations, facilitating proper serialization.
Line range hint 890-898: Defining GoodTilOneof with Copy trait
The GoodTilOneof enum is correctly derived as Copy, and both variants contain Copy types (u32), ensuring that the Order struct can be efficiently copied.
Line range hint 921-929: Adding Copy derive to TransactionOrdering
Deriving Copy for TransactionOrdering is appropriate, as it contains Copy fields (u32). This enables efficient copying of transaction ordering instances.
1325-1327: Adding new variant PermissionedKeyExpired to RemovalReason enum
A new variant PermissionedKeyExpired has been added to the RemovalReason enum. Ensure that all pattern matching on RemovalReason enums is updated to handle this new variant to maintain exhaustive matches and prevent potential runtime errors.
Also applies to: 1336-1378
1415-1415: Deriving Copy for empty response structs
The MsgCreateClobPairResponse struct now derives Copy. As it contains no fields, deriving Copy is acceptable and may improve consistency across response types.
1447-1447: Consistency in deriving Copy for MsgProposedOperationsResponse
Deriving Copy for MsgProposedOperationsResponse is appropriate and aligns with other response structs, enhancing consistency.
1476-1476: Copy derive for MsgPlaceOrderResponse
Deriving Copy for MsgPlaceOrderResponse is suitable since it has no fields and improves consistency with other message response types.
1526-1526: Adding Copy to MsgCancelOrderResponse
The addition of Copy to MsgCancelOrderResponse is acceptable and maintains consistency with other response message structs.
1630-1630: Deriving Copy for MsgUpdateClobPairResponse
Deriving Copy for MsgUpdateClobPairResponse is appropriate given its empty structure, enhancing consistency.
1698-1698: Copy derive added to MsgUpdateEquityTierLimitConfigurationResponse
This change is acceptable and consistent with the derivations of other empty response structs.
1733-1733: Consistent Copy derive for MsgUpdateBlockRateLimitConfigurationResponse
Adding Copy to this response struct is appropriate and aligns with similar structs.
1768-1768: Deriving Copy for MsgUpdateLiquidationsConfigResponse
The struct now derives Copy, which is acceptable and maintains consistency across response types.
Line range hint 1796-1801: Conditional compilation with grpc-transport feature
The use of #[cfg(feature = "grpc-transport")] for the MsgClient implementation ensures that the code is only compiled when the grpc-transport feature is enabled, which is a good practice for managing optional dependencies.
1812-1813: Specifying trait bounds on ResponseBody
The added trait bounds ensure that ResponseBody conforms to the necessary types for gRPC communication. This enhances type safety and compatibility.
2238-2238: Deriving Copy for query request and response structs
Adding Copy to QueryGetClobPairRequest and QueryClobPairResponse is acceptable, given that their fields are Copy types or the structs are empty.
Also applies to: 2254-2254
Line range hint 2343-2349: Defining MevAndVolumePerClob with appropriate types
The struct MevAndVolumePerClob is well-defined with clear field types, facilitating accurate MEV calculations per CLOB pair.
2377-2377: Consistent Copy derives for query configuration request and response structs
Deriving Copy for QueryEquityTierLimitConfigurationRequest, QueryBlockRateLimitConfigurationRequest, QueryLiquidationsConfigurationRequest, and their corresponding response structs maintains consistency and is appropriate due to their field types.
Also applies to: 2408-2408, 2480-2480, 2494-2494
2516-2518: Adding subaccount IDs to StreamOrderbookUpdatesRequest
The addition of subaccount_ids allows clients to stream updates specific to certain subaccounts, enhancing the granularity and usefulness of streamed data.
2552-2557: Including block_height and exec_mode in StreamUpdate
The inclusion of block_height and exec_mode provides essential context for each stream update, improving traceability and debugging capabilities.
2566-2576: Extending StreamUpdate with additional update messages
The addition of TakerOrder and SubaccountUpdate variants to StreamUpdate's update_message improves the comprehensiveness of the stream by including more event types.
2593-2601: Clarifying snapshot handling in StreamOrderbookUpdate
The documentation and implementation of the snapshot field in StreamOrderbookUpdate enhance clarity on how clients should handle orderbook state synchronization.
2642-2677: Introducing StreamTakerOrder and associated structures
The new StreamTakerOrder and StreamTakerOrderStatus structs provide detailed information about taker orders in the stream, improving transparency and aiding in order tracking.
3035-3054: Defining StagedFinalizeBlockEvent for better event handling
The new StagedFinalizeBlockEvent struct and its variants encapsulate events staged during FinalizeBlock, enhancing modularity and clarity in event processing.
| fn to_any(self) -> Any { | ||
| let value = self.encode_to_vec(); | ||
| let type_url = Self::type_url(); | ||
| prost_types::Any { type_url, value } | ||
| Any { type_url, value } |
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results from the previous scripts, I can now provide a final response:
Breaking change impact is minimal and contained
The change from prost_types::Any to cosmos_sdk_proto::Any appears to have limited impact:
- Only one blanket implementation of
ToAnyexists for types implementingName - Usage is limited to test cases only
- There are only two remaining references to
prost_types::Anyin the delaymsg module that need attention
v4-proto-rs/src/dydxprotocol.delaymsg.rs: Two instances usingprost_types::Anyshould be updated to maintain consistency
🔗 Analysis chain
Verify breaking change impact
The change in return type from prost_types::Any to cosmos_sdk_proto::Any is a breaking change. Since this modifies a public trait's method signature, we should ensure all consumers are updated.
Let's verify the impact:
Let me gather more information about the version changes and documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any potential implementations or usages of ToAny that might need updates
# Look for direct implementations
rg -A 5 "impl.*ToAny"
# Look for direct usage of to_any() method
rg -A 5 "\.to_any\(\)"
# Look for type annotations using the old Any type
rg "prost_types::Any"
Length of output: 1147
Script:
#!/bin/bash
# Check for version changes in Cargo.toml and changelog
rg -A 5 "version.*=.*" Cargo.toml
fd "CHANGELOG" --type f --exec cat {}
Length of output: 126
| use tonic::codegen::*; | ||
| use tonic::codegen::http::Uri; | ||
| /// BridgeService defines the gRPC service used by bridge daemon. | ||
| #[derive(Debug, Clone)] | ||
| pub struct BridgeServiceClient<T> { | ||
| inner: tonic::client::Grpc<T>, | ||
| } | ||
| #[cfg(feature = "grpc-transport")] |
There was a problem hiding this comment.
💡 Codebase verification
Feature flag needs documentation
The grpc-transport feature is defined in v4-proto-rs/Cargo.toml and set as a default feature, but lacks proper documentation explaining its purpose and impact. Since this is a breaking change that affects the connect implementation, it should be documented in:
- README.md to explain the feature's purpose and its relationship to WASM support
- Cargo.toml with a comment describing what the feature enables/disables
🔗 Analysis chain
Verify feature flag documentation.
The addition of grpc-transport feature flag is a breaking change that needs proper documentation. Let's verify this is documented in the crate's README or Cargo.toml.
Let me search for the Cargo manifest file as it might be in a different location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the feature is documented
echo "Checking documentation for grpc-transport feature..."
rg -A 5 "grpc-transport" README.md || echo "Feature not found in README.md"
rg -A 5 "grpc-transport" Cargo.toml || echo "Feature not found in Cargo.toml"
Length of output: 380
Script:
#!/bin/bash
# Find Cargo.toml files
fd Cargo.toml
# Once found, check their contents for the feature flag
fd Cargo.toml --exec sh -c 'echo "\n=== {} ==="; cat {} | grep -A 5 "grpc-transport" || echo "Feature not found"'
Length of output: 433
| let regex = "impl(.+)tonic::transport(.+)"; | ||
| let replacement = "#[cfg(feature = \"grpc-transport\")]\n \ | ||
| impl${1}tonic::transport${2}"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a Rust parser instead of regex for code modification
Using regular expressions to modify Rust code can be fragile and may not handle all edge cases correctly. Consider using a Rust parsing library like syn or quote to parse and manipulate the code in a more robust way.
| let mut path = PathBuf::from(env::var("CARGO_MANIFEST_DIR").map_err(|e| { | ||
| tonic_buf_build::error::TonicBufBuildError { | ||
| message: format!("Failed to get CARGO_MANIFEST_DIR: {}", e), | ||
| cause: None, | ||
| } | ||
| })?); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify error handling when retrieving CARGO_MANIFEST_DIR
Since the main function returns Result<(), Box<dyn std::error::Error>>, you can directly use the ? operator without custom error mapping.
Apply this diff to simplify error handling:
-let mut path = PathBuf::from(env::var("CARGO_MANIFEST_DIR").map_err(|e| {
- tonic_buf_build::error::TonicBufBuildError {
- message: format!("Failed to get CARGO_MANIFEST_DIR: {}", e),
- cause: None,
- }
-})?);
+let mut path = PathBuf::from(env::var("CARGO_MANIFEST_DIR")?);📝 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.
| let mut path = PathBuf::from(env::var("CARGO_MANIFEST_DIR").map_err(|e| { | |
| tonic_buf_build::error::TonicBufBuildError { | |
| message: format!("Failed to get CARGO_MANIFEST_DIR: {}", e), | |
| cause: None, | |
| } | |
| })?); | |
| let mut path = PathBuf::from(env::var("CARGO_MANIFEST_DIR")?); |
| #[derive(Clone, PartialEq, ::prost::Message)] | ||
| pub struct ListingVaultDepositParams { | ||
| /// Amount that will be deposited into the new market vault exclusively | ||
| #[prost(bytes = "vec", tag = "1")] | ||
| pub new_vault_deposit_amount: ::prost::alloc::vec::Vec<u8>, | ||
| /// Amount deposited into the main vault exclusively. This amount does not | ||
| /// include the amount deposited into the new vault. | ||
| #[prost(bytes = "vec", tag = "2")] | ||
| pub main_vault_deposit_amount: ::prost::alloc::vec::Vec<u8>, | ||
| /// Lockup period for this deposit | ||
| #[prost(uint32, tag = "3")] | ||
| pub num_blocks_to_lock_shares: u32, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Suggest deriving the Debug trait for structs for better debugging
Currently, the structs ListingVaultDepositParams, GenesisState, QueryMarketsHardCap, QueryMarketsHardCapResponse, QueryListingVaultDepositParams, QueryListingVaultDepositParamsResponse, MsgSetMarketsHardCap, MsgCreateMarketPermissionless, and MsgSetListingVaultDepositParams derive Clone and PartialEq. Deriving the Debug trait would allow for easier debugging and logging of these structs.
Apply the following changes to derive the Debug trait:
#[derive(Clone, PartialEq, ::prost::Message)]
+#[derive(Clone, PartialEq, Debug, ::prost::Message)]
pub struct ListingVaultDepositParams {
// fields...
}
// Repeat the above change for each of the mentioned structsAlso applies to: 27-46, 49-59, 62-75, 78-88, 91-104, 258-264, 291-298, 326-332
| self.inner = self.inner.max_decoding_message_size(limit); | ||
| self | ||
| } | ||
| /// Limits the maximum size of an encoded message. | ||
| /// | ||
| /// Default: `usize::MAX` | ||
| #[must_use] | ||
| pub fn max_encoding_message_size(mut self, limit: usize) -> Self { | ||
| self.inner = self.inner.max_encoding_message_size(limit); | ||
| self | ||
| } | ||
| /// RegisterAffiliate registers a referee-affiliate relationship | ||
| pub async fn register_affiliate( | ||
| &mut self, | ||
| request: impl tonic::IntoRequest<super::MsgRegisterAffiliate>, | ||
| ) -> std::result::Result< | ||
| tonic::Response<super::MsgRegisterAffiliateResponse>, | ||
| tonic::Status, | ||
| > { | ||
| self.inner | ||
| .ready() | ||
| .await | ||
| .map_err(|e| { | ||
| tonic::Status::unknown( | ||
| format!("Service was not ready: {}", e.into()), | ||
| ) | ||
| })?; | ||
| let codec = tonic::codec::ProstCodec::default(); | ||
| let path = http::uri::PathAndQuery::from_static( | ||
| "/dydxprotocol.affiliates.Msg/RegisterAffiliate", | ||
| ); | ||
| let mut req = request.into_request(); | ||
| req.extensions_mut() | ||
| .insert( | ||
| GrpcMethod::new("dydxprotocol.affiliates.Msg", "RegisterAffiliate"), | ||
| ); | ||
| self.inner.unary(req, path, codec).await | ||
| } | ||
| /// UpdateAffiliateTiers updates affiliate tiers | ||
| pub async fn update_affiliate_tiers( | ||
| &mut self, | ||
| request: impl tonic::IntoRequest<super::MsgUpdateAffiliateTiers>, | ||
| ) -> std::result::Result< | ||
| tonic::Response<super::MsgUpdateAffiliateTiersResponse>, | ||
| tonic::Status, | ||
| > { | ||
| self.inner | ||
| .ready() | ||
| .await | ||
| .map_err(|e| { | ||
| tonic::Status::unknown( | ||
| format!("Service was not ready: {}", e.into()), | ||
| ) | ||
| })?; | ||
| let codec = tonic::codec::ProstCodec::default(); | ||
| let path = http::uri::PathAndQuery::from_static( | ||
| "/dydxprotocol.affiliates.Msg/UpdateAffiliateTiers", | ||
| ); | ||
| let mut req = request.into_request(); | ||
| req.extensions_mut() | ||
| .insert( | ||
| GrpcMethod::new( | ||
| "dydxprotocol.affiliates.Msg", | ||
| "UpdateAffiliateTiers", | ||
| ), | ||
| ); | ||
| self.inner.unary(req, path, codec).await | ||
| } | ||
| /// UpdateAffiliateWhitelist updates affiliate whitelist | ||
| pub async fn update_affiliate_whitelist( | ||
| &mut self, | ||
| request: impl tonic::IntoRequest<super::MsgUpdateAffiliateWhitelist>, | ||
| ) -> std::result::Result< | ||
| tonic::Response<super::MsgUpdateAffiliateWhitelistResponse>, | ||
| tonic::Status, | ||
| > { | ||
| self.inner | ||
| .ready() | ||
| .await | ||
| .map_err(|e| { | ||
| tonic::Status::unknown( | ||
| format!("Service was not ready: {}", e.into()), | ||
| ) | ||
| })?; | ||
| let codec = tonic::codec::ProstCodec::default(); | ||
| let path = http::uri::PathAndQuery::from_static( | ||
| "/dydxprotocol.affiliates.Msg/UpdateAffiliateWhitelist", | ||
| ); | ||
| let mut req = request.into_request(); | ||
| req.extensions_mut() | ||
| .insert( | ||
| GrpcMethod::new( | ||
| "dydxprotocol.affiliates.Msg", | ||
| "UpdateAffiliateWhitelist", | ||
| ), | ||
| ); | ||
| self.inner.unary(req, path, codec).await | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify and DRY up code in 'msg_client' module
Similar to the QueryClient, the MsgClient methods have repetitive structures. Refactoring with macros or helper functions can adhere to the DRY (Don't Repeat Yourself) principle and improve code clarity.
| /// Deprecated since v7.x in favor of GenesisState. | ||
| #[derive(Clone, PartialEq, ::prost::Message)] | ||
| pub struct NumShares { | ||
| /// Number of shares. | ||
| #[prost(bytes = "vec", tag = "2")] | ||
| pub num_shares: ::prost::alloc::vec::Vec<u8>, | ||
| pub struct GenesisStateV6 { | ||
| /// The vaults. | ||
| #[prost(message, repeated, tag = "2")] | ||
| pub vaults: ::prost::alloc::vec::Vec<Vault>, | ||
| /// The default quoting parameters for all vaults. | ||
| #[prost(message, optional, tag = "3")] | ||
| pub default_quoting_params: ::core::option::Option<QuotingParams>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing deprecated GenesisStateV6 and VaultV6 structs
The structs GenesisStateV6 (lines 368~ to 376~) and VaultV6 (lines 389~ to 415~) are deprecated since v7.x in favor of GenesisState and Vault, respectively. If backward compatibility is not required, removing these deprecated structs can enhance code clarity and maintainability.
Also applies to: 389-415
| /// Deprecated since v6.x as is replaced by MsgUpdateDefaultQuotingParams. | ||
| #[derive(Clone, PartialEq, ::prost::Message)] | ||
| pub struct OwnerShare { | ||
| pub struct MsgUpdateParams { | ||
| #[prost(string, tag = "1")] | ||
| pub owner: ::prost::alloc::string::String, | ||
| pub authority: ::prost::alloc::string::String, | ||
| /// The parameters to update. Each field must be set. | ||
| #[prost(message, optional, tag = "2")] | ||
| pub shares: ::core::option::Option<NumShares>, | ||
| pub params: ::core::option::Option<Params>, | ||
| } | ||
| impl ::prost::Name for OwnerShare { | ||
| const NAME: &'static str = "OwnerShare"; | ||
| impl ::prost::Name for MsgUpdateParams { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing deprecated message types MsgUpdateParams and MsgSetVaultQuotingParams
The message types MsgUpdateParams (lines 1332~ to 1341~) and MsgSetVaultQuotingParams (lines 1352~ to 1371~) are deprecated since v6.x and have been replaced by MsgUpdateDefaultQuotingParams and MsgSetVaultParams, respectively. Removing these deprecated messages can help streamline the codebase and avoid potential misuse.
Also applies to: 1352-1371
| /// TIME_IN_FORCE_FILL_OR_KILL has been deprecated and will be removed in | ||
| /// future versions. |
There was a problem hiding this comment.
Deprecation notice for TIME_IN_FORCE_FILL_OR_KILL
The TIME_IN_FORCE_FILL_OR_KILL variant is marked as deprecated. Ensure that this variant is no longer used in the codebase and consider removing it in future versions to prevent confusion.
| #[deprecated] | ||
| #[prost(message, repeated, tag = "1")] | ||
| pub placed_long_term_order_ids: ::prost::alloc::vec::Vec<OrderId>, | ||
| #[prost(message, repeated, tag = "2")] | ||
| pub expired_stateful_order_ids: ::prost::alloc::vec::Vec<OrderId>, | ||
| #[prost(message, repeated, tag = "3")] | ||
| pub order_ids_filled_in_last_block: ::prost::alloc::vec::Vec<OrderId>, | ||
| #[deprecated] |
There was a problem hiding this comment.
Usage of deprecated fields in ProcessProposerMatchesEvents
Fields placed_long_term_order_ids and placed_stateful_cancellation_order_ids are marked as deprecated. Ensure that these fields are no longer used in the codebase and consider removing them in a future release to clean up the code.
Changelist
grpc-transportfeature, similarly tocosmos-sdk-proto, in an effort to optionally remove the dependency ontonic::transport, and allow compilation for targets such as WASM;Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
accountplus,affiliates, andlisting.Improvements
Copyfor better memory handling.tonic::Status::unknown.Deprecations
Documentation