Skip to content

Commit

Permalink
Ensure that trade_preimage will not succeed on input that will fail o…
Browse files Browse the repository at this point in the history
…n buy/sell/setprice #902 (#937)

* Ensure that trade_preimage will not succeed on input that will fail on setprice
* Add balance checking on trade_preimage::method = setprice
* Add an additional params checking using MakerOrderBuilder
* Fix broken tests, add red test_trade_preimage_additional_validation test

* Ensure that trade_preimage will not succeed on input that will fail on sell/buy
* Add balance checking on trade_preimage::method = sell/buy
* Add an additional params checking using TakerOrderBuilder
* Make the test_trade_preimage_additional_validation test green
* Extend test_trade_preimage_not_sufficient_balance

* Fix invalid required field in NotSufficientBalance error generated by calc_max_maker_vol
* Fix broken tests
* Extend the test_trade_preimage_not_sufficient_balance test
* Add the test_trade_preimage_deduct_fee_from_output_failed test

* Remove unused TradePreimageRpcError::MaxVolumeLessThanDust error type
* GenerateTxError::OutputValueLessThanDust converts to TradePreimageError::InternalError the value is upper bound

* Remove TODO comment

* Refactor 'TradePreimageRpcError::VolumeTooLow'
* Rename 'TradePreimageRpcError::VolumeIsTooSmall' to 'TradePreimageRpcError::VolumeTooLow'
* Add the 'coin' field to 'TradePreimageRpcError::VolumeTooLow'
  • Loading branch information
sergeyboyko0791 authored May 10, 2021
1 parent c001252 commit 73875fd
Show file tree
Hide file tree
Showing 9 changed files with 615 additions and 167 deletions.
20 changes: 12 additions & 8 deletions mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,8 @@ pub enum TradePreimageError {
available: BigDecimal,
required: BigDecimal,
},
#[display(fmt = "The amount {} is too small", amount)]
AmountIsTooSmall { amount: BigDecimal },
#[display(fmt = "The max available amount {} is too small", amount)]
UpperBoundAmountIsTooSmall { amount: BigDecimal },
#[display(fmt = "The amount {} less than minimum transaction amount {}", amount, threshold)]
AmountIsTooSmall { amount: BigDecimal, threshold: BigDecimal },
#[display(fmt = "Transport error: {}", _0)]
Transport(String),
#[display(fmt = "Internal error: {}", _0)]
Expand Down Expand Up @@ -531,12 +529,18 @@ impl TradePreimageError {
}
},
GenerateTxError::EmptyOutputs => TradePreimageError::InternalError(gen_tx_err.to_string()),
GenerateTxError::OutputValueLessThanDust { value, .. } => {
let amount = big_decimal_from_sat_unsigned(value, decimals);
GenerateTxError::OutputValueLessThanDust { value, dust } => {
if is_upper_bound {
TradePreimageError::UpperBoundAmountIsTooSmall { amount }
// If the preimage value is [`TradePreimageValue::UpperBound`], then we had to pass the account balance as the output value.
let error = format!(
"Output value {} (equal to the account balance) less than dust {}. Probably, dust is not set or outdated",
value, dust
);
TradePreimageError::InternalError(error)
} else {
TradePreimageError::AmountIsTooSmall { amount }
let amount = big_decimal_from_sat_unsigned(value, decimals);
let threshold = big_decimal_from_sat_unsigned(dust, decimals);
TradePreimageError::AmountIsTooSmall { amount, threshold }
}
},
GenerateTxError::DeductFeeFromOutputFailed {
Expand Down
261 changes: 225 additions & 36 deletions mm2src/docker_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1617,6 +1617,7 @@ mod docker_tests {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"price": 1,
"max": true,
},
})))
Expand Down Expand Up @@ -1650,6 +1651,7 @@ mod docker_tests {
"base": "MYCOIN1",
"rel": "MYCOIN",
"swap_method": "setprice",
"price": 1,
"max": true,
},
})))
Expand Down Expand Up @@ -1684,6 +1686,7 @@ mod docker_tests {
"base": "MYCOIN1",
"rel": "MYCOIN",
"swap_method": "setprice",
"price": 1,
"volume": "19.99998",
},
})))
Expand Down Expand Up @@ -1756,6 +1759,7 @@ mod docker_tests {
"rel": "MYCOIN1",
"swap_method": "sell",
"max": true,
"price": 1,
},
})))
.unwrap();
Expand All @@ -1769,23 +1773,6 @@ mod docker_tests {
};
assert_eq!(actual.error_data, Some(expected));

// `price` field is missing
let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "sell",
"volume": "10",
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let actual: RpcErrorResponse<()> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "ZeroPrice", "Unexpected error_type: {}", rc.1);

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
Expand Down Expand Up @@ -1858,12 +1845,149 @@ mod docker_tests {

#[test]
fn test_trade_preimage_not_sufficient_balance() {
#[track_caller]
fn expect_not_sufficient_balance(res: &str, available: BigDecimal, required: BigDecimal) {
let actual: RpcErrorResponse<trade_preimage_error::NotSufficientBalance> = json::from_str(res).unwrap();
assert_eq!(actual.error_type, "NotSufficientBalance");
let expected = trade_preimage_error::NotSufficientBalance {
coin: "MYCOIN".to_owned(),
available,
required,
locked_by_swaps: Some(BigDecimal::from(0)),
};
assert_eq!(actual.error_data, Some(expected));
}

let priv_key = SecretKey::random(&mut rand4::thread_rng()).serialize();

let coins = json!([
{"coin":"MYCOIN","asset":"MYCOIN","txversion":4,"overwintered":1,"txfee":1000,"protocol":{"type":"UTXO"}},
{"coin":"MYCOIN1","asset":"MYCOIN1","txversion":4,"overwintered":1,"txfee":2000,"protocol":{"type":"UTXO"}},
]);
let mm = MarketMakerIt::start(
json! ({
"gui": "nogui",
"netid": 9000,
"dht": "on", // Enable DHT without delay.
"passphrase": format!("0x{}", hex::encode(priv_key)),
"coins": coins,
"rpc_password": "pass",
"i_am_see": true,
}),
"pass".to_string(),
None,
)
.unwrap();
let (_dump_log, _dump_dashboard) = mm_dump(&mm.log_path);

log!([block_on(enable_native(&mm, "MYCOIN1", &[]))]);
log!([block_on(enable_native(&mm, "MYCOIN", &[]))]);

// Try sell the max amount with the zero balance.
let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"price": 1,
"max": true,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let available = BigDecimal::from(0);
// Required at least 0.00002 MYCOIN to pay the transaction_fee(0.00001) and to send a value not less than dust(0.00001).
let required = MmNumber::from("0.00002").to_decimal();
expect_not_sufficient_balance(&rc.1, available, required);

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"price": 1,
"volume": 0.1,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
// Required 0.00001 MYCOIN to pay the transaction fee and the specified 0.1 volume.
let available = BigDecimal::from(0);
let required = MmNumber::from("0.10001").to_decimal();
expect_not_sufficient_balance(&rc.1, available, required);

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "sell",
"price": 1,
"volume": 0.01,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let available = BigDecimal::from(0);
// `required = volume + fee_to_send_taker_payment + dex_fee + fee_to_send_dex_fee`,
// where `volume = 0.01`, `fee_to_send_taker_payment = fee_to_send_dex_fee = 0.00001`, `dex_fee = 0.0001`.
// Please note `dex_fee = 0.01 / 7770` < `min_dex_fee = 0.0001`, so `dex_fee = min_dex_fee = 0.0001`
let required = MmNumber::from("0.01") + MmNumber::from("0.00012");
expect_not_sufficient_balance(&rc.1, available, required.to_decimal());

// The max available value = balance (0.000015) - transaction_fee (0.00001) = 0.000005 that is less than dust (0.00001).
// In this case we have to receive the `NotSufficientBalance` error.
//
// vvv Fill the MYCOIN balance vvv

let low_balance = MmNumber::from("0.000015").to_decimal();
let (_ctx, mycoin) = utxo_coin_from_privkey("MYCOIN", &priv_key);
let my_address = mycoin.my_address().expect("!my_address");
fill_address(&mycoin, &my_address, low_balance.clone(), 30);

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"price": 1,
"max": true,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
// balance(0.000015)
let available = low_balance;
// balance(0.000015) + transaction_fee(0.00001)
let required = MmNumber::from("0.00002").to_decimal();
expect_not_sufficient_balance(&rc.1, available, required);
}

/// This test ensures that `trade_preimage` will not succeed on input that will fail on `buy/sell/setprice`.
/// https://github.com/KomodoPlatform/atomicDEX-API/issues/902
#[test]
fn test_trade_preimage_additional_validation() {
let priv_key = SecretKey::random(&mut rand4::thread_rng()).serialize();

let (_ctx, mycoin1) = utxo_coin_from_privkey("MYCOIN1", &priv_key);
let my_address = mycoin1.my_address().expect("!my_address");
fill_address(&mycoin1, &my_address, 20.into(), 30);

let (_ctx, mycoin) = utxo_coin_from_privkey("MYCOIN", &priv_key);
let my_address = mycoin.my_address().expect("!my_address");
fill_address(&mycoin, &my_address, 10.into(), 30);

let coins = json!([
{"coin":"MYCOIN","asset":"MYCOIN","txversion":4,"overwintered":1,"txfee":1000,"protocol":{"type":"UTXO"}},
{"coin":"MYCOIN1","asset":"MYCOIN1","txversion":4,"overwintered":1,"txfee":2000,"protocol":{"type":"UTXO"}},
Expand All @@ -1887,10 +2011,7 @@ mod docker_tests {
log!([block_on(enable_native(&mm, "MYCOIN1", &[]))]);
log!([block_on(enable_native(&mm, "MYCOIN", &[]))]);

// Try sell the max amount with the zero balance.
// This RPC call should fail, because if the `max` param is true,
// then MM2 would check the account balance, amounts locked by other swaps, etc
// and it turns out that the account balance is not sufficient for swap.
// Price is too low
let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
Expand All @@ -1899,25 +2020,25 @@ mod docker_tests {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"max": true,
"price": 0,
"volume": 0.1,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let actual: RpcErrorResponse<trade_preimage_error::NotSufficientBalance> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "NotSufficientBalance");
// Required at least 0.00001 MYCOIN to pay the transaction fee.
let required = MmNumber::from("0.00001").to_decimal();
let expected = trade_preimage_error::NotSufficientBalance {
coin: "MYCOIN".to_owned(),
available: BigDecimal::from(0),
required,
locked_by_swaps: Some(BigDecimal::from(0)),
let actual: RpcErrorResponse<trade_preimage_error::PriceTooLow> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "PriceTooLow");
// currently the minimum price is 0.00000001
let price_threshold = BigDecimal::from(1) / BigDecimal::from(100_000_000);
let expected = trade_preimage_error::PriceTooLow {
price: BigDecimal::from(0),
threshold: price_threshold,
};
assert_eq!(actual.error_data, Some(expected));

// But even if we pass the exact volume, the `trade_preimage` RPC call should not fail,
// because MYCOIN and MYCOIN1 have fixed transaction fees.
// volume 0.00001 is too low, min trading volume 0.0001
let low_volume = BigDecimal::from(1) / BigDecimal::from(100_000);

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
Expand All @@ -1926,12 +2047,78 @@ mod docker_tests {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "setprice",
"volume": 0.1,
"price": 1,
"volume": low_volume,
},
})))
.unwrap();
assert!(rc.0.is_success(), "!trade_preimage: {}", rc.1);
let _: RpcSuccessResponse<TradePreimageResult> = json::from_str(&rc.1).unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let actual: RpcErrorResponse<trade_preimage_error::VolumeTooLow> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "VolumeTooLow");
// Min MYCOIN trading volume is 0.0001.
let volume_threshold = BigDecimal::from(1) / BigDecimal::from(10_000);
let expected = trade_preimage_error::VolumeTooLow {
coin: "MYCOIN".to_owned(),
volume: low_volume.clone(),
threshold: volume_threshold,
};
assert_eq!(actual.error_data, Some(expected));

