Prevent excessive api calls #4966

ghubstan · 2020-12-17T17:33:47Z

This change provides a gRPC CallRateMeteringInterceptor to help protect the server and network against being overloaded by CLI scripting mistakes.

An interceptor instance can be configured on a gRPC service to set per method call rate limits on one or more of a service's methods. For example, the GrpcOffersService could be configured with this interceptor to set the createoffer rate limit to 5/hour, and the takeoffer call rate limit could be set to 20/day. Whenever a call rate limit is exceeded, the gRPC call is aborted and the client receives a "rate limit exceeded" error.

Below is a simple example showing how to set rate limits for only the getVersion method in GrpcVersionService.

final ServerInterceptor[] interceptors() {
    return new ServerInterceptor[]{
            new CallRateMeteringInterceptor(new HashMap<>() {{
                put("getVersion", new GrpcCallRateMeter(2, SECONDS));
            }})
    };
}

It specifies a CLI can execute getversion 2 times / second.

This is not a throttling mechanism, there is no blocking nor locking to slow call rates. When call rates are exceeded, calls are simply aborted.

This is the 10th in a chain of PRs beginning with #4884.
PR #4960 should be reviewed before this one.

BtcWalletService was changed to allow the api to override tx fee rates from the sendbsq and sendbtc methods. The api methods will still be able to use the network fee service and custom tx fee rate preference, and set / unset the custom tx fee rate preference, but the change will permit the addition of an optional txFeeRate parameter to the sendbsq and sendbtc methods (todo). A few other minor changes (style and removal of never thrown ex spec) were also made to this class. Two BtcWalletService methods were refactored. - The redundant (was always true) boolean isSendTx argument was removed from the completePreparedVoteRevealTx method signature. - The redundant (was always true) boolean useCustomTxFee was removed from the completePreparedBsqTx method signature. - The completePreparedSendBsqTx method was overloaded with a 2nd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC. - The completePreparedBsqTx method was overloaded with a 3rd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC. The following line was deleted from the completePreparedBsqTx method because txFeePerVbyte is now an argument: Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte(); This useCustomTxFee value was always true, and redudant here because getTxFeeForWithdrawalPerVbyte() returns feeService.getTxFeePerVbyte() or the custom fee rate preference. i.e., Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte(); is equivalent to Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte(); LockupTxService, UnlockTxService, BsqSendView, and BsqTransferService were adjusted to this BtcWalletService refactoring.

If present in the sendbsq command, the parameter will override the fee service and custom fee rate setting for the BSQ transaction. Also changed the sendbsq grpc return type to a lightweight TX proto wrapper. Besides some small refactoring in the CLI, all the changes are adjustments for this new sendbsq parameter and its new grpc return value.

Takes an address, amount, and optional txfeerate param, returns a lightweight TxInfo proto. Also overloaded two BtcWalletService methods to allow sendbtc to pass in the tx fee rate -- overriding the fee service and custom fee rate setting.

- Added optional memo parameter to the api's sendbtc and withdrawfunds commands. - Removed the @nullable annotation was removed because protobuf does not support null. - Visibility in two wallet check methods were changed from private to pkg protected so the CoreTradeService could use them. - Adjusted affected tests. (Asserting the memo field was set on a transaction cannot be checked from apitest yet.)

The withdrawalTxId field will be set in TradeManager#onWithdrawRequest upon successful trade completion and withdrawal of funds. Persisting withdrawalTxId will allow the api and ui to find the withdrawalTxId for a completed trade after the seller withdraws funds to an external wallet. In turn, the withdrawal tx's memo field will be accessible in a new (todo) api getTx(txID) api method. Changed: - Appended field 'string withdrawal_tx_id = 40' to pb.proto's Trade message. - Added nullable 'String withdrawalTxId' to Trade entity class. - Added trade.setWithdrawalTxId(transaction.getTxId().toString()) in TradeManager#onWithdrawRequest's callback.

