[bisq-network/bisq] Add auto-confirm feature for Monero (#4458)

chimp1984 notifications at github.com
Thu Sep 3 02:01:49 UTC 2020


I am complete with dev testing so far. Here an overview and hints for testing.

**Specification:**
This feature is only visible for XMR trades.
BTC buyer gets a new popup at payment sent button click asking for entering the XMR tx ID and XMR tx key. He can either enter valid 32byte hex strings (input validation used) or leave the fields empty. If he leaves one or both empty another popup will ask him if he is sure and give info about the requirements to provide those date in dispute case and that the peer cannot do auto-confirm if he does not provide it. But we do not enforce it. There might be users who sent from a wallet not supporting tx keys but as reciver gets the XMR there is no dispute and no follow-up problems. If we would enforce it we would cause dispute cases for such situations.
The data is sent to the BTC seller inside the trade message. Once received he starts requesting from the service(s) for the tx proof if he has the feature enabled. 

There is a news badge and a popup informing about the new feature. By default it is disabled and filled with 1 BTC trade amount limit (auto confirm only used for trades up to 1 BTC), 5 required confirmations and the default service addresses (onion addresses for tor, clearnet for localhost dev testing). Atm there is only 1 service (from @emzy) but @wiz has started to setup a service as well, so there will be 2 services soon. The user can change the service address (not containing 'http://') but cannot leave it empty, in that case it will be refilled with default values at focus out. The required confirmations are only written at focus out as well to avoid unintended values (those are read just in time at service response). The min number of success results it the number of services defined, so by default 2.

The Seller will see a new text field in the trade screen with the auto-confirm status. Initially the expected state is "Proof requests started". After a response it will show either the number of confirmations or "Transaction not seen in mem-pool yet". Once number of confirmations are >= required the service has completed with success (assuming all data are valid and proof was valid). If anything is incorrect or a connection problem the auto-conf fails (1 error or failure of any servive will terminate all). Failures do not mean that the tx was invalid, the user need to check manually in that case.

As long the tx is not in the mempool or the required confirmations are not reached we are in pending state and show how many services have succeeded and how many confirmations the tx has. It might be that both services report a diff. number of confirmations, in that case the last update is visible to the user. We do not have enough space in the UI to show the data per service but combine all in 1 text field. As long we are in that pending state the services get requested every 90 seconds for max. 12 hours. After 12 hous without success it has failed.
Once all services have succeeded the auto confirmation will get triggered. The trade is closed and in the UI there is an auto-confirmed badge. The state of the auto conf is also visible in the trade details window. The state gets persisted with the trade but in case if PENDING we reset the state when reading the persisted file as it does not make sense to show PENDING in that case. At startup the  service will get started automatically in case the trade has not completed in the meantime. If the user clicks manually the confirm button the service will get stopped and the trade completes normally.
If the user updates the required confirmations in settings, this new value will be used at the next response. Also disabling the service in settings cause terminating all services. They will not be restarted if the user enables again. Only after restart the service requests will start again.
All other values in the settings are not applied on running services and changes only have effect at new trades or at a restart.   

There is a new entry in filter to block the auto-confirm feature. If that is set, no new service can be started. Already started ones would not be stopped. It would be good to stop those in that case but that is not implemented so far and be left for later. If time permits we should try to get it into the release as the filter might be important if we discover any flaw and it would be good to ensure that the filter stops all services. Alternatively the service providers could stop their service as well and at least the users using those services will get an connection error and stop.

**How to test the feature?**

If devMode is enabled dev values for txId txKey, xmr receiver address and amount are fixed set. With that you will always receive a success result from the service. Those dev test data are in XmrTxProofModel and can be changed to different ones.
To test with real values you have to disable devMode. 

To test different states is a bit tricky and requires hacking a bit into the code. 
I will describe what I did for dev testing:
- Change `XmrTxProofRequest.REPEAT_REQUEST_PERIOD` from 90 sec to 5 sec
- Comment out the body of the method `autoConfirmFiatPaymentReceived` in `TradeManager`. This avoids that the trade gets completed and you don't need to create new trades all the time but can simply restart the app and test another scenario with the same trade.
- Use the DevTestXmrTxProofHttpClient as mock to configure the timing behaviour, the confirmations and the result. To enable that class change the binding in TradeModule from `bind(AssetTxProofHttpClient.class).to(XmrTxProofHttpClient.class);` to `bind(AssetTxProofHttpClient.class).to(DevTestXmrTxProofHttpClient.class);`
- One can also set the required confirmations 1 block higher as the confirmations from the dev data (or your own tx) and simple leave the UI running and watch if the confirmation updates at a new XMR block and triggers the auto-confirm if the required confirmations are met.

There is a unit test for the parser. No integration test for the whole feature, would be quite a bit of effort and challenge.
 

-- 
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/4458#issuecomment-686186944
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200902/221a782c/attachment-0001.html>


More information about the bisq-github mailing list