Sensitive data still visible in ClosedTrades file

[Android-X13]

This is an example excerpt from ClosedTrades file regarding an old trade:

SEPA��:Ύ�
�COUNTRY_CODE"·�
FULL_NAME��IBAN��BIC�AT�BE�BG�CH�CY�CZ�DE�DK�EE�ES�FI�FR�GB�GR�HR�HU�IE�IS�IT�LI�LT�LU�LV�MC�MT�NL�NO�PL�PT�RO�SE�SI�SKzH
�salt�@xxxxx�XXXXX"ό�{
"offerPayload": {
"id": "XXXXX",
"date": XXXXX,
"ownerNodeAddress": {
"hostName": "XXXXX.onion",
"port": 9999
},
"direction": "SELL",
"price": 0,
"marketPriceMargin": XXXXX,
"useMarketBasedPrice": true,
"amount": XXXXX,
"minAmount": XXXXX,
"baseCurrencyCode": "XXXXX",
"counterCurrencyCode": "XXXXX",
"arbitratorNodeAddresses": [],
"mediatorNodeAddresses": [
{
"hostName": "6c4cim7h7t3bm4bnchbf727qrhdfrfr6lhod25wjtizm2sifpkktvwad.onion",
"port": 9999
},
{
"hostName": "7hkpotiyaukuzcfy6faihjaols5r2mkysz7bm3wrhhbpbphzz3zbwyqd.onion",
"port": 9999
}
],
"paymentMethodId": "SEPA",
"makerPaymentAccountId": "XXXXX",
"offerFeePaymentTxId": "XXXXX",
"versionNr": "1.9.1",
"blockHeightAtOfferCreation": XXXXX,
"txFee": XXXXX,
"makerFee": XXXXX,
"isCurrencyForMakerFeeBtc": false,
"buyerSecurityDeposit": XXXXX,
"sellerSecurityDeposit": XXXXX,
"maxTradeLimit": XXXXX,
"maxTradePeriod": XXXXX,
"useAutoClose": false,
"useReOpenAfterAutoClose": false,
"lowerClosePrice": 0,
"upperClosePrice": 0,
"isPrivateOffer": false,
"extraDataMap": {
"capabilities": "0, 1, 2, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17",
"accountAgeWitnessHash": "XXXXX"
},
"protocolVersion": 4
},
"tradeAmount": XXXXX,
"tradePrice": XXXXX,
"takerFeeTxID": "XXXXX",
"buyerNodeAddress": {
"hostName": "XXXXX.onion",
"port": 9999
},
"sellerNodeAddress": {
"hostName": "XXXXX.onion",
"port": 9999
},
"mediatorNodeAddress": {
"hostName": "6c4cim7h7t3bm4bnchbf727qrhdfrfr6lhod25wjtizm2sifpkktvwad.onion",
"port": 9999
},
"isBuyerMakerAndSellerTaker": false,
"makerAccountId": "XXXXX",
"takerAccountId": "XXXXX",
"makerPayoutAddressString": "XXXXX",
"takerPayoutAddressString": "XXXXX",
"lockTime": XXXXX,
"refundAgentNodeAddress": {
"hostName": "3z5jnirlccgxzoxc6zwkcgwj66bugvqplzf6z2iyd5oxifiaorhnanqd.onion",
"port": 9999
},
"hashOfMakersPaymentAccountPayload": [
...
XXXXX
...
],
"hashOfTakersPaymentAccountPayload": [
...
XXXXX
...
],
"makerPaymentMethodId": "SEPA",
"takerPaymentMethodId": "SEPA"
}

I (and anyone who might get access to my computer) can still clearly see the personal details of the trade partner.

Field Clear sensitive data after (days) in the Settings screen is set to 1.

I am aware of #5396 but it seems to be incomplete? (Chat messages however are indeed removed from the file).

[ghost]

Account payloads and trader chats are currently removed.
In design discussions it was decided that is sufficient.
If you want to clear everything, you can switch to a new data directory or delete your ClosedTrades file.

[Android-X13]

It's not about what I want to clear on my computer... it's about what one wants to be cleared on his trade partner's computer regarding his identity. As it is now, all the info is stored on his (and everyone you've traded with) hard drive.
The issue I linked to was also mentioning this (unencrypted trade data like names and account numbers).
Full name and IBAN is a lot more sensitive than trade chats in my opinion.

[Android-X13]

Just to clarify: I'm not saying that the data should be encrypted, but it should probably be removed after X days, the sooner the better. And not all the data but just the sensitive type of data.
Is there a reason why the trade partner's full name, IBAN, BIC and country code should be kept in storage forever? Wouldn't it be ideal if ClosedTrades kept all the rest (including peer's onion address) except the fields I mentioned?

[ghost]

Good point well argued. In the original implementation I was trying to limit the risk of potential damage, but as time has passed without complaints; its a good point to work on redacting the trade contract. I'm on it.

[Android-X13]

Now that I'm thinking about it, country code maybe doesn't matter because it is already exposed and linked to an onion address, at least when the owner of that address is the Maker and publishes an offer. But the full name, IBAN and BIC are only exposed when the trade is started, so these are the details that should definitely be removed from storage afterwards.

@jmacxx as a side note could you please clarify to me what exactly is the format ClosedTrades file is saved as? There's a lot of seemingly garbage data in there which cannot be read. Is there a way to decode them outside Bisq application?

[ghost]

It turns out that the trade ProcessModel includes a copy of the peer's PaymentAccountPayload and contractAsJson. That was something missed from /pull/6001, but as already mentioned above I'm happier for changes like this being done in stages to avoid catastrophe. Making a PR to remove these two items now.

[ghost]

@jmacxx as a side note could you please clarify to me what exactly is the format ClosedTrades file is saved as? There's a lot of seemingly garbage data in there which cannot be read. Is there a way to decode them outside Bisq application?

It is saved as Protobuf. I'm not aware of any way to decode from outside the Bisq codebase.