This change was prompted by the recent changes in the main branch to allow a tx memo field to be set from the UI and API. This and the prior PR address the API's need to be able to fetch a tx (with a memo). The API can now get a completed trade's withdrawal txid and pass it as a gettransaction parameter. See previous PR "Append nullable withdrawalTxId field to Trade". bisq-network#4937 A summary of changes by file: grpc.proto - Added withdrawalTxId field to existing TradeInfo proto & wrapper. - Reordered fields in TradeInfo proto. - Added new fields to be displayed by TxInfo proto in CLI. - Fixed typo: unsetTxFeeRatePreference -> UnsetTxFeeRatePreference. - Added new GetTransaction rpc. GrpcWalletsService - Added new getTransaction gRPC boilerplate. CoreWalletsService - Added new getTransaction implementation. TxInfo - Added the new fields for displaying a tx summary from CLI. This is not intended to be more than a brief summary; a block explorer or bitcoin-core client should be used to see the complete definition. TradeInfo - Added the new withdrawalTxId field defined in grpc.proto. CliMain - Added new 'case gettransaction'. TransactionFormat - Formats a TxInfo sent from the server to CLI. ColumnHeaderConstants - Added console headers used by TransactionFormat. TradeFormat - Displays a completed trade's WithdrawalTxId if present. Apitest - Adjusted affected tests: assert tx memo is persisted and test gettransaction.

As per commit 88f26f9, do not autofill all currencies by default but keep all unselected.

@singleton

This change reduces gRPC service error handling duplication by moving it into a @singleton encapsulating everything needed to wrap an expected or unexpected core api exception into a gRPC StatusRuntimeException before sending it to the client. It also fixes some boilerpate classes were gRPC error handling was missing.

This change provides a gRPC CallRateMeteringInterceptor to help protect the server and network against being overloaded by CLI scripting mistakes. An interceptor instance can be configured on a gRPC service to set individual method call rate limits on one or more of the the service's methods. For example, the GrpcOffersService could be configured with this interceptor to set the createoffer rate limit to 5/hour, and the takeoffer call rate limit could be set to 20/day. Whenever a call rate limit is exceeded, the gRPC call is aborted and the client recieves a "rate limit exceeded" error. Below is a simple example showing how to set rate limits for one method in GrpcVersionService. final ServerInterceptor[] interceptors() { return new ServerInterceptor[]{ new CallRateMeteringInterceptor(new HashMap<>() {{ put("getVersion", new GrpcCallRateMeter(2, SECONDS)); }}) }; } It specifies a CLI can execute getversion 2 times / second. This is not a throttling mechansim, there is no blocking nor locking to slow call rates. When call rates are exceeded, calls are simply aborted.

…sient

This adds a GrpcServiceRateMeteringConfig class that can read and write rate metering interceptor config files, and configure a gRPC rate metering service interceptor at startup. This seems excessive, but we need to be able to test and tune method rate metering without having to change hard coded, default interceptor rate meters.

…ceptor Adjust to reverting to reverting 6aa385e, and fix test file conflict.

sqrrm

Mainly I would like to see a moving time window for the rate limit.

Also, I might have missed but is the api only supposed to work on *nix systems? Seems very specific in many places.

daemon/src/main/java/bisq/daemon/grpc/interceptor/CallRateMeteringInterceptor.java

daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java

daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcServiceRateMeteringConfig.java

Rewrote the GrpcCallRateMeter class and adjusted afected classes. These changes were requested in PR review bisq-network#4966 (review)

The size of the timestamp queue is the call count

ghubstan · 2020-12-22T22:48:09Z

Force travis jar download.

We need to be able to define call rate meters for time spans not limited to TimeUnit intervals of 1 SECOND, 1 HOUR, or 1 DAY. This change allows more flexibility, e.g., 10 per 5 seconds, 10 per 5 hrs, 100 per 7 days.

This will reduce the entire apitest suite's exec time

sqrrm

Functionality now looks right. Just some comments on naming.

daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java

- Change method isAllowed() -> checkAndIncrement(). - Change variable allowedCallsPerTimeUnit -> allowedCallsPerTimeWindow.

sqrrm

utACK

ghubstan added 30 commits December 4, 2020 14:17

Merge branch 'master' into 02-refactor-completePreparedSendBsqTx

900d498

Merge branch 'master' into 03-add-txFeeRate-param

2842070

Add new api method 'sendbtc' and test

6c9f0c2

Takes an address, amount, and optional txfeerate param, returns a lightweight TxInfo proto. Also overloaded two BtcWalletService methods to allow sendbtc to pass in the tx fee rate -- overriding the fee service and custom fee rate setting.

Merge branch 'master' into 04-add-sendbtc-impl

