-
Notifications
You must be signed in to change notification settings - Fork 44
feat(sdk): fetch defined token direct purchase prices #2544
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
Conversation
WalkthroughThis set of changes introduces and fully implements the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK
participant PlatformService
participant Platform
participant Drive
Client->>SDK: fetch_many(TokenPricingSchedule, token_ids)
SDK->>PlatformService: get_token_direct_purchase_prices(request)
PlatformService->>Platform: query_token_direct_purchase_prices(request, state, version)
Platform->>Drive: fetch_tokens_direct_purchase_price(token_ids, ...)
Drive-->>Platform: Map<token_id, pricing_schedule>
Platform-->>PlatformService: GetTokenDirectPurchasePricesResponse
PlatformService-->>SDK: Response (with/without proof)
SDK-->>Client: TokenDirectPurchasePrices (parsed)
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 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 review all |
|
Tip For best results, initiate chat on the files or code changes. @lklimek I've received your request to review all files in this PR. I'll initiate a full review process to examine all 35 changed files from scratch. This comprehensive review will analyze the code changes across all modified files in the PR. The review process may take several minutes to complete depending on the complexity of the changes. You'll receive notifications when the review is complete. If you have any specific questions about particular files or code sections while waiting, feel free to ask by tagging me in a comment. |
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: 2
♻️ Duplicate comments (1)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
52737-52749: Serializing fixedPrice as uint64.Same note regarding potential overflow applies here as in the chunk for reading uint64 values.
🧹 Nitpick comments (5)
packages/rs-drive/src/drive/tokens/direct_purchase/prove_tokens_direct_purchase_price/v0/mod.rs (1)
7-22: Consider removing or documenting the unuseddrive_operations.
Currently, thedrive_operationsvector is initialized but never used. If it's truly unnecessary, removing it would reduce clutter. Otherwise, document its anticipated use to clarify future plans.packages/dapi-grpc/protos/platform/v0/platform.proto (1)
1373-1380: Ensure callers validate token IDs correctly.
The request includes a repeatedbytes token_idsfield. Since these IDs must be 32 bytes, consider adding validation or mention in documentation for clients.packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/mod.rs (1)
25-25: Fix minor typo in error message.Change
"could not decode token direct pruchase prices query"to"could not decode token direct purchase prices query".- "could not decode token direct pruchase prices query".to_string(), + "could not decode token direct purchase prices query".to_string(),packages/rs-sdk/tests/fetch/token_direct_prices.rs (1)
9-9: Correct mismatched doc comments.These doc comments erroneously refer to fetching a data contract instead of token prices. Update them to reflect token direct purchase price fetching, for clarity.
-/// Given some dummy data contract ID, when I fetch data contract, I get None because it doesn't exist. +/// Given a dummy token ID, when we fetch direct purchase prices, we expect None because it doesn't exist.Also applies to: 31-31, 55-55
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (1)
396-499: Comprehensive multi-token pricing testThis test covers a variety of pricing structures (single, tiered, empty) and validates prices with and without proofs. For even richer coverage, consider an additional scenario testing tier-based token purchases or partial tiers. Overall, it’s already quite comprehensive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
packages/dapi-grpc/Cargo.toml(2 hunks)packages/dapi-grpc/build.rs(4 hunks)packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(15 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h(9 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m(5 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(15 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(57 hunks)packages/rs-dapi-client/src/transport/grpc.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs(5 hunks)packages/rs-drive-abci/src/query/service.rs(1 hunks)packages/rs-drive-abci/src/query/token_queries/mod.rs(1 hunks)packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/mod.rs(1 hunks)packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/v0/mod.rs(1 hunks)packages/rs-drive-proof-verifier/src/proof.rs(1 hunks)packages/rs-drive-proof-verifier/src/proof/token_direct_purchase.rs(1 hunks)packages/rs-drive-proof-verifier/src/types.rs(2 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/fetch_tokens_direct_purchase_price/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/fetch_tokens_direct_purchase_price/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/prove_tokens_direct_purchase_price/mod.rs(1 hunks)packages/rs-drive/src/drive/tokens/direct_purchase/prove_tokens_direct_purchase_price/v0/mod.rs(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs(0 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/mocks/v2_test.rs(1 hunks)packages/rs-sdk/README.md(1 hunks)packages/rs-sdk/src/mock/requests.rs(2 hunks)packages/rs-sdk/src/mock/sdk.rs(1 hunks)packages/rs-sdk/src/platform/fetch_many.rs(3 hunks)packages/rs-sdk/src/platform/query.rs(2 hunks)packages/rs-sdk/tests/fetch/mod.rs(1 hunks)packages/rs-sdk/tests/fetch/token_direct_prices.rs(1 hunks)
💤 Files with no reviewable changes (1)
- packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_direct_purchase_transition_action/v0/transformer.rs
🧰 Additional context used
🧠 Learnings (1)
packages/rs-sdk/src/mock/sdk.rs (1)
Learnt from: lklimek
PR: dashpay/platform#2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
🧬 Code Graph Analysis (5)
packages/rs-sdk/src/platform/fetch_many.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest(6817-6832)TokenDirectPurchasePrices(6782-6796)
packages/rs-dapi-client/src/transport/grpc.rs (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetTokenDirectPurchasePricesRequest(6817-6832)GetTokenDirectPurchasePricesResponse(6639-6654)packages/rs-drive-abci/src/query/service.rs (1)
get_token_direct_purchase_prices(743-753)
packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/mod.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (3)
Version(5163-5182)GetTokenDirectPurchasePricesRequest(6817-6832)GetTokenDirectPurchasePricesResponse(6639-6654)
packages/rs-sdk/tests/fetch/token_direct_prices.rs (3)
packages/rs-sdk/src/platform/fetch.rs (1)
fetch(91-96)packages/rs-sdk/tests/fetch/common.rs (1)
setup_logs(82-92)packages/rs-sdk/src/platform/fetch_many.rs (4)
sdk(235-235)sdk(343-344)fetch_many(146-153)fetch_many(325-361)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (4)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/methods/mod.rs (1)
new_token_change_direct_purchase_price_transition(1003-1062)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (10)
None(59-59)None(243-243)None(333-333)None(426-426)None(2885-2885)None(2972-2972)None(3066-3066)None(3186-3186)None(3287-3287)None(3379-3379)packages/rs-drive/src/verify/tokens/verify_token_direct_selling_prices/mod.rs (1)
verify_token_direct_selling_prices(40-68)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/mod.rs (1)
setup_identity(103-159)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
🔇 Additional comments (88)
packages/rs-drive-abci/src/query/token_queries/mod.rs (1)
5-5: New module declaration correctly addedThe addition of the
token_direct_purchase_pricesmodule aligns with the PR objective of implementing functionality to retrieve token direct purchase prices. The new module is appropriately positioned among other token-related query modules.packages/rs-sdk/tests/fetch/mod.rs (1)
30-30: Test module correctly added for new functionalityThe addition of the
token_direct_pricestest module is appropriate and follows the project's testing structure. This ensures the new token direct purchase prices functionality is properly tested, which is essential for maintaining code quality.packages/rs-sdk/src/mock/sdk.rs (1)
232-234: Mock support correctly implemented for new request typeThe new match arm for
GetTokenDirectPurchasePricesRequestproperly extends the mock SDK's capability to handle the new request type. The implementation follows the established pattern in the codebase, ensuring consistency and proper test support for the new functionality.packages/rs-drive-proof-verifier/src/proof.rs (1)
3-3: Proof verification module properly exposedThe addition of the
token_direct_purchasemodule to the public API is appropriate and necessary for enabling proof verification of token direct purchase prices. This integrates well with the existing proof verification infrastructure.packages/rs-sdk/README.md (1)
142-145: Checklist numbering updated correctly.The numbering of the checklist items has been updated to maintain a sequential order, showing good attention to detail in documentation maintenance.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs (1)
10-10: Good improvement to test module availability.Expanding the conditional compilation to include both
create_sdk_test_dataandtestensures the test module is available during both regular testing and when specifically generating SDK test data. This is a valuable improvement for test coverage.packages/dapi-grpc/Cargo.toml (2)
49-49: Added TLS-Ring support for secure gRPC communication.Adding the "tls-ring" feature to tonic ensures proper TLS support for secure communication in the gRPC client, which is essential for the new token direct purchase price API endpoints.
68-68: Consistent TLS-Ring feature for non-WASM targets.Good job maintaining feature consistency by adding the "tls-ring" feature to both the general dependencies and the non-WASM target-specific dependencies.
packages/rs-sdk/src/mock/requests.rs (2)
8-8: Added TokenPricingSchedule import for mock support.This import enables the implementation of mock responses for token pricing schedules, a necessary component for testing the new token direct purchase prices functionality.
450-450: Added mock response implementation for TokenPricingSchedule.Registering TokenPricingSchedule with the impl_mock_response! macro properly enables mocking for the new token pricing functionality, facilitating comprehensive testing of the feature.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs (1)
34-34: Addition of versioning for token direct purchase prices is correct.The new
token_direct_purchase_pricesfield is consistent with the versioning approach for other token queries and supports future-proofing for this feature.packages/rs-drive/src/drive/tokens/direct_purchase/mod.rs (1)
4-6: Module declarations for batch fetch/prove are correct and well-structured.The new modules are appropriately gated and named, supporting the new feature without introducing any issues.
packages/dapi-grpc/build.rs (1)
110-110: Inclusion of new gRPC request/response types is correct.Adding the new message types to the versioned arrays ensures proper code generation and versioning support for the new feature.
Also applies to: 157-157
packages/rs-drive-proof-verifier/src/types.rs (1)
633-634: Type alias for token direct purchase prices is clear and appropriate.The new alias improves code clarity and aligns with the batch fetch/proof feature for token pricing schedules.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs (1)
103-107: Addition of version bounds for token direct purchase prices is correct.The new field is consistent with the structure and initialization of other versioned queries.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (1)
201-201: Added required field for token direct purchase pricing rulesThis addition properly configures the token distribution rules to support direct purchase pricing changes. The default
ChangeControlRulesV0is appropriate for test data.packages/rs-platform-version/src/version/mocks/v2_test.rs (1)
241-245: Added versioning support for token direct purchase prices queryThe new field is correctly structured with
FeatureVersionBoundsand matches the pattern used for other query features. Setting all version values to 0 is consistent with other test version definitions.packages/rs-sdk/src/platform/query.rs (2)
28-29: Added necessary imports for token direct purchase prices queryThe import addition is appropriate for the implementation below.
684-704: Implemented Query trait for token direct purchase pricesThe implementation follows the established pattern in the codebase and correctly:
- Enforces the use of proofs (similar to other query implementations)
- Maps the slice of Identifiers to token IDs in the request
- Properly structures the version field with V0
This enables fetching token direct purchase prices through the SDK interface.
packages/rs-dapi-client/src/transport/grpc.rs (1)
583-590: Added transport request implementation for the token direct purchase prices queryThe implementation correctly uses the
impl_transport_request_grpc!macro to register the new request type with the gRPC transport system. It follows the established pattern in the file and connects the new SDK query capability to the gRPC service.packages/rs-drive-abci/src/query/service.rs (1)
743-753: Implementation looks good!The
get_token_direct_purchase_pricesmethod follows the established pattern of other query methods in this service, delegating to thehandle_blocking_queryhelper with appropriate parameters. The implementation is concise and consistent with the service architecture.packages/rs-sdk/src/platform/fetch_many.rs (4)
22-23: LGTM: Import addition for token direct purchase prices requestThe import for
GetTokenDirectPurchasePricesRequestis correctly added to support the new functionality.
34-34: LGTM: Import addition for TokenPricingScheduleThe import for
TokenPricingSchedulefrom the tokens module is correctly added.
42-43: LGTM: Import addition for TokenDirectPurchasePrices typeThe import for
TokenDirectPurchasePricesfrom drive_proof_verifier types is correctly added.
545-552: Implementation for fetching token pricing data looks goodThe implementation of
FetchManyforTokenPricingSchedulefollows the established pattern of other implementations in this file. The trait enables batch fetching of token direct purchase prices by token identifiers, which aligns with the PR objectives.packages/rs-drive/src/drive/tokens/direct_purchase/prove_tokens_direct_purchase_price/mod.rs (1)
1-42: Well-structured implementation with good documentationThe implementation of
prove_tokens_direct_purchase_priceis well-organized with comprehensive documentation. It properly handles version matching and delegation to version-specific implementation, with appropriate error handling for unsupported versions.packages/rs-drive/src/drive/tokens/direct_purchase/prove_tokens_direct_purchase_price/v0/mod.rs (1)
7-22: Logic appears correct for generating GroveDB proofs.
The function is straightforward, constructing a path query and callinggrove_get_proved_path_query. No issues stand out from a correctness standpoint.packages/dapi-grpc/protos/platform/v0/platform.proto (2)
85-86: New RPC method naming looks consistent.
AddinggetTokenDirectPurchasePricesfollows the existing naming convention of token-related RPC calls, maintaining clarity and coherence.
1339-1372: Well-structured response definition for token direct purchase prices.
Defining both fixed and variable prices in a oneof enhances flexibility. The nested messages, such asPriceForQuantityandPricingSchedule, align with typical proto design patterns.packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/v0/mod.rs (3)
26-32: Input validation is appropriate.
Requiring at least one token ID prevents needless queries and guards against empty input.
34-43: Robust conversion to 32-byte identifiers.
Using.try_into()with a custom error message handles malformed token IDs gracefully.
57-98: Implementation properly distinguishes proof vs. data fetching paths.
The separate logic for producing proofs or returning direct purchase prices is clear and straightforward. Keep an eye on performance if large sets of prices are fetched.packages/rs-drive-abci/src/query/token_queries/token_direct_purchase_prices/mod.rs (1)
14-66: Looks good overall.The version-based routing is properly handled, and the flow for returning an error on unsupported versions is clearly defined.
packages/rs-sdk/tests/fetch/token_direct_prices.rs (1)
1-8: Test structure is solid.Loading configuration, initializing SDK, and verifying results with multiple tokens is well done.
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_tokens_direct_purchase_price/v0/mod.rs (1)
1-65: Implementation appears correct.The raw path query and the structured BTreeMap result are well-structured, and error handling for unexpected elements is appropriate.
packages/rs-drive-proof-verifier/src/proof/token_direct_purchase.rs (1)
1-76: Well-structured proof verification.The logic for extracting and verifying proofs is correct, and the approach for returning an empty result when no tokens are found is clean.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs (7)
4-5: Imports look consistentAll added imports appear necessary for the test logic and are well-organized.
Also applies to: 6-6, 10-16
31-34: Initialization ofsingle_priceand contract nonceNo issues with setting a single-price schedule and the
identity_contract_nonce. These values are used clearly in subsequent state transition logic.Also applies to: 36-42
51-51: Passing the pricing schedule and nonce to the transitionThe parameters align with the
new_token_change_direct_purchase_price_transitionmethod signature. No concerns.Also applies to: 55-55
66-66: Loading platform stateFetching the platform state before processing the state transition is consistent with previous patterns. Looks good.
78-110: Buyer price fetching and proof verificationThese test steps are thorough, covering both non-proof and proof-based fetches. Assertions invoke clear error messages when mismatches occur.
501-522: Helper functionassert_priceThis utility function cleanly checks the presence and correctness of prices, ensuring highly readable test assertions.
524-591: Helper functioncreate_token_with_pricingThe flow of contract creation, configuration updating, and price setting is cohesive. Incrementing the contract nonce helps maintain correct state transition ordering.
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (3)
6741-6757: Well-structured token pricing entry with good TypeScript type definitionsThe TokenDirectPurchasePriceEntry class is well-designed, providing a clean interface to access token pricing information including the token ID and either fixed or variable pricing options.
6767-6779: Properly implemented pricing model with clear type definitionsThe pricing model implementation correctly uses TypeScript's type system to represent the pricing options, with a well-defined enum for distinguishing between fixed and variable pricing models.
6817-6872: Complete implementation of the token price request messageThe GetTokenDirectPurchasePricesRequest class follows the established pattern for versioned request messages in the system, with proper type safety and serialization support. This implementation will allow clients to retrieve pricing information for a list of token IDs.
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (14)
374-374: Exporting PriceCase for the TokenDirectPurchasePriceEntry.
This addition appears to be standard Protobuf-generated code for handling oneof fields.
4881-4882: Constructor and displayName for GetTokenDirectPurchasePricesResponse.
These definitions and initializations for the response message look correct.Also applies to: 4884-4884, 4890-4890
4902-4903: Constructor for GetTokenDirectPurchasePricesResponseV0.
Introducing the oneof groups for versioned response fields is standard. No concerns.Also applies to: 4905-4905, 4911-4911
4923-4924: PriceForQuantity message.
Auto-generated code for this nested message appears consistent with the proto definitions.Also applies to: 4926-4926, 4932-4932
4944-4945: PricingSchedule message.
Fields and repeated properties look properly generated.Also applies to: 4947-4947, 4953-4953
4965-4966: TokenDirectPurchasePriceEntry constructor and oneof assignments.
The new price oneof for fixed/variable pricing is correctly introduced.Also applies to: 4968-4968, 4974-4974
4986-4987: TokenDirectPurchasePrices container message.
Auto-generated oneof logic and repeated fields are standard.Also applies to: 4989-4989, 4995-4995
5007-5008: GetTokenDirectPurchasePricesRequest base constructor.
This code sets up the top-level version oneof for the request. Looks fine.Also applies to: 5010-5010, 5016-5016
5028-5029: GetTokenDirectPurchasePricesRequestV0 constructor.
The repeated fields and structure match the proto specs for listing multiple token IDs.Also applies to: 5031-5031, 5037-5037
52600-52600: Definition of PriceCase oneof for TokenDirectPurchasePriceEntry.
These enum assignments and oneof references align with the newly introduced fixed vs. variable price design.Also applies to: 52605-52605, 52607-52608, 52612-52612, 52614-52615
52650-52651: Mapping tokenId, fixedPrice, and variablePrice in toObject.
B64 usage and object structure comply with typical JSPB patterns.
52798-52799: Setters and clearers for fixedPrice and variablePrice.
All typical auto-generated oneof helper methods. No issues.Also applies to: 52801-52802, 52807-52812, 52819-52821, 52829-52830, 52833-52839, 52849-52849, 52856-52857, 52866-52866
53180-53561: GetTokenDirectPurchasePricesRequest and nested GetTokenDirectPurchasePricesRequestV0 logic.
This large block includes repeated bytes for token IDs plus the prove field. Everything appears standard for versioned requests in JSPB.
52693-52699:❓ Verification inconclusive
Reading fixedPrice as uint64 from the binary stream.
In JavaScript, values above 2^53 can lead to integer precision issues.Please confirm whether large pricing values might exceed 2^53. If so, you could store these numbers as strings. For example, run this script to search for references to big integer handling:
🏁 Script executed:
#!/bin/bash rg "BigInt\|long\|safe integer\|Number.MAX_SAFE_INTEGER" -A 5Length of output: 63
Action Required: Verify Handling of Large Pricing Values
The search for explicit references to BigInt, Number.MAX_SAFE_INTEGER, or other safeguards in the codebase returned no matches. This means there’s no evidence that the implementation compensates for the potential loss of precision when reading
fixedPricevalues as a uint64. Please manually verify whether pricing values might exceed 2^53. If large values are indeed possible, consider revising the implementation—such as storing these numbers as strings or using BigInt—to avoid precision issues.
- Confirm if the
fixedPricevalues can exceed JavaScript’s safe integer threshold.- Update the handling of large numbers accordingly if necessary.
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (15)
374-374: Auto-generated symbol export.This line exports the
PriceCasesymbol for the updated token direct purchase prices proto definition. No issues noted.
4881-4890: Auto-generated constructor definitions.These lines initialize
GetTokenDirectPurchasePricesResponsefor serialization/deserialization. Everything looks standard.
4902-4911: AddedGetTokenDirectPurchasePricesResponseV0.This constructor adheres to the typical protobuf pattern for versioned messages. No concerns.
4923-4932: AddedPriceForQuantityboilerplate.The constructor is consistent with protobuf message patterns. No issues.
4944-4953: AddedPricingScheduleconstructor.Defines repeated fields properly per protobuf conventions. Looks good.
4965-4974: AddedTokenDirectPurchasePriceEntryfor oneof groups.Implements the oneof for fixed vs. variable price. No concerns.
4986-4995: AddedTokenDirectPurchasePricesmessage.All fields and repeated data structures follow standard codegen.
5007-5016: IntroducedGetTokenDirectPurchasePricesRequest.Auto-generated request structure is consistent with normal protobuf usage.
5028-5037: Added versionedGetTokenDirectPurchasePricesRequestV0.Captures token IDs and prove flag. All standard.
52600-52615: DefinedPriceCaseenum in a oneof group.Enables distinction between
FIXED_PRICEandVARIABLE_PRICE. Implementation looks correct.
52650-52651: Token ID handling.These lines reference the
tokenIdfield. Nothing unusual.
52693-52699: Deserialization logic for fields.Properly handles
fixedPriceandvariablePricein the oneof. No issues.
52737-52749: Serialization logic.Oneof fields are correctly serialized. Looks good.
52798-52866: Getters, setters, and clear operations for pricing.Boilerplate methods for
fixedPriceandvariablePricefields in the oneof.
53179-53561: Complete request definition.The new
GetTokenDirectPurchasePricesRequestandGetTokenDirectPurchasePricesRequestV0messages introduce repeated token IDs and theproveflag. All code is auto-generated and looks consistent.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (8)
5584-5590: Well-designed token price structure with oneOf patternThe TokenDirectPurchasePriceEntry structure properly implements a oneOf pattern for price with either fixedPrice or variablePrice, making it flexible for different token pricing models while ensuring mutual exclusivity.
5616-5628: Clean implementation of token direct purchase prices containerThe TokenDirectPurchasePrices container properly encapsulates multiple price entries, following the established pattern for collection objects in this codebase.
5630-5669: Well-structured request message for token direct purchase pricesThe GetTokenDirectPurchasePricesRequest implementation follows the established patterns in this codebase, including proper versioning support and correct field types. The tokenIdsArray allows for efficient batch retrieval of multiple token prices in a single request.
5554-5582: Comprehensive pricing schedule implementationThe PricingSchedule and PriceForQuantity structures are well-designed to support variable pricing based on quantities purchased, allowing for volume discount pricing models or other quantity-based pricing schemes.
476-478: Improved code commentsThe enhancement of the ResponseMetadata comment to be more descriptive is a good improvement to code documentation.
2425-2429: Merged comment line fixThe response field comments that were previously split are now properly merged, improving readability.
2754-2756: Fixed comment formattingThe properly formatted comments for the proof field improve code readability.
2897-2900: Comment style consistency improvementThe previously split comment for vote choices is now properly formatted as a single continuous comment line.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (5)
14428-14442: Implementation of the repeated price array looks correct.
DefiningNSMutableArray *tokenDirectPurchasePriceArrayas a repeated field is standard practice for handling multiple token price entries in Objective-C Protobuf.
14451-14521: Repeated field descriptor for 'tokenDirectPurchasePriceArray'
The descriptor configuration for your repeated field is consistent with how Protobuf manages arrays in Objective-C. ARC with GPB is well-suited to handle any memory-related concerns here, so there appear to be no issues.
14531-14547: Optional 'prove' field
Markingproveas optional is correct if a default offalseis acceptable for most cases. Ensure the .proto file clearly documents the intent of this boolean for future maintainability.
14555-14584: Repeated token IDs usage
Storing multiple token IDs as repeated bytes (tokenIdsArray) is a common pattern for passing multiple identifiers, especially in cryptographic contexts. The generated descriptor aligns with the standard Objective-C Protobuf usage. Looks good.
14353-14412:✅ Verification successful
Confirm the domain for 'fixedPrice' and 'variablePrice' usage.
This new oneof approach is correct if the intention is to store either a fixed or a variable price, but not both. Please confirm thatuint64_tis sufficient to avoid potential overflow and that storingtokenIdasNSData *is the correct approach for your token identifier size.
🏁 Script executed:
#!/bin/bash # Verify the underlying .proto file has oneof "price" and that usage is consistent with the domain. rg -A 5 "oneof price" # Check references to tokenId usage to ensure consistent handling across the codebase. rg -A 10 "tokenId"Length of output: 66493
Fixed and Verified: Price Oneof and Token Identifier Domain Confirmed
After reviewing the proto definition and related tests, we can confirm the following:
- The proto file correctly defines a oneof group for "price" with
uint64 fixed_priceandPricingSchedule variable_price, which aligns with the intended design of storing either a fixed or variable price.- Usage of
uint64_tfor fixed prices appears sufficient and matches the proto definition, with no overflow issues flagged based on current domain expectations.- The
tokenIdis consistently handled as anNSData *and corresponds to a 32-byte identifier throughout the codebase, matching the test patterns.These findings indicate that the approach is correct and consistent across the implementation.
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_tokens_direct_purchase_price/mod.rs
Outdated
Show resolved
Hide resolved
packages/rs-drive/src/drive/tokens/direct_purchase/fetch_tokens_direct_purchase_price/mod.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rs-sdk/tests/fetch/token_direct_prices.rs(1 hunks)packages/rs-sdk/tests/vectors/test_direct_prices_duplicate_token/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json(1 hunks)packages/rs-sdk/tests/vectors/test_direct_prices_token_not_found/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json(1 hunks)packages/rs-sdk/tests/vectors/test_direct_prices_tokens_ok/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-sdk/tests/fetch/token_direct_prices.rs
🧰 Additional context used
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/test_direct_prices_duplicate_token/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_direct_prices_token_not_found/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_direct_prices_tokens_ok/quorum_pubkey-106-6e35482a0100e4cda834cf0e4c5e8de47d162a958d61ac778e6a1cc4ab9a8936.json
[error] 1-1: String values must be double quoted.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
Issue being fixed or feature implemented
As a developer, I want to retrieve information about defined direct purchase prices for a token, to allow my users to see and buy tokens for these prices.
What was done?
getTokenDirectPurchasePricesin drive-abci and dapi-grpcHow Has This Been Tested?
Added SDK tests, tested using local devnet.
TODO: test some test data.
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Style