Clear payment account payload info from closed trades. #6001

ghost · 2022-01-28T06:29:46Z

Removes sensitive payment account information and trader chat messages from closed trades and disputes.
User can specify in preferences how long to wait before the information is removed, defaults to 20 days.
The feature is enabled after the user clicks I UNDERSTAND shown in the dialog below.

Screenshots:

Settings:

The highlighted payment account details get removed:

Trade contract showing payment account information removed.

Invocation:

The list of closed trades need to be occasionally evaluated to see if any are eligible for having their sensitive data cleared. This is primarily done at app initialization, but also is done periodically whenever a trade is added to the list shown in portfolio history. That is to handle the case of clients which do not restart daily.

Likewise, the list of dispute tickets is evaluated at initialization and also whenever a dispute ticket closed message is processed.

Whenever the evaluation is done, the following log messages are written:

INFO  b.c.t.ClosedTradableManager: checking closed trades eligibility for having sensitive data cleared 
INFO  b.c.s.dispute.DisputeManager: ArbitrationManager checking closed disputes eligibility for having sensitive data cleared 
INFO  b.c.s.dispute.DisputeManager: MediationManager checking closed disputes eligibility for having sensitive data cleared 
INFO  b.c.s.dispute.DisputeManager: RefundManager checking closed disputes eligibility for having sensitive data cleared

If any actions are taken, they are logged, for example:

INFO  b.c.trade.model.bisq_v1.Trade: cleared sensitive data from contract;contractAsJson; of trade rfvUYPIx 
INFO  b.core.support.dispute.Dispute: cleared sensitive data from chat messages; of dispute for trade IOAMZBWH

pazza83 · 2022-01-29T00:23:36Z

Hi @jmacxx thanks for sorting this out.

just checking trades in mediation and arbitration will not count as completed trades?

Assuming mediated trades are completed once payout occurs.

What about arbitrated trades? What point do these become completed after payment is made to donation address?

pazza83 · 2022-01-29T00:49:38Z

Also just wondering how this would affect how a duplicate offer creation when a previous offer has had their payload info removed?

ghost · 2022-01-29T03:39:10Z

Answering your questions @pazza83 :

just checking trades in mediation and arbitration will not count as completed trades?
Assuming mediated trades are completed once payout occurs.

Correct, only trades that are Closed are considered.

What about arbitrated trades? What point do these become completed after payment is made to donation address?

Same, they transition to Closed once the arbitration ticket has been closed.

Also just wondering how this would affect how a duplicate offer creation when a previous offer has had their payload info removed?

No effect as offers do not contain account payloads or contracts, only trades do.

There's some work remaining to sanitize account payloads from closed dispute tickets which looks like the implementation will need refactoring, so moving this back into draft for a couple of days.

pazza83 · 2022-01-30T21:37:32Z

No effect as offers do not contain account payloads or contracts, only trades do.

Just so I am clear, I am assuming the payload is link to the local payment account (trading account) used to fund the trade?

When an offer moves to 'history' currently it still has the information associated to what payment account (trading account) was used to fund the trade associated to it. Will that still be the case once the payload has been cleared?

What I am getting at is once the trade contract details have been cleared, a local user should still not be able just to right click duplicate on a historic trade to see what local payment account (trading account) was used to fund the trade.

ghost · 2022-01-31T14:21:06Z

I think we still want to maintain the ability to duplicate an offer as it is quite frequently used.

Severing past offers' makerPaymentAccountId link to the account used would make duplicating an offer more of a multi-step process similar to offer entry, and is not what this PR is about.

What is being done here is removing counterparties personal information from each Bisq instance. Yes it also removes copies of your own personal information from trades, which is good too.

ripcurlx

utACK - Code changes are looking fine 👍

