-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Protocol support for Rust #2172
base: main
Are you sure you want to change the base?
Conversation
Rust proto impl From -> Any for more generated messages
Use const names
Impl Any::from MsgBatchCancel, MsgSend
Add the `ToAny` trait
Implement the Name trait and use it for ToAny
WalkthroughThe changes introduce a new Rust module for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC Server
participant Redis
participant dYdX Protocol
Client->>gRPC Server: Request to manage assets
gRPC Server->>dYdX Protocol: Process asset management request
dYdX Protocol->>Redis: Store asset data
Redis-->>dYdX Protocol: Confirm storage
dYdX Protocol-->>gRPC Server: Return asset management result
gRPC Server-->>Client: Send response
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (36)
- .gitignore (1 hunks)
- v4-proto-rs/Cargo.toml (1 hunks)
- v4-proto-rs/README.md (1 hunks)
- v4-proto-rs/build.rs (1 hunks)
- v4-proto-rs/src/_includes.rs (1 hunks)
- v4-proto-rs/src/cosmos.base.query.v1beta1.rs (1 hunks)
- v4-proto-rs/src/cosmos.base.v1beta1.rs (1 hunks)
- v4-proto-rs/src/cosmos_proto.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.assets.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.blocktime.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.bridge.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.bridge.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.liquidation.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.delaymsg.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.epochs.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.feetiers.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.govplus.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.events.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.indexer_manager.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.off_chain_updates.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.indexer.protocol.v1.rs (1 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 (1 hunks)
- v4-proto-rs/src/dydxprotocol.perpetuals.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.prices.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.ratelimit.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.rewards.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.sending.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.stats.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.subaccounts.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.vault.rs (1 hunks)
- v4-proto-rs/src/dydxprotocol.vest.rs (1 hunks)
- v4-proto-rs/src/google.api.rs (1 hunks)
- v4-proto-rs/src/lib.rs (1 hunks)
Files skipped from review due to trivial changes (5)
- .gitignore
- v4-proto-rs/Cargo.toml
- v4-proto-rs/src/_includes.rs
- v4-proto-rs/src/dydxprotocol.indexer.protocol.v1.rs
- v4-proto-rs/src/dydxprotocol.sending.rs
Additional context used
LanguageTool
v4-proto-rs/README.md
[grammar] ~1-~1: The singular proper name ‘Rust’ must be used with a third-person or a past tense verb.
Context: # Rust crate for dYdX Chain protobufs ## Usage as a...(HE_VERB_AGR)
[style] ~58-~58: In American English, abbreviations like “etc.” require a period.
Context: ...t? The main work (parsing, linking etc - have a look https://protobuf.com/docs...(ETC_PERIOD)
[uncategorized] ~64-~64: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ation-deserialization stubs for messages but we also need a client implementation (g...(COMMA_COMPOUND_SENTENCE)
[grammar] ~68-~68: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...iles. Protobuf specifications can refer 3d party protobuf specifications and use t...(THREE_D)
[grammar] ~68-~68: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...otobuf files, downloads them and allows to export (=copy) them in a specified directory. ...(ALLOW_TO)
[grammar] ~68-~68: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...files in this repository and downloaded 3d party proto files (aka "includes") are ...(THREE_D)
Markdownlint
v4-proto-rs/README.md
58-58: null
Bare URL used(MD034, no-bare-urls)
60-60: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (301)
v4-proto-rs/build.rs (1)
1-3
: LGTM!The imports are necessary for the functionality implemented in the file.
v4-proto-rs/src/lib.rs (4)
1-2
: LGTM!The re-export of
cosmos_sdk_proto
is necessary for the functionality implemented in the file.
4-4
: LGTM!The inclusion of generated code from
_includes.rs
is necessary for the functionality implemented in the file.
6-7
: LGTM!The imports of
prost::Name
andprost_types
are necessary for the functionality implemented in the file.
17-17
: LGTM!The implementation of the
ToAny
trait for types that implementprost::Name
is correct and necessary for the functionality implemented in the file.v4-proto-rs/src/cosmos.base.v1beta1.rs (4)
24-35
: Add documentation for theDecCoin
struct.Adding comments to explain the purpose of the struct would improve readability.
Apply this diff to add documentation for the
DecCoin
struct:/// DecCoin defines a token with a denomination and a decimal amount. /// /// NOTE: The amount field is an Dec which implements the custom method /// signatures required by gogoproto. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct DecCoin { #[prost(string, tag = "1")] pub denom: ::prost::alloc::string::String, #[prost(string, tag = "2")] pub amount: ::prost::alloc::string::String, }Likely invalid or redundant comment.
2-13
: Add documentation for theCoin
struct.Adding comments to explain the purpose of the struct would improve readability.
Apply this diff to add documentation for the
Coin
struct:/// Coin defines a token with a denomination and an amount. /// /// NOTE: The amount field is an Int which implements the custom method /// signatures required by gogoproto. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Coin { #[prost(string, tag = "1")] pub denom: ::prost::alloc::string::String, #[prost(string, tag = "2")] pub amount: ::prost::alloc::string::String, }Likely invalid or redundant comment.
64-71
: Add documentation for theDecProto
struct.Adding comments to explain the purpose of the struct would improve readability.
Apply this diff to add documentation for the
DecProto
struct:/// DecProto defines a Protobuf wrapper around a Dec object. /// Deprecated: Prefer to use math.LegacyDec directly. It supports binary Marshal and Unmarshal. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct DecProto { #[prost(string, tag = "1")] pub dec: ::prost::alloc::string::String, }Likely invalid or redundant comment.
46-53
: Add documentation for theIntProto
struct.Adding comments to explain the purpose of the struct would improve readability.
Apply this diff to add documentation for the
IntProto
struct:/// IntProto defines a Protobuf wrapper around an Int object. /// Deprecated: Prefer to use math.Int directly. It supports binary Marshal and Unmarshal. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct IntProto { #[prost(string, tag = "1")] pub int: ::prost::alloc::string::String, }Likely invalid or redundant comment.
v4-proto-rs/src/cosmos.base.query.v1beta1.rs (4)
1-37
: LGTM!The
PageRequest
struct is correctly defined for pagination in gRPC requests.
48-67
: LGTM!The
PageResponse
struct is correctly defined for pagination in gRPC responses.
38-47
: LGTM!The implementation of
prost::Name
forPageRequest
is correctly defined.
68-77
: LGTM!The implementation of
prost::Name
forPageResponse
is correctly defined.v4-proto-rs/src/dydxprotocol.indexer.redis.rs (3)
1-26
: LGTM!The
RedisOrder
struct is correctly defined for representing orders stored in Redis.
27-72
: LGTM!The
TickerType
enum and its methods are correctly defined.
74-83
: LGTM!The implementation of
prost::Name
forRedisOrder
is correctly defined.v4-proto-rs/README.md (3)
12-18
: LGTM!The example usage section is correctly defined.
20-25
: LGTM!The note and idiomatic Rust section is correctly defined.
1-10
: Fix grammar issue in the title.The singular proper name ‘Rust’ must be used with a third-person or a past tense verb.
Apply this diff to fix the grammar issue:
-# Rust crate for dYdX Chain protobufs +# Rust crate for dYdX Chain protobufLikely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~1-~1: The singular proper name ‘Rust’ must be used with a third-person or a past tense verb.
Context: # Rust crate for dYdX Chain protobufs ## Usage as a...(HE_VERB_AGR)
v4-proto-rs/src/cosmos_proto.rs (3)
1-27
: LGTM!The
InterfaceDescriptor
structure is well-defined and follows the protobuf conventions.
28-65
: LGTM!The
ScalarDescriptor
structure is well-defined and follows the protobuf conventions.
66-94
: LGTM!The
ScalarType
enumeration is well-defined and follows the protobuf conventions.v4-proto-rs/src/dydxprotocol.daemons.bridge.rs (3)
1-19
: LGTM!The
AddBridgeEventsRequest
structure is well-defined and follows the protobuf conventions.
20-33
: LGTM!The
AddBridgeEventsResponse
structure is well-defined and follows the protobuf conventions.
34-151
: LGTM!The client implementation for
BridgeService
is well-defined and follows the gRPC conventions.v4-proto-rs/src/dydxprotocol.indexer.shared.rs (1)
1-125
: LGTM!The
OrderRemovalReason
enumeration is well-defined and follows the protobuf conventions.v4-proto-rs/src/dydxprotocol.daemons.liquidation.rs (3)
38-52
: LGTM! But verify the usage of the struct.The
LiquidateSubaccountsResponse
struct is correctly defined. Ensure that the struct is used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
Verification successful
The
LiquidateSubaccountsResponse
struct is used correctly.The struct is appropriately used as a response type in a gRPC service, consistent with its intended purpose. No issues were found with its usage in the codebase.
- The struct is defined and used in
v4-proto-rs/src/dydxprotocol.daemons.liquidation.rs
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `LiquidateSubaccountsResponse` struct. # Test: Search for the struct usage. Expect: Correct usage of the struct. rg --type rust -A 5 $'LiquidateSubaccountsResponse'Length of output: 2330
1-27
: LGTM! But verify the usage of the struct.The
LiquidateSubaccountsRequest
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
53-169
: LGTM! But verify the usage of the methods.The
LiquidationServiceClient
struct and its methods are correctly defined. Ensure that the methods are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the methods:
v4-proto-rs/src/dydxprotocol.indexer.indexer_manager.rs (5)
1-11
: LGTM! But verify the usage of the struct.The
IndexerTendermintEventWrapper
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
Verification successful
Verification Successful: Usage of
IndexerTendermintEventWrapper
is CorrectThe
IndexerTendermintEventWrapper
struct is used appropriately within the codebase. It is part of a vector in theIndexerEventsStoreValue
struct, which aligns with its intended purpose. No issues were found in its usage.
- The struct is defined and used in
v4-proto-rs/src/dydxprotocol.indexer.indexer_manager.rs
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `IndexerTendermintEventWrapper` struct. # Test: Search for the struct usage. Expect: Correct usage of the fields. rg --type rust -A 5 $'IndexerTendermintEventWrapper'Length of output: 3067
137-161
: LGTM! But verify the usage of the struct.The
IndexerTendermintBlock
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
22-29
: LGTM! But verify the usage of the struct.The
IndexerEventsStoreValue
struct is correctly defined. Ensure that the field is used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
40-66
: LGTM! But verify the usage of the struct.The
IndexerTendermintEvent
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
70-114
: LGTM! But verify the usage of the enum.The
BlockEvent
enum is correctly defined. Ensure that the enum is used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the enum:
v4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs (5)
1-8
: LGTM! But verify the usage of the struct.The
UpdateMarketPricesRequest
struct is correctly defined. Ensure that the field is used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
Verification successful
Struct Usage Verified:
UpdateMarketPricesRequest
is used correctly.The
UpdateMarketPricesRequest
struct is utilized appropriately within a gRPC service request, confirming its correct implementation and usage in the codebase. No further issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `UpdateMarketPricesRequest` struct. # Test: Search for the struct usage. Expect: Correct usage of the field. rg --type rust -A 5 $'UpdateMarketPricesRequest'Length of output: 2224
19-32
: LGTM! But verify the usage of the struct.The
UpdateMarketPricesResponse
struct is correctly defined. Ensure that the struct is used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
Verification successful
The
UpdateMarketPricesResponse
struct is used correctly.The
UpdateMarketPricesResponse
struct is utilized as part of a gRPC service response, which aligns with its intended purpose. The usage appears consistent and correct within the codebase.
- The struct is defined and used in
v4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `UpdateMarketPricesResponse` struct. # Test: Search for the struct usage. Expect: Correct usage of the struct. rg --type rust -A 5 $'UpdateMarketPricesResponse'Length of output: 4131
33-42
: LGTM! But verify the usage of the struct.The
ExchangePrice
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
Verification successful
Usage of
ExchangePrice
Struct VerifiedThe
ExchangePrice
struct is correctly defined and used within thev4-proto-rs/src/dydxprotocol.daemons.pricefeed.rs
file. There are no additional usages found outside this file, and the struct is utilized appropriately in its context. No issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `ExchangePrice` struct. # Test: Search for the struct usage. Expect: Correct usage of the fields. rg --type rust -A 5 $'ExchangePrice'Length of output: 2649
Script:
#!/bin/bash # Description: Search for all usages of the `ExchangePrice` struct across the repository. # Test: Search for the struct usage. Expect: Correct usage of the fields. rg --type rust -A 5 'ExchangePrice'Length of output: 2649
54-61
: LGTM! But verify the usage of the struct.The
MarketPriceUpdate
struct is correctly defined. Ensure that the fields are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the struct:
73-189
: LGTM! But verify the usage of the methods.The
PriceFeedServiceClient
struct and its methods are correctly defined. Ensure that the methods are used correctly in the rest of the codebase.The code changes are approved.
Run the following script to verify the usage of the methods:
v4-proto-rs/src/dydxprotocol.indexer.socks.rs (6)
1-25
: LGTM!The code changes are approved.
26-58
: LGTM!The code changes are approved.
59-85
: LGTM!The code changes are approved.
86-106
: LGTM!The code changes are approved.
107-196
: LGTM!The code changes are approved.
125-185
: LGTM!The code changes are approved.
v4-proto-rs/src/dydxprotocol.indexer.off_chain_updates.rs (7)
1-85
: LGTM!The code changes are approved.
15-74
: LGTM!The code changes are approved.
86-178
: LGTM!The code changes are approved.
102-167
: LGTM!The code changes are approved.
179-198
: LGTM!The code changes are approved.
199-218
: LGTM!The code changes are approved.
220-256
: LGTM!The code changes are approved.
v4-proto-rs/src/dydxprotocol.govplus.rs (5)
1-15
: LGTM!The code changes are approved.
16-102
: LGTM!The code changes are approved.
104-146
: LGTM!The code changes are approved.
147-160
: LGTM!The code changes are approved.
161-274
: LGTM!The code changes are approved.
v4-proto-rs/src/dydxprotocol.epochs.rs (7)
3-41
: LGTM!The struct
EpochInfo
is well-defined and follows the protobuf conventions.
54-60
: LGTM!The struct
GenesisState
is well-defined and follows the protobuf conventions.
72-77
: LGTM!The struct
QueryGetEpochInfoRequest
is well-defined and follows the protobuf conventions.
89-93
: LGTM!The struct
QueryEpochInfoResponse
is well-defined and follows the protobuf conventions.
106-112
: LGTM!The struct
QueryAllEpochInfoRequest
is well-defined and follows the protobuf conventions.
126-133
: LGTM!The struct
QueryEpochInfoAllResponse
is well-defined and follows the protobuf conventions.
145-283
: LGTM!The module
query_client
is well-defined and follows the gRPC client implementation conventions.v4-proto-rs/src/dydxprotocol.rewards.rs (8)
3-24
: LGTM!The struct
Params
is well-defined and follows the protobuf conventions.
37-42
: LGTM!The struct
GenesisState
is well-defined and follows the protobuf conventions.
55-57
: LGTM!The struct
QueryParamsRequest
is well-defined and follows the protobuf conventions.
70-73
: LGTM!The struct
QueryParamsResponse
is well-defined and follows the protobuf conventions.
202-207
: LGTM!The struct
RewardShare
is well-defined and follows the protobuf conventions.
221-227
: LGTM!The struct
MsgUpdateParams
is well-defined and follows the protobuf conventions.
86-196
: LGTM!The module
query_client
is well-defined and follows the gRPC client implementation conventions.
Line range hint
253-370
: LGTM!The module
msg_client
is well-defined and follows the gRPC client implementation conventions.v4-proto-rs/src/dydxprotocol.assets.rs (8)
3-41
: LGTM!The struct
Asset
is well-defined and follows the protobuf conventions.
55-58
: LGTM!The struct
GenesisState
is well-defined and follows the protobuf conventions.
72-75
: LGTM!The struct
QueryAssetRequest
is well-defined and follows the protobuf conventions.
89-92
: LGTM!The struct
QueryAssetResponse
is well-defined and follows the protobuf conventions.
106-111
: LGTM!The struct
QueryAllAssetsRequest
is well-defined and follows the protobuf conventions.
125-132
: LGTM!The struct
QueryAllAssetsResponse
is well-defined and follows the protobuf conventions.
145-281
: LGTM!The module
query_client
is well-defined and follows the gRPC client implementation conventions.
285-370
: LGTM!The module
msg_client
is well-defined and follows the gRPC client implementation conventions.v4-proto-rs/src/dydxprotocol.vest.rs (10)
1-24
: LGTM!The
VestEntry
struct and its implementation are correctly defined.
35-52
: LGTM!The
GenesisState
struct and its implementation are correctly defined.
53-69
: LGTM!The
QueryVestEntryRequest
struct and its implementation are correctly defined.
70-86
: LGTM!The
QueryVestEntryResponse
struct and its implementation are correctly defined.
87-200
: LGTM!The
query_client
module and its methods are correctly defined.
201-221
: LGTM!The
MsgDeleteVestEntry
struct and its implementation are correctly defined.
222-235
: LGTM!The
MsgDeleteVestEntryResponse
struct and its implementation are correctly defined.
236-255
: LGTM!The
MsgSetVestEntry
struct and its implementation are correctly defined.
256-270
: LGTM!The
MsgSetVestEntryResponse
struct and its implementation are correctly defined.
Line range hint
271-435
: LGTM!The
msg_client
module and its methods are correctly defined.v4-proto-rs/src/google.api.rs (3)
1-31
: LGTM!The
Http
struct and its implementation are correctly defined.
32-381
: LGTM!The
HttpRule
struct and its implementation are correctly defined.
382-402
: LGTM!The
CustomHttpPattern
struct and its implementation are correctly defined.v4-proto-rs/src/dydxprotocol.feetiers.rs (8)
1-19
: LGTM!The
PerpetualFeeParams
struct and its implementation are correctly defined.
20-52
: LGTM!The
PerpetualFeeTier
struct and its implementation are correctly defined.
53-70
: LGTM!The
GenesisState
struct and its implementation are correctly defined.
71-85
: LGTM!The
QueryPerpetualFeeParamsRequest
struct and its implementation are correctly defined.
86-103
: LGTM!The
QueryPerpetualFeeParamsResponse
struct and its implementation are correctly defined.
104-120
: LGTM!The
QueryUserFeeTierRequest
struct and its implementation are correctly defined.
121-140
: LGTM!The
QueryUserFeeTierResponse
struct and its implementation are correctly defined.
141-280
: LGTM!The
query_client
module and its methods are correctly defined.v4-proto-rs/src/dydxprotocol.delaymsg.rs (13)
2-21
: LGTM!The struct
BlockMessageIds
and its implementation are correct.
22-45
: LGTM!The struct
DelayedMessage
and its implementation are correct.
46-66
: LGTM!The struct
GenesisState
and its implementation are correct.
67-81
: LGTM!The struct
QueryNextDelayedMessageIdRequest
and its implementation are correct.
82-99
: LGTM!The struct
QueryNextDelayedMessageIdResponse
and its implementation are correct.
100-116
: LGTM!The struct
QueryMessageRequest
and its implementation are correct.
117-133
: LGTM!The struct
QueryMessageResponse
and its implementation are correct.
134-151
: LGTM!The struct
QueryBlockMessageIdsRequest
and its implementation are correct.
152-169
: LGTM!The struct
QueryBlockMessageIdsResponse
and its implementation are correct.
170-341
: LGTM!The module
query_client
is correctly implemented.
343-365
: LGTM!The struct
MsgDelayMessage
and its implementation are correct.
366-383
: LGTM!The struct
MsgDelayMessageResponse
and its implementation are correct.
Line range hint
384-519
: LGTM!The module
msg_client
is correctly implemented.v4-proto-rs/src/dydxprotocol.ratelimit.rs (16)
3-24
: LGTM!The struct
LimitParams
and its implementation are correctly defined and documented.
26-53
: LGTM!The struct
Limiter
and its implementation are correctly defined and documented.
55-77
: LGTM!The struct
DenomCapacity
and its implementation are correctly defined and documented.
79-96
: LGTM!The struct
LimiterCapacity
and its implementation are correctly defined and documented.
97-114
: LGTM!The struct
GenesisState
and its implementation are correctly defined and documented.
115-134
: LGTM!The struct
PendingSendPacket
and its implementation are correctly defined and documented.
135-148
: LGTM!The struct
ListLimitParamsRequest
and its implementation are correctly defined.
149-165
: LGTM!The struct
ListLimitParamsResponse
and its implementation are correctly defined and documented.
166-183
: LGTM!The struct
QueryCapacityByDenomRequest
and its implementation are correctly defined and documented.
184-201
: LGTM!The struct
QueryCapacityByDenomResponse
and its implementation are correctly defined and documented.
202-216
: LGTM!The struct
QueryAllPendingSendPacketsRequest
and its implementation are correctly defined.
217-234
: LGTM!The struct
QueryAllPendingSendPacketsResponse
and its implementation are correctly defined and documented.
235-407
: LGTM!The module
query_client
and its implementations are correctly defined and documented.
410-429
: LGTM!The struct
MsgSetLimitParams
and its implementation are correctly defined and documented.
430-443
: LGTM!The struct
MsgSetLimitParamsResponse
and its implementation are correctly defined.
Line range hint
444-649
: LGTM!The module
msg_client
and its implementations are correctly defined and documented.v4-proto-rs/src/dydxprotocol.stats.rs (10)
3-19
: LGTM!The struct
Params
and its implementation are correctly defined and documented.
20-37
: LGTM!The struct
GenesisState
and its implementation are correctly defined and documented.
38-82
: LGTM!The struct
BlockStats
and its implementation are correctly defined and documented.
47-71
: LGTM!The module
block_stats
and its implementations are correctly defined and documented.
83-101
: LGTM!The struct
StatsMetadata
and its implementation are correctly defined and documented.
102-143
: LGTM!The struct
EpochStats
and its implementation are correctly defined and documented.
114-133
: LGTM!The module
epoch_stats
and its implementations are correctly defined and documented.
145-161
: LGTM!The struct
GlobalStats
and its implementation are correctly defined and documented.
163-183
: LGTM!The struct
UserStats
and its implementation are correctly defined and documented.
184-197
: LGTM!The struct
QueryParamsRequest
and its implementation are correctly defined.v4-proto-rs/src/dydxprotocol.vault.rs (21)
1-1
: LGTM!The file header is approved.
2-33
: LGTM!The data structure
Params
is well-defined and follows the protobuf conventions.
44-61
: LGTM!The data structure
GenesisState
is well-defined and follows the protobuf conventions.
62-82
: LGTM!The data structure
VaultId
is well-defined and follows the protobuf conventions.
83-100
: LGTM!The data structure
NumShares
is well-defined and follows the protobuf conventions.
101-129
: LGTM!The enum
VaultType
is well-defined and follows the protobuf conventions.
130-143
: LGTM!The data structure
QueryParamsRequest
is well-defined and follows the protobuf conventions.
144-160
: LGTM!The data structure
QueryParamsResponse
is well-defined and follows the protobuf conventions.
161-179
: LGTM!The data structure
QueryVaultRequest
is well-defined and follows the protobuf conventions.
180-204
: LGTM!The data structure
QueryVaultResponse
is well-defined and follows the protobuf conventions.
205-223
: LGTM!The data structure
QueryAllVaultsRequest
is well-defined and follows the protobuf conventions.
224-244
: LGTM!The data structure
QueryAllVaultsResponse
is well-defined and follows the protobuf conventions.
245-267
: LGTM!The data structure
QueryOwnerSharesRequest
is well-defined and follows the protobuf conventions.
268-286
: LGTM!The data structure
OwnerShare
is well-defined and follows the protobuf conventions.
287-307
: LGTM!The data structure
QueryOwnerSharesResponse
is well-defined and follows the protobuf conventions.
308-497
: LGTM!The module
query_client
is well-defined and follows the protobuf conventions.
500-523
: LGTM!The data structure
MsgDepositToVault
is well-defined and follows the protobuf conventions.
524-537
: LGTM!The data structure
MsgDepositToVaultResponse
is well-defined and follows the protobuf conventions.
538-557
: LGTM!The data structure
MsgUpdateParams
is well-defined and follows the protobuf conventions.
558-571
: LGTM!The data structure
MsgUpdateParamsResponse
is well-defined and follows the protobuf conventions.
572-709
: LGTM!The module
msg_client
is well-defined and follows the protobuf conventions.v4-proto-rs/src/dydxprotocol.prices.rs (31)
2-32
: LGTM!The
MarketParam
struct is well-defined and documented.
34-43
: LGTM!The implementation of
MarketParam
is correctly done.
45-58
: LGTM!The
MarketPrice
struct is well-defined and documented.
60-69
: LGTM!The implementation of
MarketPrice
is correctly done.
71-77
: LGTM!The
GenesisState
struct is well-defined and documented.
79-88
: LGTM!The implementation of
GenesisState
is correctly done.
89-95
: LGTM!The
QueryMarketPriceRequest
struct is well-defined and documented.
97-106
: LGTM!The implementation of
QueryMarketPriceRequest
is correctly done.
108-113
: LGTM!The
QueryMarketPriceResponse
struct is well-defined and documented.
115-124
: LGTM!The implementation of
QueryMarketPriceResponse
is correctly done.
125-133
: LGTM!The
QueryAllMarketPricesRequest
struct is well-defined and documented.
135-144
: LGTM!The implementation of
QueryAllMarketPricesRequest
is correctly done.
146-155
: LGTM!The
QueryAllMarketPricesResponse
struct is well-defined and documented.
157-166
: LGTM!The implementation of
QueryAllMarketPricesResponse
is correctly done.
167-173
: LGTM!The
QueryMarketParamRequest
struct is well-defined and documented.
175-184
: LGTM!The implementation of
QueryMarketParamRequest
is correctly done.
185-191
: LGTM!The
QueryMarketParamResponse
struct is well-defined and documented.
193-202
: LGTM!The implementation of
QueryMarketParamResponse
is correctly done.
203-211
: LGTM!The
QueryAllMarketParamsRequest
struct is well-defined and documented.
213-222
: LGTM!The implementation of
QueryAllMarketParamsRequest
is correctly done.
224-233
: LGTM!The
QueryAllMarketParamsResponse
struct is well-defined and documented.
235-244
: LGTM!The implementation of
QueryAllMarketParamsResponse
is correctly done.
245-434
: LGTM!The
query_client
module is well-defined and documented.
437-447
: LGTM!The
MsgCreateOracleMarket
struct is well-defined and documented.
449-458
: LGTM!The implementation of
MsgCreateOracleMarket
is correctly done.
459-462
: LGTM!The
MsgCreateOracleMarketResponse
struct is well-defined and documented.
464-472
: LGTM!The implementation of
MsgCreateOracleMarketResponse
is correctly done.
473-480
: LGTM!The
MsgUpdateMarketPrices
struct is well-defined and documented.
482-504
: LGTM!The
msg_update_market_prices
module is well-defined and documented.
506-515
: LGTM!The implementation of
MsgUpdateMarketPrices
is correctly done.
516-520
: LGTM!The
MsgUpdateMarketPricesResponse
struct is well-defined and documented.v4-proto-rs/src/dydxprotocol.bridge.rs (26)
2-18
: LGTM!The struct
BridgeEvent
and its implementation are correctly defined.Also applies to: 19-28
29-40
: LGTM!The struct
BridgeEventInfo
and its implementation are correctly defined.Also applies to: 42-51
52-65
: LGTM!The struct
EventParams
and its implementation are correctly defined.Also applies to: 67-76
77-102
: LGTM!The struct
ProposeParams
and its implementation are correctly defined.Also applies to: 104-113
114-124
: LGTM!The struct
SafetyParams
and its implementation are correctly defined.Also applies to: 126-135
136-151
: LGTM!The struct
GenesisState
and its implementation are correctly defined.Also applies to: 153-162
163-169
: LGTM!The struct
MsgAcknowledgeBridges
and its implementation are correctly defined.Also applies to: 171-180
181-185
: LGTM!The struct
MsgAcknowledgeBridgesResponse
and its implementation are correctly defined.Also applies to: 187-194
196-204
: LGTM!The struct
MsgCompleteBridge
and its implementation are correctly defined.Also applies to: 206-215
216-219
: LGTM!The struct
MsgCompleteBridgeResponse
and its implementation are correctly defined.Also applies to: 221-228
230-238
: LGTM!The struct
MsgUpdateEventParams
and its implementation are correctly defined.Also applies to: 240-249
250-253
: LGTM!The struct
MsgUpdateEventParamsResponse
and its implementation are correctly defined.Also applies to: 255-262
264-272
: LGTM!The struct
MsgUpdateProposeParams
and its implementation are correctly defined.Also applies to: 274-283
284-287
: LGTM!The struct
MsgUpdateProposeParamsResponse
and its implementation are correctly defined.Also applies to: 289-296
298-306
: LGTM!The struct
MsgUpdateSafetyParams
and its implementation are correctly defined.Also applies to: 308-317
318-321
: LGTM!The struct
MsgUpdateSafetyParamsResponse
and its implementation are correctly defined.Also applies to: 323-330
557-560
: LGTM!The struct
QueryEventParamsRequest
and its implementation are correctly defined.Also applies to: 562-569
572-576
: LGTM!The struct
QueryEventParamsResponse
and its implementation are correctly defined.Also applies to: 578-587
588-591
: LGTM!The struct
QueryProposeParamsRequest
and its implementation are correctly defined.Also applies to: 593-600
602-608
: LGTM!The struct
QueryProposeParamsResponse
and its implementation are correctly defined.Also applies to: 610-619
620-623
: LGTM!The struct
QuerySafetyParamsRequest
and its implementation are correctly defined.Also applies to: 625-632
634-639
: LGTM!The struct
QuerySafetyParamsResponse
and its implementation are correctly defined.Also applies to: 641-649
651-655
: LGTM!The struct
QueryAcknowledgedEventInfoRequest
and its implementation are correctly defined.Also applies to: 657-664
666-672
: LGTM!The struct
QueryAcknowledgedEventInfoResponse
and its implementation are correctly defined.Also applies to: 674-683
684-688
: LGTM!The struct
QueryRecognizedEventInfoRequest
and its implementation are correctly defined.Also applies to: 690-697
699-705
: LGTM!The struct
QueryRecognizedEventInfoResponse
and its implementation are correctly defined.Also applies to: 707-716
v4-proto-rs/src/dydxprotocol.indexer.events.rs (25)
1-1
: File OverviewThis file contains multiple data structures and message formats for managing various aspects of the dYdX protocol. Each struct represents a specific event or entity within the protocol.
The file structure and organization are appropriate.
2-18
: LGTM!The
FundingUpdateV1
struct is well-defined and includes appropriate fields for representing funding update events.The code changes are approved.
29-48
: LGTM!The
FundingEventV1
struct is well-defined and includes appropriate fields for representing funding events.The code changes are approved.
112-123
: LGTM!The
MarketEventV1
struct is well-defined and includes appropriate fields for representing market events.The code changes are approved.
148-158
: LGTM!The
MarketPriceUpdateEventV1
struct is well-defined and includes appropriate fields for representing market price update events.The code changes are approved.
169-179
: LGTM!The
MarketBaseEventV1
struct is well-defined and includes appropriate fields for representing shared fields between market create and modify events.The code changes are approved.
191-203
: LGTM!The
MarketCreateEventV1
struct is well-defined and includes appropriate fields for representing market create events.The code changes are approved.
215-221
: LGTM!The
MarketModifyEventV1
struct is well-defined and includes appropriate fields for representing market modify events.The code changes are approved.
233-241
: LGTM!The
SourceOfFunds
struct is well-defined and includes appropriate fields for representing the source of funds in a transfer event.The code changes are approved.
267-297
: LGTM!The
TransferEventV1
struct is well-defined and includes appropriate fields for representing transfer events.The code changes are approved.
309-335
: LGTM!The
OrderFillEventV1
struct is well-defined and includes appropriate fields for representing order fill events.The code changes are approved.
359-389
: LGTM!The
DeleveragingEventV1
struct is well-defined and includes appropriate fields for representing deleveraging events.The code changes are approved.
401-428
: LGTM!The
LiquidationOrderV1
struct is well-defined and includes appropriate fields for representing liquidation orders.The code changes are approved.
440-460
: LGTM!The
SubaccountUpdateEventV1
struct is well-defined and includes appropriate fields for representing subaccount update events.The code changes are approved.
472-481
: LGTM!The
StatefulOrderEventV1
struct is well-defined and includes appropriate fields for representing stateful order events.The code changes are approved.
638-664
: LGTM!The
AssetCreateEventV1
struct is well-defined and includes appropriate fields for representing asset create events.The code changes are approved.
676-728
: LGTM!The
PerpetualMarketCreateEventV1
struct is well-defined and includes appropriate fields for representing perpetual market create events.The code changes are approved.
740-793
: LGTM!The
PerpetualMarketCreateEventV2
struct is well-defined and includes appropriate fields for representing perpetual market create events.The code changes are approved.
805-831
: LGTM!The
LiquidityTierUpsertEventV1
struct is well-defined and includes appropriate fields for representing liquidity tier upsert events.The code changes are approved.
843-870
: LGTM!The
UpdateClobPairEventV1
struct is well-defined and includes appropriate fields for representing updates to a clob pair.The code changes are approved.
882-909
: LGTM!The
UpdatePerpetualEventV1
struct is well-defined and includes appropriate fields for representing updates to a perpetual.The code changes are approved.
921-928
: LGTM!The
TradingRewardsEventV1
struct is well-defined and includes appropriate fields for representing trading rewards events.The code changes are approved.
940-951
: LGTM!The
AddressTradingReward
struct is well-defined and includes appropriate fields for representing an instance of an address receiving a reward.The code changes are approved.
963-969
: LGTM!The
OpenInterestUpdateEventV1
struct is well-defined and includes appropriate fields for representing open interest update events.The code changes are approved.
981-990
: LGTM!The
OpenInterestUpdate
struct is well-defined and includes appropriate fields for representing a single open interest update for a perpetual.The code changes are approved.
v4-proto-rs/src/dydxprotocol.perpetuals.rs (30)
2-16
: LGTM!The struct
Perpetual
is well-defined and uses theprost
library for serialization.
17-26
: LGTM!The impl block for
Perpetual
correctly implements theName
trait.
27-59
: LGTM!The struct
PerpetualParams
is well-defined and uses theprost
library for serialization.
60-69
: LGTM!The impl block for
PerpetualParams
correctly implements theName
trait.
70-81
: LGTM!The struct
MarketPremiums
is well-defined and uses theprost
library for serialization.
82-92
: LGTM!The impl block for
MarketPremiums
correctly implements theName
trait.
93-113
: LGTM!The struct
PremiumStore
is well-defined and uses theprost
library for serialization.
114-124
: LGTM!The impl block for
PremiumStore
correctly implements theName
trait.
125-168
: LGTM!The struct
LiquidityTier
is well-defined and uses theprost
library for serialization.
169-179
: LGTM!The impl block for
LiquidityTier
correctly implements theName
trait.
180-189
: LGTM!The enum
PerpetualMarketType
is well-defined and uses theprost
library for serialization.
190-210
: LGTM!The impl block for
PerpetualMarketType
correctly implements the methods.
211-229
: LGTM!The struct
Params
is well-defined and uses theprost
library for serialization.
230-240
: LGTM!The impl block for
Params
correctly implements theName
trait.
241-250
: LGTM!The struct
GenesisState
is well-defined and uses theprost
library for serialization.
251-261
: LGTM!The impl block for
GenesisState
correctly implements theName
trait.
262-267
: LGTM!The struct
QueryPerpetualRequest
is well-defined and uses theprost
library for serialization.
268-278
: LGTM!The impl block for
QueryPerpetualRequest
correctly implements theName
trait.
279-284
: LGTM!The struct
QueryPerpetualResponse
is well-defined and uses theprost
library for serialization.
285-295
: LGTM!The impl block for
QueryPerpetualResponse
correctly implements theName
trait.
296-303
: LGTM!The struct
QueryAllPerpetualsRequest
is well-defined and uses theprost
library for serialization.
304-314
: LGTM!The impl block for
QueryAllPerpetualsRequest
correctly implements theName
trait.
315-324
: LGTM!The struct
QueryAllPerpetualsResponse
is well-defined and uses theprost
library for serialization.
325-335
: LGTM!The impl block for
QueryAllPerpetualsResponse
correctly implements theName
trait.
336-343
: LGTM!The struct
QueryAllLiquidityTiersRequest
is well-defined and uses theprost
library for serialization.
344-354
: LGTM!The impl block for
QueryAllLiquidityTiersRequest
correctly implements theName
trait.
355-364
: LGTM!The struct
QueryAllLiquidityTiersResponse
is well-defined and uses theprost
library for serialization.
365-375
: LGTM!The impl block for
QueryAllLiquidityTiersResponse
correctly implements theName
trait.
377-380
: LGTM!The struct
QueryPremiumVotesRequest
is well-defined and uses theprost
library for serialization.
381-390
: LGTM!The impl block for
QueryPremiumVotesRequest
correctly implements theName
trait.
v4-proto-rs/build.rs
Outdated
fn main() -> Result<(), tonic_buf_build::error::TonicBufBuildError> { | ||
if std::env::var("V4_PROTO_REBUILD").is_ok() { | ||
let mut config = Config::new(); | ||
config.out_dir("src"); | ||
config.include_file("_includes.rs"); | ||
config.enable_type_names(); | ||
let mut path = PathBuf::from( | ||
env::var("CARGO_MANIFEST_DIR").expect("cargo sets CARGO_MANIFEST_DIR env var"), | ||
); | ||
path.pop(); | ||
tonic_buf_build::compile_from_buf_workspace_with_config( | ||
tonic_build::configure().build_server(false), | ||
Some(config), | ||
tonic_buf_build::TonicBufConfig { | ||
buf_dir: Some(path), | ||
}, | ||
)?; | ||
} | ||
|
||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve error handling and add comments.
- Replace the
expect
call with proper error handling. - Add comments to explain the purpose of each step.
Apply this diff to improve error handling and add comments:
fn main() -> Result<(), tonic_buf_build::error::TonicBufBuildError> {
if std::env::var("V4_PROTO_REBUILD").is_ok() {
// Create a new prost_build configuration.
let mut config = Config::new();
config.out_dir("src");
config.include_file("_includes.rs");
config.enable_type_names();
// Get the path to the workspace directory.
let mut path = PathBuf::from(
- env::var("CARGO_MANIFEST_DIR").expect("cargo sets CARGO_MANIFEST_DIR env var"),
+ env::var("CARGO_MANIFEST_DIR").map_err(|e| {
+ tonic_buf_build::error::TonicBufBuildError::new(format!("Failed to get CARGO_MANIFEST_DIR: {}", e))
+ })?,
);
path.pop();
// Compile the protocol buffers using the configuration.
tonic_buf_build::compile_from_buf_workspace_with_config(
tonic_build::configure().build_server(false),
Some(config),
tonic_buf_build::TonicBufConfig {
buf_dir: Some(path),
},
)?;
}
Ok(())
}
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.
fn main() -> Result<(), tonic_buf_build::error::TonicBufBuildError> { | |
if std::env::var("V4_PROTO_REBUILD").is_ok() { | |
let mut config = Config::new(); | |
config.out_dir("src"); | |
config.include_file("_includes.rs"); | |
config.enable_type_names(); | |
let mut path = PathBuf::from( | |
env::var("CARGO_MANIFEST_DIR").expect("cargo sets CARGO_MANIFEST_DIR env var"), | |
); | |
path.pop(); | |
tonic_buf_build::compile_from_buf_workspace_with_config( | |
tonic_build::configure().build_server(false), | |
Some(config), | |
tonic_buf_build::TonicBufConfig { | |
buf_dir: Some(path), | |
}, | |
)?; | |
} | |
Ok(()) | |
} | |
fn main() -> Result<(), tonic_buf_build::error::TonicBufBuildError> { | |
if std::env::var("V4_PROTO_REBUILD").is_ok() { | |
// Create a new prost_build configuration. | |
let mut config = Config::new(); | |
config.out_dir("src"); | |
config.include_file("_includes.rs"); | |
config.enable_type_names(); | |
// Get the path to the workspace directory. | |
let mut path = PathBuf::from( | |
env::var("CARGO_MANIFEST_DIR").map_err(|e| { | |
tonic_buf_build::error::TonicBufBuildError::new(format!("Failed to get CARGO_MANIFEST_DIR: {}", e)) | |
})?, | |
); | |
path.pop(); | |
// Compile the protocol buffers using the configuration. | |
tonic_buf_build::compile_from_buf_workspace_with_config( | |
tonic_build::configure().build_server(false), | |
Some(config), | |
tonic_buf_build::TonicBufConfig { | |
buf_dir: Some(path), | |
}, | |
)?; | |
} | |
Ok(()) | |
} |
v4-proto-rs/src/lib.rs
Outdated
pub trait ToAny: Name + Sized { | ||
fn to_any(self) -> prost_types::Any { | ||
let value = self.encode_to_vec(); | ||
let type_url = Self::type_url(); | ||
prost_types::Any { type_url, value } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add documentation for the ToAny
trait.
Adding comments to explain the purpose of the trait would improve readability.
Apply this diff to add documentation for the ToAny
trait:
pub trait ToAny: Name + Sized {
+ /// Converts the type to `prost_types::Any`.
fn to_any(self) -> prost_types::Any {
let value = self.encode_to_vec();
let type_url = Self::type_url();
prost_types::Any { type_url, value }
}
}
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.
pub trait ToAny: Name + Sized { | |
fn to_any(self) -> prost_types::Any { | |
let value = self.encode_to_vec(); | |
let type_url = Self::type_url(); | |
prost_types::Any { type_url, value } | |
} | |
} | |
pub trait ToAny: Name + Sized { | |
/// Converts the type to `prost_types::Any`. | |
fn to_any(self) -> prost_types::Any { | |
let value = self.encode_to_vec(); | |
let type_url = Self::type_url(); | |
prost_types::Any { type_url, value } | |
} | |
} |
v4-proto-rs/src/lib.rs
Outdated
#[cfg(test)] | ||
mod test { | ||
use super::ToAny; | ||
use crate::cosmos_sdk_proto::cosmos::bank::v1beta1::MsgSend; | ||
use crate::dydxprotocol::clob::MsgCancelOrder; | ||
|
||
#[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); | ||
} | ||
|
||
#[test] | ||
pub fn test_any_conversion_wrapped() { | ||
let msg = MsgSend::default(); | ||
let any = msg.to_any(); | ||
let url = "/cosmos.bank.v1beta1.MsgSend"; | ||
assert_eq!(any.type_url, url); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add documentation for the tests.
Adding comments to explain the purpose of each test would improve readability.
Apply this diff to add documentation for the tests:
#[cfg(test)]
mod test {
use super::ToAny;
use crate::cosmos_sdk_proto::cosmos::bank::v1beta1::MsgSend;
use crate::dydxprotocol::clob::MsgCancelOrder;
#[test]
+ /// Tests the conversion of `MsgCancelOrder` to `prost_types::Any`.
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);
}
#[test]
+ /// Tests the conversion of `MsgSend` to `prost_types::Any`.
pub fn test_any_conversion_wrapped() {
let msg = MsgSend::default();
let any = msg.to_any();
let url = "/cosmos.bank.v1beta1.MsgSend";
assert_eq!(any.type_url, url);
}
}
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.
#[cfg(test)] | |
mod test { | |
use super::ToAny; | |
use crate::cosmos_sdk_proto::cosmos::bank::v1beta1::MsgSend; | |
use crate::dydxprotocol::clob::MsgCancelOrder; | |
#[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); | |
} | |
#[test] | |
pub fn test_any_conversion_wrapped() { | |
let msg = MsgSend::default(); | |
let any = msg.to_any(); | |
let url = "/cosmos.bank.v1beta1.MsgSend"; | |
assert_eq!(any.type_url, url); | |
} | |
#[cfg(test)] | |
mod test { | |
use super::ToAny; | |
use crate::cosmos_sdk_proto::cosmos::bank::v1beta1::MsgSend; | |
use crate::dydxprotocol::clob::MsgCancelOrder; | |
#[test] | |
/// Tests the conversion of `MsgCancelOrder` to `prost_types::Any`. | |
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); | |
} | |
#[test] | |
/// Tests the conversion of `MsgSend` to `prost_types::Any`. | |
pub fn test_any_conversion_wrapped() { | |
let msg = MsgSend::default(); | |
let any = msg.to_any(); | |
let url = "/cosmos.bank.v1beta1.MsgSend"; | |
assert_eq!(any.type_url, url); | |
} | |
} |
v4-proto-rs/README.md
Outdated
## Local development | ||
|
||
### Prerequisites | ||
1) [Rust](https://www.rust-lang.org/tools/install) | ||
2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3d-party dependencies for protobuf files | ||
3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) to compile protobuf files with their 3d-party dependencies | ||
|
||
Then for a code (re-)generation run | ||
|
||
```sh | ||
V4_PROTO_REBUILD=1 cargo build -vv | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar issue in the local development section.
The term "3d-party" should be corrected to "3rd-party".
Apply this diff to fix the grammar issue:
-2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3d-party dependencies for protobuf files
-3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) to compile protobuf files with their 3d-party dependencies
+2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3rd-party dependencies for protobuf files
+3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) to compile protobuf files with their 3rd-party dependencies
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.
## Local development | |
### Prerequisites | |
1) [Rust](https://www.rust-lang.org/tools/install) | |
2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3d-party dependencies for protobuf files | |
3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) to compile protobuf files with their 3d-party dependencies | |
Then for a code (re-)generation run | |
```sh | |
V4_PROTO_REBUILD=1 cargo build -vv | |
``` | |
## Local development | |
### Prerequisites | |
1) [Rust](https://www.rust-lang.org/tools/install) | |
2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3rd-party dependencies for protobuf files | |
3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) to compile protobuf files with their 3rd-party dependencies | |
Then for a code (re-)generation run | |
```sh | |
V4_PROTO_REBUILD=1 cargo build -vv | |
``` |
v4-proto-rs/README.md
Outdated
## Q&A | ||
|
||
1) Why do we put autogenerated files to the crate (and git) and do not (re-)generate them at compilation? | ||
|
||
For several reasons: | ||
* reproducability of the dependency | ||
* to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for a code generation) | ||
|
||
But if a user wants to (re-)generate at compilation time, he/she can set an environment variable `V4_PROTO_REBUILD` (to any value). | ||
|
||
2) Why do I need a `protoc` for this crate development? I thought `prost-build` crate generates everything natively with Rust? | ||
|
||
The main work (parsing, linking etc - have a look https://protobuf.com/docs/descriptors) is done by `protoc`. | ||
The result of the `protoc` work is a "file descriptor" (think of it as IR assembly language like LLVM IR) - a binary file. This file descriptor is an input for a language-specific code generator like `prost`. Think of `prost` crate as a compiler target which generates a ISA-specific "assembly" (in our case, Rust) as an output. | ||
`prost-build` always used the `protoc` but since version 0.11 of [prost-build](https://github.com/tokio-rs/prost?tab=readme-ov-file#protoc) it requires `protoc` (the protobuf compiler) to be already installed on the system - before the `protoc` could be compiled during the `prost-build` build (https://github.com/tokio-rs/prost/blob/v0.10.4/prost-build/build.rs#L77). | ||
|
||
3) Why do we use `tonic-build` crate and not just `prost-build`? | ||
|
||
`prost-build` generates only serialization-deserialization stubs for messages but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones. | ||
|
||
4) Why do we need `buf`? | ||
|
||
[Buf](https://buf.build/) is a tool which primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer 3d party protobuf specifications and use types declared there. Basically `buf` builds a list of all used protobuf files, downloads them and allows to export (=copy) them in a specified directory. The proto files in this repository and downloaded 3d party proto files (aka "includes") are an input for the `protoc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar and style issues in the Q&A section.
Several grammar and style issues have been flagged by static analysis tools.
Apply this diff to fix the issues:
- * reproducability of the dependency
+ * reproducibility of the dependency
- * to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for a code generation)
+ * to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for code generation)
- The main work (parsing, linking etc - have a look https://protobuf.com/docs/descriptors) is done by `protoc`.
+ The main work (parsing, linking, etc. - have a look https://protobuf.com/docs/descriptors) is done by `protoc`.
- `prost-build` generates only serialization-deserialization stubs for messages but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones.
+ `prost-build` generates only serialization-deserialization stubs for messages, but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones.
- [Buf](https://buf.build/) is a tool which primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer 3d party protobuf specifications and use types declared there. Basically `buf` builds a list of all used protobuf files, downloads them and allows to export (=copy) them in a specified directory. The proto files in this repository and downloaded 3d party proto files (aka "includes") are an input for the `protoc`.
+ [Buf](https://buf.build/) is a tool whose primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer to 3rd-party protobuf specifications and use types declared there. Basically, `buf` builds a list of all used protobuf files, downloads them, and allows exporting (=copying) them to a specified directory. The proto files in this repository and downloaded 3rd-party proto files (aka "includes") are an input for the `protoc`.
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.
## Q&A | |
1) Why do we put autogenerated files to the crate (and git) and do not (re-)generate them at compilation? | |
For several reasons: | |
* reproducability of the dependency | |
* to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for a code generation) | |
But if a user wants to (re-)generate at compilation time, he/she can set an environment variable `V4_PROTO_REBUILD` (to any value). | |
2) Why do I need a `protoc` for this crate development? I thought `prost-build` crate generates everything natively with Rust? | |
The main work (parsing, linking etc - have a look https://protobuf.com/docs/descriptors) is done by `protoc`. | |
The result of the `protoc` work is a "file descriptor" (think of it as IR assembly language like LLVM IR) - a binary file. This file descriptor is an input for a language-specific code generator like `prost`. Think of `prost` crate as a compiler target which generates a ISA-specific "assembly" (in our case, Rust) as an output. | |
`prost-build` always used the `protoc` but since version 0.11 of [prost-build](https://github.com/tokio-rs/prost?tab=readme-ov-file#protoc) it requires `protoc` (the protobuf compiler) to be already installed on the system - before the `protoc` could be compiled during the `prost-build` build (https://github.com/tokio-rs/prost/blob/v0.10.4/prost-build/build.rs#L77). | |
3) Why do we use `tonic-build` crate and not just `prost-build`? | |
`prost-build` generates only serialization-deserialization stubs for messages but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones. | |
4) Why do we need `buf`? | |
[Buf](https://buf.build/) is a tool which primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer 3d party protobuf specifications and use types declared there. Basically `buf` builds a list of all used protobuf files, downloads them and allows to export (=copy) them in a specified directory. The proto files in this repository and downloaded 3d party proto files (aka "includes") are an input for the `protoc`. | |
## Q&A | |
1) Why do we put autogenerated files to the crate (and git) and do not (re-)generate them at compilation? | |
For several reasons: | |
* reproducibility of the dependency | |
* to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for code generation) | |
But if a user wants to (re-)generate at compilation time, he/she can set an environment variable `V4_PROTO_REBUILD` (to any value). | |
2) Why do I need a `protoc` for this crate development? I thought `prost-build` crate generates everything natively with Rust? | |
The main work (parsing, linking, etc. - have a look https://protobuf.com/docs/descriptors) is done by `protoc`. | |
The result of the `protoc` work is a "file descriptor" (think of it as IR assembly language like LLVM IR) - a binary file. This file descriptor is an input for a language-specific code generator like `prost`. Think of `prost` crate as a compiler target which generates a ISA-specific "assembly" (in our case, Rust) as an output. | |
`prost-build` always used the `protoc` but since version 0.11 of [prost-build](https://github.com/tokio-rs/prost?tab=readme-ov-file#protoc) it requires `protoc` (the protobuf compiler) to be already installed on the system - before the `protoc` could be compiled during the `prost-build` build (https://github.com/tokio-rs/prost/blob/v0.10.4/prost-build/build.rs#L77). | |
3) Why do we use `tonic-build` crate and not just `prost-build`? | |
`prost-build` generates only serialization-deserialization stubs for messages, but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones. | |
4) Why do we need `buf`? | |
[Buf](https://buf.build/) is a tool whose primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer to 3rd-party protobuf specifications and use types declared there. Basically, `buf` builds a list of all used protobuf files, downloads them, and allows exporting (=copying) them to a specified directory. The proto files in this repository and downloaded 3rd-party proto files (aka "includes") are an input for the `protoc`. |
Tools
LanguageTool
[style] ~58-~58: In American English, abbreviations like “etc.” require a period.
Context: ...t? The main work (parsing, linking etc - have a look https://protobuf.com/docs...(ETC_PERIOD)
[uncategorized] ~64-~64: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ation-deserialization stubs for messages but we also need a client implementation (g...(COMMA_COMPOUND_SENTENCE)
[grammar] ~68-~68: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...iles. Protobuf specifications can refer 3d party protobuf specifications and use t...(THREE_D)
[grammar] ~68-~68: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...otobuf files, downloads them and allows to export (=copy) them in a specified directory. ...(ALLOW_TO)
[grammar] ~68-~68: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...files in this repository and downloaded 3d party proto files (aka "includes") are ...(THREE_D)
Markdownlint
58-58: null
Bare URL used(MD034, no-bare-urls)
60-60: null
Bare URL used(MD034, no-bare-urls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
v4-proto-rs/README.md (1)
14-16
: Address valid static analysis hints.Some of the static analysis hints are valid and should be addressed:
- Add a comma after "For more idiomatic Rust" at line 14.
- Add a comma after "Before publishing" at line 32.
- Replace "a ISA-specific" with "an ISA-specific" at line 53.
Apply this diff to address the valid static analysis hints:
-For more idiomatic Rust you can use conversions (`try_into` and `into`) for the following types: +For more idiomatic Rust, you can use conversions (`try_into` and `into`) for the following types: -Before publishing make sure to run (and fix all warnings and errors) +Before publishing, make sure to run (and fix all warnings and errors) - Think of `prost` crate as a compiler target which generates a ISA-specific "assembly" (in our case, Rust) as an output. + Think of `prost` crate as a compiler target which generates an ISA-specific "assembly" (in our case, Rust) as an output.The other static analysis hints can be ignored as they are false positives or not applicable.
Also applies to: 32-33, 53-54
Tools
LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ilt (see Q&A below) For more idiomatic Rust you can use conversions (try_into
and...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- v4-proto-rs/Cargo.toml (1 hunks)
- v4-proto-rs/README.md (1 hunks)
- v4-proto-rs/build.rs (1 hunks)
- v4-proto-rs/deny.toml (1 hunks)
- v4-proto-rs/src/lib.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- v4-proto-rs/Cargo.toml
Files skipped from review as they are similar to previous changes (2)
- v4-proto-rs/build.rs
- v4-proto-rs/src/lib.rs
Additional context used
LanguageTool
v4-proto-rs/README.md
[grammar] ~1-~1: The singular proper name ‘Rust’ must be used with a third-person or a past tense verb.
Context: # Rust crate for dYdX Chain protobufs ## Usage as a...(HE_VERB_AGR)
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ilt (see Q&A below) For more idiomatic Rust you can use conversions (try_into
and...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~32-~32: A comma might be missing here.
Context: ...O_REBUILD=1 cargo build -vv ``` Before publishing make sure to run (and fix all warnings ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~52-~52: Possible missing preposition found.
Context: ...k (parsing, linking, etc. - have a look https://protobuf.com/docs/descriptors) is done byprotoc
. The result of...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~53-~53: You might be missing the article “the” here.
Context: ...c code generator likeprost
. Think ofprost
crate as a compiler target which ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~53-~53: Use the indefinite article “an” before nouns that start with a vowel sound.
Context: ...te as a compiler target which generates a ISA-specific "assembly" (in our case, R...(AI_EN_LECTOR_REPLACEMENT_DETERMINER_A_AN)
Markdownlint
v4-proto-rs/README.md
52-52: null
Bare URL used(MD034, no-bare-urls)
54-54: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (7)
v4-proto-rs/deny.toml (5)
1-11
: LGTM!The
[graph]
section configuration looks good and covers a wide range of target platforms. Theall-features
andno-default-features
options being set tofalse
is a common practice.
13-14
: LGTM!The
[output]
section configuration looks good. Settingfeature-depth
to1
is a common practice to keep the output concise.
16-18
: LGTM!The
[advisories]
section configuration looks good. It uses the standard Rust advisory database.
20-40
: LGTM!The
[licenses]
section configuration looks good. It allows a standard set of permissive licenses and clarifies the license for a specific crate. The confidence threshold of 0.8 is a reasonable default. Not ignoring private registries is a good practice.
43-55
: LGTM!The
[bans]
and[sources]
sections configuration looks good. Warning for multiple versions and unknown sources is a good practice. Allowing wildcards and highlighting all bans can be helpful for visibility. Allowing only the standardcrates.io
registry and not allowing any specific organizations is a secure default.v4-proto-rs/README.md (2)
22-23
: Fix grammar issue in the local development section.The term "3d-party" should be corrected to "3rd-party".
Apply this diff to fix the grammar issue:
-2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3d-party dependencies for protobuf files -3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) - to compile protobuf files with their 3d-party dependencies +2) [Buf](https://github.com/bufbuild/buf?tab=readme-ov-file#installation) - to resolve 3rd-party dependencies for protobuf files +3) [protoc](https://github.com/protocolbuffers/protobuf#protobuf-compiler-installation) - to compile protobuf files with their 3rd-party dependencies
52-62
: Fix grammar and style issues in the Q&A section.Several grammar and style issues have been flagged by past review comments and static analysis tools.
Apply this diff to fix the issues:
- * reproducability of the dependency + * reproducibility of the dependency - * to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for a code generation) + * to avoid external dependencies for the lib users (`protoc` and `buf` are only needed for code generation) - The main work (parsing, linking etc - have a look https://protobuf.com/docs/descriptors) is done by `protoc`. + The main work (parsing, linking, etc. - have a look at https://protobuf.com/docs/descriptors) is done by `protoc`. - `prost-build` generates only serialization-deserialization stubs for messages but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones. + `prost-build` generates only serialization-deserialization stubs for messages, but we also need a client implementation (generated by `tonic-build`) because packages in other language implementations of `v4-chain` have ones. - [Buf](https://buf.build/) is a tool which primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer 3d party protobuf specifications and use types declared there. Basically `buf` builds a list of all used protobuf files, downloads them and allows to export (=copy) them in a specified directory. The proto files in this repository and downloaded 3d party proto files (aka "includes") are an input for the `protoc`. + [Buf](https://buf.build/) is a tool whose primary function is to resolve dependencies in protobuf files. Protobuf specifications can refer to 3rd-party protobuf specifications and use types declared there. Basically, `buf` builds a list of all used protobuf files, downloads them, and allows exporting (=copying) them to a specified directory. The proto files in this repository and downloaded 3rd-party proto files (aka "includes") are an input for `protoc`.Tools
LanguageTool
[uncategorized] ~52-~52: Possible missing preposition found.
Context: ...k (parsing, linking, etc. - have a look https://protobuf.com/docs/descriptors) is done byprotoc
. The result of...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~53-~53: You might be missing the article “the” here.
Context: ...c code generator likeprost
. Think ofprost
crate as a compiler target which ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~53-~53: Use the indefinite article “an” before nouns that start with a vowel sound.
Context: ...te as a compiler target which generates a ISA-specific "assembly" (in our case, R...(AI_EN_LECTOR_REPLACEMENT_DETERMINER_A_AN)
Markdownlint
52-52: null
Bare URL used(MD034, no-bare-urls)
54-54: null
Bare URL used(MD034, no-bare-urls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- v4-proto-rs/src/lib.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- v4-proto-rs/src/lib.rs
Changelist
This PR adds support for Rust as part of the work on implementing the Rust Trading Client.
The crate contains the pre-built protocol. The sources are generated during compilation only if a specific environment variable is set.
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
v4-proto-rs
with comprehensive support for handling dYdX Chain protobufs.ToAny
for encoding messages into a standardized format, improving interoperability within the Cosmos SDK ecosystem.Documentation
README.md
file with detailed instructions on using the new package and its components.Chores
.gitignore
to exclude build artifacts and lock files for cleaner version control.deny.toml
for improved dependency management and licensing compliance.