Fix required opt validation bugs in CLI

apitest/docs/README.md

@@ -3,3 +3,4 @@
  - [build-run.md](build-run.md): Build and run API tests at the command line and from Intellij.
  - [test-categories.md](test-categories.md): How to categorize a test case as `method`, `scenario` or `e2e`.
  - [regtest-port-conflicts.md](regtest-port-conflicts.md): Avoid port conflicts when running multiple bitcoin-core apps in regtest mode.
+ - [api-beta-test-guide.md](api-beta-test-guide.md): How to run the test harness and tutorial script, and beta test the API with the new CLI.

apitest/docs/api-beta-test-guide.md

@@ -152,6 +152,25 @@ CLI command unless you change the server’s `–apiPort=<listening-port>`.   In
 `9998`, Bob’s is `9999`.  When you manually test the Api using the test harness, be aware of the port numbers being
 used in the CLI commands, so you know which server (Bob’s or Alice’s) the CLI is sending requests to.
 
+### Registering Test Dispute Agents
+
+If you ran the `trade-simulation.sh` script in your currently running test harness, dispute agents have
+already been registered in the arbitration node, and you can run any of the commands described in the following
+sections.
+
+If you have not run the `trade-simulation.sh` script against the test harness, you will need to
+manually register dispute agents in the arbitration node before you can initiate a trade.  Copy, paste and run
+the following CLI commands to register a `mediator` and a `refundagent`.  Do not change the commands' port `9997`
+option (the test arbitration node's listening port).
+```
+$ ./bisq-cli --password=xyz --port=9997 registerdisputeagent --dispute-agent-type=mediator
+    --registration-key=6ac43ea1df2a290c1c8391736aa42e4339c5cb4f110ff0257a13b63211977b7a
+
+$ ./bisq-cli --password=xyz --port=9997 registerdisputeagent --dispute-agent-type=refundagent
+    --registration-key=6ac43ea1df2a290c1c8391736aa42e4339c5cb4f110ff0257a13b63211977b7a
+```
+_Note:  The API cannot be used to register dispute agents on nodes connected to `mainnet`._
+
 ### CLI Help
 
 Useful information can be found using the CLI’s `--help` option.

apitest/scripts/mainnet-test.sh

@@ -53,6 +53,8 @@
 }
 
 @test "test getversion" {
+  # Wait 1 second before calling getversion again.
+  sleep 1
   load 'version-parser'
   run ./bisq-cli --password=xyz getversion
   [ "$status" -eq 0 ]
@@ -118,6 +120,8 @@
 }
 
 @test "test setwalletpassword oldpwd newpwd" {
+  # Wait 5 seconds before calling setwalletpassword again.
+  sleep 5
   run ./bisq-cli --password=xyz setwalletpassword --wallet-password="a b c"  --new-wallet-password="d e f"
   [ "$status" -eq 0 ]
   echo "actual output:  $output" >&2
@@ -137,7 +141,7 @@
   [ "$status" -eq 0 ]
   echo "actual output:  $output" >&2
   [ "$output" = "wallet decrypted" ]
-  sleep 1
+  sleep 3
 }
 
 @test "test getbalance when wallet available & unlocked with 0 btc balance" {
@@ -151,7 +155,7 @@
 }
 
 @test "test getunusedbsqaddress" {
-  run ./bisq-cli --password=xyz getfundingaddresses
+  run ./bisq-cli --password=xyz getunusedbsqaddress
   [ "$status" -eq 0 ]
 }
 
@@ -163,6 +167,8 @@
 }
 
 @test "test getaddressbalance bogus address argument" {
+  # Wait 1 second before calling getaddressbalance again.
+  sleep 1
   run ./bisq-cli --password=xyz getaddressbalance --address=bogus
   [ "$status" -eq 1 ]
   echo "actual output:  $output" >&2
@@ -187,16 +193,22 @@
 }
 
 @test "test getoffers sell eur check return status" {
+  # Wait 1 second before calling getoffers again.
+  sleep 1
   run ./bisq-cli --password=xyz getoffers --direction=sell --currency-code=eur
   [ "$status" -eq 0 ]
 }
 
 @test "test getoffers buy eur check return status" {
+  # Wait 1 second before calling getoffers again.
+  sleep 1
   run ./bisq-cli --password=xyz getoffers --direction=buy --currency-code=eur
   [ "$status" -eq 0 ]
 }
 
 @test "test getoffers sell gbp check return status" {
+  # Wait 1 second before calling getoffers again.
+  sleep 1
   run ./bisq-cli --password=xyz getoffers --direction=sell --currency-code=gbp
   [ "$status" -eq 0 ]
 }
@@ -216,3 +228,9 @@
   [ "${lines[1]}" = "Usage: bisq-cli [options] <method> [params]" ]
   # TODO add asserts after help text is modified for new endpoints
 }
