Add holder name for new Faster Payments accounts #4046

stejbac · 2020-03-11T11:42:15Z

Use excludeFromJsonDataMap to add an optional owner full name property to Faster Payments (GBP) accounts, without affecting the trade JSON contract (for backwards compatibility). This is to fix #3976, that some banks have started checking the recipient name.

This uses the new map key: public static final String HOLDER_NAME = "holderName";

Make the full name a non-optional field for new accounts, but display the details for old Faster Payments accounts in exactly the same way as before, only showing the extra field in the info popup, account and trade step views when it is present.

Also update the now misleading payment instructions in the buyer popup. With the change, it shows as:

Also add a popup to advise the user to recreate their Faster Payments account.

Make an info popup open in the take/create offer view, upon choosing to take or make a new offer, to instruct the user to recreate their old Faster Payments account with an owner full name (and preserved salt). Also show the popup upon manual selection of any old (i.e. missing full name) Faster Payments account from the trading account combo box, analogously to the ClearXchange (Zelle) warning popup logic.

I've also copied/overwritten the two new/updated English resource bundle strings into the non-English resource files, so that the translations stay in sync with the updated information. (As it is a UK-only payment method, the non-English translations are probably not all that important anyway.)

--

Finally, I removed some redundant null-handling of the excludeFromJsonDataMap field
and constructor parameters. This touched all the PaymentAccountPayload subclasses, which was quite a lot of files (although fairly trivial changes).

Since this map is final and every PaymentAccountPayload constructor initialises it to something nonnull, the @nullable field annotation is redundant, so remove it. Further simplify the code by passing an empty map into the constructor directly from each subclass constructor, rather than mapping empties to nulls from each 'proto.getExcludeFromJsonDataMap()' result, then mapping nulls back to empties in the base constructor. This is safe, since getExcludeFromJsonDataMap() always returns a nonnull map. Finally, tidy the code slightly by replacing 'Charset.forName("UTF-8")' with StandardCharsets.UTF_8 in each PaymentAccountPayload subclass.

Use excludeFromJsonDataMap to add an optional owner full name property to Faster Payments (GBP) accounts, without affecting the trade JSON contract (for backwards compatibility). This is to fix bisq-network#3976, that some banks have started checking the recipient name. Make the name a non-optional field for _new_ accounts, but display the details for old Faster Payments accounts in exactly the same way as before, only showing the extra field in the info popup, account and trade step views when it is present. Also update the now misleading payment instructions in the buyer popup. TODO: Update translations (which probably aren't needed anyway).

Open an info popup in the take/create offer view, upon choosing to take or make a new offer, to instruct the user to recreate their old Faster Payments account with an owner full name (and preserved salt). Also show the popup upon manual selection of any old (i.e. missing full name) Faster Payments account from the trading account combo box, analogously to the ClearXchange (Zelle) warning popup logic. (Also eliminate slight differences between the private 'maybeShow[ClearXchange|FasterPayments]Warning' methods in TakeOfferView and MutableOfferView, to make the code easier to deduplicate in future.)

ripcurlx · 2020-03-26T14:53:59Z

@m52go Could you please review the wording?
I'll test it on Regtest after reviewing the code.

ripcurlx

NACK - please see my comment regarding the buyers UI during the trade process.
Also please only commit changes to the English source file all other files will be overwritten automatically by loading translations from Transifex before the next release.

ACK from a functionality perspective. Tested old and new Faster Payment Accounts in trades.

desktop/src/main/java/bisq/desktop/components/paymentmethods/FasterPaymentsForm.java

m52go

Existing text was already good. I just made it more concise, since there's already so much text in the pop-ups.

I did not suggest edits across languages, in case anyone would like to propose edits to these edits. If these edits are acceptable, I can do so.

m52go · 2020-03-27T06:12:06Z

core/src/main/resources/i18n/displayStrings.properties

+  The UK sort code and account number used to be sufficient for a Faster Payment transfer, but since then some banks \
+  have started verifying the receiver's name. For this reason, accounts created in old Bisq clients do not provide the \
+  trade partner with an owner full name. Please use trade chat to avoid any issues in such cases.


Suggested change

The UK sort code and account number used to be sufficient for a Faster Payment transfer, but since then some banks \

have started verifying the receiver's name. For this reason, accounts created in old Bisq clients do not provide the \

trade partner with an owner full name. Please use trade chat to avoid any issues in such cases.

Faster Payment accounts created in old Bisq clients do not provide the receiver's name, \

so please use trade chat to obtain it (if needed).

OK, I'll apply that change. I think the text

Some banks might require the receiver's name ...

should possibly be changed to

Some banks might verify the receiver's name ...

with the suggested change, to emphasise that the issue is that they've started checking it.