144c5a8

Remove unused imports

478c8f4

Merge branch 'master' into 05-use-memo-tx-field

259bad6

Use Bisq's UserThread.executor in gRPC server

150e2f6

Merge branch 'master' into 08-scratch

a0f1c22

Adjust create TransferwiseAccount test

0384642

As per commit 88f26f9, do not autofill all currencies by default but keep all unselected.

Disable method test to avoid repetition

4be87a6

Merge branch 'master' into 09-scratch

a341173

Merge branch 'master' into 09-refactor-grpc-error-handling

1cd47fd

Fix class level comment

c60605f

Rename gRPC exception handler class

f7c1103

Create grpc interceptor pkg, move auth interceptor into it

2572e86

Put arguments on separate lines

fa9ffa1

Change long to int, tidy up error msg

89e2187

Add license comment, stop & toString methods, and make isRunning tran…

a5ed17e

…sient

Add unit test dependencies to daemon subproject

5de910a

Add GrpcServiceRateMeteringConfigTest

455ed67

Add license note

830a5f0

Add license note and toString method

9f679de

Merge branch '09-refactor-grpc-error-handling' into 10-callrate-inter…

27efc5f

…ceptor Adjust to reverting to reverting 6aa385e, and fix test file conflict.

sqrrm requested changes Dec 22, 2020

View reviewed changes

ghubstan added 4 commits December 22, 2020 16:14

Use a simpler, time windowing call rate meter

01546ad

Rewrote the GrpcCallRateMeter class and adjusted afected classes. These changes were requested in PR review bisq-network#4966 (review)

Remove unused local var

d615212

Remove redundant callCount field

0d4ed95

The size of the timestamp queue is the call count

Fix comment

c8ef414

ghubstan closed this Dec 22, 2020

ghubstan reopened this Dec 22, 2020

ghubstan added 2 commits December 22, 2020 21:11

Support more fine grained rate metering

6356476

We need to be able to define call rate meters for time spans not limited to TimeUnit intervals of 1 SECOND, 1 HOUR, or 1 DAY. This change allows more flexibility, e.g., 10 per 5 seconds, 10 per 5 hrs, 100 per 7 days.

Disable CallRateMeteringInterceptorTest and run it from test suite

b8c5a29

This will reduce the entire apitest suite's exec time

sqrrm requested changes Dec 23, 2020

View reviewed changes

daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java Outdated Show resolved Hide resolved

daemon/src/main/java/bisq/daemon/grpc/interceptor/GrpcCallRateMeter.java Outdated Show resolved Hide resolved

Fix GrpcCallRateMeter method and variable name

10727fc

- Change method isAllowed() -> checkAndIncrement(). - Change variable allowedCallsPerTimeUnit -> allowedCallsPerTimeWindow.

sqrrm approved these changes Dec 23, 2020

View reviewed changes

sqrrm merged commit 7d7f1b0 into bisq-network:master Dec 23, 2020

ghubstan deleted the 10-callrate-interceptor branch December 23, 2020 18:59

This was referenced Dec 26, 2020

For Cycle 20 bisq-network/compensation#741

Closed

Implement simplest possible trading script #4257

Closed

ripcurlx added this to the v1.5.3 milestone Dec 27, 2020

sqrrm mentioned this pull request Dec 28, 2020

For Cycle 20 bisq-network/compensation#747

Closed

ghubstan mentioned this pull request Jan 8, 2021

Prepare roll out of API for January or February release bisq-network/projects#46

Open

14 tasks

ghubstan mentioned this pull request Jan 26, 2021

For Cycle 21 bisq-network/compensation#767

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent excessive api calls #4966

Prevent excessive api calls #4966

ghubstan commented Dec 17, 2020 •

edited

Loading

sqrrm left a comment

ghubstan commented Dec 22, 2020

sqrrm left a comment

sqrrm left a comment

Prevent excessive api calls #4966

Prevent excessive api calls #4966

Conversation

ghubstan commented Dec 17, 2020 • edited Loading

sqrrm left a comment

Choose a reason for hiding this comment

ghubstan commented Dec 22, 2020

sqrrm left a comment

Choose a reason for hiding this comment

sqrrm left a comment

Choose a reason for hiding this comment

ghubstan commented Dec 17, 2020 •

edited

Loading