mirror of
https://github.com/bisq-network/bisq.git
synced 2025-02-23 15:00:30 +01:00
Fix call rate metering interceptor bug
The gRPC interceptor was not using the correct method/ratemeter map key, and failing to find a rate meter for the server call. - Fix the rate meter key lookup bug. - Disable most strict, default call rate meters in tests. Made an adjustment to the test harness' scaffold setup so an interceptor testing or disabling config file is always picked up by bob and alice daemons. - Set arbitration daemon's registerDisputeAgent rate @ 10/second, so it does not interfere with the test harness. (There is no pre-existing arb node appDataDir before that daemon starts.) Note: The test harness cannot install the custom rate metering file in an arb daemon's appDataDir before it starts because there is no dao-setup file for that node type. TODO: Adjust api simulation scripts to interceptor bug fix.
This commit is contained in:
parent
f90d2cee57
commit
6b2c386a7c
6 changed files with 62 additions and 66 deletions
|
@ -17,11 +17,14 @@
|
||||||
|
|
||||||
package bisq.apitest;
|
package bisq.apitest;
|
||||||
|
|
||||||
|
import java.io.File;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
|
||||||
import java.util.concurrent.ExecutionException;
|
import java.util.concurrent.ExecutionException;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
|
||||||
import javax.annotation.Nullable;
|
import javax.annotation.Nullable;
|
||||||
|
|
||||||
import org.junit.jupiter.api.TestInfo;
|
import org.junit.jupiter.api.TestInfo;
|
||||||
|
@ -29,15 +32,19 @@ import org.junit.jupiter.api.TestInfo;
|
||||||
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
||||||
import static bisq.apitest.config.BisqAppConfig.arbdaemon;
|
import static bisq.apitest.config.BisqAppConfig.arbdaemon;
|
||||||
import static bisq.apitest.config.BisqAppConfig.bobdaemon;
|
import static bisq.apitest.config.BisqAppConfig.bobdaemon;
|
||||||
|
import static bisq.common.file.FileUtil.deleteFileIfExists;
|
||||||
import static java.net.InetAddress.getLoopbackAddress;
|
import static java.net.InetAddress.getLoopbackAddress;
|
||||||
import static java.util.Arrays.stream;
|
import static java.util.Arrays.stream;
|
||||||
import static java.util.concurrent.TimeUnit.MILLISECONDS;
|
import static java.util.concurrent.TimeUnit.MILLISECONDS;
|
||||||
|
import static java.util.concurrent.TimeUnit.SECONDS;
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
import bisq.apitest.config.ApiTestConfig;
|
import bisq.apitest.config.ApiTestConfig;
|
||||||
import bisq.apitest.method.BitcoinCliHelper;
|
import bisq.apitest.method.BitcoinCliHelper;
|
||||||
import bisq.cli.GrpcClient;
|
import bisq.cli.GrpcClient;
|
||||||
|
import bisq.daemon.grpc.GrpcVersionService;
|
||||||
|
import bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Base class for all test types: 'method', 'scenario' and 'e2e'.
|
* Base class for all test types: 'method', 'scenario' and 'e2e'.
|
||||||
|
@ -64,6 +71,7 @@ import bisq.cli.GrpcClient;
|
||||||
* <p>
|
* <p>
|
||||||
* Initial Bob balances & accounts: 10.0 BTC, 1500000.00 BSQ, USD PerfectMoney dummy
|
* Initial Bob balances & accounts: 10.0 BTC, 1500000.00 BSQ, USD PerfectMoney dummy
|
||||||
*/
|
*/
|
||||||
|
@Slf4j
|
||||||
public class ApiTestCase {
|
public class ApiTestCase {
|
||||||
|
|
||||||
protected static Scaffold scaffold;
|
protected static Scaffold scaffold;
|
||||||
|
@ -79,12 +87,12 @@ public class ApiTestCase {
|
||||||
|
|
||||||
public static void setUpScaffold(Enum<?>... supportingApps)
|
public static void setUpScaffold(Enum<?>... supportingApps)
|
||||||
throws InterruptedException, ExecutionException, IOException {
|
throws InterruptedException, ExecutionException, IOException {
|
||||||
scaffold = new Scaffold(stream(supportingApps).map(Enum::name)
|
String[] params = new String[]{
|
||||||
.collect(Collectors.joining(",")))
|
"--supportingApps", stream(supportingApps).map(Enum::name).collect(Collectors.joining(",")),
|
||||||
.setUp();
|
"--callRateMeteringConfigPath", defaultRateMeterInterceptorConfig().getAbsolutePath(),
|
||||||
config = scaffold.config;
|
"--enableBisqDebugging", "false"
|
||||||
bitcoinCli = new BitcoinCliHelper((config));
|
};
|
||||||
createGrpcClients();
|
setUpScaffold(params);
|
||||||
}
|
}
|
||||||
|
|
||||||
public static void setUpScaffold(String[] params)
|
public static void setUpScaffold(String[] params)
|
||||||
|
@ -139,4 +147,32 @@ public class ApiTestCase {
|
||||||
? testInfo.getTestMethod().get().getName()
|
? testInfo.getTestMethod().get().getName()
|
||||||
: "unknown test name";
|
: "unknown test name";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected static File defaultRateMeterInterceptorConfig() {
|
||||||
|
GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder();
|
||||||
|
builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(),
|
||||||
|
"getVersion",
|
||||||
|
1,
|
||||||
|
SECONDS);
|
||||||
|
// Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names.
|
||||||
|
builder.addCallRateMeter("GrpcDisputeAgentsService",
|
||||||
|
"registerDisputeAgent",
|
||||||
|
6, // Allow 6/second for test harness setup + tests.
|
||||||
|
SECONDS);
|
||||||
|
String[] serviceClassNames = new String[]{
|
||||||
|
"GrpcGetTradeStatisticsService",
|
||||||
|
"GrpcHelpService",
|
||||||
|
"GrpcOffersService",
|
||||||
|
"GrpcPaymentAccountsService",
|
||||||
|
"GrpcPriceService",
|
||||||
|
"GrpcTradesService",
|
||||||
|
"GrpcWalletsService"
|
||||||
|
};
|
||||||
|
for (String service : serviceClassNames) {
|
||||||
|
builder.addCallRateMeter(service, "disabled", 1, MILLISECONDS);
|
||||||
|
}
|
||||||
|
File file = builder.build();
|
||||||
|
file.deleteOnExit();
|
||||||
|
return file;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,9 +19,6 @@ package bisq.apitest.method;
|
||||||
|
|
||||||
import io.grpc.StatusRuntimeException;
|
import io.grpc.StatusRuntimeException;
|
||||||
|
|
||||||
import java.io.File;
|
|
||||||
import java.io.IOException;
|
|
||||||
|
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
|
||||||
import org.junit.jupiter.api.AfterAll;
|
import org.junit.jupiter.api.AfterAll;
|
||||||
|
@ -35,19 +32,9 @@ import org.junit.jupiter.api.TestMethodOrder;
|
||||||
|
|
||||||
import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind;
|
import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind;
|
||||||
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
||||||
import static bisq.common.file.FileUtil.deleteFileIfExists;
|
|
||||||
import static java.util.concurrent.TimeUnit.DAYS;
|
|
||||||
import static java.util.concurrent.TimeUnit.HOURS;
|
|
||||||
import static java.util.concurrent.TimeUnit.MINUTES;
|
|
||||||
import static java.util.concurrent.TimeUnit.SECONDS;
|
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
import bisq.daemon.grpc.GrpcVersionService;
|
|
||||||
import bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig;
|
|
||||||
|
|
||||||
@Disabled
|
@Disabled
|
||||||
@Slf4j
|
@Slf4j
|
||||||
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
|
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
|
||||||
|
@ -55,13 +42,9 @@ public class CallRateMeteringInterceptorTest extends MethodTest {
|
||||||
|
|
||||||
private static final GetVersionTest getVersionTest = new GetVersionTest();
|
private static final GetVersionTest getVersionTest = new GetVersionTest();
|
||||||
|
|
||||||
private static File callRateMeteringConfigFile;
|
|
||||||
|
|
||||||
@BeforeAll
|
@BeforeAll
|
||||||
public static void setUp() {
|
public static void setUp() {
|
||||||
callRateMeteringConfigFile = buildInterceptorConfigFile();
|
startSupportingApps(false,
|
||||||
startSupportingApps(callRateMeteringConfigFile,
|
|
||||||
false,
|
|
||||||
false,
|
false,
|
||||||
bitcoind, alicedaemon);
|
bitcoind, alicedaemon);
|
||||||
}
|
}
|
||||||
|
@ -102,37 +85,6 @@ public class CallRateMeteringInterceptorTest extends MethodTest {
|
||||||
|
|
||||||
@AfterAll
|
@AfterAll
|
||||||
public static void tearDown() {
|
public static void tearDown() {
|
||||||
try {
|
|
||||||
deleteFileIfExists(callRateMeteringConfigFile);
|
|
||||||
} catch (IOException ex) {
|
|
||||||
log.error(ex.getMessage());
|
|
||||||
}
|
|
||||||
tearDownScaffold();
|
tearDownScaffold();
|
||||||
}
|
}
|
||||||
|
|
||||||
public static File buildInterceptorConfigFile() {
|
|
||||||
GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder();
|
|
||||||
builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(),
|
|
||||||
"getVersion",
|
|
||||||
1,
|
|
||||||
SECONDS);
|
|
||||||
builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(),
|
|
||||||
"shouldNotBreakAnything",
|
|
||||||
1000,
|
|
||||||
DAYS);
|
|
||||||
// Only GrpcVersionService is @VisibleForTesting, so we hardcode the class names.
|
|
||||||
builder.addCallRateMeter("GrpcOffersService",
|
|
||||||
"createOffer",
|
|
||||||
5,
|
|
||||||
MINUTES);
|
|
||||||
builder.addCallRateMeter("GrpcTradesService",
|
|
||||||
"takeOffer",
|
|
||||||
10,
|
|
||||||
DAYS);
|
|
||||||
builder.addCallRateMeter("GrpcTradesService",
|
|
||||||
"withdrawFunds",
|
|
||||||
3,
|
|
||||||
HOURS);
|
|
||||||
return builder.build();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -71,8 +71,11 @@ public class MethodTest extends ApiTestCase {
|
||||||
boolean generateBtcBlock,
|
boolean generateBtcBlock,
|
||||||
Enum<?>... supportingApps) {
|
Enum<?>... supportingApps) {
|
||||||
try {
|
try {
|
||||||
|
// Disable call rate metering where there is no callRateMeteringConfigFile.
|
||||||
|
File callRateMeteringConfigFile = defaultRateMeterInterceptorConfig();
|
||||||
setUpScaffold(new String[]{
|
setUpScaffold(new String[]{
|
||||||
"--supportingApps", toNameList.apply(supportingApps),
|
"--supportingApps", toNameList.apply(supportingApps),
|
||||||
|
"--callRateMeteringConfigPath", callRateMeteringConfigFile.getAbsolutePath(),
|
||||||
"--enableBisqDebugging", "false"
|
"--enableBisqDebugging", "false"
|
||||||
});
|
});
|
||||||
doPostStartup(registerDisputeAgents, generateBtcBlock);
|
doPostStartup(registerDisputeAgents, generateBtcBlock);
|
||||||
|
@ -136,6 +139,7 @@ public class MethodTest extends ApiTestCase {
|
||||||
|
|
||||||
protected static void registerDisputeAgents() {
|
protected static void registerDisputeAgents() {
|
||||||
arbClient.registerDisputeAgent(MEDIATOR, DEV_PRIVILEGE_PRIV_KEY);
|
arbClient.registerDisputeAgent(MEDIATOR, DEV_PRIVILEGE_PRIV_KEY);
|
||||||
|
sleep(1001); // Can call registerdisputeagent 1x per second.
|
||||||
arbClient.registerDisputeAgent(REFUND_AGENT, DEV_PRIVILEGE_PRIV_KEY);
|
arbClient.registerDisputeAgent(REFUND_AGENT, DEV_PRIVILEGE_PRIV_KEY);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -33,7 +33,6 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind;
|
||||||
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
import static bisq.apitest.config.BisqAppConfig.alicedaemon;
|
||||||
import static bisq.apitest.config.BisqAppConfig.arbdaemon;
|
import static bisq.apitest.config.BisqAppConfig.arbdaemon;
|
||||||
import static bisq.apitest.config.BisqAppConfig.seednode;
|
import static bisq.apitest.config.BisqAppConfig.seednode;
|
||||||
import static bisq.apitest.method.CallRateMeteringInterceptorTest.buildInterceptorConfigFile;
|
|
||||||
import static bisq.common.file.FileUtil.deleteFileIfExists;
|
import static bisq.common.file.FileUtil.deleteFileIfExists;
|
||||||
import static org.junit.jupiter.api.Assertions.fail;
|
import static org.junit.jupiter.api.Assertions.fail;
|
||||||
|
|
||||||
|
@ -55,7 +54,7 @@ public class StartupTest extends MethodTest {
|
||||||
@BeforeAll
|
@BeforeAll
|
||||||
public static void setUp() {
|
public static void setUp() {
|
||||||
try {
|
try {
|
||||||
callRateMeteringConfigFile = buildInterceptorConfigFile();
|
callRateMeteringConfigFile = defaultRateMeterInterceptorConfig();
|
||||||
startSupportingApps(callRateMeteringConfigFile,
|
startSupportingApps(callRateMeteringConfigFile,
|
||||||
false,
|
false,
|
||||||
false,
|
false,
|
||||||
|
|
|
@ -59,9 +59,9 @@ class GrpcDisputeAgentsService extends DisputeAgentsGrpc.DisputeAgentsImplBase {
|
||||||
return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass())
|
return getCustomRateMeteringInterceptor(coreApi.getConfig().appDataDir, this.getClass())
|
||||||
.or(() -> Optional.of(CallRateMeteringInterceptor.valueOf(
|
.or(() -> Optional.of(CallRateMeteringInterceptor.valueOf(
|
||||||
new HashMap<>() {{
|
new HashMap<>() {{
|
||||||
// You can only register mainnet dispute agents in the UI.
|
// Do not limit devs' ability to test agent registration
|
||||||
// Do not limit devs' ability to register test agents.
|
// and call validation in regtest arbitration daemons.
|
||||||
put("registerDisputeAgent", new GrpcCallRateMeter(1, SECONDS));
|
put("registerDisputeAgent", new GrpcCallRateMeter(10, SECONDS));
|
||||||
}}
|
}}
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
|
|
|
@ -27,7 +27,6 @@ import org.apache.commons.lang3.StringUtils;
|
||||||
|
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
@ -35,6 +34,7 @@ import lombok.extern.slf4j.Slf4j;
|
||||||
import static io.grpc.Status.PERMISSION_DENIED;
|
import static io.grpc.Status.PERMISSION_DENIED;
|
||||||
import static java.lang.String.format;
|
import static java.lang.String.format;
|
||||||
import static java.util.stream.Collectors.joining;
|
import static java.util.stream.Collectors.joining;
|
||||||
|
import static org.apache.commons.lang3.StringUtils.uncapitalize;
|
||||||
|
|
||||||
@Slf4j
|
@Slf4j
|
||||||
public final class CallRateMeteringInterceptor implements ServerInterceptor {
|
public final class CallRateMeteringInterceptor implements ServerInterceptor {
|
||||||
|
@ -106,11 +106,16 @@ public final class CallRateMeteringInterceptor implements ServerInterceptor {
|
||||||
}
|
}
|
||||||
|
|
||||||
private String getRateMeterKey(ServerCall<?, ?> serverCall) {
|
private String getRateMeterKey(ServerCall<?, ?> serverCall) {
|
||||||
// Get the rate meter map key from the full rpc service name. The key name
|
// Get the rate meter map key from the server call method descriptor. The
|
||||||
// is hard coded in the Grpc*Service interceptors() method.
|
// returned key (e.g., 'createOffer') is defined in the 'serviceCallRateMeters'
|
||||||
String fullServiceName = serverCall.getMethodDescriptor().getServiceName();
|
// constructor argument. It is extracted from the gRPC fullMethodName, e.g.,
|
||||||
return StringUtils.uncapitalize(Objects.requireNonNull(fullServiceName)
|
// 'io.bisq.protobuffer.Offers/CreateOffer'.
|
||||||
.substring("io.bisq.protobuffer.".length()));
|
String fullServiceMethodName = serverCall.getMethodDescriptor().getFullMethodName();
|
||||||
|
if (fullServiceMethodName.contains("/"))
|
||||||
|
return uncapitalize(fullServiceMethodName.split("/")[1]);
|
||||||
|
else
|
||||||
|
throw new IllegalStateException("Could not extract rate meter key from "
|
||||||
|
+ fullServiceMethodName + ".");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
Loading…
Add table
Reference in a new issue