+
+@test "test takeoffer method --help" {
+  run ./bisq-cli --password=xyz takeoffer --help
+  [ "$status" -eq 0 ]
+  [ "${lines[0]}" = "takeoffer" ]
+}

apitest/scripts/trade-simulation-utils.sh

@@ -267,9 +267,9 @@ istradepaymentsent() {
     MAKER_OR_TAKER="$2"
     if [ "$MAKER_OR_TAKER" = "MAKER" ]
     then
-        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $11}')
-    else
         ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $12}')
+    else
+        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}')
     fi
     commandalert $? "Could not parse istradepaymentsent from trade detail."
     echo "$ANSWER"
@@ -280,9 +280,9 @@ istradepaymentreceived() {
     MAKER_OR_TAKER="$2"
     if [ "$MAKER_OR_TAKER" = "MAKER" ]
     then
-        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $12}')
-    else
         ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}')
+    else
+        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $14}')
     fi
     commandalert $? "Could not parse istradepaymentreceived from trade detail."
     echo "$ANSWER"
@@ -293,9 +293,9 @@ istradepayoutpublished() {
     MAKER_OR_TAKER="$2"
     if [ "$MAKER_OR_TAKER" = "MAKER" ]
     then
-        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $13}')
-    else
         ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $14}')
+    else
+        ANSWER=$(echo "$TRADE_DETAIL" | awk '{print $15}')
     fi
     commandalert $? "Could not parse istradepayoutpublished from trade detail."
     echo "$ANSWER"

apitest/src/test/java/bisq/apitest/ApiTestCase.java

@@ -17,27 +17,35 @@
 
 package bisq.apitest;
 
+import java.io.File;
 import java.io.IOException;
 
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 
+import lombok.extern.slf4j.Slf4j;
+
 import javax.annotation.Nullable;
 
 import org.junit.jupiter.api.TestInfo;
 
 import static bisq.apitest.config.BisqAppConfig.alicedaemon;
 import static bisq.apitest.config.BisqAppConfig.arbdaemon;
 import static bisq.apitest.config.BisqAppConfig.bobdaemon;
+import static bisq.proto.grpc.DisputeAgentsGrpc.getRegisterDisputeAgentMethod;
+import static bisq.proto.grpc.GetVersionGrpc.getGetVersionMethod;
 import static java.net.InetAddress.getLoopbackAddress;
 import static java.util.Arrays.stream;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
 
 
 
 import bisq.apitest.config.ApiTestConfig;
 import bisq.apitest.method.BitcoinCliHelper;
 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'.
@@ -64,6 +72,7 @@
  * <p>
  * Initial Bob balances & accounts:    10.0 BTC, 1500000.00 BSQ, USD PerfectMoney dummy
  */
