From a10147026410fcb2e820772458a29f944c3f0db0 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Tue, 21 Sep 2021 13:27:51 +0200 Subject: [PATCH 1/9] dApp: split transfer action into two Due to recent decisions, the user must always be able to see the full costs of his transactions before confirming them. This means that the fully automated transfer by the SDK can not be used anymore as it might end up in a mediated transfer with additional hidden costs. To solve this issue the dApp must always decide to do a direct or mediated transfer. Therefore the former transfer action got replaced by two new actions: direct transfer and mediated transfer. The direct transfer will also construct the to use direct route itself. This is necessary to ensure it will be a direct transfer. Furthermore it is fine to put this into the action itself, as it will not cause any hidden costs. This is different for the mediated transfer. Here the user must decide himself to first query a route (paying the pathfinding service) and then use this route with the additional fees. Therefore the mediated transfer action is expecting the route as parameter. --- .../transfer/DirectTransferAction.vue | 63 +++++++++++ ...rAction.vue => MediatedTransferAction.vue} | 14 ++- raiden-dapp/src/locales/en.json | 16 ++- .../transfer/direct-transfer-action.spec.ts | 100 ++++++++++++++++++ ...ec.ts => mediated-transfer-action.spec.ts} | 16 ++- 5 files changed, 197 insertions(+), 12 deletions(-) create mode 100644 raiden-dapp/src/components/transfer/DirectTransferAction.vue rename raiden-dapp/src/components/transfer/{TransferAction.vue => MediatedTransferAction.vue} (73%) create mode 100644 raiden-dapp/tests/unit/components/transfer/direct-transfer-action.spec.ts rename raiden-dapp/tests/unit/components/transfer/{transfer-action.spec.ts => mediated-transfer-action.spec.ts} (81%) diff --git a/raiden-dapp/src/components/transfer/DirectTransferAction.vue b/raiden-dapp/src/components/transfer/DirectTransferAction.vue new file mode 100644 index 0000000000..3381b51f6a --- /dev/null +++ b/raiden-dapp/src/components/transfer/DirectTransferAction.vue @@ -0,0 +1,63 @@ + diff --git a/raiden-dapp/src/components/transfer/TransferAction.vue b/raiden-dapp/src/components/transfer/MediatedTransferAction.vue similarity index 73% rename from raiden-dapp/src/components/transfer/TransferAction.vue rename to raiden-dapp/src/components/transfer/MediatedTransferAction.vue index 76b651dfdd..9a934e9bac 100644 --- a/raiden-dapp/src/components/transfer/TransferAction.vue +++ b/raiden-dapp/src/components/transfer/MediatedTransferAction.vue @@ -2,29 +2,34 @@ import type { BigNumber } from 'ethers'; import { Component, Mixins, Prop } from 'vue-property-decorator'; +import type { RaidenPaths } from 'raiden-ts'; + import ActionMixin from '@/mixins/action-mixin'; import type { ActionProgressStep } from '@/model/types'; const cleanTransferStep: ActionProgressStep = { - title: 'transfer-action.steps.transfer.title', - description: 'transfer-action.steps.transfer.description', + title: 'mediated-transfer-action.steps.transfer.title', + description: 'mediated-transfer-action.steps.transfer.description', active: false, completed: false, failed: false, }; @Component -export default class ChannelOpenAndTransferAction extends Mixins(ActionMixin) { +export default class MediatedTransferAction extends Mixins(ActionMixin) { @Prop({ required: true }) readonly transferTokenAmount!: BigNumber; @Prop({ required: true }) readonly paymentIdentifier!: BigNumber; + @Prop({ required: true }) + readonly route!: RaidenPaths[number]; + transferStep = Object.assign({}, cleanTransferStep); get confirmButtonLabel(): string { - return this.$t('transfer-action.confirm-button-label') as string; + return this.$t('mediated-transfer-action.confirm-button-label') as string; } get steps(): ActionProgressStep[] { @@ -43,6 +48,7 @@ export default class ChannelOpenAndTransferAction extends Mixins(ActionMixin) { options.partnerAddress, this.transferTokenAmount, this.paymentIdentifier, + [this.route], ); this.transferStep.completed = true; diff --git a/raiden-dapp/src/locales/en.json b/raiden-dapp/src/locales/en.json index c798dae4e3..d15800b459 100644 --- a/raiden-dapp/src/locales/en.json +++ b/raiden-dapp/src/locales/en.json @@ -304,12 +304,22 @@ "label": "Amount" } }, - "transfer-action": { + "mediated-transfer-action": { "confirm-button-label": "Confirm Payment", "steps": { "transfer": { - "title": "Transfering tokens", - "description": "Please wait" + "title": "Transfering tokens via route", + "description": "Transfer in progress" + } + } + }, + "direct-transfer-action": { + "confirm-button-label": "Confirm Payment", + "no-route-error": "Please verify that the recipient is online and receives payments before trying again.", + "steps": { + "transfer": { + "title": "Transfering tokens directly", + "description": "Transfer in progress" } } }, diff --git a/raiden-dapp/tests/unit/components/transfer/direct-transfer-action.spec.ts b/raiden-dapp/tests/unit/components/transfer/direct-transfer-action.spec.ts new file mode 100644 index 0000000000..fa676aca1a --- /dev/null +++ b/raiden-dapp/tests/unit/components/transfer/direct-transfer-action.spec.ts @@ -0,0 +1,100 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { $t } from '../../utils/mocks'; + +import type { Wrapper } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; +import type { BigNumber } from 'ethers'; +import { constants } from 'ethers'; +import flushPromises from 'flush-promises'; + +import DirectTransferAction from '@/components/transfer/DirectTransferAction.vue'; + +const defaultTransferOptions = { + tokenAddress: '0xToken', + partnerAddress: '0xPartner', +}; + +function createWrapper(options?: { + transferTokenAmount?: BigNumber; + paymentIdentifier?: BigNumber; + directRoute?: (...args: any[]) => void; + transfer?: (...args: any[]) => void; +}): Wrapper { + const $raiden = { + directRoute: options?.directRoute ?? jest.fn().mockResolvedValue(['no-actual-route']), + transfer: options?.transfer ?? jest.fn(), + }; + + return shallowMount(DirectTransferAction, { + propsData: { + transferTokenAmount: options?.transferTokenAmount ?? constants.One, + paymentIdentifier: options?.paymentIdentifier ?? constants.Two, + completionDelayTimeout: 0, + }, + mocks: { $raiden, $t }, + }); +} + +describe('DirectTransferAction', () => { + afterEach(() => { + flushPromises(); + }); + + test('fetches route from raiden service with correct arguments', async () => { + const directRoute = jest.fn(); + const wrapper = createWrapper({ directRoute, transferTokenAmount: constants.One }); + + await (wrapper.vm as any).runAction({ + tokenAddress: '0xToken', + partnerAddress: '0xPartner', + }); + + expect(directRoute).toHaveBeenCalledTimes(1); + expect(directRoute).toHaveBeenLastCalledWith('0xToken', '0xPartner', constants.One); + }); + + test('executes raiden service deposit with correct arguments', async () => { + const directRoute = jest.fn().mockResolvedValue(['no-actual-route']); + const transfer = jest.fn(); + const wrapper = createWrapper({ + directRoute, + transfer, + transferTokenAmount: constants.One, + paymentIdentifier: constants.Two, + }); + + await (wrapper.vm as any).runAction({ + tokenAddress: '0xToken', + partnerAddress: '0xPartner', + }); + + expect(transfer).toHaveBeenCalledTimes(1); + expect(transfer).toHaveBeenLastCalledWith( + '0xToken', + '0xPartner', + constants.One, + constants.Two, + ['no-actual-route'], + ); + }); + + test('sets transfer step to be active before calling raiden service', async () => { + const directRoute = jest.fn().mockReturnValue(new Promise(() => undefined)); + const wrapper = createWrapper({ directRoute }); + + (wrapper.vm as any).runAction(defaultTransferOptions); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(wrapper.vm.$data.transferStep.active).toBeTruthy(); + }); + + test('completes transfer step when transfer finishes', async () => { + const wrapper = createWrapper(); + + (wrapper.vm as any).runAction(defaultTransferOptions); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(wrapper.vm.$data.transferStep.completed).toBeTruthy(); + expect(wrapper.vm.$data.transferStep.active).toBeFalsy(); + }); +}); diff --git a/raiden-dapp/tests/unit/components/transfer/transfer-action.spec.ts b/raiden-dapp/tests/unit/components/transfer/mediated-transfer-action.spec.ts similarity index 81% rename from raiden-dapp/tests/unit/components/transfer/transfer-action.spec.ts rename to raiden-dapp/tests/unit/components/transfer/mediated-transfer-action.spec.ts index f832f1dc47..013ad03752 100644 --- a/raiden-dapp/tests/unit/components/transfer/transfer-action.spec.ts +++ b/raiden-dapp/tests/unit/components/transfer/mediated-transfer-action.spec.ts @@ -7,7 +7,9 @@ import type { BigNumber } from 'ethers'; import { constants } from 'ethers'; import flushPromises from 'flush-promises'; -import TransferAction from '@/components/transfer/TransferAction.vue'; +import type { RaidenPaths } from 'raiden-ts'; + +import MediatedTransfer from '@/components/transfer/MediatedTransferAction.vue'; const defaultTransferOptions = { tokenAddress: '0xToken', @@ -17,32 +19,35 @@ const defaultTransferOptions = { function createWrapper(options?: { transferTokenAmount?: BigNumber; paymentIdentifier?: BigNumber; + route?: RaidenPaths[number]; transfer?: (...args: any[]) => void; -}): Wrapper { +}): Wrapper { const $raiden = { transfer: options?.transfer ?? jest.fn(), }; - return shallowMount(TransferAction, { + return shallowMount(MediatedTransfer, { propsData: { transferTokenAmount: options?.transferTokenAmount ?? constants.One, paymentIdentifier: options?.paymentIdentifier ?? constants.Two, + route: options?.route ?? 'no-actual-route', completionDelayTimeout: 0, }, mocks: { $raiden, $t }, }); } -describe('TransferAction', () => { +describe('MediatedTransferAction', () => { afterEach(() => { flushPromises(); }); - test('executes raiden service deposit with correct arguments', async () => { + test('executes raiden service transfer with correct arguments', async () => { const transfer = jest.fn().mockResolvedValue(undefined); const wrapper = createWrapper({ transferTokenAmount: constants.One, paymentIdentifier: constants.Two, + route: 'no-actual-route' as unknown as RaidenPaths[number], transfer, }); @@ -57,6 +62,7 @@ describe('TransferAction', () => { '0xPartner', constants.One, constants.Two, + ['no-actual-route'], ); }); From 513025688945f26dbc510acc955d918020ffba59 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Tue, 21 Sep 2021 13:53:46 +0200 Subject: [PATCH 2/9] dApp: rework additional action component props The action components, based on the action mixin are quiet some special components. But some of them are slightly different than the others. These are those which include (also) a transfer. So far action are quite tightly coupled to the usage with action forms in may cases. Though this might become improved in future, at the moment there is the need for some actions to take more options to perform their action then the action form provides (e.g. payment identifier). These have been provided as special properties which are only defined on a couple of those actions. This lead to different interfaces of the actions, which is not nice. Especially where actions are used interchangeable like for the quick pay feature it is necessary to have a unified interface. So far this situation was accepted, because all actions that were different where different the same. And on the quick pay route only those special actions where used. But with the recent addition of direct and mediated transfer actions, this has changed. The mediated transfer now also needs to be provided with a route. This also causes issues in the quick pay route. To solve it, a new property has been added to the base action mixin. This piece of data works the same as the generic `options` argument to the `handleAction` function of the actions. It is an object that can be specified individually by each implementing action. The same goes now for the `fixedRunOptions`. They are supposed to be provided as an optional component property and can be used as any other data in the respective actions implementation. Those actions who don't need any additional options just ignore them. After all there are some downsides to this approach: type safety. Having this generic new properties does not allow for any static type checks. The action will basically fail (or not) when the respective action logic can not handle the undefined parameters which are expected. But it could also lead to unintended behavior if they can but work differently then. After all typical JavaScript issues. We accept these issues for the moment. The actions were not safe beforehand as well as this is just the same approach as we took for the direct/"flexible" action options. There are different approaches to tackle this issue to detect miss-usage early. Though probably not on a static code analysis base. Unfortunately none of these approaches is really the best and it requires more thoughts how to solve it completely with something that works for both option datasets. --- .../ChannelDepositAndTransferAction.vue | 26 ++++++++------- .../channels/ChannelOpenAndTransferAction.vue | 30 +++++++++-------- .../transfer/DirectTransferAction.vue | 32 ++++++++++--------- .../transfer/MediatedTransferAction.vue | 30 ++++++++--------- raiden-dapp/src/mixins/action-mixin.ts | 3 ++ ...hannel-deposit-and-transfer-action.spec.ts | 11 ++++--- .../channel-open-and-transfer-action.spec.ts | 11 ++++--- .../transfer/direct-transfer-action.spec.ts | 11 ++++--- .../transfer/mediated-transfer-action.spec.ts | 13 ++++---- 9 files changed, 90 insertions(+), 77 deletions(-) diff --git a/raiden-dapp/src/components/channels/ChannelDepositAndTransferAction.vue b/raiden-dapp/src/components/channels/ChannelDepositAndTransferAction.vue index 6a1627f091..978ddedc2b 100644 --- a/raiden-dapp/src/components/channels/ChannelDepositAndTransferAction.vue +++ b/raiden-dapp/src/components/channels/ChannelDepositAndTransferAction.vue @@ -1,10 +1,15 @@ diff --git a/raiden-dapp/tests/unit/components/amount-display.spec.ts b/raiden-dapp/tests/unit/components/amount-display.spec.ts index 81622108a0..bc23d40ff1 100644 --- a/raiden-dapp/tests/unit/components/amount-display.spec.ts +++ b/raiden-dapp/tests/unit/components/amount-display.spec.ts @@ -1,10 +1,12 @@ import type { Wrapper } from '@vue/test-utils'; import { mount } from '@vue/test-utils'; +import type { BigNumber } from 'ethers'; import { constants } from 'ethers'; import Vue from 'vue'; import AmountDisplay from '@/components/AmountDisplay.vue'; import Filters from '@/filters'; +import type { Token } from '@/model/types'; import { generateToken } from '../utils/data-generator'; @@ -13,18 +15,22 @@ Vue.filter('displayFormat', Filters.displayFormat); const token = generateToken({ balance: constants.One }); function createWrapper(options?: { + token?: Token; + amount?: BigNumber; exactAmount?: boolean; sign?: string; label?: string; + slot?: string; }): Wrapper { return mount(AmountDisplay, { propsData: { - token: token, - amount: token.balance, + token: options?.token ?? token, + amount: options?.amount ?? constants.One, exactAmount: options?.exactAmount, sign: options?.sign, label: options?.label, }, + slots: { default: options?.slot ?? '' }, }); } @@ -38,42 +44,68 @@ async function triggerHoverEvent( describe('AmountDisplay.vue', () => { test('does not display full amount when "exactAmount" prop is set to false', async () => { - const wrapper = createWrapper({ exactAmount: false }); + const token = generateToken({ decimals: 7 }); + const wrapper = createWrapper({ exactAmount: false, token, amount: constants.One }); + const formattedAmount = wrapper.get('.amount-display__formatted-amount'); await triggerHoverEvent(wrapper); - expect(wrapper.text()).toContain('<0.000001'); + expect(formattedAmount.text()).toContain('<0.000001'); }); test('does display full amount when "exactAmount" prop is set to true', async () => { - const wrapper = createWrapper({ exactAmount: true }); + const token = generateToken({ decimals: 7 }); + const wrapper = createWrapper({ exactAmount: true, token, amount: constants.One }); + const formattedAmount = wrapper.get('.amount-display__formatted-amount'); await triggerHoverEvent(wrapper); - expect(wrapper.text()).toContain('0.000000000000000001'); + expect(formattedAmount.text()).toContain('0.0000001'); }); test('toggles between displaying full amount and not displaying full amount', async () => { - const wrapper = createWrapper({ exactAmount: true }); + const token = generateToken({ decimals: 7 }); + const wrapper = createWrapper({ exactAmount: true, token, amount: constants.One }); + const formattedAmount = wrapper.get('.amount-display__formatted-amount'); await triggerHoverEvent(wrapper, 'mouseover'); - expect(wrapper.text()).toContain('0.000000000000000001'); + expect(formattedAmount.text()).toContain('0.0000001'); await triggerHoverEvent(wrapper, 'mouseleave'); - expect(wrapper.text()).toContain('<0.000001'); + expect(formattedAmount.text()).toContain('<0.000001'); }); test('displays sign if propery is set', () => { const wrapper = createWrapper({ sign: '+' }); + const formattedAmount = wrapper.get('.amount-display__formatted-amount'); - expect(wrapper.text()).toContain('+'); + expect(formattedAmount.text()).toContain('+'); }); test('displays label if propery is set', () => { const wrapper = createWrapper({ label: 'test label' }); + const label = wrapper.find('.amount-display__label'); - expect(wrapper.text()).toContain('test label'); + expect(label.exists()).toBeTruthy(); + expect(label.text()).toBe('test label'); + }); + + test('displays also an empty label if propery is set', () => { + const wrapper = createWrapper({ label: '' }); + const label = wrapper.find('.amount-display__label'); + + expect(label.exists()).toBeTruthy(); + expect(label.text()).toBe(''); + }); + + test('displays slot instead of amount if defined', () => { + const wrapper = createWrapper({ slot: "test" }); + const slot = wrapper.find('#test'); + const formattedAmount = wrapper.find('.amount-display__formatted-amount'); + + expect(slot.exists()).toBeTruthy(); + expect(formattedAmount.exists()).toBeFalsy(); }); }); From e33328f6a85a1a5cea5c938d4af8220295afe45f Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Tue, 21 Sep 2021 14:45:29 +0200 Subject: [PATCH 4/9] dApp: allow action form to have no focus The action form component automatically focused the first input field that is editable. But this might not be always the desired behavior. Therefore a new property has been added to disable autofocus completely. --- .../src/components/channels/ChannelActionForm.vue | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/raiden-dapp/src/components/channels/ChannelActionForm.vue b/raiden-dapp/src/components/channels/ChannelActionForm.vue index f280200459..ab3e338d47 100644 --- a/raiden-dapp/src/components/channels/ChannelActionForm.vue +++ b/raiden-dapp/src/components/channels/ChannelActionForm.vue @@ -168,6 +168,9 @@ export default class ChannelActionForm extends Vue { @Prop({ type: Boolean, default: false }) readonly stickyButton!: boolean; + @Prop({ type: Boolean, default: false }) + readonly autofocusDisabled!: boolean; + @Prop({ required: true }) readonly runAction!: (options: unknown) => Promise; @@ -203,15 +206,18 @@ export default class ChannelActionForm extends Vue { } get tokenAddressInputFocused(): boolean { - return this.tokenAddressEditable; + return !this.autofocusDisabled && this.tokenAddressEditable; } get partnerAddressInputFocused(): boolean { - return !this.tokenAddressInputFocused && this.partnerAddressEditable; + return ( + !this.autofocusDisabled && !this.tokenAddressInputFocused && this.partnerAddressEditable + ); } get tokenAmountInputFocused(): boolean { return ( + !this.autofocusDisabled && !this.tokenAddressInputFocused && !this.partnerAddressInputFocused && this.tokenAmountEditable From e913065083494162fede94f2c8f288315526e0b5 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Wed, 22 Sep 2021 17:11:20 +0200 Subject: [PATCH 5/9] dApp: add warning feature to amount display Sometimes an amount is associated with a warning. Prominent examples are when a balance is too low or a price too high. To simply visualize this, the amount display component has a new flag property to toggle the warning. This will apply special coloring to the formatted amount then. --- raiden-dapp/src/components/AmountDisplay.vue | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/raiden-dapp/src/components/AmountDisplay.vue b/raiden-dapp/src/components/AmountDisplay.vue index 0a9a097650..de3d07f792 100644 --- a/raiden-dapp/src/components/AmountDisplay.vue +++ b/raiden-dapp/src/components/AmountDisplay.vue @@ -13,7 +13,10 @@ - + {{ formattedAmount }} @@ -50,6 +53,9 @@ export default class AmountDisplay extends Vue { @Prop({ type: Boolean, default: false }) fullWidth!: boolean; + @Prop({ type: Boolean, default: false }) + warning!: boolean; + displayExactAmount = false; get showLabel(): boolean { @@ -76,6 +82,8 @@ export default class AmountDisplay extends Vue { From a093c6b9f0533b513de3c5cf040255a09f2e5942 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Wed, 22 Sep 2021 17:17:31 +0200 Subject: [PATCH 6/9] dApp: adapt quick pay feature for transparency Due to recent decisions, the user must always be able to see the full costs of his transactions before confirming them. This means that the fully automated transfer by the SDK can not be used anymore as it might end up in a mediated transfer with additional hidden costs. To solve this issue the dApp must always decide to do a direct or mediated transfer. As a result of this, the quick pay route must manually fetch the available pathfinding services and select the cheapest one. In case a direct transfer is not possible but a mediated one, the user will get displayed more payment information. This includes a new section for the mediation. Here the user sees the price of the cheapest service once it is known. Then the user can decide himself to spend the money to fetch a route. Once the possible routes are fetched, the cheapest one gets selected and the user can see the mediation fees for this route. The user can now see the complete of costs the payment will charge him. While the user has not yet asked to query the pathfinding service for a route, the quick pay view will show him the alternative payment solution. This will either be a channel deposit and direct transfer or a channel open and direct transfer. Once he has received a valid route, this will switch to a mediated transfer. So far the user can not decide against the fetched route and go back to the alternative action. As workaround he would need to reload the page and go straight for it. The whole section of mediation stuff includes some nice features like visual progress states, and the detecting if a pathfinding service is too expensive with visual effects and disabling. The logic to handle failed transfer got removed as it is not necessary anymore. It was only used to check that a mediated transfer was not possible and then decide on an alternative action. This is now build in from the very beginning and the user has the choice between both. I'm happy about the user flow we have designed here. But I'm not too happy with the code. The quick pay route has become way too big and complex. Though it is quite hard to split it in a helpful way. Somehow it would be nice to outsource the whole payment information stuff with the new mediation interactive logic. But it is to much tied with all the other stuff that it produces more overhead than actual gain. Furthermore have the latest updates shown that the combination of actions with the channel action form is not flexible enough. While a nice feature, it must become improved with a more generic form approach. --- raiden-dapp/src/components/icons/Spinner.vue | 2 +- raiden-dapp/src/locales/en.json | 10 +- raiden-dapp/src/views/QuickPayRoute.vue | 257 +++++++-- .../tests/unit/views/quick-pay-route.spec.ts | 523 +++++++++++------- 4 files changed, 529 insertions(+), 263 deletions(-) diff --git a/raiden-dapp/src/components/icons/Spinner.vue b/raiden-dapp/src/components/icons/Spinner.vue index b36808d723..6aefaec6d6 100644 --- a/raiden-dapp/src/components/icons/Spinner.vue +++ b/raiden-dapp/src/components/icons/Spinner.vue @@ -1,5 +1,5 @@ diff --git a/raiden-dapp/src/locales/en.json b/raiden-dapp/src/locales/en.json index d15800b459..dc4fd0262a 100644 --- a/raiden-dapp/src/locales/en.json +++ b/raiden-dapp/src/locales/en.json @@ -887,10 +887,13 @@ } }, "quick-pay": { - "transfer-information-labels": { + "transfer-information": { "header": "Payment Information", "target-address": "Receiver", - "token-amount": "Payment amount" + "token-amount": "Payment amount", + "pathfinding-service-price": "Mediation possible, price for requesting route", + "fetch-route-trigger": "Pay to find route", + "fetch-route-error": "No route available" }, "action-titles": { "channel-deposit": "Channel Deposit", @@ -899,7 +902,8 @@ "channel-open-and-transfer": "Channel Open and Pay" }, "action-messages": { - "transfer": "Please confirm the above payment information.", + "direct-transfer": "Please confirm the above payment information.", + "mediated-transfer": "Please confirm the above payment information including the mediation fees for the to use route.", "channel-deposit-and-transfer": "{deposit} will be deposited into the existing channel and {payment} will be paid out.", "channel-open-and-transfer": "{deposit} will be deposited into a new channel and {payment} will be paid out." }, diff --git a/raiden-dapp/src/views/QuickPayRoute.vue b/raiden-dapp/src/views/QuickPayRoute.vue index 540bfca22b..9ff718dd9f 100644 --- a/raiden-dapp/src/views/QuickPayRoute.vue +++ b/raiden-dapp/src/views/QuickPayRoute.vue @@ -2,43 +2,83 @@
- {{ $t('quick-pay.transfer-information-labels.header') }} + {{ $t('quick-pay.transfer-information.header') }} + +
+ + + + + + + + + + + {{ $t('quick-pay.transfer-information.fetch-route-error') }} + + +
-
+
{{ actionHeader }} import { BigNumber, constants } from 'ethers'; import { Component, Mixins, Watch } from 'vue-property-decorator'; -import { mapGetters } from 'vuex'; +import { createNamespacedHelpers, mapGetters } from 'vuex'; -import type { RaidenChannel, RaidenError } from 'raiden-ts'; -import { ChannelState, ErrorCodes } from 'raiden-ts'; +import type { RaidenChannel, RaidenPaths, RaidenPFS } from 'raiden-ts'; +import { ChannelState } from 'raiden-ts'; -import ActionProgressCard from '@/components/ActionProgressCard.vue'; +import ActionButton from '@/components/ActionButton.vue'; import AddressDisplay from '@/components/AddressDisplay.vue'; import AmountDisplay from '@/components/AmountDisplay.vue'; import ChannelActionForm from '@/components/channels/ChannelActionForm.vue'; @@ -89,21 +130,18 @@ import ChannelOpenAndTransferAction from '@/components/channels/ChannelOpenAndTr import ErrorDialog from '@/components/dialogs/ErrorDialog.vue'; import Spinner from '@/components/icons/Spinner.vue'; import TokenInformation from '@/components/TokenInformation.vue'; -import TransferAction from '@/components/transfer/TransferAction.vue'; +import DirectTransferAction from '@/components/transfer/DirectTransferAction.vue'; +import MediatedTransferAction from '@/components/transfer/MediatedTransferAction.vue'; import NavigationMixin from '@/mixins/navigation-mixin'; import type { Token } from '@/model/types'; import { BalanceUtils } from '@/utils/balance-utils'; import { getAddress, getAmount, getPaymentId } from '@/utils/query-params'; -function isRecoverableTransferError(error: RaidenError): boolean { - return [ErrorCodes.PFS_NO_ROUTES_FOUND, ErrorCodes.PFS_NO_ROUTES_BETWEEN_NODES].includes( - error.message, - ); -} +const { mapState: mapUserDepositContractState } = createNamespacedHelpers('userDepositContract'); @Component({ components: { - ActionProgressCard, + ActionButton, AddressDisplay, AmountDisplay, ChannelActionForm, @@ -112,24 +150,36 @@ function isRecoverableTransferError(error: RaidenError): boolean { ErrorDialog, Spinner, TokenInformation, - TransferAction, + DirectTransferAction, + MediatedTransferAction, }, computed: { ...mapGetters({ getChannels: 'channels', getToken: 'token', }), + ...mapUserDepositContractState({ + serviceToken: 'token', + }), }, }) export default class QuickPayRoute extends Mixins(NavigationMixin) { getChannels!: (tokenAddress: string) => RaidenChannel[]; getToken!: (tokenAddress: string) => Token | null; + serviceToken!: Token; + pathfindingService: RaidenPFS | null = null; + serviceTokenCapacity = constants.Zero; + fetchMediationRouteInProgress = false; + fetchMediationRouteFailed = false; + mediationRoute: RaidenPaths[number] | null = null; depositAmountInput = constants.Zero; actionComponent: - | typeof TransferAction + | typeof DirectTransferAction + | typeof MediatedTransferAction | typeof ChannelOpenAndTransferAction - | typeof ChannelDepositAndTransferAction = TransferAction; + | typeof ChannelDepositAndTransferAction + | null = null; actionInProgress = false; actionFailed = false; error: Error | null = null; // Note that action errors are not set here. @@ -170,7 +220,11 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { } get tokenAmountInputHidden(): boolean { - return this.actionComponent === TransferAction; + return ( + this.actionComponent === null || + this.actionComponent === DirectTransferAction || + this.actionComponent === MediatedTransferAction + ); } get paymentIdentifier(): BigNumber | undefined { @@ -222,9 +276,44 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { return channelsWithEnoughCapacity.length > 0; } + get mediationTransferIsPossible(): boolean { + return this.hasAnyChannelWithEnoughCapacity && !this.directChannelHasEnoughCapacity; + } + + get pathfindingServicePrice(): BigNumber | undefined { + return this.pathfindingService?.price + ? BigNumber.from(this.pathfindingService.price) + : undefined; + } + + get pathfindingServiceTooExpensive(): boolean { + if (this.pathfindingServicePrice === undefined) { + return false; + } else { + return this.pathfindingServicePrice.gt(this.serviceTokenCapacity); + } + } + + get showFetchMediationRouteButton(): boolean { + return ( + !this.mediationRoute && + !this.fetchMediationRouteInProgress && + !this.fetchMediationRouteFailed + ); + } + + get fetchMediationRouteDisabled(): boolean { + return this.pathfindingService === null || this.pathfindingServiceTooExpensive; + } + + get mediationFees(): BigNumber | undefined { + return this.mediationRoute?.fee ? BigNumber.from(this.mediationRoute.fee) : undefined; + } + get actionHeader(): string { switch (this.actionComponent) { - case TransferAction: + case DirectTransferAction: + case MediatedTransferAction: return ''; // Empty on purpose. No header for this case. case ChannelDepositAndTransferAction: return this.$t('quick-pay.action-titles.channel-deposit') as string; @@ -235,9 +324,18 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { } } + get actionFixedRunOptions(): { [key: string]: unknown } { + return { + transferTokenAmount: this.tokenAmount, + paymentIdentifier: this.paymentIdentifier, + route: this.mediationRoute, + }; + } + get actionDialogTitle(): string { switch (this.actionComponent) { - case TransferAction: + case DirectTransferAction: + case MediatedTransferAction: return ''; // Empty on purpose. No title for this case. case ChannelDepositAndTransferAction: return this.$t('quick-pay.action-titles.channel-deposit-and-transfer') as string; @@ -250,8 +348,10 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { get actionMessagePath(): string { switch (this.actionComponent) { - case TransferAction: - return 'quick-pay.action-messages.transfer'; + case DirectTransferAction: + return 'quick-pay.action-messages.direct-transfer'; + case MediatedTransferAction: + return 'quick-pay.action-messages.mediated-transfer'; case ChannelDepositAndTransferAction: return 'quick-pay.action-messages.channel-deposit-and-transfer'; case ChannelOpenAndTransferAction: @@ -261,6 +361,50 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { } } + get actionFormAutofocusDisabled(): boolean { + // In this case the it could be that the user can fetch a route for + // a mediated transfer. In such a case the active action and its form will + // be either the channel deposit and transfer or channel open and transfer. + // Focusing their form in such a case it not nice as the user theoretically + // should prefer the mediated transfer if possible. + // In case of a direct transfer this does basically nothing. + // In case no direct transfer nor a mediated transfer is possible, this will + // still autofocus the respective form. + return this.hasAnyChannelWithEnoughCapacity; + } + + created(): void { + this.fetchServiceTokenCapacity(); + this.fetchCheapestPathfindingService(); + } + + async fetchServiceTokenCapacity(): Promise { + this.serviceTokenCapacity = await this.$raiden.getUDCCapacity(); + } + + async fetchCheapestPathfindingService(): Promise { + const allServiceSorted = await this.$raiden.fetchServices(); + this.pathfindingService = allServiceSorted[0]; + } + + async fetchCheapestMediationRoute(): Promise { + try { + this.fetchMediationRouteFailed = false; + this.fetchMediationRouteInProgress = true; + const allRoutesSorted = await this.$raiden.findRoutes( + this.tokenAddress, + this.targetAddress, + this.tokenAmount!, + this.pathfindingService!, + ); + this.mediationRoute = allRoutesSorted[0]; + } catch (error) { + this.fetchMediationRouteFailed = true; + } finally { + this.fetchMediationRouteInProgress = false; + } + } + @Watch('$route.query', { immediate: true }) onRouteQueryChanged(): void { if (this.anyRequiredQueryParameterInvalid) { @@ -271,6 +415,11 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { } } + @Watch('mediationRoute') + onMediationRouteChanged(): void { + this.decideOnActionComponent(); + } + onActionFromInputsChanged(event: { tokenAmount: string }): void { this.depositAmountInput = BalanceUtils.parse(event.tokenAmount, this.token?.decimals ?? 18); } @@ -279,30 +428,27 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { this.actionInProgress = true; } - decideOnActionComponent(): void { - this.actionComponent = this.hasAnyChannelWithEnoughCapacity - ? TransferAction - : this.directChannelWithTarget - ? ChannelDepositAndTransferAction - : ChannelOpenAndTransferAction; + setActionFailed(): void { + this.actionFailed = true; } - decideToDepositOrOpenChannel(): void { - this.actionComponent = this.directChannelWithTarget + decideOnActionComponent(): void { + if (this.actionInProgress) { + return; + } + + // Note that the mediated transfer takes precedence here because the user + // manually fetches a route. If a direct transfer is possible, the user + // can't fetch such a route, so this case will never happen. + this.actionComponent = this.mediationRoute + ? MediatedTransferAction + : this.directChannelHasEnoughCapacity + ? DirectTransferAction + : this.directChannelWithTarget ? ChannelDepositAndTransferAction : ChannelOpenAndTransferAction; } - async handleActionError(error: RaidenError): Promise { - this.actionInProgress = false; - - if (isRecoverableTransferError(error)) { - this.decideToDepositOrOpenChannel(); - } else { - this.actionFailed = true; - } - } - async redirect(): Promise { if (this.redirectionTarget) { // Save getter into variable to preserve it over the automatic unloading @@ -351,13 +497,34 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { &__transfer-information { &__token, &__target, - &__amount { + &__amount, + &__mediation { height: 48px; margin: 10px 0; padding: 0 22px 0 16px; border-radius: 8px; background-color: $input-background; } + + &__mediation { + display: flex; + flex-direction: column; + justify-content: center; + height: 80px; + width: 100%; + + &__button { + color: $primary-color; + + &:disabled { + color: $primary-disabled-color; + } + } + + &__error { + color: $error-color; + } + } } } diff --git a/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts b/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts index 607b668c7f..2bf845ab69 100644 --- a/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts +++ b/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts @@ -3,6 +3,7 @@ import { $t } from '../utils/mocks'; import type { Wrapper } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils'; import { constants } from 'ethers'; +import flushPromises from 'flush-promises'; import Vuex, { Store } from 'vuex'; import type { Address, RaidenChannel } from 'raiden-ts'; @@ -10,7 +11,9 @@ import type { Address, RaidenChannel } from 'raiden-ts'; import ChannelDepositAndTransferAction from '@/components/channels/ChannelDepositAndTransferAction.vue'; import ChannelOpenAndTransferAction from '@/components/channels/ChannelOpenAndTransferAction.vue'; import ErrorDialog from '@/components/dialogs/ErrorDialog.vue'; -import TransferAction from '@/components/transfer/TransferAction.vue'; +import Spinner from '@/components/icons/Spinner.vue'; +import DirectTransferAction from '@/components/transfer/DirectTransferAction.vue'; +import MediatedTransferAction from '@/components/transfer/MediatedTransferAction.vue'; import type { Token } from '@/model/types'; import { RouteNames } from '@/router/route-names'; import QuickPayRoute from '@/views/QuickPayRoute.vue'; @@ -20,6 +23,8 @@ import { generateChannel, generateRoute, generateToken } from '../utils/data-gen const localVue = createLocalVue(); localVue.use(Vuex); +const userDepositToken = generateToken({ balance: constants.One }); + function createWrapper(options?: { tokenAddress?: string; targetAddress?: string; @@ -29,6 +34,9 @@ function createWrapper(options?: { token?: Token; channels?: RaidenChannel[]; accountAddress?: string; + getUDCCapacity?: () => void; + fetchServices?: () => void; + findRoutes?: () => void; disconnect?: () => void; push?: () => void; }): Wrapper { @@ -39,11 +47,23 @@ function createWrapper(options?: { token: () => () => token, }; - const store = new Store({ getters }); + const userDepositContractModule = { + namespaced: true, + state: { token: userDepositToken }, + }; + + const store = new Store({ + getters, + modules: { userDepositContract: userDepositContractModule }, + }); const $raiden = { getAccount: () => options?.accountAddress, disconnect: options?.disconnect ?? jest.fn(), + getUDCCapacity: options?.getUDCCapacity ?? jest.fn().mockResolvedValue(constants.Two), + fetchServices: + options?.fetchServices ?? jest.fn().mockResolvedValue([{ price: constants.One }]), + findRoutes: options?.findRoutes ?? jest.fn().mockResolvedValue(['no-actual-route']), }; const $route = generateRoute({ @@ -68,51 +88,57 @@ function createWrapper(options?: { }); } -const channelWithTarget = generateChannel({ - partner: '0x1F916ab5cf1B30B22f24Ebf435f53Ee665344Acf' as Address, - capacity: constants.Zero, -}); +async function clickGetRoutesButton(wrapper: Wrapper): Promise { + const getRouteButton = wrapper.get('.quick-pay__transfer-information__mediation__button'); + await flushPromises(); // Fetching services must complete + getRouteButton.trigger('click'); + await flushPromises(); // Fetching routes must complete +} + +const partnerAddress = '0x1F916ab5cf1B30B22f24Ebf435f53Ee665344Acf'; +const foreignAddress = '0xdA75b5Cd196C6C8292B567Fdc78B33A886ce0d80'; -const otherChannel = generateChannel({ - partner: '0x3D389c9B67cA85F1de2EbeF648C54d740365c366' as Address, - capacity: constants.Two, +const channelWithPartner = generateChannel({ + partner: partnerAddress as Address, + capacity: constants.One, }); -const optionsForStraightTransfer = { +const optionsForDirectTransfer = { + targetAddress: partnerAddress, tokenAmount: '1', - channels: [otherChannel], -}; - -const optionsForChannelDepositAndTransfer = { - targetAddress: '0x1F916ab5cf1B30B22f24Ebf435f53Ee665344Acf', - tokenAmount: '5', - channels: [channelWithTarget, otherChannel], + channels: [channelWithPartner], }; -const optionsForChannelDepositAndTransferWithTransferFirst = { - ...optionsForChannelDepositAndTransfer, +const optionsForMediatedTransfer = { + targetAddress: foreignAddress, tokenAmount: '1', + channels: [channelWithPartner], }; -const optionsForChannelOpenAndTransfer = { - targetAddress: '0xdA75b5Cd196C6C8292B567Fdc78B33A886ce0d80', +const optionsForChannelDepositAndTransfer = { + targetAddress: partnerAddress, tokenAmount: '5', - channels: [otherChannel], + channels: [channelWithPartner], }; -const optionsForChannelOpenAndTransferWithTransferFirst = { - ...optionsForChannelOpenAndTransfer, +const optionsForChannelOpenAndTransfer = { + targetAddress: foreignAddress, tokenAmount: '1', + channels: [], }; describe('QuickPayRoute.vue', () => { + afterEach(() => { + flushPromises(); + }); + describe('transfer information', () => { test('displays header', () => { const wrapper = createWrapper(); const header = wrapper.find('.quick-pay__transfer-information__header'); expect(header.exists()).toBeTruthy(); - expect(header.text()).toContain('quick-pay.transfer-information-labels.header'); + expect(header.text()).toContain('quick-pay.transfer-information.header'); }); test('displays token information', () => { @@ -132,9 +158,7 @@ describe('QuickPayRoute.vue', () => { expect(addressDisplay.exists()).toBeTruthy(); expect(addressDisplay.html()).toContain('0x1F916ab5cf1B30B22f24Ebf435f53Ee665344Acf'); - expect(addressDisplay.html()).toContain( - 'quick-pay.transfer-information-labels.target-address', - ); + expect(addressDisplay.html()).toContain('quick-pay.transfer-information.target-address'); }); test('displays token amount', () => { @@ -145,7 +169,7 @@ describe('QuickPayRoute.vue', () => { expect(amountDisplay.exists()).toBeTruthy(); expect(amountDisplay.html()).toContain('1'); expect(amountDisplay.html()).toContain(token.toString()); - expect(amountDisplay.html()).toContain('quick-pay.transfer-information-labels.token-amount'); + expect(amountDisplay.html()).toContain('quick-pay.transfer-information.token-amount'); }); }); @@ -211,57 +235,209 @@ describe('QuickPayRoute.vue', () => { }); }); - describe('transfer action', () => { - test('displays no header', () => { - const wrapper = createWrapper(optionsForStraightTransfer); - const header = wrapper.find('.quick-pay__action__header'); + describe('direct transfer', () => { + test('displays direct transfer action if direct channel exists and has enough capacity', () => { + const wrapper = createWrapper(optionsForDirectTransfer); + const action = wrapper.findComponent(DirectTransferAction); - expect(header.exists()).toBeFalsy(); + expect(action.exists()).toBeTruthy(); }); - test('displays action component if any channel has enough capacity', () => { - const wrapper = createWrapper({ ...optionsForStraightTransfer, paymentIdentifier: '501' }); - const action = wrapper.findComponent(TransferAction); + test('displays no action header', () => { + const wrapper = createWrapper(optionsForDirectTransfer); + const header = wrapper.find('.quick-pay__action__header'); - expect(action.exists()).toBeTruthy(); - expect(action.html()).toContain('1'); // token amount - expect(action.html()).toContain('501'); // payment identifier + expect(header.exists()).toBeFalsy(); }); - test('displays message', () => { - const wrapper = createWrapper(optionsForStraightTransfer); + test('displays correct action message', () => { + const wrapper = createWrapper(optionsForDirectTransfer); const message = wrapper.find('.quick-pay__action__message'); expect(message.exists()).toBeTruthy(); - expect(message.html()).toContain('quick-pay.action-messages.transfer'); + expect(message.html()).toContain('quick-pay.action-messages.direct-transfer'); }); }); - describe('channel deposit and transfer action', () => { - test('displays header', () => { - const wrapper = createWrapper(optionsForChannelDepositAndTransfer); - const header = wrapper.find('.quick-pay__action__header'); + describe('mediated transfer', () => { + test('displays medidation information if any channel has enough capacity but not the direct one', () => { + const wrapper = createWrapper(optionsForMediatedTransfer); + const mediationInformation = wrapper.find('.quick-pay__transfer-information__mediation'); - expect(header.exists()).toBeTruthy(); - expect(header.text()).toBe('quick-pay.action-titles.channel-deposit'); + expect(mediationInformation.exists()).toBeTruthy(); }); - test('displays action component if no channel has enoug capacity but there is a direct one', () => { + test('initially fetches user deposit capacity', () => { + const getUDCCapacity = jest.fn(); + createWrapper({ getUDCCapacity }); + + expect(getUDCCapacity).toHaveBeenCalledTimes(1); + }); + + test('initially fetches pathfinding service', () => { + const fetchServices = jest.fn().mockResolvedValue(['no-pathfinding-serivce']); + createWrapper({ fetchServices }); + + expect(fetchServices).toHaveBeenCalledTimes(1); + }); + + test('shows spinner while pathfinding service is fetched', async () => { + const fetchServices = jest.fn(() => new Promise(() => undefined)); + const wrapper = createWrapper({ fetchServices, ...optionsForMediatedTransfer }); + const spinner = wrapper.findComponent(Spinner); + + expect(spinner.exists()).toBeTruthy(); + }); + + test('disables get route button while pathfinding service is fetched', async () => { + const fetchServices = jest.fn(() => new Promise(() => undefined)); + const wrapper = createWrapper({ fetchServices, ...optionsForMediatedTransfer }); + const getRouteButton = wrapper.get('.quick-pay__transfer-information__mediation__button'); + + expect(getRouteButton.attributes('disabled')).toBeTruthy(); + }); + + test('selects the first returned pathfinding service as cheapest', async () => { + const fetchServices = jest + .fn() + .mockResolvedValue([{ price: constants.One }, { price: constants.Two }]); + const wrapper = createWrapper({ fetchServices, ...optionsForMediatedTransfer }); + const pathfindingServicePrice = wrapper.find( + '.quick-pay__transfer-information__mediation__pathfinding-service-price', + ); + + await flushPromises(); // Fetch services must complete. + + expect(pathfindingServicePrice.exists()).toBeTruthy(); + expect(pathfindingServicePrice.html()).toContain('1'); + }); + + test('marks pathfinding service price with a warning when price is higher than user deposit capacity', async () => { + const getUDCCapacity = jest.fn().mockResolvedValue(constants.One); + const fetchServices = jest.fn().mockResolvedValue([{ price: constants.Two }]); const wrapper = createWrapper({ - ...optionsForChannelDepositAndTransfer, - tokenAmount: '5', - paymentIdentifier: '501', + getUDCCapacity, + fetchServices, + ...optionsForMediatedTransfer, }); + const pathfindingServicePrice = wrapper.find( + '.quick-pay__transfer-information__mediation__pathfinding-service-price', + ); + + await flushPromises(); // Fetch services must complete. + + expect(pathfindingServicePrice.html()).toContain('warning'); + }); + + test('disables get route button when pathfinding service price is higher than user deposit capacity', () => { + const getUDCCapacity = jest.fn().mockResolvedValue(constants.One); + const fetchServices = jest.fn().mockResolvedValue([{ price: constants.Two }]); + const wrapper = createWrapper({ + getUDCCapacity, + fetchServices, + ...optionsForMediatedTransfer, + }); + const getRouteButton = wrapper.get('.quick-pay__transfer-information__mediation__button'); + + expect(getRouteButton.attributes('disabled')).toBeTruthy(); + }); + + test('fetches routes from cheapest pathfinding service when user click button', async () => { + const fetchServices = jest.fn().mockResolvedValue(['no-actual-service']); + const findRoutes = jest.fn(); + const wrapper = createWrapper({ + ...optionsForMediatedTransfer, + tokenAddress: '0x59105441977ecD9d805A4f5b060E34676F50F806', + targetAddress: '0xdA75b5Cd196C6C8292B567Fdc78B33A886ce0d80', + tokenAmount: '1', + fetchServices, + findRoutes, + }); + + await clickGetRoutesButton(wrapper); + + expect(findRoutes).toHaveBeenCalledTimes(1); + expect(findRoutes).toHaveBeenLastCalledWith( + '0x59105441977ecD9d805A4f5b060E34676F50F806', + '0xdA75b5Cd196C6C8292B567Fdc78B33A886ce0d80', + constants.One, + 'no-actual-service', + ); + }); + + test('shows warning message if no route was found', async () => { + const findRoutes = jest.fn().mockRejectedValue(new Error('No route between nodes')); + const wrapper = createWrapper({ findRoutes, ...optionsForMediatedTransfer }); + + await clickGetRoutesButton(wrapper); + + const mediationError = wrapper.find('.quick-pay__transfer-information__mediation__error'); + expect(mediationError.exists()).toBeTruthy(); + expect(mediationError.text()).toBe('quick-pay.transfer-information.fetch-route-error'); + }); + + test('selects the first returned route as cheapest', async () => { + const findRoutes = jest + .fn() + .mockResolvedValue([{ fee: constants.One }, { fee: constants.Two }]); + const wrapper = createWrapper({ findRoutes, ...optionsForMediatedTransfer }); + + await clickGetRoutesButton(wrapper); + + const mediationFees = wrapper.find('.quick-pay__transfer-information__mediation__fees'); + expect(mediationFees.exists()).toBeTruthy(); + expect(mediationFees.html()).toContain('1'); + }); + + test('displays mediated transfer action when a route has been found', async () => { + const findRoutes = jest.fn().mockResolvedValue(['no-actual-route']); + const wrapper = createWrapper({ findRoutes, ...optionsForMediatedTransfer }); + + await clickGetRoutesButton(wrapper); + + const action = wrapper.findComponent(MediatedTransferAction); + expect(action.exists()).toBeTruthy(); + }); + + test('displays no action header', async () => { + const findRoutes = jest.fn().mockResolvedValue(['no-actual-route']); + const wrapper = createWrapper({ findRoutes, ...optionsForMediatedTransfer }); + + await clickGetRoutesButton(wrapper); + + const header = wrapper.find('.quick-pay__action__header'); + expect(header.exists()).toBeFalsy(); + }); + + test('displays correct action message', async () => { + const findRoutes = jest.fn().mockResolvedValue(['no-actual-route']); + const wrapper = createWrapper({ findRoutes, ...optionsForMediatedTransfer }); + + await clickGetRoutesButton(wrapper); + + const message = wrapper.find('.quick-pay__action__message'); + expect(message.exists()).toBeTruthy(); + expect(message.html()).toContain('quick-pay.action-messages.mediated-transfer'); + }); + }); + + describe('channel deposit and transfer', () => { + test('displays channel deposit and transfer action if no channel has enough capacity but there is a direct one', () => { + const wrapper = createWrapper(optionsForChannelDepositAndTransfer); const action = wrapper.findComponent(ChannelDepositAndTransferAction); expect(action.exists()).toBeTruthy(); - expect(action.html()).toContain('5'); - expect(action.html()).toContain('501'); - expect(action.html()).toContain('showprogressindialog'); - expect(action.html()).toContain('quick-pay.action-titles.channel-deposit-and-transfer'); }); - test('displays message', () => { + test('displays correct action header', () => { + const wrapper = createWrapper(optionsForChannelDepositAndTransfer); + const header = wrapper.find('.quick-pay__action__header'); + + expect(header.exists()).toBeTruthy(); + expect(header.text()).toBe('quick-pay.action-titles.channel-deposit'); + }); + + test('displays correct action message', () => { const wrapper = createWrapper(optionsForChannelDepositAndTransfer); const message = wrapper.find('.quick-pay__action__message'); @@ -270,32 +446,23 @@ describe('QuickPayRoute.vue', () => { }); }); - describe('channel open and transfer action', () => { - test('displays header', () => { + describe('channel open and transfer', () => { + test('displays channel open and transfer action if no channel has enough capacity and there is no direct one', () => { const wrapper = createWrapper(optionsForChannelOpenAndTransfer); - const header = wrapper.find('.quick-pay__action__header'); + const action = wrapper.findComponent(ChannelOpenAndTransferAction); - expect(header.exists()).toBeTruthy(); - expect(header.text()).toBe('quick-pay.action-titles.channel-open'); + expect(action.exists()).toBeTruthy(); }); - test('displays action component if no channel has enoug capacity and there is no direct one', () => { - const wrapper = createWrapper({ - ...optionsForChannelOpenAndTransfer, - tokenAmount: '5', - paymentIdentifier: '501', - }); - const action = wrapper.findComponent(ChannelOpenAndTransferAction); + test('displays correct action header', () => { + const wrapper = createWrapper(optionsForChannelOpenAndTransfer); + const header = wrapper.find('.quick-pay__action__header'); - expect(action.exists()).toBeTruthy(); - expect(action.exists()).toBeTruthy(); - expect(action.html()).toContain('5'); - expect(action.html()).toContain('501'); - expect(action.html()).toContain('showprogressindialog'); - expect(action.html()).toContain('quick-pay.action-titles.channel-open-and-transfer'); + expect(header.exists()).toBeTruthy(); + expect(header.text()).toBe('quick-pay.action-titles.channel-open'); }); - test('displays message', () => { + test('displays correct action message', () => { const wrapper = createWrapper(optionsForChannelOpenAndTransfer); const message = wrapper.find('.quick-pay__action__message'); @@ -304,161 +471,89 @@ describe('QuickPayRoute.vue', () => { }); }); - describe('action events', () => { - describe('redirections', () => { - const originalWindowLocation = global.window.location; - let windowReplaceSpy: jest.Mock; - - beforeAll(() => { - windowReplaceSpy = jest.fn(); - Reflect.deleteProperty(global.window, 'location'); - global.window.location = { - ...originalWindowLocation, - replace: windowReplaceSpy, - }; - }); + describe('redirections after action has finished', () => { + const originalWindowLocation = global.window.location; + let windowReplaceSpy: jest.Mock; + + beforeAll(() => { + windowReplaceSpy = jest.fn(); + Reflect.deleteProperty(global.window, 'location'); + global.window.location = { + ...originalWindowLocation, + replace: windowReplaceSpy, + }; + }); - beforeEach(() => { - jest.resetAllMocks(); - }); + beforeEach(() => { + jest.resetAllMocks(); + }); - afterAll(() => { - global.window.location = originalWindowLocation; - }); + afterAll(() => { + global.window.location = originalWindowLocation; + }); - test('includes payment information parameters to specified target on completed action', async () => { - const wrapper = createWrapper({ - redirectTo: 'https://redirect.target', - paymentIdentifier: '501', - accountAddress: '0x3D389c9B67cA85F1de2EbeF648C54d740365c366', - }); - const action = wrapper.get('.quick-pay__action__component'); - - action.vm.$emit('completed'); - await wrapper.vm.$nextTick(); - - expect(windowReplaceSpy).toHaveBeenCalledTimes(1); - expect(windowReplaceSpy).toHaveBeenLastCalledWith( - 'https://redirect.target?identifier=501&payerAddress=0x3D389c9B67cA85F1de2EbeF648C54d740365c366', - ); + test('includes payment information parameters to specified target on completed action', async () => { + const wrapper = createWrapper({ + redirectTo: 'https://redirect.target', + paymentIdentifier: '501', + accountAddress: '0x3D389c9B67cA85F1de2EbeF648C54d740365c366', }); + const action = wrapper.get('.quick-pay__action__component'); - test('shuts down raiden before redirect', async () => { - const disconnect = jest.fn(); - const wrapper = createWrapper({ disconnect, redirectTo: 'https://redirect.target' }); - const action = wrapper.get('.quick-pay__action__component'); + action.vm.$emit('completed'); + await wrapper.vm.$nextTick(); - action.vm.$emit('completed'); - await wrapper.vm.$nextTick(); + expect(windowReplaceSpy).toHaveBeenCalledTimes(1); + expect(windowReplaceSpy).toHaveBeenLastCalledWith( + 'https://redirect.target?identifier=501&payerAddress=0x3D389c9B67cA85F1de2EbeF648C54d740365c366', + ); + }); - expect(disconnect).toHaveBeenCalledTimes(1); - }); + test('shuts down raiden before redirect', async () => { + const disconnect = jest.fn(); + const wrapper = createWrapper({ disconnect, redirectTo: 'https://redirect.target' }); + const action = wrapper.get('.quick-pay__action__component'); - test('redirects to transfer screen of given token if no redirect target given', async () => { - const push = jest.fn(); - const wrapper = createWrapper({ - push, - redirectTo: '', - tokenAddress: '0x59105441977ecD9d805A4f5b060E34676F50F806', - }); - const action = wrapper.get('.quick-pay__action__component'); - - action.vm.$emit('completed'); - await wrapper.vm.$nextTick(); - - expect(push).toHaveBeenCalledTimes(1); - expect(push).toHaveBeenLastCalledWith({ - name: RouteNames.TRANSFER, - params: { token: '0x59105441977ecD9d805A4f5b060E34676F50F806' }, - }); - }); + action.vm.$emit('completed'); + await wrapper.vm.$nextTick(); - test('includes failed state if there is an error and the user dismisses the dialog', async () => { - const wrapper = createWrapper({ - redirectTo: 'https://redirect.target', - paymentIdentifier: '501', - }); - const action = wrapper.get('.quick-pay__action__component'); - - action.vm.$emit('failed', new Error()); - action.vm.$emit('dialogClosed'); - await wrapper.vm.$nextTick(); - - expect(windowReplaceSpy).toHaveBeenCalledTimes(1); - expect(windowReplaceSpy).toHaveBeenLastCalledWith( - 'https://redirect.target?identifier=501&failed=true', - ); - }); + expect(disconnect).toHaveBeenCalledTimes(1); }); - describe('failed action handling', () => { - test('remains same action when error is not recoverable', async () => { - const wrapper = createWrapper(optionsForStraightTransfer); - const firstAction = wrapper.getComponent(TransferAction); - - const error = new Error('The requested target is offline.'); - firstAction.vm.$emit('failed', error); - await wrapper.vm.$nextTick(); - - const secondAction = wrapper.get('.quick-pay__action__component'); - expect(secondAction.vm).toEqual(firstAction.vm); + test('redirects to transfer screen of given token if no redirect target given', async () => { + const push = jest.fn(); + const wrapper = createWrapper({ + push, + redirectTo: '', + tokenAddress: '0x59105441977ecD9d805A4f5b060E34676F50F806', }); + const action = wrapper.get('.quick-pay__action__component'); - describe('can recover from no valid routes found error', () => { - test('switches to channel deposit and transfer action if there is a direct channel', async () => { - const wrapper = createWrapper(optionsForChannelDepositAndTransferWithTransferFirst); - const transferAction = wrapper.getComponent(TransferAction); - - const error = new Error('No valid routes found.'); - transferAction.vm.$emit('failed', error); - await wrapper.vm.$nextTick(); + action.vm.$emit('completed'); + await wrapper.vm.$nextTick(); - const channelDepositAndTransferAction = wrapper.findComponent( - ChannelDepositAndTransferAction, - ); - expect(channelDepositAndTransferAction.exists()).toBeTruthy(); - }); - - test('switches to channel open and transfer action if there is no direct channel', async () => { - const wrapper = createWrapper(optionsForChannelOpenAndTransferWithTransferFirst); - const transferAction = wrapper.getComponent(TransferAction); - - const error = new Error('No valid routes found.'); - transferAction.vm.$emit('failed', error); - await wrapper.vm.$nextTick(); - - const channelOpenAndTransferAction = wrapper.findComponent(ChannelOpenAndTransferAction); - expect(channelOpenAndTransferAction.exists()).toBeTruthy(); - }); + expect(push).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenLastCalledWith({ + name: RouteNames.TRANSFER, + params: { token: '0x59105441977ecD9d805A4f5b060E34676F50F806' }, }); + }); - describe('can recover from no route between nodes found error', () => { - test('switches to channel deposit and transfer action if there is a direct channel', async () => { - const wrapper = createWrapper(optionsForChannelDepositAndTransferWithTransferFirst); - const transferAction = wrapper.getComponent(TransferAction); - - const error = new Error('No route between nodes found.'); - transferAction.vm.$emit('failed', error); - await wrapper.vm.$nextTick(); - - const channelDepositAndTransferAction = wrapper.findComponent( - ChannelDepositAndTransferAction, - ); - expect(channelDepositAndTransferAction.exists()).toBeTruthy(); - }); - - test('switches to channel open and transfer action if there is no direct channel', async () => { - const wrapper = createWrapper(optionsForChannelOpenAndTransferWithTransferFirst); - const transferAction = wrapper.getComponent(TransferAction); + test('includes failed state if there is an error and the user dismisses the dialog', async () => { + const wrapper = createWrapper({ + redirectTo: 'https://redirect.target', + paymentIdentifier: '501', + }); + const action = wrapper.get('.quick-pay__action__component'); - const error = new Error('No route between nodes found.'); - transferAction.vm.$emit('failed', error); - await wrapper.vm.$nextTick(); + action.vm.$emit('failed', new Error()); + action.vm.$emit('dialogClosed'); + await wrapper.vm.$nextTick(); - const channelOpenAndTransferAction = wrapper.findComponent(ChannelOpenAndTransferAction); - expect(channelOpenAndTransferAction.exists()).toBeTruthy(); - }); - }); + expect(windowReplaceSpy).toHaveBeenCalledTimes(1); + expect(windowReplaceSpy).toHaveBeenLastCalledWith( + 'https://redirect.target?identifier=501&failed=true', + ); }); }); }); From c61ddfdf67efb234127aa42a2e420eaa0b6ed6ce Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Wed, 13 Oct 2021 12:12:47 +0200 Subject: [PATCH 7/9] dapp: add exception to no connected token There is a special routing guard that checks if the user has any connected token. If not, it will automatically forces him to route to the screen where he can connect. There are a few exceptions to this like the account settings. This does also prevent a new user to use the quick pay feature right away. But as the quick pay works also for new users, this must be a new exception to the guard. --- raiden-dapp/src/router/guards/no-connected-token.ts | 3 ++- .../tests/unit/router/guards/no-connected-token.spec.ts | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/raiden-dapp/src/router/guards/no-connected-token.ts b/raiden-dapp/src/router/guards/no-connected-token.ts index f3b7a85f0e..0640808d44 100644 --- a/raiden-dapp/src/router/guards/no-connected-token.ts +++ b/raiden-dapp/src/router/guards/no-connected-token.ts @@ -20,10 +20,11 @@ export const redirectIfNoConnectedToken: NavigationGuardChild = (to, store) => { const { tokensWithChannels } = store.getters; const noTokenWithChannel = Object.keys(tokensWithChannels).length === 0; const routingToNoTokens = to.name === RouteNames.NO_CONNECTED_TOKEN; + const routingToQuickPayRoute = to.name === RouteNames.QUICK_PAY; const routingToAccountRoute = accountRoutes.includes(to.name ?? ''); const routingToConnectTokenRoute = connectTokenRoutes.includes(to.name ?? ''); - if (routingToAccountRoute) return undefined; + if (routingToAccountRoute || routingToQuickPayRoute) return undefined; if (routingToConnectTokenRoute) return null; diff --git a/raiden-dapp/tests/unit/router/guards/no-connected-token.spec.ts b/raiden-dapp/tests/unit/router/guards/no-connected-token.spec.ts index 8e783f4887..e7dd519e8f 100644 --- a/raiden-dapp/tests/unit/router/guards/no-connected-token.spec.ts +++ b/raiden-dapp/tests/unit/router/guards/no-connected-token.spec.ts @@ -21,6 +21,7 @@ const { [RouteNames.SELECT_TOKEN]: selectTokenRoute, [RouteNames.SELECT_HUB]: selectHubRoute, [RouteNames.OPEN_CHANNEL]: openChannelRoute, + [RouteNames.QUICK_PAY]: quickPayRoute, ...protectedRoutes } = transformRouteConfigsToRoutes(); @@ -97,4 +98,10 @@ describe('redirectIfNoConnectedToken()', () => { expect(redirectIfNoConnectedToken(route, store)).toBeUndefined(); }); }); + + test('do nothing if navigating to quick pay route', () => { + const store = createStore(); + + expect(redirectIfNoConnectedToken(quickPayRoute, store)).toBeUndefined(); + }); }); From 630ebee2fbdeaf36ca3eaadb1f8b21de0a74d723 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Wed, 27 Oct 2021 11:09:58 +0200 Subject: [PATCH 8/9] dapp: add tests for channel action form autofocus The autofocus features of the channel action form component were not tested yet. This has become especially important since it is possible to disable the automatic focus. --- .../channels/channel-action-form.spec.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/raiden-dapp/tests/unit/components/channels/channel-action-form.spec.ts b/raiden-dapp/tests/unit/components/channels/channel-action-form.spec.ts index f87c3283d1..04111a6e4c 100644 --- a/raiden-dapp/tests/unit/components/channels/channel-action-form.spec.ts +++ b/raiden-dapp/tests/unit/components/channels/channel-action-form.spec.ts @@ -39,6 +39,7 @@ async function createWrapper(options?: { minimumTokenAmount?: BigNumber; confirmButtonLabel?: string; stickyButton?: boolean; + autofocusDisabled?: boolean; runAction?: () => void; inputsAreValid?: boolean; fetchAndUpdateTokenData?: () => void; @@ -75,6 +76,7 @@ async function createWrapper(options?: { minimumTokenAmount: options?.minimumTokenAmount, confirmButtonLabel: options?.confirmButtonLabel ?? 'confirm label', stickyButton: options?.stickyButton, + autofocusDisabled: options?.autofocusDisabled, runAction: options?.runAction ?? jest.fn(), }, stubs: ['v-form'], @@ -313,6 +315,48 @@ describe('ChannelActionForm.vue', () => { }); }); + describe('autofocus', () => { + test('focuses the partner address input if it is editable', async () => { + const wrapper = await createWrapper({ partnerAddressEditable: true }); + const input = wrapper.get( + '.channel-action-form__partner-address.channel-action-form__editable-input', + ); + + expect(input.attributes('autofocus')).toBeTruthy(); + }); + + test('focuses the token amount input if it is editable, but partner address is not editable', async () => { + const wrapper = await createWrapper({ + tokenAmountEditable: true, + partnerAddressEditable: false, + }); + const input = wrapper.get( + '.channel-action-form__token-amount.channel-action-form__editable-input', + ); + + expect(input.attributes('autofocus')).toBeTruthy(); + }); + + test('focuses no input if autofocus is disabled, even if inputs are editable', async () => { + const wrapper = await createWrapper({ + autofocusDisabled: true, + tokenAmountEditable: true, + partnerAddressEditable: true, + }); + + const partnerAddressInput = wrapper.get( + '.channel-action-form__partner-address.channel-action-form__editable-input', + ); + + const tokenAmountInput = wrapper.get( + '.channel-action-form__token-amount.channel-action-form__editable-input', + ); + + expect(partnerAddressInput.attributes('autofocus')).toBeFalsy(); + expect(tokenAmountInput.attributes('autofocus')).toBeFalsy(); + }); + }); + describe('emit input update event', () => { test('emits event', async () => { const wrapper = await createWrapper({ From cb460320b7ff86e1886f1fa905619caca37db496 Mon Sep 17 00:00:00 2001 From: Thore Strassburg Date: Thu, 28 Oct 2021 18:35:43 +0200 Subject: [PATCH 9/9] dapp: add new warning message to quick pay route When the pathfinding servie is too expensive, the price was shown in red and the button to fetch a route was disabled. This leaves the user with no information why. To improve this, instead of the button a warning message gets displayed. This bascially makes the former 'no routes' approach more generic an allows to use it for other messages as well. The price will not be shown in red color anymore as this would be too much red then. --- raiden-dapp/src/locales/en.json | 3 +- raiden-dapp/src/views/QuickPayRoute.vue | 18 ++++++++++-- .../tests/unit/views/quick-pay-route.spec.ts | 28 +++++++------------ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/raiden-dapp/src/locales/en.json b/raiden-dapp/src/locales/en.json index dc4fd0262a..8f1f42b48f 100644 --- a/raiden-dapp/src/locales/en.json +++ b/raiden-dapp/src/locales/en.json @@ -893,7 +893,8 @@ "token-amount": "Payment amount", "pathfinding-service-price": "Mediation possible, price for requesting route", "fetch-route-trigger": "Pay to find route", - "fetch-route-error": "No route available" + "fetch-route-error-too-expensive": "UDC balance too low", + "fetch-route-error-no-route": "No route available" }, "action-titles": { "channel-deposit": "Channel Deposit", diff --git a/raiden-dapp/src/views/QuickPayRoute.vue b/raiden-dapp/src/views/QuickPayRoute.vue index 9ff718dd9f..2347ce5672 100644 --- a/raiden-dapp/src/views/QuickPayRoute.vue +++ b/raiden-dapp/src/views/QuickPayRoute.vue @@ -29,7 +29,6 @@ :label="$t('quick-pay.transfer-information.pathfinding-service-price')" :amount="pathfindingServicePrice" :token="serviceToken" - :warning="pathfindingServiceTooExpensive" exact-amount full-width > @@ -56,10 +55,10 @@ - {{ $t('quick-pay.transfer-information.fetch-route-error') }} + {{ fetchMediationRouteError }}
@@ -294,9 +293,22 @@ export default class QuickPayRoute extends Mixins(NavigationMixin) { } } + get showFetchMediationRouteError(): boolean { + return this.fetchMediationRouteFailed || this.pathfindingServiceTooExpensive; + } + + get fetchMediationRouteError(): string | undefined { + if (this.pathfindingServiceTooExpensive) { + return this.$t('quick-pay.transfer-information.fetch-route-error-too-expensive') as string; + } else if (this.fetchMediationRouteFailed) { + return this.$t('quick-pay.transfer-information.fetch-route-error-no-route') as string; + } + } + get showFetchMediationRouteButton(): boolean { return ( !this.mediationRoute && + !this.pathfindingServiceTooExpensive && !this.fetchMediationRouteInProgress && !this.fetchMediationRouteFailed ); diff --git a/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts b/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts index 2bf845ab69..04a12e43f3 100644 --- a/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts +++ b/raiden-dapp/tests/unit/views/quick-pay-route.spec.ts @@ -312,7 +312,7 @@ describe('QuickPayRoute.vue', () => { expect(pathfindingServicePrice.html()).toContain('1'); }); - test('marks pathfinding service price with a warning when price is higher than user deposit capacity', async () => { + test('shows warning message when pathfinding service price is higher than user deposit capacity', async () => { const getUDCCapacity = jest.fn().mockResolvedValue(constants.One); const fetchServices = jest.fn().mockResolvedValue([{ price: constants.Two }]); const wrapper = createWrapper({ @@ -320,26 +320,15 @@ describe('QuickPayRoute.vue', () => { fetchServices, ...optionsForMediatedTransfer, }); - const pathfindingServicePrice = wrapper.find( - '.quick-pay__transfer-information__mediation__pathfinding-service-price', - ); await flushPromises(); // Fetch services must complete. - expect(pathfindingServicePrice.html()).toContain('warning'); - }); - - test('disables get route button when pathfinding service price is higher than user deposit capacity', () => { - const getUDCCapacity = jest.fn().mockResolvedValue(constants.One); - const fetchServices = jest.fn().mockResolvedValue([{ price: constants.Two }]); - const wrapper = createWrapper({ - getUDCCapacity, - fetchServices, - ...optionsForMediatedTransfer, - }); - const getRouteButton = wrapper.get('.quick-pay__transfer-information__mediation__button'); + const mediationError = wrapper.find('.quick-pay__transfer-information__mediation__error'); - expect(getRouteButton.attributes('disabled')).toBeTruthy(); + expect(mediationError.exists()).toBeTruthy(); + expect(mediationError.text()).toBe( + 'quick-pay.transfer-information.fetch-route-error-too-expensive', + ); }); test('fetches routes from cheapest pathfinding service when user click button', async () => { @@ -372,8 +361,11 @@ describe('QuickPayRoute.vue', () => { await clickGetRoutesButton(wrapper); const mediationError = wrapper.find('.quick-pay__transfer-information__mediation__error'); + expect(mediationError.exists()).toBeTruthy(); - expect(mediationError.text()).toBe('quick-pay.transfer-information.fetch-route-error'); + expect(mediationError.text()).toBe( + 'quick-pay.transfer-information.fetch-route-error-no-route', + ); }); test('selects the first returned route as cheapest', async () => {