Skip to content

Comments

fix(sdk): propagate PutSettings in token freeze/mint/unfreeze/set_price transitions#3132

Open
thepastaclaw wants to merge 1 commit intodashpay:v3.1-devfrom
thepastaclaw:fix/token-put-settings-propagation
Open

fix(sdk): propagate PutSettings in token freeze/mint/unfreeze/set_price transitions#3132
thepastaclaw wants to merge 1 commit intodashpay:v3.1-devfrom
thepastaclaw:fix/token-put-settings-propagation

Conversation

@thepastaclaw
Copy link
Contributor

@thepastaclaw thepastaclaw commented Feb 20, 2026

Issue being fixed or feature implemented

Token transition methods for freeze, mint, unfreeze, and set_price_for_direct_purchase pass None to broadcast_and_wait instead of the builder's settings field. This silently ignores any caller-provided PutSettings (retry config, timeout, etc.).

Split out from #3092 per review feedback ("1 PR = 1 thing").

What was done?

Extract settings from the transition builder before sign() consumes it, then pass it to broadcast_and_wait. This matches the pattern already used in destroy_frozen_funds.

Before:

let state_transition = freeze_tokens_transition_builder
    .sign(self, signing_key, signer, platform_version).await?;
let proof_result = state_transition
    .broadcast_and_wait::<StateTransitionProofResult>(self, None)  // ← settings dropped
    .await?;

After:

let put_settings = freeze_tokens_transition_builder.settings;  // ← extract before sign
let state_transition = freeze_tokens_transition_builder
    .sign(self, signing_key, signer, platform_version).await?;
let proof_result = state_transition
    .broadcast_and_wait::<StateTransitionProofResult>(self, put_settings)  // ← propagated
    .await?;

Applied to: freeze.rs, mint.rs, unfreeze.rs, set_price_for_direct_purchase.rs

All other token transitions (burn, claim, config_update, direct_purchase, emergency_action, transfer, destroy_frozen_funds) already propagate settings correctly.

How Has This Been Tested?

Code review — the fix is mechanical and matches the existing correct pattern in destroy_frozen_funds.rs.

Breaking Changes

None. This is a bug fix — callers who set PutSettings on these builders will now have them respected as intended.

Summary by CodeRabbit

Bug Fixes

  • Token transition operations including freeze, mint, set price for direct purchase, and unfreeze now properly utilize configured settings during proof broadcasting, replacing previous null value defaults. This ensures consistent and reliable verification behavior across all token-related transactions while maintaining backward compatibility without affecting the public API surface or existing functionality.

Validation

  1. What was tested

    • CI checks on this PR (Rust packages linting, tests, builds)
  2. Results

    • All Rust CI checks passing
  3. Environment

    • GitHub Actions CI for dashpay/platform

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

📝 Walkthrough

Walkthrough

Four token transition files now extract settings from their respective builders and pass them to broadcast calls instead of None, enabling proper settings utilization during state transition proof broadcasting across freeze, mint, unfreeze, and price-setting operations.

Changes

Cohort / File(s) Summary
Token Transition Settings Integration
packages/rs-sdk/src/platform/tokens/transitions/freeze.rs, mint.rs, set_price_for_direct_purchase.rs, unfreeze.rs
Each file now extracts put_settings from its corresponding transition builder and passes it to broadcast_and_wait instead of None, enabling builder-provided settings to be utilized during proof broadcasting. No changes to public API signatures or control flow structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Four transitions now align,
Settings flow through each design,
No more None to block the way,
Builder's wisdom guides the play,
Token proofs dance true today! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: propagating PutSettings across four token transition types (freeze, mint, unfreeze, set_price).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

thepastaclaw added a commit to thepastaclaw/platform that referenced this pull request Feb 20, 2026
The PutSettings propagation bug fix for freeze/mint/unfreeze/set_price
transitions has been split into its own PR (dashpay#3132) per review feedback.
This PR now only contains the transition hash exposure feature.
@thepastaclaw
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…ce transitions

Token transition methods for freeze, mint, unfreeze, and
set_price_for_direct_purchase were passing None to broadcast_and_wait
instead of the builder's settings field. This silently ignored any
caller-provided PutSettings (retry config, timeout, etc.).

Extract settings from the builder before sign() consumes it, matching
the pattern already used in destroy_frozen_funds.
@thepastaclaw thepastaclaw force-pushed the fix/token-put-settings-propagation branch from b2ec44d to 1eb6a89 Compare February 21, 2026 19:25
@github-actions github-actions bot added this to the v3.1.0 milestone Feb 21, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/rs-sdk/src/platform/tokens/transitions/unfreeze.rs (1)

80-82: ⚠️ Potential issue | 🟡 Minor

Pre-existing copy-paste comment — "freezing" should be "unfreezing".

-            // This means the token keeps freezing history
+            // This means the token keeps unfreezing history
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/rs-sdk/src/platform/tokens/transitions/unfreeze.rs` around lines 80
- 82, The comment above the UnfreezeResult::HistoricalDocument branch
incorrectly says "freezing" — update the comment to use "unfreezing" (e.g.,
change "// This means the token keeps freezing history" to "// This means the
token keeps unfreezing history" or a clearer phrase like "// This means the
token keeps unfreezing history/document history") near the match arm that
returns Ok(UnfreezeResult::HistoricalDocument(doc)) so the wording accurately
reflects the Unfreeze flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/rs-sdk/src/platform/tokens/transitions/unfreeze.rs`:
- Around line 80-82: The comment above the UnfreezeResult::HistoricalDocument
branch incorrectly says "freezing" — update the comment to use "unfreezing"
(e.g., change "// This means the token keeps freezing history" to "// This means
the token keeps unfreezing history" or a clearer phrase like "// This means the
token keeps unfreezing history/document history") near the match arm that
returns Ok(UnfreezeResult::HistoricalDocument(doc)) so the wording accurately
reflects the Unfreeze flow.

Copy link
Collaborator

@shumkov shumkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants