[r2r] enhanced logging in spv and rpc_client mods #1594

borngraced · 2023-01-02T14:42:07Z

fixes: #1481

Signed-off-by: borngraced samiodev@icloud.com

Signed-off-by: borngraced <samiodev@icloud.com>

sergeyboyko0791

Awesome! I have only one minor suggestion

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

sergeyboyko0791

Thanks for the fixes! Please consider my comments

sergeyboyko0791 · 2023-01-03T11:42:12Z

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

@@ -518,7 +545,14 @@ async fn update_single_order(
    let resp = update_maker_order(ctx, req)
        .await
        .map_to_mm(OrderProcessingError::OrderUpdateError)?;
-    info!("Successfully update order for {} - uuid: {}", key_trade_pair, resp.uuid);
+    let (volume, _) = calculate_volume(ctx, &cfg, Some(&base_balance), &rates.base_price).await?;


We can reuse the volume returning from prepare_order function:

- let (min_vol, _, calculated_price, is_max, base_balance) = prepare_order(rates, &cfg, &key_trade_pair, ctx).await?; + let (min_vol, volume, calculated_price, is_max, base_balance) = prepare_order(rates, &cfg, &key_trade_pair, ctx).await?;

sergeyboyko0791 · 2023-01-03T11:43:00Z

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

+//async fn prepare_order(cfg) {
+//    let mut is_max = cfg.max.unwrap_or(false);
+//    let (vol, is_max) = calculate_volume(ctx, cfg, base_balance, price).await?;
+//    let is_max = cfg.max.unwrap_or_default() | is_max;
+//}


This can be removed

sergeyboyko0791 · 2023-01-03T11:44:10Z

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

@@ -491,21 +518,21 @@ async fn prepare_order(
        None => None,
    };

-    Ok((min_vol, volume, calculated_price, is_max))
+    Ok((min_vol, volume, calculated_price, is_max, base_balance))


I believe that after these changes:

- let (min_vol, _, calculated_price, is_max, base_balance) = prepare_order(rates, &cfg, &key_trade_pair, ctx).await?; + let (min_vol, volume, calculated_price, is_max, base_balance) = prepare_order(rates, &cfg, &key_trade_pair, ctx).await?;

the base_balance is no longed required to be returned from prepare_order

Signed-off-by: borngraced <samiodev@icloud.com>

sergeyboyko0791

Awesome, thanks!

sergeyboyko0791 · 2023-01-03T13:41:35Z

@smk762 could you please check if this extends the logs as you asked at #1481

smk762 · 2023-01-03T15:44:47Z

I see

03 15:37:20, simple_market_maker:577] INFO Successfully placed order for LTC-KMD - uuid: 0aa2e82c-0e09-4520-b727-c6b7a021f2bf - rate: (403.5584 LTC-KMD) - volume 0
03 15:37:21, simple_market_maker:577] INFO Successfully placed order for KMD-LTC - uuid: 41a73431-dc82-40dd-bbb6-1a8854a649a0 - rate: (0.0025 KMD-LTC) - volume 0

and in my_orders

"price": "0.002567964983164983164983164983164983164983164983164983164983164983164983164983164983164983164983164983",
"available_amount": "5.00490775"

"price": "403.5584623598505072076882007474639615589962626801922050186865990389749065670048051254671649759743727",
"available_amount": "0.20379761"

Seems volume not calculated, as logs show zero. Can we please extend the logs to 8 decimal places?

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced · 2023-01-03T20:09:30Z

I see
03 15:37:20, simple_market_maker:577] INFO Successfully placed order for LTC-KMD - uuid: 0aa2e82c-0e09-4520-b727-c6b7a021f2bf - rate: (403.5584 LTC-KMD) - volume 0
03 15:37:21, simple_market_maker:577] INFO Successfully placed order for KMD-LTC - uuid: 41a73431-dc82-40dd-bbb6-1a8854a649a0 - rate: (0.0025 KMD-LTC) - volume 0
and in my_orders

"price": "0.002567964983164983164983164983164983164983164983164983164983164983164983164983164983164983164983164983", "available_amount": "5.00490775"

"price": "403.5584623598505072076882007474639615589962626801922050186865990389749065670048051254671649759743727", "available_amount": "0.20379761"

Seems volume not calculated, as logs show zero. Can we please extend the logs to 8 decimal places?

@smk762 this is ready for another round of test..

smk762 · 2023-01-04T06:18:34Z

Thanks, I see extra decimals now

04 06:16:56, simple_market_maker:576] INFO Successfully placed order for LTC-KMD - uuid: a09e210e-f5f9-48de-9a8c-586acc703010 - rate: (414.88914893 LTC-KMD) - volume 0
04 06:16:58, simple_market_maker:576] INFO Successfully placed order for KMD-LTC - uuid: 22210f77-ac77-4d19-ab05-bc4c8c65acaf - rate: (0.00249783 KMD-LTC) - volume 0

Volume still zero tho

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced · 2023-01-05T12:10:46Z

