Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support base URL with path #138

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Support base URL with path #138

merged 1 commit into from
Apr 7, 2022

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Apr 7, 2022

#132 introduced regressions that caused the order alerter to constantly fail to retrieve orders. This is because the public API URL contains /$network path prefix that wasn't supported by the alerter.

This PR adds support for this. Basically, it just joins instead of replacing the path entirely with set_path.

Test Plan

Run the alerter and see it make successful API requests:

% cargo run -p alerter -- --min-order-age 1 --time-without-trade 1
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/alerter --min-order-age 1 --time-without-trade 1`
2022-04-07T11:24:09.456Z  INFO alerter: running alerter with Arguments {
    time_without_trade: 1s,
    min_order_age: 1s,
    min_alert_interval: 1799.99997952s,
    errors_in_a_row_before_alert: 5,
    orderbook_api: "https://api.cow.fi/mainnet/",
}
2022-04-07T11:24:09.701Z DEBUG alerter: found 16 open orders
2022-04-07T11:24:09.701Z DEBUG alerter: found no fulfilled orders
2022-04-07T11:24:39.747Z DEBUG alerter: found 14 open orders
2022-04-07T11:24:39.802Z DEBUG alerter: found no fulfilled orders
2022-04-07T11:24:41.905Z DEBUG alerter: 0x url="https://api.0x.org/swap/v1/price?sellToken=0x6b175474e89094c44da98b954eedeac495271d0f&buyToken=0xdac17f958d2ee523a2206206994597c13d831ec7&sellAmount=811063811362965485568" response=Response { sell_amount: 811063811362965485568, buy_amount: 810420605 }
2022-04-07T11:24:43.441Z DEBUG alerter: 0x url="https://api.0x.org/swap/v1/price?sellToken=0x6b175474e89094c44da98b954eedeac495271d0f&buyToken=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48&sellAmount=475833914861854859264" response=Response { sell_amount: 475833914861854859264, buy_amount: 474553897 }
2022-04-07T11:24:43.952Z DEBUG alerter: 0x url="https://api.0x.org/swap/v1/price?sellToken=0xab846fb6c81370327e784ae7cbb6d6a6af6ff4bf&buyToken=0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2&sellAmount=45974334753652705821" response=Response { sell_amount: 45974334753652705821, buy_amount: 19342714737093254 }
2022-04-07T11:25:13.994Z DEBUG alerter: found 13 open orders
2022-04-07T11:25:14.084Z DEBUG alerter: found no fulfilled orders
2022-04-07T11:25:15.393Z DEBUG alerter: 0x url="https://api.0x.org/swap/v1/price?sellToken=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48&buyToken=0xdac17f958d2ee523a2206206994597c13d831ec7&sellAmount=220029560" response=Response { sell_amount: 220029560, buy_amount: 218997589 }
...

@nlordell nlordell requested a review from a team as a code owner April 7, 2022 11:28
@@ -12,7 +12,7 @@ reqwest = { version = "0.11", features = ["json"] }
serde = { version = "1.0", features = ["derive"] }
shared = { path = "../shared" }
clap = { version = "3.1", features = ["derive", "env"] }
tokio = { version = "1.15", features = ["macros", "time"] }
tokio = { version = "1.15", features = ["macros", "time", "rt-multi-thread"] }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, but I needed it when building the alerter crate individually, otherwise:

% cargo build -p alerter
...
   Compiling alerter v0.1.0 (/Users/nlordell/Developer/cowprotocol/services/crates/alerter)
error: The default runtime flavor is `multi_thread`, but the `rt-multi-thread` feature is disabled.
   --> crates/alerter/src/main.rs:277:1
    |
277 | #[tokio::main]
    | ^^^^^^^^^^^^^^
    |
    = note: this error originates in the attribute macro `tokio::main` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `alerter` due to previous error

@nlordell nlordell enabled auto-merge (squash) April 7, 2022 11:30
@nlordell nlordell merged commit e8580c5 into main Apr 7, 2022
@nlordell nlordell deleted the fix-alerter-startup-args branch April 7, 2022 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants