From cfaa539a5e5fffb2c9d732b7281769bde49516c6 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Thu, 4 Mar 2021 13:03:04 -0300 Subject: [PATCH 01/15] Fix opt validation bugs in CLI Unset empty string default values on some the createoffer method's required option declarations in CreateOfferOptionParser, and adds a check for presence of required currency-code option. - Removes default "" value from required opt direction. - Removes default "" value from required opt currency-code. - Removes default "" value from required opt amount. - Removes default "" value from required opt min-amount. - Removes default "" value from required opt fixed-price. - Removes default "" value from required opt security-deposit. - Check for required currency-code option. Unset empty string default values on some of the createoffer method's required option declarations in TakeOfferOptionParser. - Removes default "" value from required opt offer-id. - Removes default "" value from required opt payment-account. Other opt parser default values removed from: - CancelOfferOptionParser#offer-id - CreatePaymentAcctOptionParser#payment-account-form - GetAddressBalanceOptionParser#address - GetBTCMarketPriceOptionParser#currency-code - GetOfferOptionParser#offer-id - GetOffersOptionParser#direction - GetOffersOptionParser#currency-code - GetPaymentAcctFormOptionParser#payment-method-id - GetTradeOptionParser#trade-id - GetTransactionOptionParser#transaction-id - RegisterDisputeAgentOptionParser#registration-key - RegisterDisputeAgentOptionParser#dispute-agent-type - RemoveWalletPasswordOptionParser#wallet-password - SendBsqOptionParser#address - SendBsqOptionParser#amount - SendBtcOptionParser#address - SendBtcOptionParser#amount - SetTxFeeRateOptionParser#tx-fee-rate - SetWalletPasswordOptionParser#wallet-password - UnlockWalletOptionParser#wallet-password - UnlockWalletOptionParser#timeout - WithdrawFundsOptionParser#trade-id - WithdrawFundsOptionParser#address --- .../cli/opts/CancelOfferOptionParser.java | 4 +--- .../cli/opts/CreateOfferOptionParser.java | 20 +++++++++---------- .../opts/CreatePaymentAcctOptionParser.java | 4 +--- .../opts/GetAddressBalanceOptionParser.java | 4 +--- .../opts/GetBTCMarketPriceOptionParser.java | 4 +--- .../bisq/cli/opts/GetOfferOptionParser.java | 4 +--- .../bisq/cli/opts/GetOffersOptionParser.java | 7 ++----- .../opts/GetPaymentAcctFormOptionParser.java | 4 +--- .../bisq/cli/opts/GetTradeOptionParser.java | 4 +--- .../cli/opts/GetTransactionOptionParser.java | 4 +--- .../RegisterDisputeAgentOptionParser.java | 7 ++----- .../RemoveWalletPasswordOptionParser.java | 4 +--- .../bisq/cli/opts/SendBsqOptionParser.java | 6 ++---- .../bisq/cli/opts/SendBtcOptionParser.java | 6 ++---- .../cli/opts/SetTxFeeRateOptionParser.java | 4 +--- .../opts/SetWalletPasswordOptionParser.java | 3 +-- .../bisq/cli/opts/TakeOfferOptionParser.java | 7 ++----- .../cli/opts/UnlockWalletOptionParser.java | 4 +--- .../cli/opts/WithdrawFundsOptionParser.java | 9 +++++---- 19 files changed, 36 insertions(+), 73 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java index 24ebc744211..5ed33ed41fb 100644 --- a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; -import static joptsimple.internal.Strings.EMPTY; public class CancelOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to cancel") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public CancelOfferOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java index d4d7c05c7ad..bbaf68bab36 100644 --- a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java @@ -33,20 +33,16 @@ public class CreateOfferOptionParser extends AbstractMethodOptionParser implemen .defaultsTo(EMPTY); final OptionSpec directionOpt = parser.accepts(OPT_DIRECTION, "offer direction (buy|sell)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency code (eur|usd|...)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of btc to buy or sell") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec minAmountOpt = parser.accepts(OPT_MIN_AMOUNT, "minimum amount of btc to buy or sell") - .withOptionalArg() - .defaultsTo(EMPTY); + .withOptionalArg(); final OptionSpec mktPriceMarginOpt = parser.accepts(OPT_MKT_PRICE_MARGIN, "market btc price margin (%)") .withOptionalArg() @@ -54,11 +50,10 @@ public class CreateOfferOptionParser extends AbstractMethodOptionParser implemen final OptionSpec fixedPriceOpt = parser.accepts(OPT_FIXED_PRICE, "fixed btc price") .withOptionalArg() - .defaultsTo(EMPTY); + .defaultsTo("0"); final OptionSpec securityDepositOpt = parser.accepts(OPT_SECURITY_DEPOSIT, "maker security deposit (%)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec makerFeeCurrencyCodeOpt = parser.accepts(OPT_FEE_CURRENCY, "maker fee currency code (bsq|btc)") .withOptionalArg() @@ -81,6 +76,9 @@ public CreateOfferOptionParser parse() { if (!options.has(directionOpt)) throw new IllegalArgumentException("no direction (buy|sell) specified"); + if (!options.has(currencyCodeOpt)) + throw new IllegalArgumentException("no currency code specified"); + if (!options.has(amountOpt)) throw new IllegalArgumentException("no btc amount specified"); diff --git a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java index 21fb01bcec5..1d4a85ed497 100644 --- a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java @@ -25,14 +25,12 @@ import static bisq.cli.opts.OptLabel.OPT_PAYMENT_ACCOUNT_FORM; import static java.lang.String.format; -import static joptsimple.internal.Strings.EMPTY; public class CreatePaymentAcctOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec paymentAcctFormPathOpt = parser.accepts(OPT_PAYMENT_ACCOUNT_FORM, "path to json payment account form") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public CreatePaymentAcctOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java index 80537ffc89d..254fed91e65 100644 --- a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_ADDRESS; -import static joptsimple.internal.Strings.EMPTY; public class GetAddressBalanceOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "wallet btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetAddressBalanceOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java index c54f65f42e9..da66b6b41f1 100644 --- a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_CURRENCY_CODE; -import static joptsimple.internal.Strings.EMPTY; public class GetBTCMarketPriceOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency-code") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetBTCMarketPriceOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java index 600e7672c45..159f459be2d 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to get") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetOfferOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java index 29360886f87..dbc1bf83a99 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java @@ -22,17 +22,14 @@ import static bisq.cli.opts.OptLabel.OPT_CURRENCY_CODE; import static bisq.cli.opts.OptLabel.OPT_DIRECTION; -import static joptsimple.internal.Strings.EMPTY; public class GetOffersOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec directionOpt = parser.accepts(OPT_DIRECTION, "offer direction (buy|sell)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec currencyCodeOpt = parser.accepts(OPT_CURRENCY_CODE, "currency code (eur|usd|...)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetOffersOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java index ef5bd5b454c..52fdfd52999 100644 --- a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java @@ -21,14 +21,12 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_PAYMENT_METHOD_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetPaymentAcctFormOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec paymentMethodIdOpt = parser.accepts(OPT_PAYMENT_METHOD_ID, "id of payment method type used by a payment account") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetPaymentAcctFormOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java index 2b7681f3c69..3ea344a06ee 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java @@ -22,13 +22,11 @@ import static bisq.cli.opts.OptLabel.OPT_SHOW_CONTRACT; import static bisq.cli.opts.OptLabel.OPT_TRADE_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetTradeOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec tradeIdOpt = parser.accepts(OPT_TRADE_ID, "id of trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec showContractOpt = parser.accepts(OPT_SHOW_CONTRACT, "show trade's json contract") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java index d4266eb9ff7..83cc4f48fa7 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_TRANSACTION_ID; -import static joptsimple.internal.Strings.EMPTY; public class GetTransactionOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec txIdOpt = parser.accepts(OPT_TRANSACTION_ID, "id of transaction") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public GetTransactionOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java index 428555a3493..9a3a8e7525d 100644 --- a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java @@ -22,17 +22,14 @@ import static bisq.cli.opts.OptLabel.OPT_DISPUTE_AGENT_TYPE; import static bisq.cli.opts.OptLabel.OPT_REGISTRATION_KEY; -import static joptsimple.internal.Strings.EMPTY; public class RegisterDisputeAgentOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec disputeAgentTypeOpt = parser.accepts(OPT_DISPUTE_AGENT_TYPE, "dispute agent type") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec registrationKeyOpt = parser.accepts(OPT_REGISTRATION_KEY, "registration key") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public RegisterDisputeAgentOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java index 5b9a3915941..dd1de67f057 100644 --- a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java @@ -21,13 +21,11 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_WALLET_PASSWORD; -import static joptsimple.internal.Strings.EMPTY; public class RemoveWalletPasswordOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public RemoveWalletPasswordOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java index 3bffce785c4..73025a17bf0 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java @@ -28,12 +28,10 @@ public class SendBsqOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination bsq address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of bsq to send") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "optional tx fee rate (sats/byte)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java index 8c3f9762019..e49e961a1f5 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java @@ -29,12 +29,10 @@ public class SendBtcOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec amountOpt = parser.accepts(OPT_AMOUNT, "amount of btc to send") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "optional tx fee rate (sats/byte)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java index 9d4b5e71b3e..f6b9f7c7a06 100644 --- a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java @@ -21,14 +21,12 @@ import joptsimple.OptionSpec; import static bisq.cli.opts.OptLabel.OPT_TX_FEE_RATE; -import static joptsimple.internal.Strings.EMPTY; public class SetTxFeeRateOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec feeRateOpt = parser.accepts(OPT_TX_FEE_RATE, "tx fee rate preference (sats/byte)") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); public SetTxFeeRateOptionParser(String[] args) { super(args); diff --git a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java index d55b1bf33b4..483d56f649e 100644 --- a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java @@ -27,8 +27,7 @@ public class SetWalletPasswordOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec newPasswordOpt = parser.accepts(OPT_NEW_WALLET_PASSWORD, "new bisq wallet password") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java index 75ef2885b04..589860cd63e 100644 --- a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java @@ -23,17 +23,14 @@ import static bisq.cli.opts.OptLabel.OPT_FEE_CURRENCY; import static bisq.cli.opts.OptLabel.OPT_OFFER_ID; import static bisq.cli.opts.OptLabel.OPT_PAYMENT_ACCOUNT; -import static joptsimple.internal.Strings.EMPTY; public class TakeOfferOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec offerIdOpt = parser.accepts(OPT_OFFER_ID, "id of offer to take") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec paymentAccountIdOpt = parser.accepts(OPT_PAYMENT_ACCOUNT, "id of payment account used for trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec takerFeeCurrencyCodeOpt = parser.accepts(OPT_FEE_CURRENCY, "taker fee currency code (bsq|btc)") .withOptionalArg() diff --git a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java index 4446138dd37..db2f1aa2606 100644 --- a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java @@ -22,13 +22,11 @@ import static bisq.cli.opts.OptLabel.OPT_TIMEOUT; import static bisq.cli.opts.OptLabel.OPT_WALLET_PASSWORD; -import static joptsimple.internal.Strings.EMPTY; public class UnlockWalletOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec passwordOpt = parser.accepts(OPT_WALLET_PASSWORD, "bisq wallet password") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec unlockTimeoutOpt = parser.accepts(OPT_TIMEOUT, "wallet unlock timeout (s)") .withRequiredArg() diff --git a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java index f47432fa860..b60829e9bef 100644 --- a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java @@ -28,12 +28,10 @@ public class WithdrawFundsOptionParser extends AbstractMethodOptionParser implements MethodOpts { final OptionSpec tradeIdOpt = parser.accepts(OPT_TRADE_ID, "id of trade") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec addressOpt = parser.accepts(OPT_ADDRESS, "destination btc address") - .withRequiredArg() - .defaultsTo(EMPTY); + .withRequiredArg(); final OptionSpec memoOpt = parser.accepts(OPT_MEMO, "optional tx memo") .withOptionalArg() @@ -53,6 +51,9 @@ public WithdrawFundsOptionParser parse() { if (!options.has(tradeIdOpt)) throw new IllegalArgumentException("no trade id specified"); + if (!options.has(addressOpt)) + throw new IllegalArgumentException("no destination address specified"); + return this; } From 62ff79d718fda4bb8c4fed0e73d33b557149e45f Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:09:41 -0300 Subject: [PATCH 02/15] Update cli getoffers smoke test to posix style opts --- .../test/java/bisq/cli/GetOffersSmokeTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java b/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java index 49f849e0cc7..f613aea358c 100644 --- a/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java +++ b/cli/src/test/java/bisq/cli/GetOffersSmokeTest.java @@ -16,24 +16,24 @@ public class GetOffersSmokeTest { public static void main(String[] args) { out.println(">>> getoffers buy usd"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "usd"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=usd"}); out.println(">>> getoffers sell usd"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "usd"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=usd"}); out.println(">>> getoffers buy eur"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "eur"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=eur"}); out.println(">>> getoffers sell eur"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "eur"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=eur"}); out.println(">>> getoffers buy gbp"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "gbp"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=gbp"}); out.println(">>> getoffers sell gbp"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "gbp"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=gbp"}); out.println(">>> getoffers buy brl"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "buy", "brl"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=buy", "--currency-code=brl"}); out.println(">>> getoffers sell brl"); - CliMain.main(new String[]{"--password=xyz", "getoffers", "sell", "brl"}); + CliMain.main(new String[]{"--password=xyz", "getoffers", "--direction=sell", "--currency-code=brl"}); } } From d01a7b7feec7167b66ec808529d2e63ffe248442 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:10:42 -0300 Subject: [PATCH 03/15] Improve required-argument opt validation Make sure non-empty opt values are present in the parsers' custom validation, or jsimpleopt will throw exceptions with stylistically inconsisent messages. --- .../bisq/cli/opts/CancelOfferOptionParser.java | 2 +- .../bisq/cli/opts/CreateOfferOptionParser.java | 16 +++++++++++----- .../cli/opts/CreatePaymentAcctOptionParser.java | 2 +- .../cli/opts/GetAddressBalanceOptionParser.java | 2 +- .../cli/opts/GetBTCMarketPriceOptionParser.java | 2 +- .../java/bisq/cli/opts/GetOfferOptionParser.java | 2 +- .../bisq/cli/opts/GetOffersOptionParser.java | 4 ++-- .../cli/opts/GetPaymentAcctFormOptionParser.java | 2 +- .../java/bisq/cli/opts/GetTradeOptionParser.java | 2 +- .../cli/opts/GetTransactionOptionParser.java | 2 +- .../opts/RegisterDisputeAgentOptionParser.java | 4 ++-- .../opts/RemoveWalletPasswordOptionParser.java | 2 +- .../java/bisq/cli/opts/SendBsqOptionParser.java | 4 ++-- .../java/bisq/cli/opts/SendBtcOptionParser.java | 4 ++-- .../bisq/cli/opts/SetTxFeeRateOptionParser.java | 2 +- .../cli/opts/SetWalletPasswordOptionParser.java | 2 +- .../bisq/cli/opts/TakeOfferOptionParser.java | 4 ++-- .../bisq/cli/opts/UnlockWalletOptionParser.java | 2 +- .../bisq/cli/opts/WithdrawFundsOptionParser.java | 4 ++-- 19 files changed, 35 insertions(+), 29 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java index 5ed33ed41fb..c35ffc7bfb4 100644 --- a/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CancelOfferOptionParser.java @@ -38,7 +38,7 @@ public CancelOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java index bbaf68bab36..42cf8ad1550 100644 --- a/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreateOfferOptionParser.java @@ -70,22 +70,28 @@ public CreateOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentAccountIdOpt)) + if (!options.has(paymentAccountIdOpt) || options.valueOf(paymentAccountIdOpt).isEmpty()) throw new IllegalArgumentException("no payment account id specified"); - if (!options.has(directionOpt)) + if (!options.has(directionOpt) || options.valueOf(directionOpt).isEmpty()) throw new IllegalArgumentException("no direction (buy|sell) specified"); - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no btc amount specified"); if (!options.has(mktPriceMarginOpt) && !options.has(fixedPriceOpt)) throw new IllegalArgumentException("no market price margin or fixed price specified"); - if (!options.has(securityDepositOpt)) + if (options.has(mktPriceMarginOpt) && options.valueOf(mktPriceMarginOpt).isEmpty()) + throw new IllegalArgumentException("no market price margin specified"); + + if (options.has(fixedPriceOpt) && options.valueOf(fixedPriceOpt).isEmpty()) + throw new IllegalArgumentException("no fixed price specified"); + + if (!options.has(securityDepositOpt) || options.valueOf(securityDepositOpt).isEmpty()) throw new IllegalArgumentException("no security deposit specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java index 1d4a85ed497..907f7ca0ed5 100644 --- a/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/CreatePaymentAcctOptionParser.java @@ -43,7 +43,7 @@ public CreatePaymentAcctOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentAcctFormPathOpt)) + if (!options.has(paymentAcctFormPathOpt) || options.valueOf(paymentAcctFormPathOpt).isEmpty()) throw new IllegalArgumentException("no path to json payment account form specified"); Path path = Paths.get(options.valueOf(paymentAcctFormPathOpt)); diff --git a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java index 254fed91e65..4ad693ca4c6 100644 --- a/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetAddressBalanceOptionParser.java @@ -38,7 +38,7 @@ public GetAddressBalanceOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no address specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java index da66b6b41f1..8d6585631bb 100644 --- a/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetBTCMarketPriceOptionParser.java @@ -38,7 +38,7 @@ public GetBTCMarketPriceOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java index 159f459be2d..1a849654cf7 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOfferOptionParser.java @@ -38,7 +38,7 @@ public GetOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java index dbc1bf83a99..f8a4dee839f 100644 --- a/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetOffersOptionParser.java @@ -42,10 +42,10 @@ public GetOffersOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(directionOpt)) + if (!options.has(directionOpt) || options.valueOf(directionOpt).isEmpty()) throw new IllegalArgumentException("no direction (buy|sell) specified"); - if (!options.has(currencyCodeOpt)) + if (!options.has(currencyCodeOpt) || options.valueOf(currencyCodeOpt).isEmpty()) throw new IllegalArgumentException("no currency code specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java index 52fdfd52999..508069c2f30 100644 --- a/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetPaymentAcctFormOptionParser.java @@ -39,7 +39,7 @@ public GetPaymentAcctFormOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(paymentMethodIdOpt)) + if (!options.has(paymentMethodIdOpt) || options.valueOf(paymentMethodIdOpt).isEmpty()) throw new IllegalArgumentException("no payment method id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java index 3ea344a06ee..1419f3ed6a6 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTradeOptionParser.java @@ -44,7 +44,7 @@ public GetTradeOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(tradeIdOpt)) + if (!options.has(tradeIdOpt) || options.valueOf(tradeIdOpt).isEmpty()) throw new IllegalArgumentException("no trade id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java index 83cc4f48fa7..0b245cb1560 100644 --- a/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/GetTransactionOptionParser.java @@ -38,7 +38,7 @@ public GetTransactionOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(txIdOpt)) + if (!options.has(txIdOpt) || options.valueOf(txIdOpt).isEmpty()) throw new IllegalArgumentException("no tx id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java index 9a3a8e7525d..b1c8f0bba8f 100644 --- a/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RegisterDisputeAgentOptionParser.java @@ -42,10 +42,10 @@ public RegisterDisputeAgentOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(disputeAgentTypeOpt)) + if (!options.has(disputeAgentTypeOpt) || options.valueOf(disputeAgentTypeOpt).isEmpty()) throw new IllegalArgumentException("no dispute agent type specified"); - if (!options.has(registrationKeyOpt)) + if (!options.has(registrationKeyOpt) || options.valueOf(registrationKeyOpt).isEmpty()) throw new IllegalArgumentException("no registration key specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java index dd1de67f057..db556a0fd89 100644 --- a/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/RemoveWalletPasswordOptionParser.java @@ -38,7 +38,7 @@ public RemoveWalletPasswordOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java index 73025a17bf0..ad9ab87cbba 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBsqOptionParser.java @@ -48,10 +48,10 @@ public SendBsqOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no bsq address specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no bsq amount specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java index e49e961a1f5..f7d8fd56835 100644 --- a/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SendBtcOptionParser.java @@ -53,10 +53,10 @@ public SendBtcOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no btc address specified"); - if (!options.has(amountOpt)) + if (!options.has(amountOpt) || options.valueOf(amountOpt).isEmpty()) throw new IllegalArgumentException("no btc amount specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java index f6b9f7c7a06..f7ed113cb3c 100644 --- a/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetTxFeeRateOptionParser.java @@ -39,7 +39,7 @@ public SetTxFeeRateOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(feeRateOpt)) + if (!options.has(feeRateOpt) || options.valueOf(feeRateOpt).isEmpty()) throw new IllegalArgumentException("no tx fee rate specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java index 483d56f649e..1caa09232c1 100644 --- a/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/SetWalletPasswordOptionParser.java @@ -44,7 +44,7 @@ public SetWalletPasswordOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java index 589860cd63e..67fbdd8c86d 100644 --- a/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/TakeOfferOptionParser.java @@ -47,10 +47,10 @@ public TakeOfferOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(offerIdOpt)) + if (!options.has(offerIdOpt) || options.valueOf(offerIdOpt).isEmpty()) throw new IllegalArgumentException("no offer id specified"); - if (!options.has(paymentAccountIdOpt)) + if (!options.has(paymentAccountIdOpt) || options.valueOf(paymentAccountIdOpt).isEmpty()) throw new IllegalArgumentException("no payment account id specified"); return this; diff --git a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java index db2f1aa2606..2908be42bfd 100644 --- a/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/UnlockWalletOptionParser.java @@ -44,7 +44,7 @@ public UnlockWalletOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(passwordOpt)) + if (!options.has(passwordOpt) || options.valueOf(passwordOpt).isEmpty()) throw new IllegalArgumentException("no password specified"); if (!options.has(unlockTimeoutOpt) || options.valueOf(unlockTimeoutOpt) <= 0) diff --git a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java index b60829e9bef..bdba6437603 100644 --- a/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/WithdrawFundsOptionParser.java @@ -48,10 +48,10 @@ public WithdrawFundsOptionParser parse() { if (options.has(helpOpt)) return this; - if (!options.has(tradeIdOpt)) + if (!options.has(tradeIdOpt) || options.valueOf(tradeIdOpt).isEmpty()) throw new IllegalArgumentException("no trade id specified"); - if (!options.has(addressOpt)) + if (!options.has(addressOpt) || options.valueOf(addressOpt).isEmpty()) throw new IllegalArgumentException("no destination address specified"); return this; From 304781ce92c13df187b26d1ac2ceec5f5eac3825 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:15:01 -0300 Subject: [PATCH 04/15] Add jupiter test support to :cli subproject --- build.gradle | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/build.gradle b/build.gradle index a533c31f374..921f074addf 100644 --- a/build.gradle +++ b/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() } } From 9c12b310199cce330977fddca6d93c9d767552f2 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 07:16:36 -0300 Subject: [PATCH 05/15] Add new cli option parser test Does not check every combination of options for every method, but we finally have an option parser test case to build on. --- .../java/bisq/cli/opt/OptionParsersTest.java | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 cli/src/test/java/bisq/cli/opt/OptionParsersTest.java diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java new file mode 100644 index 00000000000..9e7cd45cb0e --- /dev/null +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -0,0 +1,160 @@ +package bisq.cli.opt; + +import org.junit.jupiter.api.Test; + +import static bisq.cli.Method.canceloffer; +import static bisq.cli.Method.createoffer; +import static bisq.cli.Method.createpaymentacct; +import static bisq.cli.opts.OptLabel.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + + + +import bisq.cli.opts.CancelOfferOptionParser; +import bisq.cli.opts.CreateOfferOptionParser; +import bisq.cli.opts.CreatePaymentAcctOptionParser; + + +public class OptionParsersTest { + + private static final String PASSWORD_OPT = "--" + OPT_PASSWORD + "=" + "xyz"; + + // CancelOffer opt parsing tests + + @Test + public void testCancelOfferWithMissingOfferIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name() + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("no offer id specified", exception.getMessage()); + } + + @Test + public void testCancelOfferWithEmptyOfferIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID + "=" // missing opt value + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("no offer id specified", exception.getMessage()); + } + + @Test + public void testCancelOfferWithMissingOfferIdValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID // missing equals sign & opt value + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CancelOfferOptionParser(args).parse()); + assertEquals("Option offer-id requires an argument", exception.getMessage()); + } + + @Test + public void testValidCancelOfferOpts() { + String[] args = new String[]{ + PASSWORD_OPT, + canceloffer.name(), + "--" + OPT_OFFER_ID + "=" + "ABC-OFFER-ID" + }; + new CancelOfferOptionParser(args).parse(); + } + + // CreateOffer opt parsing tests + + @Test + public void testCreateOfferOptParserWithMissingPaymentAccountIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name() + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no payment account id specified", exception.getMessage()); + } + + @Test + public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no direction (buy|sell) specified", exception.getMessage()); + } + + @Test + public void testCreateOfferOptParserWithMissingDirectionOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123", + "--" + OPT_DIRECTION + "=" + "" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("no direction (buy|sell) specified", exception.getMessage()); + } + + @Test + public void testValidCreateOfferOpts() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + "=" + "abc-payment-acct-id-123", + "--" + OPT_DIRECTION + "=" + "BUY", + "--" + OPT_CURRENCY_CODE + "=" + "EUR", + "--" + OPT_AMOUNT + "=" + "0.125", + "--" + OPT_MKT_PRICE_MARGIN + "=" + "0.0", + "--" + OPT_SECURITY_DEPOSIT + "=" + "25.0" + }; + } + + // CreatePaymentAcct opt parser tests + + @Test + public void testCreatePaymentAcctOptParserWithMissingPaymentFormOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name() + // OPT_PAYMENT_ACCOUNT_FORM + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("no path to json payment account form specified", exception.getMessage()); + } + + @Test + public void testCreatePaymentAcctOptParserWithMissingPaymentFormOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name(), + "--" + OPT_PAYMENT_ACCOUNT_FORM + "=" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("no path to json payment account form specified", exception.getMessage()); + } + + @Test + public void testCreatePaymentAcctOptParserWithInvalidPaymentFormOptValueShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createpaymentacct.name(), + "--" + OPT_PAYMENT_ACCOUNT_FORM + "=" + "/tmp/milkyway/solarsystem/mars" + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreatePaymentAcctOptionParser(args).parse()); + assertEquals("json payment account form '/tmp/milkyway/solarsystem/mars' could not be found", + exception.getMessage()); + } +} From a13ef79e6e606cefa04587e56bcff1181ecfb335 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 5 Mar 2021 12:28:10 -0300 Subject: [PATCH 06/15] Handle require-arg options missing the = sign This change ensure a stylistically consistent error message is printed to the CLI console if an option is present, but without the '=' sign. --- .../cli/opts/AbstractMethodOptionParser.java | 25 ++++++++++++++++--- .../java/bisq/cli/opt/OptionParsersTest.java | 15 ++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java b/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java index 4c495c06e5e..25256eb6a99 100644 --- a/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java +++ b/cli/src/main/java/bisq/cli/opts/AbstractMethodOptionParser.java @@ -17,11 +17,13 @@ package bisq.cli.opts; +import joptsimple.OptionException; import joptsimple.OptionParser; import joptsimple.OptionSet; import joptsimple.OptionSpec; import java.util.List; +import java.util.function.Function; import lombok.Getter; @@ -48,12 +50,29 @@ protected AbstractMethodOptionParser(String[] args) { } public AbstractMethodOptionParser parse() { - options = parser.parse(new ArgumentList(args).getMethodArguments()); - nonOptionArguments = (List) options.nonOptionArguments(); - return this; + try { + options = parser.parse(new ArgumentList(args).getMethodArguments()); + //noinspection unchecked + nonOptionArguments = (List) options.nonOptionArguments(); + return this; + } catch (OptionException ex) { + throw new IllegalArgumentException(cliExceptionMessageStyle.apply(ex), ex); + } } public boolean isForHelp() { return options.has(helpOpt); } + + private final Function cliExceptionMessageStyle = (ex) -> { + if (ex.getMessage() == null) + return null; + + var optionToken = "option "; + var cliMessage = ex.getMessage().toLowerCase(); + if (cliMessage.startsWith(optionToken) && cliMessage.length() > optionToken.length()) { + cliMessage = cliMessage.substring(cliMessage.indexOf(" ") + 1); + } + return cliMessage; + }; } diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java index 9e7cd45cb0e..d853da66d6a 100644 --- a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -54,7 +54,7 @@ public void testCancelOfferWithMissingOfferIdValueShouldThrowException() { }; Throwable exception = assertThrows(RuntimeException.class, () -> new CancelOfferOptionParser(args).parse()); - assertEquals("Option offer-id requires an argument", exception.getMessage()); + assertEquals("offer-id requires an argument", exception.getMessage()); } @Test @@ -80,6 +80,18 @@ public void testCreateOfferOptParserWithMissingPaymentAccountIdOptShouldThrowExc assertEquals("no payment account id specified", exception.getMessage()); } + @Test + public void testCreateOfferOptParserWithEmptyPaymentAccountIdOptShouldThrowException() { + String[] args = new String[]{ + PASSWORD_OPT, + createoffer.name(), + "--" + OPT_PAYMENT_ACCOUNT + }; + Throwable exception = assertThrows(RuntimeException.class, () -> + new CreateOfferOptionParser(args).parse()); + assertEquals("payment-account requires an argument", exception.getMessage()); + } + @Test public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException() { String[] args = new String[]{ @@ -92,6 +104,7 @@ public void testCreateOfferOptParserWithMissingDirectionOptShouldThrowException( assertEquals("no direction (buy|sell) specified", exception.getMessage()); } + @Test public void testCreateOfferOptParserWithMissingDirectionOptValueShouldThrowException() { String[] args = new String[]{ From 99fea74e096af799d53648caa31d9c0923d0658b Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 7 Mar 2021 19:51:29 -0300 Subject: [PATCH 07/15] Validate offer <-> payment-acct in createoffer This change prohibits creation of new offers with incompatible payment accounts with the api. - PaymentAccountUtil Renamed isAnyTakerPaymentAccountValidForOffer -> isAnyPaymentAccountValidForOffer. Renmaed isTakerPaymentAccountValidForOffer -> isPaymentAccountValidForOffer. Deleted commented code. - PaymentAccounts: Adjusted to PaymentAccountUtil method name changes. - OfferFilter: Adjusted to PaymentAccountUtil method name changes. - OfferBookViewModelTest: Adjusted to PaymentAccountUtil method name changes. - Add CoreOffersService#verifyPaymentAccountIsValidForOffer - ValidateCreateOfferTest, OfferTest: Added test cases. --- .../method/offer/ValidateCreateOfferTest.java | 37 ++++++++++++++++ .../java/bisq/apitest/scenario/OfferTest.java | 2 + .../java/bisq/core/api/CoreOffersService.java | 12 +++++ .../java/bisq/core/offer/OfferFilter.java | 2 +- .../bisq/core/payment/PaymentAccountUtil.java | 31 +++---------- .../bisq/core/payment/PaymentAccounts.java | 2 +- .../offerbook/OfferBookViewModelTest.java | 44 +++++++++---------- 7 files changed, 81 insertions(+), 49 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java b/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java index a979b7bf239..65f8c83f607 100644 --- a/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/offer/ValidateCreateOfferTest.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.TestMethodOrder; import static bisq.core.btc.wallet.Restrictions.getDefaultBuyerSecurityDepositAsPercent; +import static java.lang.String.format; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -55,4 +56,40 @@ public void testAmtTooLargeShouldThrowException() { assertEquals("UNKNOWN: An error occurred at task: ValidateOffer", exception.getMessage()); } + + @Test + @Order(2) + public void testNoMatchingEURPaymentAccountShouldThrowException() { + PaymentAccount chfAccount = createDummyF2FAccount(aliceClient, "ch"); + @SuppressWarnings("ResultOfMethodCallIgnored") + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + aliceClient.createFixedPricedOffer("buy", + "eur", + 10000000L, + 10000000L, + "40000.0000", + getDefaultBuyerSecurityDepositAsPercent(), + chfAccount.getId(), + "btc")); + String expectedError = format("UNKNOWN: cannot create EUR offer with payment account %s", chfAccount.getId()); + assertEquals(expectedError, exception.getMessage()); + } + + @Test + @Order(2) + public void testNoMatchingCADPaymentAccountShouldThrowException() { + PaymentAccount audAccount = createDummyF2FAccount(aliceClient, "au"); + @SuppressWarnings("ResultOfMethodCallIgnored") + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + aliceClient.createFixedPricedOffer("buy", + "cad", + 10000000L, + 10000000L, + "63000.0000", + getDefaultBuyerSecurityDepositAsPercent(), + audAccount.getId(), + "btc")); + String expectedError = format("UNKNOWN: cannot create CAD offer with payment account %s", audAccount.getId()); + assertEquals(expectedError, exception.getMessage()); + } } diff --git a/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java b/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java index b01a9486ea5..c82cbaef90e 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/OfferTest.java @@ -42,6 +42,8 @@ public class OfferTest extends AbstractOfferTest { public void testAmtTooLargeShouldThrowException() { ValidateCreateOfferTest test = new ValidateCreateOfferTest(); test.testAmtTooLargeShouldThrowException(); + test.testNoMatchingEURPaymentAccountShouldThrowException(); + test.testNoMatchingCADPaymentAccountShouldThrowException(); } @Test diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 63420e0bbb1..3e6c6f621a2 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -54,6 +54,7 @@ import static bisq.core.locale.CurrencyUtil.isCryptoCurrency; import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; +import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -174,6 +175,8 @@ void createAndPlaceOffer(String currencyCode, buyerSecurityDeposit, paymentAccount); + verifyPaymentAccountIsValidForNewOffer(offer, paymentAccount); + // We don't support atm funding from external wallet to keep it simple. boolean useSavingsWallet = true; //noinspection ConstantConditions @@ -219,6 +222,15 @@ void cancelOffer(String id) { }); } + private void verifyPaymentAccountIsValidForNewOffer(Offer offer, PaymentAccount paymentAccount) { + if (!isPaymentAccountValidForOffer(offer, paymentAccount)) { + String error = String.format("cannot create %s offer with payment account %s", + offer.getOfferPayload().getCounterCurrencyCode(), + paymentAccount.getId()); + throw new IllegalStateException(error); + } + } + private void placeOffer(Offer offer, double buyerSecurityDeposit, long triggerPrice, diff --git a/core/src/main/java/bisq/core/offer/OfferFilter.java b/core/src/main/java/bisq/core/offer/OfferFilter.java index c22231de5bb..bfd37f2af12 100644 --- a/core/src/main/java/bisq/core/offer/OfferFilter.java +++ b/core/src/main/java/bisq/core/offer/OfferFilter.java @@ -134,7 +134,7 @@ public Result canTakeOffer(Offer offer, boolean isTakerApiUser) { public boolean isAnyPaymentAccountValidForOffer(Offer offer) { return user.getPaymentAccounts() != null && - PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer(offer, user.getPaymentAccounts()); + PaymentAccountUtil.isAnyPaymentAccountValidForOffer(offer, user.getPaymentAccounts()); } public boolean hasSameProtocolVersion(Offer offer) { diff --git a/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java b/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java index 08a96db151b..fc810b017d4 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccountUtil.java @@ -41,10 +41,10 @@ @Slf4j public class PaymentAccountUtil { - public static boolean isAnyTakerPaymentAccountValidForOffer(Offer offer, - Collection takerPaymentAccounts) { - for (PaymentAccount takerPaymentAccount : takerPaymentAccounts) { - if (isTakerPaymentAccountValidForOffer(offer, takerPaymentAccount)) + public static boolean isAnyPaymentAccountValidForOffer(Offer offer, + Collection paymentAccounts) { + for (PaymentAccount paymentAccount : paymentAccounts) { + if (isPaymentAccountValidForOffer(offer, paymentAccount)) return true; } return false; @@ -55,7 +55,7 @@ public static ObservableList getPossiblePaymentAccounts(Offer of AccountAgeWitnessService accountAgeWitnessService) { ObservableList result = FXCollections.observableArrayList(); result.addAll(paymentAccounts.stream() - .filter(paymentAccount -> isTakerPaymentAccountValidForOffer(offer, paymentAccount)) + .filter(paymentAccount -> isPaymentAccountValidForOffer(offer, paymentAccount)) .filter(paymentAccount -> isAmountValidForOffer(offer, paymentAccount, accountAgeWitnessService)) .collect(Collectors.toList())); return result; @@ -79,7 +79,7 @@ public static String getInfoForMismatchingPaymentMethodLimits(Offer offer, Payme "Payment method from offer: " + offer.getPaymentMethod().toString(); } - public static boolean isTakerPaymentAccountValidForOffer(Offer offer, PaymentAccount paymentAccount) { + public static boolean isPaymentAccountValidForOffer(Offer offer, PaymentAccount paymentAccount) { return new ReceiptValidator(offer, paymentAccount).isValid(); } @@ -144,23 +144,4 @@ public static Optional findPaymentAccount(PaymentAccountPayload filter(e -> e.getPaymentAccountPayload().equals(paymentAccountPayload)) .findAny(); } - - // TODO no code duplication found in UI code (added for API) - // That is optional and set to null if not supported (AltCoins,...) - /* public static String getCountryCode(PaymentAccount paymentAccount) { - if (paymentAccount instanceof CountryBasedPaymentAccount) { - Country country = ((CountryBasedPaymentAccount) paymentAccount).getCountry(); - return country != null ? country.code : null; - } else { - return null; - } - }*/ - - // TODO no code duplication found in UI code (added for API) - /*public static long getMaxTradeLimit(AccountAgeWitnessService accountAgeWitnessService, PaymentAccount paymentAccount, String currencyCode) { - if (paymentAccount != null) - return accountAgeWitnessService.getMyTradeLimit(paymentAccount, currencyCode); - else - return 0; - }*/ } diff --git a/core/src/main/java/bisq/core/payment/PaymentAccounts.java b/core/src/main/java/bisq/core/payment/PaymentAccounts.java index c27c883449d..6f871ce20f9 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccounts.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccounts.java @@ -41,7 +41,7 @@ class PaymentAccounts { private final BiFunction validator; PaymentAccounts(Set accounts, AccountAgeWitnessService accountAgeWitnessService) { - this(accounts, accountAgeWitnessService, PaymentAccountUtil::isTakerPaymentAccountValidForOffer); + this(accounts, accountAgeWitnessService, PaymentAccountUtil::isPaymentAccountValidForOffer); } PaymentAccounts(Set accounts, AccountAgeWitnessService accountAgeWitnessService, diff --git a/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java b/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java index dc48c8c5459..68b8718e35c 100644 --- a/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java +++ b/desktop/src/test/java/bisq/desktop/main/offer/offerbook/OfferBookViewModelTest.java @@ -105,46 +105,46 @@ public void testIsAnyPaymentAccountValidForOffer() { Collection paymentAccounts; paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "DE", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // empty paymentAccounts paymentAccounts = new ArrayList<>(); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer(getSEPAPaymentMethod("EUR", "AT", + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer(getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // simple cases: same payment methods // offer: alipay paymentAccount: alipay - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getAliPayAccount("CNY"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getAliPayPaymentMethod("EUR"), paymentAccounts)); // offer: ether paymentAccount: ether - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getCryptoAccount("ETH"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getBlockChainsPaymentMethod("ETH"), paymentAccounts)); // offer: sepa paymentAccount: sepa - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "AT", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // offer: nationalBank paymentAccount: nationalBank - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // offer: SameBank paymentAccount: SameBank - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSameBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // offer: sepa paymentAccount: sepa - diff. country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "DE", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); @@ -152,79 +152,79 @@ public void testIsAnyPaymentAccountValidForOffer() { // offer: sepa paymentAccount: sepa - same country, same currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSepaAccount("EUR", "AT", "1212324", new ArrayList<>(Arrays.asList("AT", "DE"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // offer: sepa paymentAccount: nationalBank - same country, same currency // wrong method paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "AT", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("USD", "US", "XXX"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "FR", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // sepa wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("EUR", "CH", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // sepa wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getNationalBankAccount("CHF", "DE", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getSEPAPaymentMethod("EUR", "AT", new ArrayList<>(Arrays.asList("AT", "DE")), "PSK"), paymentAccounts)); // same bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "PSK"))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // not same bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "AT", "Raika"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // same bank, wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("EUR", "DE", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // same bank, wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSameBankAccount("USD", "AT", "PSK"))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "AT", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertTrue(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertTrue(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, missing bank paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "AT", "PSK", new ArrayList<>(FXCollections.singletonObservableList("Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, wrong country paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("EUR", "FR", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); // spec. bank, wrong currency paymentAccounts = new ArrayList<>(FXCollections.singletonObservableList(getSpecificBanksAccount("USD", "AT", "PSK", new ArrayList<>(Arrays.asList("PSK", "Raika"))))); - assertFalse(PaymentAccountUtil.isAnyTakerPaymentAccountValidForOffer( + assertFalse(PaymentAccountUtil.isAnyPaymentAccountValidForOffer( getNationalBankPaymentMethod("EUR", "AT", "PSK"), paymentAccounts)); //TODO add more tests From baca7001ea5081784691e20f1fcf2ad2e1fb4a84 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sun, 7 Mar 2021 20:05:39 -0300 Subject: [PATCH 08/15] Fix unnecessary use of fully qualified name 'String.format' --- core/src/main/java/bisq/core/api/CoreOffersService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 3e6c6f621a2..8a5b95cf31e 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -55,6 +55,7 @@ import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; +import static java.lang.String.*; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -224,7 +225,7 @@ void cancelOffer(String id) { private void verifyPaymentAccountIsValidForNewOffer(Offer offer, PaymentAccount paymentAccount) { if (!isPaymentAccountValidForOffer(offer, paymentAccount)) { - String error = String.format("cannot create %s offer with payment account %s", + String error = format("cannot create %s offer with payment account %s", offer.getOfferPayload().getCounterCurrencyCode(), paymentAccount.getId()); throw new IllegalStateException(error); From b4f4d90e05d4b3779fed765dce0a200b796aa1d4 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 14:47:30 -0300 Subject: [PATCH 09/15] Add tradeCurrencies field to Transferwise acct form These changes make tradeCurrencies a required twise acct form field. Addresses issue https://github.com/bisq-network/bisq/issues/5281 - Added comment to PaymentAccountForm about special 'tradeCurrencies' field handling. - Added support for Transferwise 'tradeCurrencies' to PaymentAccountTypeAdapter. - Added instance of isTransferwiseAccount check to PaymentAccount. - Added methods to CurrencyUtil to convert comma delimited code list to TradeCurrency list. - Added methods to ReflectionUtil for getting fields and methods on class. - Added required field check to CorePaymentAccountsService. - Added CreatePaymentAccount test cases. --- .../payment/AbstractPaymentAccountTest.java | 1 + .../payment/CreatePaymentAccountTest.java | 95 ++++++++++++++-- .../apitest/scenario/PaymentAccountTest.java | 18 ++- .../bisq/common/util/ReflectionUtils.java | 32 ++++++ .../core/api/CorePaymentAccountsService.java | 10 ++ .../core/api/model/PaymentAccountForm.java | 2 +- .../api/model/PaymentAccountTypeAdapter.java | 107 +++++++++++++++--- .../java/bisq/core/locale/CurrencyUtil.java | 28 +++++ .../bisq/core/payment/PaymentAccount.java | 4 + 9 files changed, 261 insertions(+), 36 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java index 44c56f08a05..034bc3aeb02 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/AbstractPaymentAccountTest.java @@ -85,6 +85,7 @@ public class AbstractPaymentAccountTest extends MethodTest { static final String PROPERTY_NAME_SALT = "salt"; static final String PROPERTY_NAME_SORT_CODE = "sortCode"; static final String PROPERTY_NAME_STATE = "state"; + static final String PROPERTY_NAME_TRADE_CURRENCIES = "tradeCurrencies"; static final String PROPERTY_NAME_USERNAME = "userName"; static final Gson GSON = new GsonBuilder() diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index a194c4aa1f9..2cc5bfcf857 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -17,6 +17,7 @@ package bisq.apitest.method.payment; +import bisq.core.locale.TradeCurrency; import bisq.core.payment.AdvancedCashAccount; import bisq.core.payment.AliPayAccount; import bisq.core.payment.AustraliaPayid; @@ -50,8 +51,12 @@ import bisq.core.payment.payload.SameBankAccountPayload; import bisq.core.payment.payload.SpecificBanksAccountPayload; +import io.grpc.StatusRuntimeException; + import java.io.File; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import lombok.extern.slf4j.Slf4j; @@ -65,12 +70,11 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; -import static bisq.core.locale.CurrencyUtil.getAllAdvancedCashCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllMoneyGramCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllRevolutCurrencies; -import static bisq.core.locale.CurrencyUtil.getAllUpholdCurrencies; +import static bisq.core.locale.CurrencyUtil.*; import static bisq.core.payment.payload.PaymentMethod.*; +import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -737,27 +741,98 @@ public void testCreateSwishAccount(TestInfo testInfo) { } @Test - public void testCreateTransferwiseAccount(TestInfo testInfo) { + public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); verifyEmptyForm(emptyForm, TRANSFERWISE_ID, PROPERTY_NAME_EMAIL); COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); - COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jan@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "eur"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); String jsonString = getCompletedFormAsJsonString(); TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); - // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa - // Do not autofill all currencies by default but keep all unselected. - // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); - assertEquals(0, paymentAccount.getTradeCurrencies().size()); + assertEquals(1, paymentAccount.getTradeCurrencies().size()); + List expectedTradeCurrencies = singletonList(getTradeCurrency("EUR").get()); + verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); } + @Test + public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "ars, cad, hrk, czk, eur, hkd, idr, jpy, chf, nzd"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); + verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); + assertEquals(10, paymentAccount.getTradeCurrencies().size()); + List expectedTradeCurrencies = new ArrayList<>() {{ + add(getTradeCurrency("ARS").get()); + add(getTradeCurrency("CAD").get()); + add(getTradeCurrency("HRK").get()); + add(getTradeCurrency("CZK").get()); + add(getTradeCurrency("EUR").get()); + add(getTradeCurrency("HKD").get()); + add(getTradeCurrency("IDR").get()); + add(getTradeCurrency("JPY").get()); + add(getTradeCurrency("CHF").get()); + add(getTradeCurrency("NZD").get()); + }}; + verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); + verifyCommonFormEntries(paymentAccount); + assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); + log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + } + + @Test + public void testCreateTransferwiseAccountWithInvalidBrlTradeCurrencyShouldThrowException(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, "eur, hkd, idr, jpy, chf, nzd, brl, gbp"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + createPaymentAccount(aliceClient, jsonString)); + assertEquals("INVALID_ARGUMENT: BRL is not a member of valid currencies list", + exception.getMessage()); + } + + @Test + public void testCreateTransferwiseAccountWithoutTradeCurrenciesShouldThrowException(TestInfo testInfo) { + File emptyForm = getEmptyForm(testInfo, TRANSFERWISE_ID); + verifyEmptyForm(emptyForm, + TRANSFERWISE_ID, + PROPERTY_NAME_EMAIL); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_PAYMENT_METHOD_ID, TRANSFERWISE_ID); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_ACCOUNT_NAME, "Transferwise Acct"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_TRADE_CURRENCIES, ""); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_EMAIL, "jane@doe.info"); + COMPLETED_FORM_MAP.put(PROPERTY_NAME_SALT, ""); + String jsonString = getCompletedFormAsJsonString(); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> + createPaymentAccount(aliceClient, jsonString)); + assertEquals("INVALID_ARGUMENT: no trade currencies defined for transferwise payment account", + exception.getMessage()); + } + @Test public void testCreateUpholdAccount(TestInfo testInfo) { File emptyForm = getEmptyForm(testInfo, UPHOLD_ID); diff --git a/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java index e3ce0a0806b..c3eb41343a9 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/PaymentAccountTest.java @@ -13,8 +13,6 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.seednode; -import static java.util.Objects.requireNonNull; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -27,10 +25,6 @@ @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class PaymentAccountTest extends AbstractPaymentAccountTest { - // Two dummy (usd +eth) accounts are set up as defaults in regtest / dao mode, - // then we add 28 more payment accounts in testCreatePaymentAccount(). - private static final int EXPECTED_NUM_PAYMENT_ACCOUNTS = 2 + 28; - @BeforeAll public static void setUp() { try { @@ -75,14 +69,18 @@ public void testCreatePaymentAccount(TestInfo testInfo) { test.testCreateSepaAccount(testInfo); test.testCreateSpecificBanksAccount(testInfo); test.testCreateSwishAccount(testInfo); - test.testCreateTransferwiseAccount(testInfo); + + // TransferwiseAccount is only PaymentAccount with a + // tradeCurrencies field in the json form. + test.testCreateTransferwiseAccountWith1TradeCurrency(testInfo); + test.testCreateTransferwiseAccountWith10TradeCurrencies(testInfo); + test.testCreateTransferwiseAccountWithInvalidBrlTradeCurrencyShouldThrowException(testInfo); + test.testCreateTransferwiseAccountWithoutTradeCurrenciesShouldThrowException(testInfo); + test.testCreateUpholdAccount(testInfo); test.testCreateUSPostalMoneyOrderAccount(testInfo); test.testCreateWeChatPayAccount(testInfo); test.testCreateWesternUnionAccount(testInfo); - - var paymentAccounts = requireNonNull(aliceClient).getPaymentAccounts(); - assertEquals(EXPECTED_NUM_PAYMENT_ACCOUNTS, paymentAccounts.size()); } @AfterAll diff --git a/common/src/main/java/bisq/common/util/ReflectionUtils.java b/common/src/main/java/bisq/common/util/ReflectionUtils.java index e70162ecb23..c9f83a8f41a 100644 --- a/common/src/main/java/bisq/common/util/ReflectionUtils.java +++ b/common/src/main/java/bisq/common/util/ReflectionUtils.java @@ -28,9 +28,13 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import lombok.extern.slf4j.Slf4j; + +import static java.lang.String.format; import static java.util.Arrays.stream; import static org.apache.commons.lang3.StringUtils.capitalize; +@Slf4j public class ReflectionUtils { /** @@ -105,4 +109,32 @@ else if (Modifier.isPublic(field.getModifiers())) else return ""; } + + public static Field getField(String name, Class clazz) { + Optional field = stream(clazz.getDeclaredFields()) + .filter(f -> f.getName().equals(name)).findFirst(); + return field.orElseThrow(() -> + new IllegalArgumentException(format("field %s not found in class %s", + name, + clazz.getSimpleName()))); + } + + public static Method getMethod(String name, Class clazz) { + Optional method = stream(clazz.getDeclaredMethods()) + .filter(m -> m.getName().equals(name)).findFirst(); + return method.orElseThrow(() -> + new IllegalArgumentException(format("method %s not found in class %s", + name, + clazz.getSimpleName()))); + } + + public static void handleSetFieldValueError(Object object, + Field field, + ReflectiveOperationException ex) { + String errMsg = format("cannot set value of field %s, on class %s", + field.getName(), + object.getClass().getSimpleName()); + log.error(capitalize(errMsg) + ".", ex); + throw new IllegalStateException("programmer error: " + errMsg); + } } diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index 095bf2d1f2d..b915b3ab4be 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -35,6 +35,8 @@ import lombok.extern.slf4j.Slf4j; +import static java.lang.String.format; + @Singleton @Slf4j class CorePaymentAccountsService { @@ -54,6 +56,7 @@ public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessServ PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); + verifyPaymentAccountHasRequiredFields(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -82,4 +85,11 @@ String getPaymentAccountFormAsString(String paymentMethodId) { File getPaymentAccountForm(String paymentMethodId) { return paymentAccountForm.getPaymentAccountForm(paymentMethodId); } + + private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount) { + // Do checks here to make sure required fields are populated. + if (paymentAccount.isTransferwiseAccount() && paymentAccount.getTradeCurrencies().isEmpty()) + throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", + paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); + } } diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java b/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java index dae437d265d..777e48987d5 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountForm.java @@ -136,7 +136,7 @@ public class PaymentAccountForm { "paymentMethod", "paymentMethodId", // This field will be included, but handled differently. "selectedTradeCurrency", - "tradeCurrencies", + "tradeCurrencies", // This field may be included, but handled differently. "HOLDER_NAME", "SALT" // This field will be included, but handled differently. }; diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java index f49809a50cd..d8d2e19c8da 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java @@ -20,6 +20,7 @@ import bisq.core.locale.Country; import bisq.core.locale.FiatCurrency; +import bisq.core.locale.TradeCurrency; import bisq.core.payment.CountryBasedPaymentAccount; import bisq.core.payment.MoneyGramAccount; import bisq.core.payment.PaymentAccount; @@ -35,12 +36,11 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; import java.util.function.Predicate; import java.lang.reflect.Constructor; @@ -50,16 +50,20 @@ import lombok.extern.slf4j.Slf4j; -import static bisq.common.util.ReflectionUtils.getSetterMethodForFieldInClassHierarchy; -import static bisq.common.util.ReflectionUtils.getVisibilityModifierAsString; -import static bisq.common.util.ReflectionUtils.isSetterOnClass; -import static bisq.common.util.ReflectionUtils.loadFieldListForClassHierarchy; +import static bisq.common.util.ReflectionUtils.*; import static bisq.common.util.Utilities.decodeFromHex; import static bisq.core.locale.CountryUtil.findCountryByCode; +import static bisq.core.locale.CurrencyUtil.getAllTransferwiseCurrencies; import static bisq.core.locale.CurrencyUtil.getCurrencyByCountryCode; +import static bisq.core.locale.CurrencyUtil.getTradeCurrencies; +import static bisq.core.locale.CurrencyUtil.getTradeCurrenciesInList; import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.String.format; +import static java.util.Arrays.stream; +import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableMap; import static java.util.Comparator.comparing; +import static java.util.stream.Collectors.toList; @Slf4j class PaymentAccountTypeAdapter extends TypeAdapter { @@ -96,7 +100,7 @@ public PaymentAccountTypeAdapter(Class paymentAccountT public PaymentAccountTypeAdapter(Class paymentAccountType, String[] excludedFields) { this.paymentAccountType = paymentAccountType; this.paymentAccountPayloadType = getPaymentAccountPayloadType(); - this.isExcludedField = (f) -> Arrays.stream(excludedFields).anyMatch(e -> e.equals(f.getName())); + this.isExcludedField = (f) -> stream(excludedFields).anyMatch(e -> e.equals(f.getName())); this.fieldSettersMap = getFieldSetterMap(); } @@ -128,6 +132,9 @@ public void write(JsonWriter out, PaymentAccount account) throws IOException { } private void writeInnerMutableFields(JsonWriter out, PaymentAccount account) { + if (account.isTransferwiseAccount()) + writeTradeCurrenciesField(out, account); + fieldSettersMap.forEach((field, value) -> { try { // Write out a json element if there is a @Setter for this field. @@ -151,6 +158,25 @@ private void writeInnerMutableFields(JsonWriter out, PaymentAccount account) { }); } + // In some cases (TransferwiseAccount), we need to include a 'tradeCurrencies' + // field in the json form, though the 'tradeCurrencies' field has no setter method in + // the PaymentAccount class hierarchy. At of time of this change, TransferwiseAccount + // is the only known exception to the rule. + private void writeTradeCurrenciesField(JsonWriter out, PaymentAccount account) { + try { + String fieldName = "tradeCurrencies"; + log.debug("Append form with non-settable field: {}", fieldName); + out.name(fieldName); + out.value("comma delimited currency code list, e.g., gbp,eur"); + } catch (Exception ex) { + String errMsg = format("cannot create a new %s json form", + account.getClass().getSimpleName()); + log.error(StringUtils.capitalize(errMsg) + ".", ex); + throw new IllegalStateException("programmer error: " + errMsg); + } + } + + @Override public PaymentAccount read(JsonReader in) throws IOException { PaymentAccount account = initNewPaymentAccount(); @@ -158,6 +184,11 @@ public PaymentAccount read(JsonReader in) throws IOException { while (in.hasNext()) { String currentFieldName = in.nextName(); + // The tradeCurrency field is common to all payment account types, + // but has no setter. + if (didReadTradeCurrenciesField(in, account, currentFieldName)) + continue; + // Some of the fields are common to all payment account types. if (didReadCommonField(in, account, currentFieldName)) continue; @@ -199,17 +230,13 @@ private void invokeSetterMethod(PaymentAccount account, Field field, JsonReader account.getClass().getSimpleName()); throw new IllegalStateException(errMsg); } - } catch (IllegalAccessException | InvocationTargetException ex) { - String errMsg = format("cannot set field value for %s on %s", - field.getName(), - account.getClass().getSimpleName()); - log.error(StringUtils.capitalize(errMsg) + ".", ex); - throw new IllegalStateException("programmer error: " + errMsg); + } catch (ReflectiveOperationException ex) { + handleSetFieldValueError(account, field, ex); } } else { throw new IllegalStateException( format("programmer error: cannot de-serialize json to a '%s' " - + " because there is no setter method for field %s.", + + " because field value cannot be set %s.", account.getClass().getSimpleName(), field.getName())); } @@ -232,7 +259,7 @@ private Map> getFieldSetterMap() { .or(() -> getSetterMethodForFieldInClassHierarchy(field, paymentAccountPayloadType)); map.put(field, setter); } - return Collections.unmodifiableMap(map); + return unmodifiableMap(map); } private List getOrderedFields() { @@ -274,6 +301,56 @@ private Long nextLongOrNull(JsonReader in) { } } + private final Predicate isCommaDelimitedCurrencyList = (s) -> s != null && s.contains(","); + private final Function> commaDelimitedCodesToList = (s) -> { + if (isCommaDelimitedCurrencyList.test(s)) + return stream(s.split(",")).map(a -> a.trim().toUpperCase()).collect(toList()); + else if (s != null && !s.isEmpty()) + return singletonList(s.trim().toUpperCase()); + else + return new ArrayList<>(); + }; + + private boolean didReadTradeCurrenciesField(JsonReader in, + PaymentAccount account, + String fieldName) { + // The PaymentAccount.tradeCurrencies field is a special case because it has + // no setter, and we add currencies to the List here. Normally, it is an + // excluded field, TransferwiseAccount excepted. + if (fieldName.equals("tradeCurrencies")) { + try { + String fieldValue = nextStringOrNull(in); + List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); + + Optional> tradeCurrencies; + if (account.isTransferwiseAccount()) + tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); + else + tradeCurrencies = getTradeCurrencies(currencyCodes); + + if (tradeCurrencies.isPresent()) { + Method addCurrencyMethod = getMethod("addCurrency", PaymentAccount.class); + for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { + addCurrencyMethod.invoke(account, tradeCurrency); + } + } else { + // Log a warning. We should not throw an exception here because the + // gson library will not pass it up to the calling Bisq class as it + // would be defined here. Do a check in a calling class to make sure + // the tradeCurrencies field is populated in the PaymentAccount + // object, if it is required for the payment account method. + log.warn("No trade currencies were found in the {} account form.", + account.getPaymentMethod().getDisplayString()); + } + return true; + } catch (ReflectiveOperationException ex) { + Field field = getField("tradeCurrencies", PaymentAccount.class); + handleSetFieldValueError(account, field, ex); + } + } + return false; + } + private boolean didReadCommonField(JsonReader in, PaymentAccount account, String fieldName) throws IOException { diff --git a/core/src/main/java/bisq/core/locale/CurrencyUtil.java b/core/src/main/java/bisq/core/locale/CurrencyUtil.java index e94127cef1b..9ed9bb1a967 100644 --- a/core/src/main/java/bisq/core/locale/CurrencyUtil.java +++ b/core/src/main/java/bisq/core/locale/CurrencyUtil.java @@ -43,6 +43,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -51,6 +52,7 @@ import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.String.format; @Slf4j public class CurrencyUtil { @@ -486,6 +488,32 @@ public static Optional getTradeCurrency(String currencyCode) { return Optional.empty(); } + public static Optional> getTradeCurrencies(List currencyCodes) { + List tradeCurrencies = new ArrayList<>(); + currencyCodes.stream().forEachOrdered(c -> + tradeCurrencies.add(getTradeCurrency(c).orElseThrow(() -> + new IllegalArgumentException(format("%s is not a valid trade currency code", c))))); + return tradeCurrencies.isEmpty() + ? Optional.empty() + : Optional.of(tradeCurrencies); + } + + public static Optional> getTradeCurrenciesInList(List currencyCodes, + List validCurrencies) { + Optional> tradeCurrencies = getTradeCurrencies(currencyCodes); + Consumer> validateCandidateCurrencies = (list) -> { + for (TradeCurrency tradeCurrency : list) { + if (!validCurrencies.contains(tradeCurrency)) { + throw new IllegalArgumentException( + format("%s is not a member of valid currencies list", + tradeCurrency.getCode())); + } + } + }; + tradeCurrencies.ifPresent(validateCandidateCurrencies); + return tradeCurrencies; + } + public static FiatCurrency getCurrencyByCountryCode(String countryCode) { if (countryCode.equals("XK")) return new FiatCurrency("EUR"); diff --git a/core/src/main/java/bisq/core/payment/PaymentAccount.java b/core/src/main/java/bisq/core/payment/PaymentAccount.java index a540641319d..1e481e44583 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccount.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccount.java @@ -198,6 +198,10 @@ public boolean isMoneyGramAccount() { return this instanceof MoneyGramAccount; } + public boolean isTransferwiseAccount() { + return this instanceof TransferwiseAccount; + } + /** * Return an Optional of the trade currency for this payment account, or * Optional.empty() if none is found. If this payment account has a selected From de59c0a5f9a159ef06f0aa4d6399f80fcacd775c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 15:22:28 -0300 Subject: [PATCH 10/15] Assign selected trading currency on new payment accts (api only) If a new account has a trading currency, assign it as the selected trading currency. Also fixed a payment acct console formatting issue (left justify the ccy code). --- .../payment/CreatePaymentAccountTest.java | 70 +++++++++++-------- cli/src/main/java/bisq/cli/TableFormat.java | 2 +- .../core/api/CorePaymentAccountsService.java | 11 ++- 3 files changed, 50 insertions(+), 33 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 2cc5bfcf857..b2911aa842e 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -32,6 +32,7 @@ import bisq.core.payment.MoneyBeamAccount; import bisq.core.payment.MoneyGramAccount; import bisq.core.payment.NationalBankAccount; +import bisq.core.payment.PaymentAccount; import bisq.core.payment.PerfectMoneyAccount; import bisq.core.payment.PopmoneyAccount; import bisq.core.payment.PromptPayAccount; @@ -63,13 +64,13 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestMethodOrder; import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; +import static bisq.cli.TableFormat.formatPaymentAcctTbl; import static bisq.core.locale.CurrencyUtil.*; import static bisq.core.payment.payload.PaymentMethod.*; import static java.util.Collections.singletonList; @@ -78,7 +79,7 @@ import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; -@Disabled +// @Disabled @Slf4j @TestMethodOrder(OrderAnnotation.class) public class CreatePaymentAccountTest extends AbstractPaymentAccountTest { @@ -109,7 +110,7 @@ public void testCreateAdvancedCashAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -128,7 +129,7 @@ public void testCreateAliPayAccount(TestInfo testInfo) { verifyAccountSingleTradeCurrency("CNY", paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -150,7 +151,7 @@ public void testCreateAustraliaPayidAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_PAY_ID), paymentAccount.getPayid()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NAME), paymentAccount.getBankAccountName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -202,7 +203,7 @@ public void testCreateCashDepositAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_REQUIREMENTS), payload.getRequirements()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -247,7 +248,7 @@ public void testCreateBrazilNationalBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -269,7 +270,7 @@ public void testCreateChaseQuickPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -292,7 +293,7 @@ public void testCreateClearXChangeAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL_OR_MOBILE_NR), paymentAccount.getEmailOrMobileNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -321,7 +322,7 @@ public void testCreateF2FAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_CITY), paymentAccount.getCity()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_CONTACT), paymentAccount.getContact()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EXTRA_INFO), paymentAccount.getExtraInfo()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -344,7 +345,7 @@ public void testCreateFasterPaymentsAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SORT_CODE), paymentAccount.getSortCode()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -363,7 +364,7 @@ public void testCreateHalCashAccount(TestInfo testInfo) { verifyAccountSingleTradeCurrency("EUR", paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_MOBILE_NR), paymentAccount.getMobileNr()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -392,7 +393,7 @@ public void testCreateInteracETransferAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_QUESTION), paymentAccount.getQuestion()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ANSWER), paymentAccount.getAnswer()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -429,7 +430,7 @@ public void testCreateJapanBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NAME), paymentAccount.getBankAccountName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_TYPE), paymentAccount.getBankAccountType()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BANK_ACCOUNT_NUMBER), paymentAccount.getBankAccountNumber()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -449,7 +450,7 @@ public void testCreateMoneyBeamAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -478,7 +479,7 @@ public void testCreateMoneyGramAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_COUNTRY), Objects.requireNonNull(paymentAccount.getCountry()).code); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_STATE), paymentAccount.getState()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -498,7 +499,7 @@ public void testCreatePerfectMoneyAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -520,7 +521,7 @@ public void testCreatePopmoneyAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -540,7 +541,7 @@ public void testCreatePromptPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_PROMPT_PAY_ID), paymentAccount.getPromptPayId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -559,7 +560,7 @@ public void testCreateRevolutAccount(TestInfo testInfo) { verifyAccountTradeCurrencies(getAllRevolutCurrencies(), paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_USERNAME), paymentAccount.getUserName()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -604,7 +605,7 @@ public void testCreateSameBankAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -635,7 +636,7 @@ public void testCreateSepaInstantAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBic()); // bankId == bic assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBankId()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -667,7 +668,7 @@ public void testCreateSepaAccount(TestInfo testInfo) { // bankId == bic assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_BIC), paymentAccount.getBankId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -714,7 +715,7 @@ public void testCreateSpecificBanksAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), payload.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_TAX_ID), payload.getHolderTaxId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_NATIONAL_ACCOUNT_ID), payload.getNationalAccountId()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -737,7 +738,7 @@ public void testCreateSwishAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_MOBILE_NR), paymentAccount.getMobileNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -759,7 +760,7 @@ public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -792,7 +793,7 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -850,7 +851,7 @@ public void testCreateUpholdAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_ID), paymentAccount.getAccountId()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -872,7 +873,7 @@ public void testCreateUSPostalMoneyOrderAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_HOLDER_NAME), paymentAccount.getHolderName()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_POSTAL_ADDRESS), paymentAccount.getPostalAddress()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -892,7 +893,7 @@ public void testCreateWeChatPayAccount(TestInfo testInfo) { verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_ACCOUNT_NR), paymentAccount.getAccountNr()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_SALT), paymentAccount.getSaltAsHex()); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @Test @@ -924,11 +925,18 @@ public void testCreateWesternUnionAccount(TestInfo testInfo) { assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_COUNTRY), Objects.requireNonNull(paymentAccount.getCountry()).code); - log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + print(paymentAccount); } @AfterAll public static void tearDown() { tearDownScaffold(); } + + private void print(PaymentAccount paymentAccount) { + if (log.isDebugEnabled()) { + log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); + log.debug("\n{}", formatPaymentAcctTbl(singletonList(paymentAccount.toProtoMessage()))); + } + } } diff --git a/cli/src/main/java/bisq/cli/TableFormat.java b/cli/src/main/java/bisq/cli/TableFormat.java index 8064e9f6962..dca4aa06994 100644 --- a/cli/src/main/java/bisq/cli/TableFormat.java +++ b/cli/src/main/java/bisq/cli/TableFormat.java @@ -163,7 +163,7 @@ public static String formatPaymentAcctTbl(List paymentAccounts) + padEnd(COL_HEADER_PAYMENT_METHOD, paymentMethodColWidth, ' ') + COL_HEADER_DELIMITER + COL_HEADER_UUID + COL_HEADER_DELIMITER + "\n"; String colDataFormat = "%-" + nameColWidth + "s" // left justify - + " %" + COL_HEADER_CURRENCY.length() + "s" // right justify + + " %-" + COL_HEADER_CURRENCY.length() + "s" // left justify + " %-" + paymentMethodColWidth + "s" // left justify + " %-" + COL_HEADER_UUID.length() + "s"; // left justify return headerLine diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index b915b3ab4be..dae19ea9fae 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -44,19 +44,23 @@ class CorePaymentAccountsService { private final AccountAgeWitnessService accountAgeWitnessService; private final PaymentAccountForm paymentAccountForm; private final User user; + private final boolean isApiUser; @Inject - public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessService, + public CorePaymentAccountsService(CoreContext coreContext, + AccountAgeWitnessService accountAgeWitnessService, PaymentAccountForm paymentAccountForm, User user) { this.accountAgeWitnessService = accountAgeWitnessService; this.paymentAccountForm = paymentAccountForm; this.user = user; + this.isApiUser = coreContext.isApiUser(); } PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); verifyPaymentAccountHasRequiredFields(paymentAccount); + maybeAssignSelectedTradeCurrency(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -92,4 +96,9 @@ private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); } + + private void maybeAssignSelectedTradeCurrency(PaymentAccount paymentAccount) { + if (isApiUser && paymentAccount.getSelectedTradeCurrency() == null) + paymentAccount.setSelectedTradeCurrency(paymentAccount.getTradeCurrency().orElse(null)); + } } From 75d81d9095d46ff9dba0f339cef642f065dcdfda Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 15:26:15 -0300 Subject: [PATCH 11/15] Avoid test run repetition, add warning supressions --- .../bisq/apitest/method/payment/CreatePaymentAccountTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index b2911aa842e..8f05b2a5da8 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -64,6 +64,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestMethodOrder; @@ -79,7 +80,8 @@ import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; -// @Disabled +@SuppressWarnings({"OptionalGetWithoutIsPresent", "ConstantConditions"}) +@Disabled @Slf4j @TestMethodOrder(OrderAnnotation.class) public class CreatePaymentAccountTest extends AbstractPaymentAccountTest { From 04b52f873e4ae233c5ccfab5dc497b05197f644f Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:02:18 -0300 Subject: [PATCH 12/15] Fix setSelectedCurrency bug, and replace unnecessary reflection usage Commit de59c0a5f9a159ef06f0aa4d6399f80fcacd775c did not persist the assigned selected currency, this fixes the bug. --- .../payment/CreatePaymentAccountTest.java | 8 ++- .../core/api/CorePaymentAccountsService.java | 11 +--- .../api/model/PaymentAccountTypeAdapter.java | 50 +++++++++---------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 8f05b2a5da8..3caef6ee391 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -758,7 +758,9 @@ public void testCreateTransferwiseAccountWith1TradeCurrency(TestInfo testInfo) { TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(aliceClient, jsonString); verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); assertEquals(1, paymentAccount.getTradeCurrencies().size()); - List expectedTradeCurrencies = singletonList(getTradeCurrency("EUR").get()); + TradeCurrency expectedCurrency = getTradeCurrency("EUR").get(); + assertEquals(expectedCurrency, paymentAccount.getSelectedTradeCurrency()); + List expectedTradeCurrencies = singletonList(expectedCurrency); verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); @@ -781,7 +783,7 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo verifyUserPayloadHasPaymentAccountWithId(aliceClient, paymentAccount.getId()); assertEquals(10, paymentAccount.getTradeCurrencies().size()); List expectedTradeCurrencies = new ArrayList<>() {{ - add(getTradeCurrency("ARS").get()); + add(getTradeCurrency("ARS").get()); // 1st in list = selected ccy add(getTradeCurrency("CAD").get()); add(getTradeCurrency("HRK").get()); add(getTradeCurrency("CZK").get()); @@ -793,6 +795,8 @@ public void testCreateTransferwiseAccountWith10TradeCurrencies(TestInfo testInfo add(getTradeCurrency("NZD").get()); }}; verifyAccountTradeCurrencies(expectedTradeCurrencies, paymentAccount); + TradeCurrency expectedSelectedCurrency = expectedTradeCurrencies.get(0); + assertEquals(expectedSelectedCurrency, paymentAccount.getSelectedTradeCurrency()); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); print(paymentAccount); diff --git a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java index dae19ea9fae..b915b3ab4be 100644 --- a/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java +++ b/core/src/main/java/bisq/core/api/CorePaymentAccountsService.java @@ -44,23 +44,19 @@ class CorePaymentAccountsService { private final AccountAgeWitnessService accountAgeWitnessService; private final PaymentAccountForm paymentAccountForm; private final User user; - private final boolean isApiUser; @Inject - public CorePaymentAccountsService(CoreContext coreContext, - AccountAgeWitnessService accountAgeWitnessService, + public CorePaymentAccountsService(AccountAgeWitnessService accountAgeWitnessService, PaymentAccountForm paymentAccountForm, User user) { this.accountAgeWitnessService = accountAgeWitnessService; this.paymentAccountForm = paymentAccountForm; this.user = user; - this.isApiUser = coreContext.isApiUser(); } PaymentAccount createPaymentAccount(String jsonString) { PaymentAccount paymentAccount = paymentAccountForm.toPaymentAccount(jsonString); verifyPaymentAccountHasRequiredFields(paymentAccount); - maybeAssignSelectedTradeCurrency(paymentAccount); user.addPaymentAccountIfNotExists(paymentAccount); accountAgeWitnessService.publishMyAccountAgeWitness(paymentAccount.getPaymentAccountPayload()); log.info("Saved payment account with id {} and payment method {}.", @@ -96,9 +92,4 @@ private void verifyPaymentAccountHasRequiredFields(PaymentAccount paymentAccount throw new IllegalArgumentException(format("no trade currencies defined for %s payment account", paymentAccount.getPaymentMethod().getDisplayString().toLowerCase())); } - - private void maybeAssignSelectedTradeCurrency(PaymentAccount paymentAccount) { - if (isApiUser && paymentAccount.getSelectedTradeCurrency() == null) - paymentAccount.setSelectedTradeCurrency(paymentAccount.getTradeCurrency().orElse(null)); - } } diff --git a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java index d8d2e19c8da..b566ac10b0d 100644 --- a/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java +++ b/core/src/main/java/bisq/core/api/model/PaymentAccountTypeAdapter.java @@ -318,35 +318,31 @@ private boolean didReadTradeCurrenciesField(JsonReader in, // no setter, and we add currencies to the List here. Normally, it is an // excluded field, TransferwiseAccount excepted. if (fieldName.equals("tradeCurrencies")) { - try { - String fieldValue = nextStringOrNull(in); - List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); - - Optional> tradeCurrencies; - if (account.isTransferwiseAccount()) - tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); - else - tradeCurrencies = getTradeCurrencies(currencyCodes); - - if (tradeCurrencies.isPresent()) { - Method addCurrencyMethod = getMethod("addCurrency", PaymentAccount.class); - for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { - addCurrencyMethod.invoke(account, tradeCurrency); - } - } else { - // Log a warning. We should not throw an exception here because the - // gson library will not pass it up to the calling Bisq class as it - // would be defined here. Do a check in a calling class to make sure - // the tradeCurrencies field is populated in the PaymentAccount - // object, if it is required for the payment account method. - log.warn("No trade currencies were found in the {} account form.", - account.getPaymentMethod().getDisplayString()); + String fieldValue = nextStringOrNull(in); + List currencyCodes = commaDelimitedCodesToList.apply(fieldValue); + + Optional> tradeCurrencies; + if (account.isTransferwiseAccount()) + tradeCurrencies = getTradeCurrenciesInList(currencyCodes, getAllTransferwiseCurrencies()); + else + tradeCurrencies = getTradeCurrencies(currencyCodes); + + if (tradeCurrencies.isPresent()) { + for (TradeCurrency tradeCurrency : tradeCurrencies.get()) { + account.addCurrency(tradeCurrency); } - return true; - } catch (ReflectiveOperationException ex) { - Field field = getField("tradeCurrencies", PaymentAccount.class); - handleSetFieldValueError(account, field, ex); + // For api users, define a selected currency. + account.setSelectedTradeCurrency(account.getTradeCurrency().orElse(null)); + } else { + // Log a warning. We should not throw an exception here because the + // gson library will not pass it up to the calling Bisq class as it + // would be defined here. Do a check in a calling class to make sure + // the tradeCurrencies field is populated in the PaymentAccount + // object, if it is required for the payment account method. + log.warn("No trade currencies were found in the {} account form.", + account.getPaymentMethod().getDisplayString()); } + return true; } return false; } From d6c79fca70f437e93348050f902a4bd5b06fac78 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:26:16 -0300 Subject: [PATCH 13/15] Filter out 'my' offers from 'available' offers --- core/src/main/java/bisq/core/api/CoreOffersService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index 8a5b95cf31e..be429709c69 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -55,7 +55,6 @@ import static bisq.core.offer.OfferPayload.Direction; import static bisq.core.offer.OfferPayload.Direction.BUY; import static bisq.core.payment.PaymentAccountUtil.isPaymentAccountValidForOffer; -import static java.lang.String.*; import static java.lang.String.format; import static java.util.Comparator.comparing; @@ -103,6 +102,7 @@ public CoreOffersService(CoreContext coreContext, Offer getOffer(String id) { return offerBookService.getOffers().stream() .filter(o -> o.getId().equals(id)) + .filter(o -> !o.isMyOffer(keyRing)) .filter(o -> offerFilter.canTakeOffer(o, isApiUser).isValid()) .findAny().orElseThrow(() -> new IllegalStateException(format("offer with id '%s' not found", id))); @@ -118,6 +118,7 @@ Offer getMyOffer(String id) { List getOffers(String direction, String currencyCode) { return offerBookService.getOffers().stream() + .filter(o -> !o.isMyOffer(keyRing)) .filter(o -> offerMatchesDirectionAndCurrency(o, direction, currencyCode)) .filter(o -> offerFilter.canTakeOffer(o, isApiUser).isValid()) .sorted(priceComparator(direction)) From 2161c936f171f0a0a8106408a172200ad190771c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 8 Mar 2021 16:36:01 -0300 Subject: [PATCH 14/15] Adjust canceloffer to getmyoffer bugfix (commit d6c79fc) --- core/src/main/java/bisq/core/api/CoreOffersService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/bisq/core/api/CoreOffersService.java b/core/src/main/java/bisq/core/api/CoreOffersService.java index be429709c69..9942380abca 100644 --- a/core/src/main/java/bisq/core/api/CoreOffersService.java +++ b/core/src/main/java/bisq/core/api/CoreOffersService.java @@ -215,7 +215,7 @@ Offer editOffer(String offerId, } void cancelOffer(String id) { - Offer offer = getOffer(id); + Offer offer = getMyOffer(id); openOfferManager.removeOffer(offer, () -> { }, From 389abf459d87f748dc0998490bc7332ac5a7ff3c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 9 Mar 2021 10:43:05 -0300 Subject: [PATCH 15/15] Check valid createoffer opt values with asserts --- cli/src/test/java/bisq/cli/opt/OptionParsersTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java index d853da66d6a..1f939198d69 100644 --- a/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java +++ b/cli/src/test/java/bisq/cli/opt/OptionParsersTest.java @@ -130,6 +130,13 @@ public void testValidCreateOfferOpts() { "--" + OPT_MKT_PRICE_MARGIN + "=" + "0.0", "--" + OPT_SECURITY_DEPOSIT + "=" + "25.0" }; + CreateOfferOptionParser parser = new CreateOfferOptionParser(args).parse(); + assertEquals("abc-payment-acct-id-123", parser.getPaymentAccountId()); + assertEquals("BUY", parser.getDirection()); + assertEquals("EUR", parser.getCurrencyCode()); + assertEquals("0.125", parser.getAmount()); + assertEquals("0.0", parser.getMktPriceMargin()); + assertEquals("25.0", parser.getSecurityDeposit()); } // CreatePaymentAcct opt parser tests