@smk762, if max: true, we can't print a correct volume as it's calculated later outside of the market bot context, So I guess the behavior is normal in this case as you set max to be true in your config.

PS. I handled this case to print "max volume" instead of volume in the log

smk762 · 2023-01-05T12:38:29Z

Cheers, here's latest logs with config change to not be max: true

05 12:37:10, simple_market_maker:591] INFO Successfully placed order for LTC-KMD - uuid: 26647352-fe5a-4bf5-940a-1602f18b02a5 - rate: (407.63481046 LTC-KMD) - volume 0.06666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666667
05 12:37:11, simple_market_maker:585] INFO Successfully update order for KMD-LTC - uuid: 05780fcd-e9e7-4f53-8b96-025db434550b - rate: (0.00254228 KMD-LTC) - max volume

Could we please likewise limit the volume to 8 decimal places?

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced · 2023-01-05T12:49:36Z

Cheers, here's latest logs with config change to not be max: true

05 12:37:10, simple_market_maker:591] INFO Successfully placed order for LTC-KMD - uuid: 26647352-fe5a-4bf5-940a-1602f18b02a5 - rate: (407.63481046 LTC-KMD) - volume 0.06666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666667
05 12:37:11, simple_market_maker:585] INFO Successfully update order for KMD-LTC - uuid: 05780fcd-e9e7-4f53-8b96-025db434550b - rate: (0.00254228 KMD-LTC) - max volume

Could we please likewise limit the volume to 8 decimal places?

all done now.. @smk762

smk762

Thanks, log output is now nice and clean and informative

05 13:17:13, simple_market_maker:592] INFO Successfully placed order for LTC-KMD - uuid: a69504ed-7ae1-4ea2-9918-0a23c8de6750 - rate: (406.00490928 LTC-KMD) - volume: 0.06689858
05 13:17:15, simple_market_maker:586] INFO Successfully update order for KMD-LTC - uuid: 27997929-260b-4be4-b024-1aa8a39041be - rate: (0.00255249 KMD-LTC) - max volume

sergeyboyko0791

A few more changes please

sergeyboyko0791 · 2023-01-06T11:54:33Z

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

+    if is_max {
+        info!(
+            "Successfully update order for {key_trade_pair} - uuid: {} - rate: ({:.8} {key_trade_pair}) - max volume",
+            resp.uuid,
+            calculated_price.to_decimal()
+        );
+    } else {


Could you please refactor this as

let vol_info = if is_max { "max volume".to_string() } else { format!("volume: {:.8}", volume.to_decimal()) }; info!("Successfully update order for {key_trade_pair} - uuid: {} - rate: ({:.8} {key_trade_pair}) - {vol_info}", resp.uuid, calculated_price.to_decimal());

is this really needed? done anyways

Do you think the changed version looks more clear and readable?
It also helped to to shorten by 4 lines

sergeyboyko0791 · 2023-01-06T11:56:14Z

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

+    if is_max {
+        info!(
+            "Successfully update order for {key_trade_pair} - uuid: {} - rate: ({:.8} {key_trade_pair}) - max volume",
+            resp.uuid,
+            calculated_price.to_decimal()
+        );


The same here please

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs

Signed-off-by: borngraced <samiodev@icloud.com>

sergeyboyko0791

Thank you! LGTM!

sergeyboyko0791 · 2023-01-10T10:15:40Z

@shamardy could you please also take a look?

shamardy

Only one suggestion :)

mm2src/coins/utxo/rpc_clients.rs

Signed-off-by: borngraced <samiodev@icloud.com>

shamardy

Just a small change!

mm2src/mm2_bitcoin/spv_validation/src/helpers_validation.rs

Signed-off-by: borngraced <samiodev@icloud.com>

shamardy

Thanks for the fixes!

sergeyboyko0791

Re-approve

enhanced logging in spv and rpc mods

eca3b31

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced requested a review from sergeyboyko0791 January 2, 2023 14:42

borngraced self-assigned this Jan 2, 2023

borngraced changed the title ~~enhanced logging in spv and rpc_client mods~~ [r2r] enhanced logging in spv and rpc_client mods Jan 2, 2023

sergeyboyko0791 reviewed Jan 2, 2023

View reviewed changes

mm2src/mm2_main/src/lp_ordermatch/simple_market_maker.rs Show resolved Hide resolved

sergeyboyko0791 added this to Todo in MM 2.0 via automation Jan 2, 2023

sergeyboyko0791 moved this from Todo to In progress in MM 2.0 Jan 2, 2023

sergeyboyko0791 linked an issue Jan 2, 2023 that may be closed by this pull request

[FR] Logging enhancements #1481

Closed

borngraced added 3 commits January 2, 2023 20:12

include maxVolume in log

770799a

create helper function for max volume calculation

2e217f9

fix clippy warning

8f7d8de

sergeyboyko0791 suggested changes Jan 3, 2023

View reviewed changes

borngraced added 3 commits January 3, 2023 13:55

remove base_balance from calculate_max_volume return type

ea9a73b

Signed-off-by: borngraced <samiodev@icloud.com>

update prepare_order

7c230bf

Signed-off-by: borngraced <samiodev@icloud.com>

remove calculate_volume

eceec15

Signed-off-by: borngraced <samiodev@icloud.com>

sergeyboyko0791 previously approved these changes Jan 3, 2023

View reviewed changes

extend volume decimal

cbc1e8b

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced dismissed sergeyboyko0791’s stale review via cbc1e8b January 3, 2023 20:08

borngraced changed the title ~~[r2r] enhanced logging in spv and rpc_client mods~~ [wip] enhanced logging in spv and rpc_client mods Jan 5, 2023

minor changes

f45eb77

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced changed the title ~~[wip] enhanced logging in spv and rpc_client mods~~ [r2r] enhanced logging in spv and rpc_client mods Jan 5, 2023

limit volume decimals

a868072

Signed-off-by: borngraced <samiodev@icloud.com>

smk762 self-requested a review January 5, 2023 13:17

smk762 previously approved these changes Jan 5, 2023

View reviewed changes

artemii235 moved this from In progress to PR review in MM 2.0 Jan 5, 2023

sergeyboyko0791 suggested changes Jan 6, 2023

View reviewed changes

remove cloning cfg and minor changes

5dd35cc

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced dismissed smk762’s stale review via 5dd35cc January 6, 2023 15:13

sergeyboyko0791 previously approved these changes Jan 6, 2023

View reviewed changes

smk762 self-requested a review January 6, 2023 16:05

smk762 previously approved these changes Jan 6, 2023

View reviewed changes

sergeyboyko0791 requested a review from shamardy January 10, 2023 10:15

shamardy reviewed Jan 10, 2023

View reviewed changes

mm2src/coins/utxo/rpc_clients.rs Outdated Show resolved Hide resolved

add coin to UnableToGetMerkle

2b26226

Signed-off-by: borngraced <samiodev@icloud.com>

borngraced dismissed stale reviews from smk762 and sergeyboyko0791 via 2b26226 January 10, 2023 17:20

shamardy reviewed Jan 11, 2023

View reviewed changes

mm2src/mm2_bitcoin/spv_validation/src/helpers_validation.rs Outdated Show resolved Hide resolved

borngraced added 2 commits January 11, 2023 13:02

minor changes

f45d622

Signed-off-by: borngraced <samiodev@icloud.com>

Merge remote-tracking branch 'origin/dev' into loggin-enhancements

bd33eca

shamardy approved these changes Jan 11, 2023

View reviewed changes

sergeyboyko0791 approved these changes Jan 13, 2023

View reviewed changes

sergeyboyko0791 merged commit 11194fe into dev Jan 13, 2023

MM 2.0 automation moved this from PR review to Done Jan 13, 2023

sergeyboyko0791 deleted the loggin-enhancements branch January 13, 2023 14:48

borngraced mentioned this pull request Feb 17, 2023

[release] 1.0.0-beta #1576

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[r2r] enhanced logging in spv and rpc_client mods #1594

[r2r] enhanced logging in spv and rpc_client mods #1594

borngraced commented Jan 2, 2023

sergeyboyko0791 left a comment

sergeyboyko0791 left a comment

sergeyboyko0791 Jan 3, 2023

borngraced Jan 3, 2023

sergeyboyko0791 Jan 3, 2023

sergeyboyko0791 Jan 3, 2023

borngraced Jan 3, 2023

sergeyboyko0791 left a comment

sergeyboyko0791 commented Jan 3, 2023

smk762 commented Jan 3, 2023

borngraced commented Jan 3, 2023

smk762 commented Jan 4, 2023

borngraced commented Jan 5, 2023

smk762 commented Jan 5, 2023

borngraced commented Jan 5, 2023

smk762 left a comment

sergeyboyko0791 left a comment

sergeyboyko0791 Jan 6, 2023

borngraced Jan 6, 2023

sergeyboyko0791 Jan 6, 2023

sergeyboyko0791 Jan 6, 2023

sergeyboyko0791 left a comment

sergeyboyko0791 commented Jan 10, 2023

shamardy left a comment

shamardy left a comment

shamardy left a comment •

edited

sergeyboyko0791 left a comment

[r2r] enhanced logging in spv and rpc_client mods #1594

[r2r] enhanced logging in spv and rpc_client mods #1594

Conversation

borngraced commented Jan 2, 2023

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 commented Jan 3, 2023

smk762 commented Jan 3, 2023

borngraced commented Jan 3, 2023

smk762 commented Jan 4, 2023

borngraced commented Jan 5, 2023

smk762 commented Jan 5, 2023

borngraced commented Jan 5, 2023

smk762 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 commented Jan 10, 2023

shamardy left a comment

Choose a reason for hiding this comment

shamardy left a comment

Choose a reason for hiding this comment

shamardy left a comment • edited

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

shamardy left a comment •

edited