<p></p>
<p><b>@sqrrm</b> commented on this pull request.</p>

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

<p>In <a href="https://github.com/bisq-network/bisq/pull/5573#discussion_r656391304">desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java</a>:</p>
<pre style='color:#555'>> @@ -72,7 +74,7 @@
 
     public DisputeAgentView(DisputeManager<? extends DisputeList<Dispute>> disputeManager,
                             KeyRing keyRing,
-                            TradeManager tradeManager,
+                            P2PService p2PService, TradeManager tradeManager,
</pre>
<p>Line break</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5573#discussion_r656394360">desktop/src/main/java/bisq/desktop/main/portfolio/failedtrades/FailedTradesDataModel.java</a>:</p>
<pre style='color:#555'>> +        if (isTradeWithSameCurrentNodeAddress(trade)) {
+            failedTradesManager.removeTrade(trade);
+            tradeManager.addFailedTradeToPendingTrades(trade);
+            return true;
+        } else {
+            return false;
+        }
</pre>
<p>In general, would prefer to return early as it reduces nesting</p>
<pre><code>if (!isTradeWithSameCurrentNodeAddress(trade)) {
    return false;
}
failedTradesManager.removeTrade(trade);
tradeManager.addFailedTradeToPendingTrades(trade);
return true;

</code></pre>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5573#discussion_r656407117">desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java</a>:</p>
<pre style='color:#555'>> +        if (OfferRestrictions.requiresNodeAddressUpdate() && !Utils.isV3Address(disputeNodeAddress.getHostName())) {
+            return false;
+        }
</pre>
<p>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 <code>hasOkAddress()</code> to catch all the conditions.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/5573#discussion_r656408360">core/src/main/java/bisq/core/offer/OfferRestrictions.java</a>:</p>
<pre style='color:#555'>> @@ -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);
</pre>
<p>Is this right, always showing the node address update for regtest?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/5573#pullrequestreview-689745390">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNQZEOOI6HJWN2HMKZTTUC5WVANCNFSM46VOS6LA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNX3RFLAGGRYBXPTL23TUC5WVA5CNFSM46VOS6LKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOFEOK33Q.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/5573#pullrequestreview-689745390",
"url": "https://github.com/bisq-network/bisq/pull/5573#pullrequestreview-689745390",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>