(Although my bank still accepts dummy names - I'm not sure how much longer, though.)

m52go · 2020-03-27T06:12:11Z

core/src/main/resources/i18n/displayStrings.properties

+payment.fasterPayments.newRequirements.info=When using Faster Payments some banks have started verifying the receiver''s \
+  full name. Your current Faster Payments account only specifies a UK bank account number and sort code. This used to be \
+  sufficient, but may not be now for some sender''s banks. New Faster Payments accounts in Bisq now include an owner \
+  full name. Please consider recreating your account in Bisq to provide future {0} buyers with a recipient name.\n\n\
+  When recreating a Faster Payments account, take care to copy the account age verification salt from your old account \
+  and use the same sort code and account number. (The owner full name or account name need not be the same, however.) \
+  This preserves the public hash, so that the new account need not be aged or signed from scratch.


Suggested change

payment.fasterPayments.newRequirements.info=When using Faster Payments some banks have started verifying the receiver''s \

full name. Your current Faster Payments account only specifies a UK bank account number and sort code. This used to be \

sufficient, but may not be now for some sender''s banks. New Faster Payments accounts in Bisq now include an owner \

full name. Please consider recreating your account in Bisq to provide future {0} buyers with a recipient name.\n\n\

When recreating a Faster Payments account, take care to copy the account age verification salt from your old account \

and use the same sort code and account number. (The owner full name or account name need not be the same, however.) \

This preserves the public hash, so that the new account need not be aged or signed from scratch.

payment.fasterPayments.newRequirements.info=Some banks have started verifying the receiver''s \

full name for Faster Payments transfers. Your current Faster Payments account does not specify a full name.\n\n \

Please consider recreating your Faster Payments account in Bisq to provide future {0} buyers with a full name.\n\n\

When you recreate the account, make sure to copy the precise sort code, account number, and account age \

verification salt values from your old account to your new account. This will ensure your existing account's age and \

signing status are preserved.

OK, I'll apply the suggested simplification.

stejbac · 2020-03-27T07:41:34Z

I've just done a force push to drop the last commit which made the unwanted translation changes, and replaced it with a commit to apply @ripcurlx's suggestion.

I just noticed the text change suggestions - will push those shortly.

Display the account number on the same row as the sort code in the trade step view, to prevent scrolling with the extra name field (as suggested in the code review). (This also affects the layout of old accounts without the extra field.) Also apply the suggested popup text simplifications.

stejbac · 2020-03-27T08:11:36Z

I've just force pushed again to include the popup text simplifications in the last commit.

stejbac added 2 commits March 7, 2020 20:08

stejbac mentioned this pull request Mar 11, 2020

For Cycle 11 bisq-network/compensation#509

Closed

stejbac marked this pull request as ready for review March 25, 2020 16:15

ripcurlx suggested changes Mar 26, 2020

View reviewed changes

desktop/src/main/java/bisq/desktop/components/paymentmethods/FasterPaymentsForm.java Show resolved Hide resolved

ripcurlx added the waiting for author label Mar 26, 2020

m52go reviewed Mar 27, 2020

View reviewed changes

stejbac force-pushed the add-faster-payments-holder-name branch from 0ec168b to e2f5d7f Compare March 27, 2020 07:15

stejbac force-pushed the add-faster-payments-holder-name branch from e2f5d7f to d84130f Compare March 27, 2020 08:10

ripcurlx approved these changes Mar 27, 2020

View reviewed changes

ripcurlx merged commit b321cd9 into bisq-network:master Mar 27, 2020

ripcurlx added is:priority PR or issue marked with this label is up for compensation and removed waiting for author labels Mar 27, 2020

ripcurlx added this to the v1.3.0 milestone Apr 1, 2020

stejbac mentioned this pull request Apr 13, 2020

For Cycle 12 bisq-network/compensation#534

Closed

ripcurlx mentioned this pull request Apr 15, 2020

For Cycle 12 bisq-network/compensation#542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add holder name for new Faster Payments accounts #4046

Add holder name for new Faster Payments accounts #4046

stejbac commented Mar 11, 2020 •

edited

Loading

ripcurlx commented Mar 26, 2020

ripcurlx left a comment

m52go left a comment

m52go Mar 27, 2020

stejbac Mar 27, 2020

m52go Mar 27, 2020

stejbac Mar 27, 2020

stejbac commented Mar 27, 2020

stejbac commented Mar 27, 2020

Add holder name for new Faster Payments accounts #4046

Add holder name for new Faster Payments accounts #4046

Conversation

stejbac commented Mar 11, 2020 • edited Loading

ripcurlx commented Mar 26, 2020

ripcurlx left a comment

Choose a reason for hiding this comment

m52go left a comment

Choose a reason for hiding this comment

m52go Mar 27, 2020

Choose a reason for hiding this comment

stejbac Mar 27, 2020

Choose a reason for hiding this comment

m52go Mar 27, 2020

Choose a reason for hiding this comment

stejbac Mar 27, 2020

Choose a reason for hiding this comment

stejbac commented Mar 27, 2020

stejbac commented Mar 27, 2020

stejbac commented Mar 11, 2020 •

edited

Loading