[bisq-network/bisq] Add hint to upgrade to tor v3 (#5573)

sqrrm notifications at github.com
Tue Jun 22 18:52:58 CEST 2021


@sqrrm commented on this pull request.

Some comments but looks mostly good. I haven't tested and looks somewhat annoying to test on regtest, have you tested it?

> @@ -72,7 +74,7 @@
 
     public DisputeAgentView(DisputeManager<? extends DisputeList<Dispute>> disputeManager,
                             KeyRing keyRing,
-                            TradeManager tradeManager,
+                            P2PService p2PService, TradeManager tradeManager,

Line break

> +        if (isTradeWithSameCurrentNodeAddress(trade)) {
+            failedTradesManager.removeTrade(trade);
+            tradeManager.addFailedTradeToPendingTrades(trade);
+            return true;
+        } else {
+            return false;
+        }

In general, would prefer to return early as it reduces nesting
```
if (!isTradeWithSameCurrentNodeAddress(trade)) {
    return false;
}
failedTradesManager.removeTrade(trade);
tradeManager.addFailedTradeToPendingTrades(trade);
return true;

```

> +        if (OfferRestrictions.requiresNodeAddressUpdate() && !Utils.isV3Address(disputeNodeAddress.getHostName())) {
+            return false;
+        }

This doesn't really fit within the scope of the name of this method since it's failing same addresses if they're V2 after the deactivation date. Perhaps renaming this method to something more generic like `hasOkAddress()` to catch all the conditions.

> @@ -33,7 +34,7 @@
     private static final Date REQUIRE_TOR_NODE_ADDRESS_V3_DATE = Utilities.getUTCDate(2021, GregorianCalendar.AUGUST, 15);
 
     public static boolean requiresNodeAddressUpdate() {
-        return new Date().after(REQUIRE_TOR_NODE_ADDRESS_V3_DATE);
+        return Config.baseCurrencyNetwork().isRegtest() || new Date().after(REQUIRE_TOR_NODE_ADDRESS_V3_DATE);

Is this right, always showing the node address update for regtest?

-- 
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/5573#pullrequestreview-689745390
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210622/16d5ec8c/attachment-0001.htm>


More information about the bisq-github mailing list