let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "sell",
"price": 1,
"volume": low_volume,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let actual: RpcErrorResponse<trade_preimage_error::VolumeTooLow> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "VolumeTooLow");
// Min MYCOIN trading volume is 0.0001.
let volume_threshold = BigDecimal::from(1) / BigDecimal::from(10_000);
let expected = trade_preimage_error::VolumeTooLow {
coin: "MYCOIN".to_owned(),
volume: low_volume,
threshold: volume_threshold,
};
assert_eq!(actual.error_data, Some(expected));

// rel volume is too low
// Min MYCOIN trading volume is 0.0001.
let volume = BigDecimal::from(1) / BigDecimal::from(10_000);
let low_price = BigDecimal::from(1) / BigDecimal::from(10);
// Min MYCOIN1 trading volume is 0.0001, but the actual volume is 0.00001
let low_rel_volume = &volume * &low_price;
let rc = block_on(mm.rpc(json!({
"userpass": mm.userpass,
"mmrpc": "2.0",
"method": "trade_preimage",
"params": {
"base": "MYCOIN",
"rel": "MYCOIN1",
"swap_method": "sell",
"price": low_price,
"volume": volume,
},
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
let actual: RpcErrorResponse<trade_preimage_error::VolumeTooLow> = json::from_str(&rc.1).unwrap();
assert_eq!(actual.error_type, "VolumeTooLow");
// Min MYCOIN1 trading volume is 0.0001.
let volume_threshold = BigDecimal::from(1) / BigDecimal::from(10_000);
let expected = trade_preimage_error::VolumeTooLow {
coin: "MYCOIN1".to_owned(),
volume: low_rel_volume,
threshold: volume_threshold,
};
assert_eq!(actual.error_data, Some(expected));
}

#[test]
Expand Down Expand Up @@ -1974,6 +2161,7 @@ mod docker_tests {
"rel": "MYCOIN1",
"swap_method": "setprice",
"max": true,
"price": "1",
})))
.unwrap();
assert!(rc.0.is_success(), "!trade_preimage: {}", rc.1);
Expand Down Expand Up @@ -2004,6 +2192,7 @@ mod docker_tests {
"rel": "MYCOIN1",
"swap_method": "sell",
"max": true,
"price": "1",
})))
.unwrap();
assert!(!rc.0.is_success(), "trade_preimage success, but should fail: {}", rc.1);
Expand Down
Loading

0 comments on commit 73875fd

Please sign in to comment.