fix(sdk): added signing_withdrawal_key_to_use to withdraw sdk call by QuantumExplorer · Pull Request #2234 · dashpay/platform

QuantumExplorer · 2024-10-10T03:30:06Z

Issue being fixed or feature implemented

SDK withdrawal call did allow to specify the key to use, this is sometimes important.

What was done?

Added an option to allow to specify the key

How Has This Been Tested?

Will test.

Breaking Changes

None

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

Summary by CodeRabbit

New Features
- Enhanced withdrawal process by allowing selection of the signing withdrawal key.
- Integrated user fee increases directly from settings for improved flexibility.
Bug Fixes
- Streamlined control flow in the withdrawal operation for better performance.

coderabbitai · 2024-10-10T03:30:15Z

Walkthrough

The changes in this pull request involve modifications to the WithdrawFromIdentity trait and its implementation for the Identity struct in the withdraw_from_identity.rs file. The user_fee_increase parameter has been replaced with signing_withdrawal_key_to_use, altering the method signature of the withdraw function. The new implementation derives the user_fee_increase from the settings parameter, enhancing the withdrawal process by integrating user fee increases directly from the settings.

Changes

File Path	Change Summary
`packages/rs-sdk/src/platform/transition/withdraw_from_identity.rs`	Updated `WithdrawFromIdentity` trait and `Identity` implementation to replace `user_fee_increase` with `signing_withdrawal_key_to_use`. Modified `withdraw` method signature and logic to derive `user_fee_increase` from `settings`.

Possibly related PRs

feat(sdk): enable withdrawals v1 in JS SDK #2201: Involves changes to the IdentityCreditWithdrawalTransition, directly related to the modifications in the withdraw function.
test(test-suite): enable withdrawal tests #2202: Updates the test suite for withdrawals, aligning with the changes in the main PR regarding withdrawal parameters.
fix(drive): set sign height when rebroadcasting #2210: Focuses on rebroadcasting expired withdrawal documents, relevant to the overall withdrawal logic.
feat(dpp): add a convenience method to get the public key data for a private key depending on the key type #2214: Introduces a method for converting private key data to public key data, tangentially related to handling withdrawal keys.
feat(sdk): fix client tls connections #2223: Fixes TLS connections, indirectly relating to the SDK's overall functionality, including withdrawals.

Suggested labels

enhancement, js-sdk

Suggested reviewers

lklimek
pshenmic

🐇 In the realm of code, where changes take flight,
A signing key now guides us, through day and night.
With fees from the settings, we weave a new tale,
For withdrawals so smooth, they’ll surely prevail!
Hops of enhancement, in the SDK’s dance,
Let’s celebrate progress, give change a chance! 🎉

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?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)

packages/rs-sdk/src/platform/transition/withdraw_from_identity.rs (1)

22-23: Clarify the Documentation Comment

The documentation can be rephrased for better clarity. Consider updating it to:

If signing_withdrawal_key_to_use is not set, the method will attempt to use a key available for withdrawal in the signer.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a646669 and 16c2803.

📒 Files selected for processing (1)

packages/rs-sdk/src/platform/transition/withdraw_from_identity.rs (4 hunks)

🧰 Additional context used

🔇 Additional comments (4)

packages/rs-sdk/src/platform/transition/withdraw_from_identity.rs (4)

6-6: Importing IdentityPublicKey for New Parameter

The IdentityPublicKey import is added to support the new signing_withdrawal_key_to_use parameter introduced in the code. This is appropriate and necessary for the implementation.

30-30: Adding signing_withdrawal_key_to_use Parameter to Trait Method

The new parameter signing_withdrawal_key_to_use: Option<&IdentityPublicKey> is appropriately added to the trait method signature, allowing callers to specify the key used for withdrawal.

44-44: Including signing_withdrawal_key_to_use in Implementation

The implementation now includes the signing_withdrawal_key_to_use parameter, matching the updated trait definition. This ensures consistency between the trait and its implementation.

61-61: Passing signing_withdrawal_key_to_use to try_from_identity

The signing_withdrawal_key_to_use parameter is correctly passed to the try_from_identity method. This allows the withdrawal transition to utilize the specified signing key.

coderabbitai · 2024-10-10T03:32:46Z

+        let user_fee_increase = settings
+            .map(|settings| settings.user_fee_increase)
+            .flatten();


🛠️ Refactor suggestion

Simplify Extraction of user_fee_increase

You can simplify the extraction of user_fee_increase from settings by using and_then instead of map followed by flatten:

let user_fee_increase = settings.and_then(|settings| settings.user_fee_increase);

This approach is more idiomatic and concise in Rust.

fix: added signing_withdrawal_key_to_use to withdraw sdk call

16c2803

QuantumExplorer requested a review from shumkov as a code owner October 10, 2024 03:30

shumkov approved these changes Oct 10, 2024

View reviewed changes

QuantumExplorer added this to the v1.4.0 milestone Oct 10, 2024

QuantumExplorer added the rs-sdk label Oct 10, 2024

QuantumExplorer self-assigned this Oct 10, 2024

coderabbitai Bot reviewed Oct 10, 2024

View reviewed changes

QuantumExplorer merged commit 3ca878d into v1.4-dev Oct 10, 2024

QuantumExplorer deleted the fix/withdrawalFixes branch October 10, 2024 03:35

This was referenced Oct 10, 2024

feat(dpp): added identity public key private key validation methods #2235

Merged

fix(sdk): testnet chain sync failed #2236

Merged

This was referenced Oct 18, 2024

feat(sdk)!: detect stale nodes #2254

Merged

feat(sdk): sdk-level retry logic for fetch and fetch_many #2266

Merged

feat(sdk)!: return consensus errors from broadcast methods #2274

Merged

This was referenced Oct 27, 2024

fix: document query start to support pagination #2284

Merged

chore(drive): increase withdrawal limits to 2000 Dash per day #2287

Merged

feat(sdk): added transfer transition to rs-sdk #2289

Merged

coderabbitai Bot mentioned this pull request Nov 4, 2024

feat: hardcoded identity transfers in strategy tests #2312

Merged

6 tasks

coderabbitai Bot mentioned this pull request Nov 15, 2024

feat(sdk): including grovedb proof bytes when error in proof verification #2332

Merged

6 tasks

coderabbitai Bot mentioned this pull request Dec 3, 2024

feat(sdk)!: allow setting CA cert #1924

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sdk): added signing_withdrawal_key_to_use to withdraw sdk call#2234

fix(sdk): added signing_withdrawal_key_to_use to withdraw sdk call#2234
QuantumExplorer merged 1 commit into
v1.4-devfrom
fix/withdrawalFixes

QuantumExplorer commented Oct 10, 2024 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2024 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

QuantumExplorer commented Oct 10, 2024 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested labels

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

QuantumExplorer commented Oct 10, 2024 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 10, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)