Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix payment account deserialize issue (e.g. CHASE_QUICK_PAY) #5614

Merged
merged 1 commit into from Jul 13, 2021
Merged

Fix payment account deserialize issue (e.g. CHASE_QUICK_PAY) #5614

merged 1 commit into from Jul 13, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jul 10, 2021

Fixes #5613

When loading payment accounts which have been deprecated, an exception is thrown which is not handled. This causes the entire set of payment accounts to be lost.

To fix this we check for an exception when deserializing a payment account, and log a warning with the details. The erroneous payment account will therefore be dropped from the list, which is what is expected.

I tested using the following procedure on both regtest & mainnet Bisq.

Testing:

  • Create a new Bisq account using v1.6.5 (or earlier).
  • Add Amazon Giftcard and Chase QuickPay accounts for the user.
  • Close Bisq, log in again using the same version and check that both accounts are listed.
  • Close Bisq, upgrade to (v1.7.0 + this PR) and run it.
  • Check that the Chase QuickPay account is no longer in the list, but the Amazon Giftcard account is present.

Check that a log warning is written, like this:

Jul-10 10:12:05.316 [PersistenceManager-read-UserPayload] WARN  b.core.payment.PaymentAccount: Could not load account: CHASE_QUICK_PAY, exception: java.lang.RuntimeException: Not supported PaymentMethod: PaymentMethod(id=N/A, maxTradePeriod=0, maxTradeLimit=0)

(It will actually be logged 2 times since UserPreferences deserializes 2 times).


@ripcurlx ripcurlx added this to the v1.7.1 milestone Jul 12, 2021
@@ -132,6 +132,7 @@ public static UserPayload fromProto(protobuf.UserPayload proto, CoreProtoResolve
ProtoUtil.stringOrNullFromProto(proto.getAccountId()),
proto.getPaymentAccountsList().isEmpty() ? new HashSet<>() : new HashSet<>(proto.getPaymentAccountsList().stream()
.map(e -> PaymentAccount.fromProto(e, coreProtoResolver))
.filter(e -> e != null)
Copy link
Contributor

@ripcurlx ripcurlx Jul 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.filter(e -> e != null)
.filter(Objects::nonNull)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT ☝️

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! change applied.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx merged commit 23a3c63 into bisq-network:master Jul 13, 2021
@ghost ghost mentioned this pull request Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Payment accounts deleted on upgrade to 1.7.0
1 participant