[bisq-network/bisq] Add filter support on network level (#5038)

sqrrm notifications at github.com
Sun Jan 3 23:58:05 CET 2021


@sqrrm commented on this pull request.

Looks good in general but esp naming comment would be good to fix to avoid future confusion

>                  .append(sep));
         sb.append("#################################################################");
         log.info(sb.toString());
     }
 
+    private void addDetails(Map<String, Tuple2<AtomicInteger, AtomicInteger>> numPayloadsByClassName,
+                            NetworkPayload networkPayload, String className) {
+        numPayloadsByClassName.putIfAbsent(className, new Tuple2<>(new AtomicInteger(0),
+                new AtomicInteger(0)));
+        numPayloadsByClassName.get(className).first.getAndIncrement();
+        numPayloadsByClassName.get(className).second.getAndAdd(networkPayload.toProtoMessage().getSerializedSize());

Might be worth adding a comment that this log info comes at a cost for in case it turns out to be a problem in the future

> @@ -137,10 +135,11 @@ public void processArgs(String[] args) {
                     checkArgument(arg.contains(":") && arg.split(":").length > 1 && arg.split(":")[1].length() > 3,
                             "Wrong program argument " + arg);
                     List<String> list = Arrays.asList(arg.split(","));
+                    Set<NodeAddress> bannedPeers = new HashSet<>();

This set isn't used so why even gather the banned peers?

> @@ -91,6 +95,11 @@
     // added at v1.3.8
     private final boolean disableAutoConf;
 
+    // added at v1.5.5
+    // In contrast to bannedNodeAddress those addresses cannot access the network but are rejected immediately.
+    // Should be only used in case of dos attacks
+    private final Set<String> nodeAddressesBannedFromNetwork;

Naming is confusing since there is a `bannedNodeAddress` and a `nodeAddressesBannedFromNetwork`, better to have more descriptive names.

> @@ -677,6 +677,7 @@ message Filter {
     repeated string bannedPrivilegedDevPubKeys = 23;
     bool disable_auto_conf = 24;
     repeated string banned_auto_conf_explorers = 25;
+    repeated string node_addresses_banned_from_network = 26;

Filter hash won't be backwards compatible so care is needed when this is rolled out

-- 
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/5038#pullrequestreview-560729237
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210103/c7e8626a/attachment.htm>


More information about the bisq-github mailing list