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

<p>Mainly I would like to see a moving time window for the rate limit.</p>
<p>Also, I might have missed but is the api only supposed to work on *nix systems? Seems very specific in many places.</p><hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4966#discussion_r547247125">daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java</a>:</p>
<pre style='color:#555'>> +    private void handleInterceptorConfigErrorAndCloseCall(ServerCall<?, ?> serverCall)
+            throws StatusRuntimeException {
+        String methodName = getRateMeterKey(serverCall);
+        String msg = format("%s's rate metering interceptor is incorrectly configured;"
+                        + "  its rate meter cannot be found ",
+                methodName);
+        log.error(StringUtils.capitalize(msg) + ".");
+        serverCall.close(FAILED_PRECONDITION.withDescription(msg), new Metadata());
+    }
+
+    private void handlePermissionDeniedErrorAndCloseCall(String methodName,
+                                                         GrpcCallRateMeter rateMeter,
+                                                         ServerCall<?, ?> serverCall)
+            throws StatusRuntimeException {
+        String msg = getDefaultRateExceededError(methodName, rateMeter);
+        log.error(StringUtils.capitalize(msg) + ".");
</pre>
<p>This should probably be a <code>log.warn</code> as it's not an error but rather expected when the rate limit is exceeded. Might consider changing the method name as well for the same reason.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4966#discussion_r547260969">daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java</a>:</p>
<pre style='color:#555'>> +import java.util.concurrent.TimeUnit;
+
+import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
+
+import javax.annotation.Nullable;
+
+import static java.lang.String.format;
+
+@Slf4j
+public final class GrpcCallRateMeter {
+
+    @Getter
+    private final long allowedCallsPerTimeUnit;
+    @Getter
+    private final TimeUnit timeUnit;
</pre>
<p>I find this a bit odd. I think it makes more sense to set the time window in seconds and just have a <code>int</code> with number of seconds in the time window.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4966#discussion_r547262513">daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java</a>:</p>
<pre style='color:#555'>> +    @Getter
+    private boolean isRunning;
+
+    @Nullable
+    private Timer timer;
+
+    public GrpcCallRateMeter(long allowedCallsPerTimeUnit, TimeUnit timeUnit) {
+        this.allowedCallsPerTimeUnit = allowedCallsPerTimeUnit;
+        this.timeUnit = timeUnit;
+    }
+
+    public void start() {
+        if (timer != null)
+            timer.stop();
+
+        timer = UserThread.runPeriodically(() -> callsCount = 0, 1, timeUnit);
</pre>
<p>My expectation is to have a moving time window with the rate limit. The way it's done now I can send double the max amount if I send all at the end of the first period and then continues at the beginning of the second period.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4966#discussion_r547274637">daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfig.java</a>:</p>
<pre style='color:#555'>> +                                     TimeUnit timeUnit) {
+            log.info("Adding call rate metering definition {}.{} ({}/{}).",
+                    grpcServiceClassName,
+                    methodName,
+                    maxCalls,
+                    StringUtils.chop(timeUnit.name().toLowerCase()));
+            rateMeterConfigs.stream().filter(c -> c.isConfigForGrpcService(grpcServiceClassName))
+                    .findFirst().ifPresentOrElse(
+                    (config) -> config.addMethodCallRateMeter(methodName, maxCalls, timeUnit),
+                    () -> rateMeterConfigs.add(new GrpcServiceRateMeteringConfig(grpcServiceClassName)
+                            .addMethodCallRateMeter(methodName, maxCalls, timeUnit)));
+        }
+
+        public File build() {
+            File tmpFile = serializeRateMeterDefinitions();
+            File configFile = new File("/tmp/ratemeters.json");
</pre>
<p>Not sure this is right unless it's only supposed to work on *nix.</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/4966#pullrequestreview-557040093">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNRAY2JYKFMS6N4WFO3SWCOXFANCNFSM4U772BAQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNRD5HLY3K33DS25FZTSWCOXFA5CNFSM4U772BA2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOEEZ4DXI.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/4966#pullrequestreview-557040093",
"url": "https://github.com/bisq-network/bisq/pull/4966#pullrequestreview-557040093",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>