From f4a4084d12cd1cd222523dcfbb39ca9ad9e18ac4 Mon Sep 17 00:00:00 2001 From: Stefan Thomas Date: Tue, 26 Aug 2014 14:36:42 -0700 Subject: [PATCH 1/2] [FIX] Fix currency choice restriction with federation. Previously the account_currencies restriction would overwrite any federation currency restriction. This patch implements the correct behavior where the Ripple network currency restrictions are intersected with the federation restrictions. Also adds a loading message while currency restrictions are being determined. Fixes a race condition where an old account_currencies request can succeed later, even if the destination has since changed. --- src/jade/tabs/send.jade | 8 ++- src/js/tabs/send.js | 126 ++++++++++++++++++++++++++-------------- 2 files changed, 89 insertions(+), 45 deletions(-) diff --git a/src/jade/tabs/send.jade b/src/jade/tabs/send.jade index a6881c637..a79b0f310 100644 --- a/src/jade/tabs/send.jade +++ b/src/jade/tabs/send.jade @@ -112,7 +112,7 @@ section.single.content(ng-controller='SendCtrl') ng-model='field.value' ng-required='{{field.required}}') option(ng-repeat='option in field.options', ng-bind='option.label', value='{{option.value}}', ng-selected='option.selected', ng-disabled='option.disabled') - .form-group + .form-group(ng-show="send.currency_choices.length") label(for='send_amount', l10n) Recipient will receive .row.amount(ng-if="!send.currency_force") .col-xs-12.col-sm-6.col-md-2 @@ -177,6 +177,8 @@ section.single.content(ng-controller='SendCtrl') | Checking p.literal(ng-show="send.path_status == 'fed-check'", rp-spinner="4", l10n) | Analyzing address + p.literal(ng-show="send.path_status == 'account-currencies'", rp-spinner="4", l10n) + | Scanning accepted currencies p.literal(ng-show="send.path_status == 'bridge-quote'", rp-spinner="4", l10n) | Requesting quote p.literal(ng-show="send.path_status == 'pending' && send.currency_code != 'XRP'", rp-spinner="4", l10n) @@ -186,7 +188,9 @@ section.single.content(ng-controller='SendCtrl') p.literal(ng-show="send.path_status == 'no-path' && send.currency_code != 'XRP'", l10n) | You cannot send {{send.amount}} {{send.currency}} to | {{send.recipient}}. Either your account has insufficient funds, - | or {{send.recipient}} doesn't accept {{send.currency}}. + | or {{send.recipient}} doesn't accept {{send.currency}}. + p.literal(ng-show="send.path_status == 'error-no-currency'", l10n) + | There are no valid currency choices for this destination. p.literal(ng-show="send.path_status == 'error-quote'", l10n) | Error while retrieving quote for outbound payment. span(ng-show="send.quote_error") The outbound bridge reported: "{{send.quote_error | rpheavynormalize}}" diff --git a/src/js/tabs/send.js b/src/js/tabs/send.js index 6794845ec..99ca1ebdb 100644 --- a/src/js/tabs/send.js +++ b/src/js/tabs/send.js @@ -67,7 +67,12 @@ SendTab.prototype.angular = function (module) }, true); $scope.$watch('send.currency', function () { - $scope.send.currency_code = ripple.Currency.from_json($scope.send.currency).to_human().toUpperCase(); + var currency = ripple.Currency.from_json($scope.send.currency); + if ($scope.send.currency !== '' && currency.is_valid()) { + $scope.send.currency_code = currency.to_human().toUpperCase(); + } else { + $scope.send.currency_code = ''; + } $scope.update_currency(); }, true); @@ -159,6 +164,12 @@ SendTab.prototype.angular = function (module) if ($scope.sendForm && $scope.sendForm.send_destination) $scope.sendForm.send_destination.$setValidity("federation", true); + // If there was a previous federation request, we need to clean it up here. + if (send.federation_record) { + send.federation_record = null; + send.dt = null; + } + if (send.federation) { send.path_status = "fed-check"; $federation.check_email(recipient) @@ -289,40 +300,50 @@ SendTab.prototype.angular = function (module) send.currency_choices = $scope.currencies_all; send.currency_force = false; + send.currency_choices_constraints = {}; + // Federation response can specific a fixed amount if (send.federation_record && "undefined" !== typeof send.federation_record.amount) { send.force_amount = Amount.from_json(send.federation_record.amount); send.amount = send.force_amount.to_text(); - send.currency_choices = [send.force_amount.currency().to_json()]; - send.currency_force = send.force_amount.currency().to_json(); - send.currency = send.currency_force; - } + send.currency_choices_constraints.federation = [send.force_amount.currency().to_json()]; // Apply federation currency restrictions - if (send.federation_record && + } else if (send.federation_record && $.isArray(send.federation_record.currencies) && send.federation_record.currencies.length >= 1 && "object" === typeof send.federation_record.currencies[0] && "string" === typeof send.federation_record.currencies[0].currency) { // XXX Do some validation on this - send.currency_choices = []; + send.currency_choices_constraints.federation = []; $.each(send.federation_record.currencies, function () { - send.currency_choices.push(this.currency); + send.currency_choices_constraints.federation.push(this.currency); }); - send.currency_force = send.currency_choices[0]; - send.currency = send.currency_choices[0]; } if (!send.recipient_info.loaded) return; if (send.recipient_info.exists) { // Check allowed currencies for this address - $network.remote.request_account_currencies(send.recipient_address) + var requestedRecipientAddress = send.recipient_address; + send.currency_choices_constraints.accountLines = 'pending'; + $network.remote.request_account_currencies(requestedRecipientAddress) .on('success', function (data) { - if (data.receive_currencies) { - $scope.update_currency_choices(data.receive_currencies); - } + $scope.$apply(function () { + if (data.receive_currencies && + // We need to make sure the destination account hasn't changed + send.recipient_address === requestedRecipientAddress) { + send.currency_choices_constraints.accountLines = data.receive_currencies; + + // add XRP if it's allowed + if (!$scope.send.recipient_info.disallow_xrp) { + send.currency_choices_constraints.accountLines.unshift('XRP'); + } + + $scope.update_currency_choices(); + } + }); }) .on('error', function () {}) .request(); @@ -336,48 +357,56 @@ SendTab.prototype.angular = function (module) // Do nothing } else { // If the account doesn't exist, we can only send XRP - send.currency_choices = ["XRP"]; - send.currency_force = "XRP"; - send.currency = "XRP"; + send.currency_choices_constraints.accountLines = ["XRP"]; } - $scope.update_currency(); + $scope.update_currency_choices(); }; - $scope.update_currency_choices = function(receive_currencies) { - - $scope.$apply(function () { - // Generate list of accepted currencies - var currencies = _.uniq(_.compact(_.map(receive_currencies, function (currency) { - return currency; - }))); - - // add XRP if it's allowed - if (!$scope.send.recipient_info.disallow_xrp) { - currencies.unshift('XRP'); - } + // Generate list of accepted currencies + $scope.update_currency_choices = function() { + var send = $scope.send; - // create a currency object for each of the currency codes - for (var i=0; i < currencies.length; i++) { - currencies[i] = ripple.Currency.from_json(currencies[i]); + var currencies = []; - if (i === 0) { - $scope.send.currency_code = currencies[i].get_iso(); - } - } + // Make sure none of the currency_choices_constraints are pending + if (_.values(send.currency_choices_constraints).indexOf('pending') !== -1) { + send.path_status = 'account-currencies'; + send.currency_choices = []; + return; + } else { + // The possible currencies are the intersection of all provided currency + // constraints. + currencies = _.intersection.apply(_, _.values(send.currency_choices_constraints)); + currencies = _.uniq(_.compact(currencies)); // create the display version of the currencies currencies = _.map(currencies, function (currency) { - if ($scope.currencies_all_keyed[currency.get_iso()]) { - return currency.to_human({full_name:$scope.currencies_all_keyed[currency.get_iso()].name}); + // create a currency object for each of the currency codes + var currencyObj = ripple.Currency.from_json(currency); + if ($scope.currencies_all_keyed[currencyObj.get_iso()]) { + return currencyObj.to_human({full_name:$scope.currencies_all_keyed[currencyObj.get_iso()].name}); } else { - return currency.to_human(); + return currencyObj.to_human(); } }); + } - $scope.send.currency_choices = currencies; - $scope.send.currency = currencies[0]; - }); + if (currencies.length === 1) { + send.currency = send.currency_force = currencies[0]; + } else if (currencies.length === 0) { + send.path_status = 'error-no-currency'; + send.currency = ''; + } else { + send.currency_force = false; + + if (currencies.indexOf(send.currency) === -1) { + send.currency = currencies[0]; + } + } + + $scope.send.currency_choices = currencies; + $scope.update_currency(); }; // Reset anything that depends on the currency @@ -398,6 +427,11 @@ SendTab.prototype.angular = function (module) return; } + if (!send.currency_choices || + send.currency_choices.length === 0) { + return; + } + $scope.update_amount(); }; @@ -414,6 +448,12 @@ SendTab.prototype.angular = function (module) $scope.update_amount = function () { var send = $scope.send; var recipient = send.recipient_actual || send.recipient_address; + + if (!send.currency_choices || + send.currency_choices.length === 0) { + return; + } + var currency = ripple.Currency.from_human(send.currency); var matchedCurrency = currency.has_interest() ? currency.to_hex() : currency.get_iso(); From a4c6b976eb152abd80ce2efe44ecf5589a61a2be Mon Sep 17 00:00:00 2001 From: Stefan Thomas Date: Tue, 26 Aug 2014 14:54:36 -0700 Subject: [PATCH 2/2] [TEST] Update test cases for new currency choices logic. --- test/unit/tabs/sendTabSpec.js | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/test/unit/tabs/sendTabSpec.js b/test/unit/tabs/sendTabSpec.js index 76d0c7338..d8131afc7 100644 --- a/test/unit/tabs/sendTabSpec.js +++ b/test/unit/tabs/sendTabSpec.js @@ -129,12 +129,12 @@ describe('SendCtrl', function(){ }); it("should update the currency_choices with the receiver's options", function(done) { + scope.send.currency_choices_constraints = {}; + scope.send.currency_choices_constraints.accountLines = ["XRP", "BEE", "BRL", "DPD", "FLO", "FRI", "GUM", "LTC", "NMC", "PPC", "SFO", "TDO", "TIK", "TIM", "USD", "XAU", "015841551A748AD2C1F76FF6ECB0CCCD00000000"]; + scope.update_currency_choices(); - var recipient_currencies = ["BEE", "BRL", "DPD", "FLO", "FRI", "GUM", "LTC", "NMC", "PPC", "SFO", "TDO", "TIK", "TIM", "USD", "XAU", "015841551A748AD2C1F76FF6ECB0CCCD00000000"]; - scope.update_currency_choices(recipient_currencies); - - // +1 since XRP is added - assert.strictEqual(scope.send.currency_choices.length, recipient_currencies.length + 1); + assert.strictEqual(scope.send.currency_choices.length, + scope.send.currency_choices_constraints.accountLines.length); assert.isTrue(_.contains(scope.send.currency_choices, "XRP - Ripples")); assert.isTrue(_.contains(scope.send.currency_choices, "XAU - Gold")); @@ -144,18 +144,6 @@ describe('SendCtrl', function(){ done(); }); - it("shouldn't add XRP as an option if the receiver disallows XRP", function(done) { - scope.send.recipient_info.disallow_xrp = 1; - scope.$apply(); - - var recipient_currencies = ["BEE", "BRL", "DPD", "FLO", "FRI", "GUM", "LTC", "NMC", "PPC", "SFO", "TDO", "TIK", "TIM", "USD", "XAU", "015841551A748AD2C1F76FF6ECB0CCCD00000000"]; - scope.update_currency_choices(recipient_currencies); - - assert.strictEqual(scope.send.currency_choices.length, recipient_currencies.length); - - done(); - }); - // currently not working, filters prohibit the recipient watch to be fired it.skip("should reset if the recipient is not valid", function(done) { scope.send.recipient = "asdfas";