+@Slf4j
 public class ApiTestCase {
 
     protected static Scaffold scaffold;
@@ -79,12 +88,12 @@ public class ApiTestCase {
 
     public static void setUpScaffold(Enum<?>... supportingApps)
             throws InterruptedException, ExecutionException, IOException {
-        scaffold = new Scaffold(stream(supportingApps).map(Enum::name)
-                .collect(Collectors.joining(",")))
-                .setUp();
-        config = scaffold.config;
-        bitcoinCli = new BitcoinCliHelper((config));
-        createGrpcClients();
+        String[] params = new String[]{
+                "--supportingApps", stream(supportingApps).map(Enum::name).collect(Collectors.joining(",")),
+                "--callRateMeteringConfigPath", defaultRateMeterInterceptorConfig().getAbsolutePath(),
+                "--enableBisqDebugging", "false"
+        };
+        setUpScaffold(params);
     }
 
     public static void setUpScaffold(String[] params)
@@ -139,4 +148,37 @@ protected final String testName(TestInfo testInfo) {
                 ? testInfo.getTestMethod().get().getName()
                 : "unknown test name";
     }
+
+    protected static File defaultRateMeterInterceptorConfig() {
+        GrpcServiceRateMeteringConfig.Builder builder = new GrpcServiceRateMeteringConfig.Builder();
+        builder.addCallRateMeter(GrpcVersionService.class.getSimpleName(),
+                getGetVersionMethod().getFullMethodName(),
+                1,
+                SECONDS);
+        // Only GrpcVersionService is @VisibleForTesting, so we need to
+        // hardcode other grpcServiceClassName parameter values used in
+        // builder.addCallRateMeter(...).
+        builder.addCallRateMeter("GrpcDisputeAgentsService",
+                getRegisterDisputeAgentMethod().getFullMethodName(),
+                10, // Same as default.
+                SECONDS);
+        // Define rate meters for non-existent method 'disabled', to override other grpc
+        // services' default rate meters -- defined in their rateMeteringInterceptor()
+        // methods.
+        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;
+    }
 }

apitest/src/test/java/bisq/apitest/method/CallRateMeteringInterceptorTest.java

@@ -19,8 +19,6 @@
 
 import io.grpc.StatusRuntimeException;
 
-import java.io.File;
-
 import lombok.extern.slf4j.Slf4j;
 
 import org.junit.jupiter.api.AfterAll;
@@ -34,18 +32,9 @@
 
 import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind;
 import static bisq.apitest.config.BisqAppConfig.alicedaemon;
-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.assertThrows;
 
-
-
-import bisq.daemon.grpc.GrpcVersionService;
-import bisq.daemon.grpc.interceptor.GrpcServiceRateMeteringConfig;
-
 @Disabled
 @Slf4j
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
@@ -55,9 +44,7 @@ public class CallRateMeteringInterceptorTest extends MethodTest {
 
     @BeforeAll
     public static void setUp() {
-        File callRateMeteringConfigFile = buildInterceptorConfigFile();
-        startSupportingApps(callRateMeteringConfigFile,
-                false,
+        startSupportingApps(false,
                 false,
                 bitcoind, alicedaemon);
     }
@@ -100,30 +87,4 @@ public void testGetVersionCall4IsAllowed() {
     public static void tearDown() {
         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("GrpcOffersService",
-                "takeOffer",
-                10,
-                DAYS);
-        builder.addCallRateMeter("GrpcTradesService",
-                "withdrawFunds",
-                3,
-                HOURS);
-        return builder.build();
-    }
 }

apitest/src/test/java/bisq/apitest/method/MethodTest.java

@@ -71,8 +71,11 @@ public static void startSupportingApps(boolean registerDisputeAgents,
                                            boolean generateBtcBlock,
                                            Enum<?>... supportingApps) {
         try {
+            // Disable call rate metering where there is no callRateMeteringConfigFile.
+            File callRateMeteringConfigFile = defaultRateMeterInterceptorConfig();
             setUpScaffold(new String[]{
                     "--supportingApps", toNameList.apply(supportingApps),
+                    "--callRateMeteringConfigPath", callRateMeteringConfigFile.getAbsolutePath(),
                     "--enableBisqDebugging", "false"
             });
             doPostStartup(registerDisputeAgents, generateBtcBlock);

apitest/src/test/java/bisq/apitest/scenario/StartupTest.java

@@ -18,6 +18,7 @@
 package bisq.apitest.scenario;
 
 import java.io.File;
+import java.io.IOException;
 
 import lombok.extern.slf4j.Slf4j;
 
@@ -32,7 +33,7 @@
 import static bisq.apitest.config.BisqAppConfig.alicedaemon;
 import static bisq.apitest.config.BisqAppConfig.arbdaemon;
 import static bisq.apitest.config.BisqAppConfig.seednode;
-import static bisq.apitest.method.CallRateMeteringInterceptorTest.buildInterceptorConfigFile;
+import static bisq.common.file.FileUtil.deleteFileIfExists;
 import static org.junit.jupiter.api.Assertions.fail;
 
 
@@ -48,10 +49,12 @@
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 public class StartupTest extends MethodTest {
 
+    private static File callRateMeteringConfigFile;
+
     @BeforeAll
     public static void setUp() {
         try {
-            File callRateMeteringConfigFile = buildInterceptorConfigFile();
+            callRateMeteringConfigFile = defaultRateMeterInterceptorConfig();
             startSupportingApps(callRateMeteringConfigFile,
                     false,
                     false,
@@ -102,6 +105,11 @@ public void testGetCreateOfferHelp() {
 
     @AfterAll
     public static void tearDown() {
+        try {
+            deleteFileIfExists(callRateMeteringConfigFile);
+        } catch (IOException ex) {
+            log.error(ex.getMessage());
+        }
         tearDownScaffold();
     }
 }

build.gradle

@@ -378,6 +378,17 @@ configure(project(':cli')) {
         implementation "ch.qos.logback:logback-classic:$logbackVersion"
         compileOnly "org.projectlombok:lombok:$lombokVersion"
         annotationProcessor "org.projectlombok:lombok:$lombokVersion"
+
+        testImplementation "org.junit.jupiter:junit-jupiter-api:$jupiterVersion"
+        testImplementation "org.junit.jupiter:junit-jupiter-params:$jupiterVersion"
+        testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:$jupiterVersion")
+        testAnnotationProcessor "org.projectlombok:lombok:$lombokVersion"
+        testCompileOnly "org.projectlombok:lombok:$lombokVersion"
+        testRuntime "javax.annotation:javax.annotation-api:$javaxAnnotationVersion"
+    }
+
+    test {
+        useJUnitPlatform()
     }
 }