-
Notifications
You must be signed in to change notification settings - Fork 43
feat(sdk): token purchase and set price transitions #2613
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
WalkthroughTwo new modules, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Builder (Purchase/SetPrice)
participant SDK
participant Signer
Caller->>Builder (Purchase/SetPrice): new(...).with_...(optional).sign(...)
Builder->>SDK: get_identity_contract_nonce(...)
SDK-->>Builder: nonce
Builder->>Signer: sign(transition_data)
Signer-->>Builder: signature
Builder-->>Caller: StateTransition (signed)
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 3
🧹 Nitpick comments (4)
packages/rs-sdk/src/platform/transition/fungible_tokens/mod.rs (1)
10-11
: Consider keepingpub mod
declarations alphabetically ordered
For long‐lived modules, keeping the list alphabetised (burn, claim, …, transfer, unfreeze) makes merge-conflicts easier to resolve and helps readers spot missing modules quickly.packages/rs-sdk/src/platform/transition/fungible_tokens/purchase.rs (1)
34-48
: Doc-comment parameters are out of sync with the function signature
issuer_id
is mentioned, but the argument is namedactor_id
.
total_agreed_price
is accepted by the function but not documented.
Keeping the docs accurate prevents misuse by downstream developers.packages/rs-sdk/src/platform/transition/fungible_tokens/set_price.rs (2)
35-39
: Stale documentation
The doc-comment still referencesamount
even though this builder changes price, not quantity, and mentionsissuer_id
instead ofactor_id
.
Synchronise the docs with the code to avoid developer confusion.
100-106
: Group-action auto-selection left as TODO
If group information is mandatory for certain schedules, forgetting to fill this value will create runtime errors difficult to trace.
Either implement the lookup here or fail fast with a descriptive error until the feature is complete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
packages/rs-sdk/src/platform/transition/fungible_tokens/mod.rs
(1 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/purchase.rs
(1 hunks)packages/rs-sdk/src/platform/transition/fungible_tokens/set_price.rs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/rs-sdk/src/platform/transition/fungible_tokens/purchase.rs (1)
49-50
:TODO
left for production-critical validation
Validatingtoken_position
against the contract layout is essential to prevent malformed or malicious transitions.
Please either implement the check now or create a tracked issue so it does not get forgotten.packages/rs-sdk/src/platform/transition/fungible_tokens/set_price.rs (1)
49-50
: Validation oftoken_position
is still TODO
As with the purchase builder, the absence of this check can let invalid transitions through.
let token_id = Identifier::from(calculate_token_id( | ||
self.data_contract.id().as_bytes(), | ||
self.token_position, | ||
)); | ||
|
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.
🛠️ Refactor suggestion
No check that token_id
actually belongs to data_contract
Although calculate_token_id(...)
is called, we don’t ensure the resulting token_id
is present in the contract’s token definitions. An invalid position would silently create an incoherent transition.
Consider validating:
if self.token_position >= self.data_contract.tokens().len() {
return Err(Error::InvalidTokenPosition(self.token_position));
}
🤖 Prompt for AI Agents
In packages/rs-sdk/src/platform/transition/fungible_tokens/purchase.rs around
lines 110 to 114, the code calculates a token_id from a token_position without
verifying that the position is valid within the data_contract's tokens. To fix
this, add a check before calculating token_id to ensure self.token_position is
less than the length of self.data_contract.tokens(). If the position is invalid,
return an appropriate error such as Error::InvalidTokenPosition with the invalid
position value.
Issue being fixed or feature implemented
Add the token purchase and set price transitions to the sdk
What was done?
How Has This Been Tested?
Not yet
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit