Repository navigation
fix(wallets): use selector ceiling for asset-lock Max #937
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
Changes from 21 commits
3493f71
807bad5
6bed93f
c2abaa4
fcd4c0f
40938d1
e96f20f
aa53542
e8d2c26
8d6f1c9
7ec1505
d38b544
d02fd53
64cd544
48e457f
0d0e762
c127b38
45c5a0b
4d7a3e3
85e9f46
d652f8f
33622bb
1bc0d96
03ec120
8f5c140
4e826bc
792e50e
bb5234a
4a61f3e
9a5105d
c4a985f
8ea190d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,24 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). | |
|
|
||
| ### Fixed | ||
|
|
||
| - **"Max" now matches what your Core wallet can actually send**: pressing | ||
| "Max" when shielding DASH, funding a Platform address, sending directly to | ||
| an identity, or funding an identity (creating or topping up, from your | ||
| wallet balance or a received deposit) could suggest an amount larger than | ||
| the wallet could actually send, so the transaction was rejected no matter | ||
| how you adjusted it. Max and the amount check now ask the wallet directly | ||
| what it can send instead of estimating from an on-screen balance, so the | ||
| two stay in agreement, and both reserve room for the fee. Funding from a | ||
| received deposit is also now capped by what actually arrived at that | ||
| deposit address, never by unrelated funds elsewhere in the wallet. While | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| the check is running, the amount field shows "Checking the available | ||
| amount…"; if it fails, "The available amount could not be checked." | ||
| appears with a "Retry available amount check" button, and you can still | ||
| switch to a different funding method at any point. For a very large | ||
| wallet, this check is capped at a few seconds; if it can't finish in | ||
| time, the amount field explains that and suggests sending a smaller | ||
| amount or trying again shortly. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking: The changelog overpromises absolute sendability The entry says Max matches what the wallet can actually send, Max and validation stay in agreement, and the large-wallet check is capped at a few seconds. The cache currently validates against stale ceilings after wallet-input changes, and the timeout still waits for an active BranchAndBound call to finish. A builder quote is also inherently a point-in-time result whose reservation state can change before dispatch. Rephrase the changelog and SND-017 as a builder-derived point-in-time ceiling that reduces over-limit attempts, and claim a bounded wait only after the backend actually returns at the deadline. source: ['codex'] |
||
|
|
||
| - **Wallet rename consistency**: renaming a wallet no longer overwrites other | ||
| saved wallet details when metadata cannot be read. Overlapping renames and | ||
| wallet removals also keep displayed aliases and deleted-wallet metadata | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |||||||||||||||||||||
| //! `Debug` → variant name + fields (logged and shown in collapsible details). | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use crate::model::fee_estimation::format_credits_as_dash; | ||||||||||||||||||||||
| use crate::model::wallet::WalletSeedHash; | ||||||||||||||||||||||
| use dash_sdk::Error as SdkError; | ||||||||||||||||||||||
| use dash_sdk::dapi_client::DapiClientError; | ||||||||||||||||||||||
| use dash_sdk::dapi_client::transport::TransportError; | ||||||||||||||||||||||
|
|
@@ -234,6 +235,23 @@ pub enum TaskError { | |||||||||||||||||||||
| >, | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// The non-broadcasting asset-lock builder probe exceeded its bounded wait. | ||||||||||||||||||||||
| #[error( | ||||||||||||||||||||||
| "We couldn't work out the maximum amount you can send from this wallet in time. Try sending a smaller amount, or wait a few minutes and try again." | ||||||||||||||||||||||
| )] | ||||||||||||||||||||||
| AssetLockMaxAmountTimedOut { seed_hash: WalletSeedHash }, | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Timeout guidance recommends an action the UI still blocks After this error, each screen marks the quote request as failed, and all affected dispatch paths still require
Suggested change
source: ['codex']
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread. |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// The non-broadcasting asset-lock builder probe could not determine a safe Max. | ||||||||||||||||||||||
| #[error( | ||||||||||||||||||||||
| "The wallet's available amount could not be checked. Wait a moment and try again." | ||||||||||||||||||||||
| )] | ||||||||||||||||||||||
| AssetLockBalanceQueryFailed { | ||||||||||||||||||||||
| #[source] | ||||||||||||||||||||||
| source: Box< | ||||||||||||||||||||||
| dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::transaction_builder::BuilderError, | ||||||||||||||||||||||
| >, | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// The payment would need more individual unspent outputs than fit in a | ||||||||||||||||||||||
| /// single standard transaction. | ||||||||||||||||||||||
| #[error( | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| //! Stateless asset-lock amount calculations shared by UI flows. | ||
|
|
||
| /// Why an asset-lock amount is outside the builder-derived ceiling. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum AssetLockAmountError { | ||
| /// Adding the operation-specific reserve exceeded the amount range. | ||
| Overflow, | ||
| /// The requested amount plus its reserve exceeds the builder ceiling. | ||
| ExceedsMaximum { maximum_amount_duffs: u64 }, | ||
| } | ||
|
|
||
| /// Largest user-entered amount after reserving operation-specific fees. | ||
| /// | ||
| /// Both arguments must use the same unit (duffs or Platform credits). | ||
| pub fn asset_lock_user_max_amount(builder_max: u64, reserve: u64) -> u64 { | ||
| builder_max.saturating_sub(reserve) | ||
| } | ||
|
|
||
| /// Validate a user-entered amount against the live builder-derived ceiling. | ||
| pub fn validate_asset_lock_amount( | ||
| amount_duffs: u64, | ||
| reserve_duffs: u64, | ||
| builder_max_duffs: u64, | ||
| ) -> Result<(), AssetLockAmountError> { | ||
| let required_duffs = amount_duffs | ||
| .checked_add(reserve_duffs) | ||
| .ok_or(AssetLockAmountError::Overflow)?; | ||
| if required_duffs > builder_max_duffs { | ||
| return Err(AssetLockAmountError::ExceedsMaximum { | ||
| maximum_amount_duffs: asset_lock_user_max_amount(builder_max_duffs, reserve_duffs), | ||
| }); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{AssetLockAmountError, asset_lock_user_max_amount, validate_asset_lock_amount}; | ||
|
|
||
| #[test] | ||
| fn builder_ceiling_validation_reserves_operation_fee() { | ||
| assert_eq!(asset_lock_user_max_amount(10_000, 1_000), 9_000); | ||
| assert_eq!(validate_asset_lock_amount(9_000, 1_000, 10_000), Ok(())); | ||
| assert_eq!( | ||
| validate_asset_lock_amount(9_001, 1_000, 10_000), | ||
| Err(AssetLockAmountError::ExceedsMaximum { | ||
| maximum_amount_duffs: 9_000, | ||
| }) | ||
| ); | ||
| } | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 RUST-005 · MEDIUM — the request/status block and the validate/report block are copy-pasted across three screens Two verbatim duplications and a third partial one. Block A — 25 lines, byte-identical apart from the Block B — ~28 lines, verbatim: Note the Fix:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed this is duplicated across the three funding screens, and your diagnosis that it's directly why the three screens drifted in behavior is exactly right. Deferring the full extraction as a larger refactor rather than doing it under this PR's scope, but the round of fixes just pushed did bring the three screens' patterns back into alignment with each other, which reduces (doesn't eliminate) the risk you're flagging. 🤖 Co-authored by Claudius the Magnificent AI Agent |
Uh oh!
There was an error while loading. Please reload this page.