Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix call rate metering interceptor bug #5250

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions apitest/scripts/mainnet-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down Expand Up @@ -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
Expand All @@ -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" {
Expand All @@ -151,7 +155,7 @@
}

@test "test getunusedbsqaddress" {
run ./bisq-cli --password=xyz getfundingaddresses
run ./bisq-cli --password=xyz getunusedbsqaddress
[ "$status" -eq 0 ]
}

Expand All @@ -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
Expand All @@ -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 ]
}
Expand All @@ -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" ]
}
12 changes: 6 additions & 6 deletions apitest/scripts/trade-simulation-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down
54 changes: 48 additions & 6 deletions apitest/src/test/java/bisq/apitest/ApiTestCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import io.grpc.StatusRuntimeException;

import java.io.File;

import lombok.extern.slf4j.Slf4j;

import org.junit.jupiter.api.AfterAll;
Expand All @@ -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)
Expand All @@ -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);
}
Expand Down Expand Up @@ -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();
}
}
3 changes: 3 additions & 0 deletions apitest/src/test/java/bisq/apitest/method/MethodTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions apitest/src/test/java/bisq/apitest/scenario/StartupTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package bisq.apitest.scenario;

import java.io.File;
import java.io.IOException;

import lombok.extern.slf4j.Slf4j;

Expand All @@ -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;


Expand All @@ -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,
Expand Down Expand Up @@ -102,6 +105,11 @@ public void testGetCreateOfferHelp() {

@AfterAll
public static void tearDown() {
try {
deleteFileIfExists(callRateMeteringConfigFile);
} catch (IOException ex) {
log.error(ex.getMessage());
}
tearDownScaffold();
}
}
10 changes: 5 additions & 5 deletions cli/src/main/java/bisq/cli/ColumnHeaderConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class ColumnHeaderConstants {

// Table column header format specs, right padded with two spaces. In some cases
// such as COL_HEADER_CREATION_DATE, COL_HEADER_VOLUME and COL_HEADER_UUID, the
// expected max data string length is accounted for. In others, the column header length
// are expected to be greater than any column value length.
// expected max data string length is accounted for. In others, column header
// lengths are expected to be greater than any column value length.
static final String COL_HEADER_ADDRESS = padEnd("%-3s Address", 52, ' ');
static final String COL_HEADER_AMOUNT = padEnd("BTC(min - max)", 24, ' ');
static final String COL_HEADER_AVAILABLE_BALANCE = "Available Balance";
Expand All @@ -49,17 +49,17 @@ class ColumnHeaderConstants {
static final String COL_HEADER_PAYMENT_METHOD = "Payment Method";
static final String COL_HEADER_PRICE = "Price in %-3s for 1 BTC";
static final String COL_HEADER_TRADE_AMOUNT = padStart("Amount(%-3s)", 12, ' ');
static final String COL_HEADER_TRADE_BUYER_COST = padEnd("Buyer Cost(%-3s)", 15, ' ');
static final String COL_HEADER_TRADE_DEPOSIT_CONFIRMED = "Deposit Confirmed";
static final String COL_HEADER_TRADE_DEPOSIT_PUBLISHED = "Deposit Published";
static final String COL_HEADER_TRADE_FIAT_SENT = "Fiat Sent";
static final String COL_HEADER_TRADE_FIAT_RECEIVED = "Fiat Received";
static final String COL_HEADER_TRADE_PAYMENT_SENT = padEnd("%-3s Sent", 8, ' ');
static final String COL_HEADER_TRADE_PAYMENT_RECEIVED = padEnd("%-3s Received", 12, ' ');
static final String COL_HEADER_TRADE_PAYOUT_PUBLISHED = "Payout Published";
static final String COL_HEADER_TRADE_WITHDRAWN = "Withdrawn";
static final String COL_HEADER_TRADE_ROLE = "My Role";
static final String COL_HEADER_TRADE_SHORT_ID = "ID";
static final String COL_HEADER_TRADE_TX_FEE = "Tx Fee(%-3s)";
static final String COL_HEADER_TRADE_TAKER_FEE = "Taker Fee(%-3s)";
static final String COL_HEADER_TRADE_WITHDRAWAL_TX_ID = "Withdrawal TX ID";

static final String COL_HEADER_TX_ID = "Tx ID";
static final String COL_HEADER_TX_INPUT_SUM = "Tx Inputs (BTC)";
Expand Down
Loading