[bisq-network/bisq] Autoconfirm XMR trade if tx check was validated by proof service (#4421)

chimp1984 notifications at github.com
Wed Aug 26 15:26:49 UTC 2020


@chimp1984 requested changes on this pull request.


We need at least 2 providers for DEFAULT_XMR_PROOF_PROVIDERS. Can you push wiz and emzy that they have the updated versions running? One need to run with clearnet.

Clearnet handling of provider for non-tor dev setup is missing.

In Trade:
Do not set xmrProofResult in constructor as it is only used in special circumstances. Keeping it null is more clear. (-> remove xmrProofResult = new XmrProofResult(0, 0, XmrProofResult.State.FEATURE_DISABLED);)
Makr xmrProofResult transient and @Nullable
setXmrProofResult can be package scope.

  	// xmrProofResult is not persisted yet
    @Getter
    @Nullable
    private transient XmrProofResult xmrProofResult;

    void setXmrProofResult(XmrProofResult xmrProofResult) {
        this.xmrProofResult = xmrProofResult;
        xmrProofResultProperty.setValue(xmrProofResult);
    }
    
We should consider to persist XmrProofResult in trade. for trade history it might be important to have some "proof" if autoconfirm was used or not, as well in dispute cases.
if we use propbug for XmrProofResult the enum should not be used as enum in protobuf as protobuf enums are cumbersome (have global scope). use the name instead. see other enum serialisations...(ProtoUtil.enumFromProto)

Still capitalized:
setting.preferences.autoConfirmXMR=XMR Auto Confirm
setting.preferences.autoConfirmEnabled=Enabled
setting.preferences.autoConfirmRequiredConfirmations=Required Confirmations
setting.preferences.autoConfirmMaxTradeSize=Max Trade Size (BTC)
setting.preferences.autoConfirmServiceAddresses=Service Addresses
setting.info.headline=New XMR Auto confirm feature
filterWindow.disableAutoConf=Disable Auto-confirmation (altcoins)


setXMRTxKeyWindow.txHash=Transaction id -> setXMRTxKeyWindow.txHash=Transaction ID
setXMRTxKeyWindow.txKey=Tx key -> setXMRTxKeyWindow.txKey=Transaction key

The second popup at xmr key input is removing the shaded background. You need to start that second popup with a short delay (UserThread.runAfter with 50 ms or so).
Auto confirm settings should be better visible. Either move it on top or reduce number of entries in currency list.

handleProofResult:
This handling need more thought/work:
 if (!walletsSetup.hasSufficientPeersForBroadcast()) {
            return failure;
        }
        if (!walletsSetup.isDownloadComplete()) {
            return failure;
        }

> @@ -62,7 +62,7 @@ public AddressValidationResult validate(String address) {
     }
 
     // obtain the raw hex version of a monero address
-    public static String addressToHex(String address) {
+    public static String convert(String address) {

if u name method more specific it does not require the comment. e.g. convertToRawHex()

> @@ -642,7 +642,11 @@ portfolio.pending.step2_buyer.fasterPaymentsHolderNameInfo=Some banks might veri
 portfolio.pending.step2_buyer.confirmStart.headline=Confirm that you have started the payment
 portfolio.pending.step2_buyer.confirmStart.msg=Did you initiate the {0} payment to your trading partner?
 portfolio.pending.step2_buyer.confirmStart.yes=Yes, I have started the payment
-
+portfolio.pending.step2_buyer.confirmStart.warningTitle=You have not provided proof of payment
+portfolio.pending.step2_buyer.confirmStart.warning=You have not entered the transaction ID and the transaction key.\n\n\

Suggestion: portfolio.pending.step2_buyer.confirmStart.warning=You have not entered the transaction ID and the transaction key.\n\n\
  By not providing this data the peer cannot use the auto confirm feature to release the BTC as soon the XMR has been received.\n\
  Beside that, Bisq requires that the sender of the XMR transaction is able to provide this information to the mediator or arbitrator in case of a dispute. 

> @@ -1240,9 +1244,11 @@ setting.about.shortcuts.sendFilter=Set Filter (privileged activity)
 setting.about.shortcuts.sendPrivateNotification=Send private notification to peer (privileged activity)
 setting.about.shortcuts.sendPrivateNotification.value=Open peer info at avatar or dispute and press: {0}
 
-setting.info.headline=New XMR Autoconfirm feature
-setting.info.msg=When selling BTC for XMR you can use the autoconfirm feature to verify that the correct amount of XMR has been deposited to your wallet and automatically confirm the trade, releasing BTC to the counterparty.\n\n\
-  This can make the trade process more efficient for both peers.  Limits can also be set on the max amount of BTC that can be auto-confirmed.  See the auto-confirm section on this page for more info and to enable the feature.
+setting.info.headline=New XMR Auto confirm feature
+setting.info.msg=When selling BTC for XMR you can use the auto confirm feature to verify that the correct amount of XMR has been deposited to your wallet and automatically confirm the trade, releasing BTC to the counterparty.\n\n\
+  This can make the trade process more efficient for both peers.  Limits can also be set on the max amount of BTC that can be auto-confirmed.  See the auto-confirm settings on this page for more info and to enable the feature.\n\n\
+  Auto confirmation relies on Bisq checking the XMR transaction on an explorer API.  For maximum security it is recommended that you point Bisq to your own explorer service, but there are also services available run by Bisq (the default setting).\n\n\
+  For more information and details on how to setup, see the blog post here: [.....]
 

Suggestions:
setting.info.msg=When selling BTC for XMR you can use the auto confirm feature to verify that the correct amount of XMR has been deposited to your wallet and automatically confirm the XMR receipt and releasing the BTC to the counterparty.\n\n\
  This can make the trade process more efficient for both peers.  Limits can also be set on the max amount of BTC per trade that can be auto-confirmed as well as the number of required confirmations. \n\n\
  Auto confirmation relies on Bisq checking the XMR transaction on an XMR explorer node using the private transaction key provided by the XMR sender. Bisq requests by default from at least 2 nodes operated by Bisq contributors. Both need to provide the same result for triggering the confirmation.
  For maximum security and privacy it is recommended that you run your own XMR explorer node and set that address in the settings.\n\n\
  More information and details on how to setup your own node can be found in the Bisq wiki at: [.....]

@m52go Could you review that?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4421#pullrequestreview-475524751
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200826/6cc9e5b7/attachment-0001.html>


More information about the bisq-github mailing list