diff --git a/apps/amm/tests/qml/tst_TokenAmountInput.qml b/apps/amm/tests/qml/tst_TokenAmountInput.qml index 128ff95..0a7dea7 100644 --- a/apps/amm/tests/qml/tst_TokenAmountInput.qml +++ b/apps/amm/tests/qml/tst_TokenAmountInput.qml @@ -117,7 +117,7 @@ TestCase { compare(input.selectedHolding.balanceRaw, "42") } - function test_multipleMatchingHoldingsRequireSelection() { + function test_multipleMatchingHoldingsSelectHighestBalance() { var input = createTemporaryObject(inputComponent, testCase, { "tokenData": { "definitionId": enabledId, @@ -142,8 +142,8 @@ TestCase { verify(input) tryCompare(input, "hasHoldingFunds", true) - compare(input.holdingReady, false) - compare(input.selectedHoldingId, "") + tryCompare(input, "selectedHoldingId", "holding-a") + compare(input.holdingReady, true) input.setHoldingSelection("holding-b") compare(input.selectedHoldingId, "holding-b") diff --git a/apps/shared/wallet/qml/ProgramAccountSelector.qml b/apps/shared/wallet/qml/ProgramAccountSelector.qml index c3d6fba..797f10f 100644 --- a/apps/shared/wallet/qml/ProgramAccountSelector.qml +++ b/apps/shared/wallet/qml/ProgramAccountSelector.qml @@ -30,6 +30,8 @@ Item { property color borderColor: "#52525b" property color focusColor: "#f26a21" property int modelRevision: 0 + property bool selectionWasAutomatic: false + property string reconciledCriteriaKey: "" readonly property bool criteriaReady: root.accountType.length > 0 && (root.stateField.length === 0 @@ -247,6 +249,16 @@ Item { if (root.accountIdFor(row).length > 0) result.push(row) } + result.sort(function(left, right) { + const balanceOrder = root.compareUnsignedDecimals( + root.valueFor(left, "balanceRaw"), + root.valueFor(right, "balanceRaw")) + if (balanceOrder !== 0) + return -balanceOrder + const leftId = root.accountIdFor(left) + const rightId = root.accountIdFor(right) + return leftId < rightId ? -1 : leftId > rightId ? 1 : 0 + }) return result } @@ -266,6 +278,22 @@ Item { return value === undefined || value === null ? "" : String(value) } + function normalizedUnsignedDecimal(value) { + const text = root.scalarText(value).trim() + if (!/^[0-9]+$/.test(text)) + return "0" + const normalized = text.replace(/^0+/, "") + return normalized.length > 0 ? normalized : "0" + } + + function compareUnsignedDecimals(left, right) { + const leftText = root.normalizedUnsignedDecimal(left) + const rightText = root.normalizedUnsignedDecimal(right) + if (leftText.length !== rightText.length) + return leftText.length < rightText.length ? -1 : 1 + return leftText < rightText ? -1 : leftText > rightText ? 1 : 0 + } + function accountIdFor(row) { return String(root.valueFor(row, "accountId") || root.valueFor(row, "displayAddress") @@ -325,37 +353,54 @@ Item { return text.length > 14 ? text.slice(0, 7) + "..." + text.slice(-5) : text } - function setSelection(accountId, createNew) { + function setSelection(accountId, createNew, automatic) { const nextId = String(accountId || "") const nextCreate = createNew === true - if (root.selectedAccountId === nextId && root.createNewSelected === nextCreate) + const nextAutomatic = automatic === true + if (root.selectedAccountId === nextId && root.createNewSelected === nextCreate) { + root.selectionWasAutomatic = nextAutomatic return + } + root.selectionWasAutomatic = nextAutomatic root.selectedAccountId = nextId root.createNewSelected = nextCreate root.selectionChanged(nextId, nextCreate) } + function criteriaKey() { + return root.accountType + "\u0000" + + root.stateField + "\u0000" + + root.scalarText(root.stateValue) + "\u0000" + + String(root.selectionMode) + } + function reconcileSelection() { + const nextCriteriaKey = root.criteriaKey() + const criteriaChanged = root.reconciledCriteriaKey !== nextCriteriaKey + root.reconciledCriteriaKey = nextCriteriaKey if (!root.criteriaReady) { - root.setSelection("", false) + root.setSelection("", false, true) return } - if (root.selectionValid) - return + if (!criteriaChanged && !root.selectionWasAutomatic) { + if (root.selectionValid) + return + if (root.selectionMode === ProgramAccountSelector.Output + && root.createNewSelected) { + return + } + } if (root.selectionMode === ProgramAccountSelector.Input) { - root.setSelection(root.matchingAccounts.length === 1 + root.setSelection(root.matchingAccounts.length > 0 ? root.accountIdFor(root.matchingAccounts[0]) : "", - false) + false, + true) return } - if (root.createNewSelected) - return if (root.matchingAccounts.length === 0) { - root.setSelection("", true) - } else if (root.matchingAccounts.length === 1) { - root.setSelection(root.accountIdFor(root.matchingAccounts[0]), false) + root.setSelection("", true, true) } else { - root.setSelection("", false) + root.setSelection(root.accountIdFor(root.matchingAccounts[0]), false, true) } } } diff --git a/apps/shared/wallet/tests/qml/tst_ProgramAccountSelector.qml b/apps/shared/wallet/tests/qml/tst_ProgramAccountSelector.qml index 2d8a21b..e27ed36 100644 --- a/apps/shared/wallet/tests/qml/tst_ProgramAccountSelector.qml +++ b/apps/shared/wallet/tests/qml/tst_ProgramAccountSelector.qml @@ -75,7 +75,7 @@ Item { compare(selector.selectedBalanceRaw, "120") } - function test_inputMultipleHoldingsRequiresSelection() { + function test_inputMultipleHoldingsSelectsHighestBalance() { const selector = createTemporaryObject(selectorComponent, root, { "sourceModel": [root.holdingA, root.holdingB], "selectionMode": Wallet.ProgramAccountSelector.Input @@ -83,12 +83,16 @@ Item { verify(!!selector, "Component exists") tryCompare(selector, "showCombo", true) compare(selector.matchingAccounts.length, 2) - compare(selector.ready, false) + tryCompare(selector, "selectedAccountId", "holding-a") + compare(selector.selectedBalanceRaw, "120") + compare(selector.ready, true) selector.setSelection("holding-b", false) compare(selector.selectedAccountId, "holding-b") compare(selector.selectedBalanceRaw, "80") compare(selector.ready, true) + selector.reconcileSelection() + compare(selector.selectedAccountId, "holding-b") } function test_outputNoHoldingSelectsCreateNew() { @@ -120,7 +124,7 @@ Item { compare(selector.ready, true) } - function test_outputMultipleHoldingsRequiresDestination() { + function test_outputMultipleHoldingsSelectsHighestAndOffersCreateNew() { const selector = createTemporaryObject(selectorComponent, root, { "sourceModel": [root.holdingA, root.holdingB], "selectionMode": Wallet.ProgramAccountSelector.Output @@ -128,9 +132,50 @@ Item { verify(!!selector, "Component exists") tryCompare(selector, "showCombo", true) compare(selector.choices.length, 3) + tryCompare(selector, "selectedAccountId", "holding-a") + compare(selector.createNewSelected, false) + compare(selector.ready, true) + + selector.setSelection("", true) compare(selector.selectedAccountId, "") + compare(selector.createNewSelected, true) + compare(selector.ready, true) + } + + function test_highestBalanceComparisonPreservesU128Precision() { + const selector = createTemporaryObject(selectorComponent, root, { + "sourceModel": [ + { + "accountId": "holding-lower", + "accountType": "TokenHolding", + "definitionId": "token-a", + "balanceRaw": "900719925474099299999" + }, + { + "accountId": "holding-higher", + "accountType": "TokenHolding", + "definitionId": "token-a", + "balanceRaw": "900719925474099300000" + } + ], + "selectionMode": Wallet.ProgramAccountSelector.Input + }) + verify(!!selector, "Component exists") + tryCompare(selector, "selectedAccountId", "holding-higher") + compare(selector.matchingAccounts[0].accountId, "holding-higher") + } + + function test_automaticCreateNewChangesToHoldingAfterWalletLoads() { + const selector = createTemporaryObject(selectorComponent, root, { + "sourceModel": [], + "selectionMode": Wallet.ProgramAccountSelector.Output + }) + verify(!!selector, "Component exists") + tryCompare(selector, "createNewSelected", true) + + selector.sourceModel = [root.holdingB, root.holdingA] + tryCompare(selector, "selectedAccountId", "holding-a") compare(selector.createNewSelected, false) - compare(selector.ready, false) } function test_matchesNumericZeroState() { diff --git a/modules/amm/ffi/src/api/holding.rs b/modules/amm/ffi/src/api/holding.rs index d1b59d8..53cdb37 100644 --- a/modules/amm/ffi/src/api/holding.rs +++ b/modules/amm/ffi/src/api/holding.rs @@ -55,7 +55,7 @@ pub(super) fn select_holding( ) -> Option { let options = holding_options(holdings, definition_id); let Some(requested_id) = requested_id else { - return (options.len() == 1).then(|| options[0].clone()); + return options.first().cloned(); }; let requested_id = account_id_from_hex(requested_id, "holding id") .or_else(|_| parse_base58_id(requested_id, "holding id")) @@ -75,6 +75,11 @@ pub(super) fn holding_options( .filter(|holding| holding.definition_id == definition_id) .cloned() .collect::>(); - options.sort_by_key(|holding| holding.id); + options.sort_by(|left, right| { + right + .balance + .cmp(&left.balance) + .then_with(|| left.id.cmp(&right.id)) + }); options } diff --git a/modules/amm/ffi/src/api/quote.rs b/modules/amm/ffi/src/api/quote.rs index 38095b9..9995948 100644 --- a/modules/amm/ffi/src/api/quote.rs +++ b/modules/amm/ffi/src/api/quote.rs @@ -637,30 +637,20 @@ fn select_lp_destination( error, }; } - match options.as_slice() { - [] => LpDestination { + if options.is_empty() { + LpDestination { options, selected: None, requires_fresh: true, error: None, - }, - [only] => LpDestination { - selected: Some(only.clone()), + } + } else { + LpDestination { + selected: options.first().cloned(), options, requires_fresh: false, error: None, - }, - _ => LpDestination { - error: Some(issue( - "lp_destination_required", - "Select an LP TokenHolding destination.", - &["lpHoldingId", "createFreshLp"], - json!({ "available": options.len() }), - )), - options, - selected: None, - requires_fresh: false, - }, + } } } diff --git a/modules/amm/ffi/src/api/tests.rs b/modules/amm/ffi/src/api/tests.rs index c2ac6c1..7f30e9a 100644 --- a/modules/amm/ffi/src/api/tests.rs +++ b/modules/amm/ffi/src/api/tests.rs @@ -404,7 +404,7 @@ fn minimum_pair_is_minimal_on_price_base_side() { } #[test] -fn holding_selection_requires_a_choice_when_multiple_exist() { +fn holding_selection_defaults_to_highest_balance_then_lowest_id() { let definition = AccountId::new([9; 32]); let holding = |id: u8, balance| SelectedHolding { id: AccountId::new([id; 32]), @@ -419,7 +419,10 @@ fn holding_selection_requires_a_choice_when_multiple_exist() { ), }; let holdings = [holding(4, 10), holding(2, 20), holding(1, 20)]; - assert!(select_holding(&holdings, definition, None).is_none()); + assert_eq!( + select_holding(&holdings, definition, None).unwrap().id, + AccountId::new([1; 32]) + ); let selected = select_holding( &holdings, definition, @@ -687,27 +690,28 @@ fn missing_pool_quote_accepts_large_direct_raw_amounts() { } #[test] -fn quote_requires_explicit_input_holding_when_multiple_match() { +fn quote_defaults_to_highest_balance_input_holding_when_multiple_match() { let mut scenario = Scenario::devnet(); let extra_holding = AccountId::new([63; 32]); scenario.snapshot.wallet_accounts.push(account_read( extra_holding, - &token_holding(scenario.pair.token_a, 1_000_000), + &token_holding(scenario.pair.token_a, 2_000_000), )); - let ambiguous = scenario.quote(); - assert_eq!(ambiguous["canSubmit"], false); - assert!(ambiguous["errors"].as_array().unwrap().iter().any(|error| { - error["code"] == "holding_selection_required" - && error["blockingFields"] == json!(["holdingAId"]) - })); + let defaulted = scenario.quote(); + assert_eq!(defaulted["canSubmit"], true); + assert_eq!( + defaulted["accountPreview"][6]["accountId"], + extra_holding.to_string() + ); - scenario.request.holding_a_id = Some(extra_holding.to_string()); + let original_holding = AccountId::new([61; 32]); + scenario.request.holding_a_id = Some(original_holding.to_string()); let selected = scenario.quote(); assert_eq!(selected["canSubmit"], true); assert_eq!( selected["accountPreview"][6]["accountId"], - extra_holding.to_string() + original_holding.to_string() ); } @@ -762,20 +766,16 @@ fn active_pool_quote_uses_ratio_and_existing_lp_holding() { } #[test] -fn active_pool_quote_requires_lp_destination_when_multiple_match() { +fn active_pool_quote_defaults_to_highest_balance_lp_destination() { let lp_a = AccountId::new([64; 32]); let lp_b = AccountId::new([65; 32]); let mut scenario = active_scenario(&[(lp_a, 500), (lp_b, 200)]); - let ambiguous = scenario.quote(); - assert_eq!(ambiguous["canSubmit"], false); - assert_eq!(ambiguous["lpDestinationRequired"], true); - assert_eq!(ambiguous["lpHoldingOptions"].as_array().unwrap().len(), 2); - assert!(ambiguous["errors"] - .as_array() - .unwrap() - .iter() - .any(|error| error["code"] == "lp_destination_required")); + let defaulted = scenario.quote(); + assert_eq!(defaulted["canSubmit"], true); + assert_eq!(defaulted["lpDestinationRequired"], false); + assert_eq!(defaulted["lpHoldingOptions"].as_array().unwrap().len(), 2); + assert_eq!(defaulted["selectedLpHoldingId"], lp_a.to_string()); scenario.request.lp_holding_id = Some(lp_b.to_string()); let selected = scenario.quote();