Was getting an NPE with closed trades older than v1.7.0. makerPaymentMethodId was new at that point (null for older trades). makerPaymentAccountPayload can be null for trades where we have removed account info for privacy reasons (see #6001). The payment method ID can always be obtained from offerPayload.

ripcurlx · 2022-02-24T10:28:15Z

@jmacxx I think we should add an empty state to the chat popup if all messages have been deleted. Otherwise it looks like an error if someone doesn't remember this privacy feature. WDYT?

ripcurlx · 2022-02-24T10:30:57Z

@jmacxx I think we should add an empty state to the chat popup if all messages have been deleted. Otherwise it looks like an error if someone doesn't remember this privacy feature. WDYT?

The additional weird thing about this is that it was a mediation case from today which seems to have lost all chat messages. It only happened on this side of the dispute. The other side still has the messages. Also the arbitrator still sees the messages. Application setting is on the default 20 days.

ripcurlx · 2022-02-24T10:43:05Z

I do see the expected logs e.g. for 30534

Feb.-09 11:12:16.044 [JavaFX Application Thread] INFO  b.c.s.d.Dispute: cleared sensitive data from contract;contractAsJson;chat messages; of dispute for trade 30534

But not for the recent trade 4900341, so I'm not sure how the chat messages were deleted.

ripcurlx · 2022-02-24T10:43:46Z

Contract details are still available.

ghost · 2022-02-24T18:04:42Z

But not for the recent trade 4900341, so I'm not sure how the chat messages were deleted.

I have not come across that issue before. I'm trying to reproduce it, so far not been able to.

I wonder if anything can be seen in your log when it removed the chat messages from 4900341? I would expect a log message about it.

Fixes #6001 (comment)

ripcurlx · 2022-02-25T15:17:45Z

core/src/main/java/bisq/core/support/dispute/DisputeManager.java

+                .filter(e -> e.isClosed())
+                .filter(e -> e.getTradeDate().toInstant().isBefore(safeDate))


Maybe you want to use:

.filter(Dispute::isClosed) .filter(e -> e.getOpeningDate().toInstant().isBefore(safeDate))

instead?

ripcurlx · 2022-02-25T15:18:09Z

But not for the recent trade 4900341, so I'm not sure how the chat messages were deleted.

I have not come across that issue before. I'm trying to reproduce it, so far not been able to.

I wonder if anything can be seen in your log when it removed the chat messages from 4900341? I would expect a log message about it.

ah - I think I know what happened. I might have missed the log, but the trade I've put into mediation was already quite old. So at the moment when the dispute was closed, all chat messages were cleared as you are using the trade date instead of the dispute opening date. See https://github.com/bisq-network/bisq/pull/6001/files#r814853854

pazza83 · 2022-03-12T23:02:09Z

Tested this and it works well. Many thanks.

ghost marked this pull request as draft January 29, 2022 03:40

Clear account payload info & chats from closed trades & disputes.

57821b7

ghost marked this pull request as ready for review January 31, 2022 21:14

ripcurlx added this to the v1.8.3 milestone Feb 1, 2022

ripcurlx approved these changes Feb 1, 2022

View reviewed changes

ripcurlx merged commit 8a846c4 into bisq-network:master Feb 1, 2022

pazza83 mentioned this pull request Feb 10, 2022

Move info from 'Peer Info' to 'Trade Information Screen' and 'Detail Data' #6036

Closed

This was referenced Feb 21, 2022

For Cycle 33 bisq-network/compensation#1009

Closed

[1.8.3 fix] Get payment method ID from offerPayload #6069

Merged

ripcurlx mentioned this pull request Feb 23, 2022

For Cycle 33 bisq-network/compensation#1016

Closed

ghost mentioned this pull request Feb 24, 2022

[1.8.3] Keep first dispute chat msg when clearing sensitive data. #6076

Merged

ripcurlx pushed a commit that referenced this pull request Feb 25, 2022

Keep first dispute chat msg when clearing sensitive data.

3bc0964

Fixes #6001 (comment)

ripcurlx reviewed Feb 25, 2022

View reviewed changes

ghost mentioned this pull request Feb 25, 2022

[1.8.3] Clear sensitive dispute data based on dispute opening date. #6078

Merged

ghost mentioned this pull request Apr 4, 2022

Check failed trades for sensitive data purge. #6132

Merged

This was referenced Jul 6, 2022

Sensitive data still visible in ClosedTrades file #6278

Closed

Clear sensitive data from trade ProcessModel #6283

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear payment account payload info from closed trades. #6001

Clear payment account payload info from closed trades. #6001

ghost commented Jan 28, 2022 •

edited by ghost

Loading

pazza83 commented Jan 29, 2022

pazza83 commented Jan 29, 2022 •

edited

Loading

ghost commented Jan 29, 2022

pazza83 commented Jan 30, 2022

ghost commented Jan 31, 2022

ripcurlx left a comment

ripcurlx commented Feb 24, 2022 •

edited

Loading

ripcurlx commented Feb 24, 2022

ripcurlx commented Feb 24, 2022

ripcurlx commented Feb 24, 2022

ghost commented Feb 24, 2022

ripcurlx Feb 25, 2022

ripcurlx commented Feb 25, 2022

pazza83 commented Mar 12, 2022

		.filter(e -> e.isClosed())
		.filter(e -> e.getTradeDate().toInstant().isBefore(safeDate))

Clear payment account payload info from closed trades. #6001

Clear payment account payload info from closed trades. #6001

Conversation

ghost commented Jan 28, 2022 • edited by ghost Loading

Screenshots:

Invocation:

pazza83 commented Jan 29, 2022

pazza83 commented Jan 29, 2022 • edited Loading

ghost commented Jan 29, 2022

pazza83 commented Jan 30, 2022

ghost commented Jan 31, 2022

ripcurlx left a comment

Choose a reason for hiding this comment

ripcurlx commented Feb 24, 2022 • edited Loading

ripcurlx commented Feb 24, 2022

ripcurlx commented Feb 24, 2022

ripcurlx commented Feb 24, 2022

ghost commented Feb 24, 2022

ripcurlx Feb 25, 2022

Choose a reason for hiding this comment

ripcurlx commented Feb 25, 2022

pazza83 commented Mar 12, 2022

ghost commented Jan 28, 2022 •

edited by ghost

Loading

pazza83 commented Jan 29, 2022 •

edited

Loading

ripcurlx commented Feb 24, 2022 •

edited

Loading