refactor: remove circular dependencies (#290)

rust-analyzer does not play well with circular dependencies. See
rust-lang/rust-analyzer#14167 for comments
from many rust-analyzer maintainers around this issue.

Fundamentally, Cargo has (fragile) support for dev-dependencies having
circular dependencies. This works because Cargo makes a distinction
between the integration tests and the actual contract code. This allows
us to not have circular dependencies when building our code, even though
they may exist without an implementation akin to the Cargo build system.

Since rust-analyzer (and likely other tools in the Rust ecosystem) don't
fare well with circular dependencies, we lift the relevant code up to
separate crates (which only contain tests) to ensure there is no
cyclic dependencies.

In the future, we could take a look at removing white-whale-testing,
since it does not provide much value (and likely will lead to a higher
chance of circular deps).

Cargo.toml

@@ -2,16 +2,16 @@
 resolver = "2"
 
 members = [
-  "packages/*",
-  "contracts/liquidity_hub/pool-network/*",
-  "contracts/liquidity_hub/fee_collector",
-  "contracts/liquidity_hub/fee_distributor",
-  "contracts/liquidity_hub/fee-distributor-mock",
-  "contracts/liquidity_hub/whale_lair",
-  "contracts/liquidity_hub/vault-network/*",
-  "contracts/liquidity_hub/pool-manager",
-  "contracts/liquidity_hub/epoch-manager",
-  "contracts/liquidity_hub/vault-manager",
+	"packages/*",
+	"contracts/liquidity_hub/pool-network/*",
+	"contracts/liquidity_hub/fee_collector",
+	"contracts/liquidity_hub/fee_distributor",
+	"contracts/liquidity_hub/fee-distributor-mock",
+	"contracts/liquidity_hub/whale_lair",
+	"contracts/liquidity_hub/vault-network/*",
+	"contracts/liquidity_hub/pool-manager",
+	"contracts/liquidity_hub/epoch-manager",
+	"contracts/liquidity_hub/vault-manager",
 ]
 
 [workspace.package]
@@ -49,7 +49,7 @@ sha2 = { version = "=0.10.8" }
 sha256 = { version = "1.4.0" }
 protobuf = { version = "=3.2.0", features = ["with-bytes"] }
 prost = { version = "0.11.9", default-features = false, features = [
-  "prost-derive",
+	"prost-derive",
 ] }
 white-whale-std = { version = "1.1.1", path = "packages/white-whale-std" }
 

contracts/liquidity_hub/epoch-manager/tests/config.rs

@@ -21,7 +21,7 @@ fn update_config_successfully() {
     mock_instantiation(deps.as_mut(), info.clone()).unwrap();
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Config {}).unwrap();
-    let config_res: ConfigResponse = from_json(&query_res).unwrap();
+    let config_res: ConfigResponse = from_json(query_res).unwrap();
     assert_eq!(
         EpochConfig {
             duration: Uint64::new(86400),
@@ -42,7 +42,7 @@ fn update_config_successfully() {
     execute(deps.as_mut(), mock_env(), info, msg).unwrap();
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Config {}).unwrap();
-    let config_res: ConfigResponse = from_json(&query_res).unwrap();
+    let config_res: ConfigResponse = from_json(query_res).unwrap();
     assert_eq!(
         EpochConfig {
             duration: Uint64::new(172800),
@@ -61,7 +61,7 @@ fn update_config_unsuccessfully() {
     mock_instantiation(deps.as_mut(), info.clone()).unwrap();
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Config {}).unwrap();
-    let config_res: ConfigResponse = from_json(&query_res).unwrap();
+    let config_res: ConfigResponse = from_json(query_res).unwrap();
     assert_eq!(
         EpochConfig {
             duration: Uint64::new(86400),
@@ -90,7 +90,7 @@ fn update_config_unsuccessfully() {
     }
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Config {}).unwrap();
-    let config_res: ConfigResponse = from_json(&query_res).unwrap();
+    let config_res: ConfigResponse = from_json(query_res).unwrap();
 
     // has not changed
     assert_eq!(

contracts/liquidity_hub/epoch-manager/tests/epoch.rs

@@ -27,7 +27,7 @@ fn create_new_epoch_successfully() {
     let res = execute(deps.as_mut(), env, info, msg).unwrap();
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::CurrentEpoch {}).unwrap();
-    let epoch_response: EpochResponse = from_json(&query_res).unwrap();
+    let epoch_response: EpochResponse = from_json(query_res).unwrap();
 
     let current_epoch = EpochV2 {
         id: 124,
@@ -46,12 +46,12 @@ fn create_new_epoch_successfully() {
     );
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Epoch { id: 124 }).unwrap();
-    let epoch_response: EpochResponse = from_json(&query_res).unwrap();
+    let epoch_response: EpochResponse = from_json(query_res).unwrap();
 
     assert_eq!(epoch_response.epoch, current_epoch);
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Epoch { id: 123 }).unwrap();
-    let epoch_response: EpochResponse = from_json(&query_res).unwrap();
+    let epoch_response: EpochResponse = from_json(query_res).unwrap();
 
     assert_eq!(
         epoch_response.epoch,

contracts/liquidity_hub/epoch-manager/tests/hooks.rs

@@ -27,8 +27,8 @@ fn add_hook_successfully() {
         },
     )
     .unwrap();
-    let hook_registered: bool = from_json(&query_res).unwrap();
-    assert_eq!(hook_registered, false);
+    let hook_registered: bool = from_json(query_res).unwrap();
+    assert!(!hook_registered);
 
     execute(deps.as_mut(), mock_env(), info.clone(), msg).unwrap();
 
@@ -40,7 +40,7 @@ fn add_hook_successfully() {
         },
     )
     .unwrap();
-    let hook_registered: bool = from_json(&query_res).unwrap();
+    let hook_registered: bool = from_json(query_res).unwrap();
     assert!(hook_registered);
 
     for i in 2..10 {
@@ -51,7 +51,7 @@ fn add_hook_successfully() {
     }
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Hooks {}).unwrap();
-    let hooks_response: HooksResponse = from_json(&query_res).unwrap();
+    let hooks_response: HooksResponse = from_json(query_res).unwrap();
     assert_eq!(
         hooks_response,
         HooksResponse {
@@ -109,7 +109,7 @@ fn remove_hook_successfully() {
         },
     )
     .unwrap();
-    let hook_registered: bool = from_json(&query_res).unwrap();
+    let hook_registered: bool = from_json(query_res).unwrap();
     assert!(hook_registered);
 
     execute(deps.as_mut(), mock_env(), info, msg).unwrap();
@@ -122,8 +122,8 @@ fn remove_hook_successfully() {
         },
     )
     .unwrap();
-    let hook_registered: bool = from_json(&query_res).unwrap();
-    assert_eq!(hook_registered, false);
+    let hook_registered: bool = from_json(query_res).unwrap();
+    assert!(!hook_registered);
 }
 
 #[test]
@@ -145,7 +145,7 @@ fn remove_hook_unsuccessfully() {
         },
     )
     .unwrap();
-    let hook_registered: bool = from_json(&query_res).unwrap();
+    let hook_registered: bool = from_json(query_res).unwrap();
     assert!(hook_registered);
 
     let info = mock_info("unauthorized", &[]);

contracts/liquidity_hub/epoch-manager/tests/instantiate.rs

@@ -30,7 +30,7 @@ fn instantiation_successful() {
     instantiate(deps.as_mut(), mock_env(), info, msg).unwrap();
 
     let query_res = query(deps.as_ref(), mock_env(), QueryMsg::Config {}).unwrap();
-    let config_res: ConfigResponse = from_json(&query_res).unwrap();
+    let config_res: ConfigResponse = from_json(query_res).unwrap();
     assert_eq!(
         EpochConfig {
             duration: Uint64::new(86400),

contracts/liquidity_hub/fee_collector/Cargo.toml

@@ -11,9 +11,9 @@ documentation.workspace = true
 publish.workspace = true
 
 exclude = [
-  # Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication.
-  "contract.wasm",
-  "hash.txt",
+	# Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication.
+	"contract.wasm",
+	"hash.txt",
 ]
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
@@ -41,16 +41,3 @@ serde.workspace = true
 thiserror.workspace = true
 white-whale-std.workspace = true
 cosmwasm-schema.workspace = true
-
-[dev-dependencies]
-cw-multi-test.workspace = true
-terraswap-router = { path = "../pool-network/terraswap_router" }
-terraswap-factory = { path = "../pool-network/terraswap_factory" }
-terraswap-pair = { path = "../pool-network/terraswap_pair" }
-stableswap-3pool = { path = "../pool-network/stableswap_3pool" }
-terraswap-token = { path = "../pool-network/terraswap_token" }
-fee_distributor = { path = "../fee_distributor" }
-whale-lair = { path = "../whale_lair" }
-vault_factory = { version = "1.0.0", path = "../vault-network/vault_factory" }
-vault = { version = "1.0.0", path = "../vault-network/vault" }
-cw20.workspace = true

contracts/liquidity_hub/fee_collector/src/lib.rs

@@ -5,8 +5,4 @@ mod migrations;
 mod queries;
 pub mod state;
 
-#[cfg(test)]
-#[cfg(not(target_arch = "wasm32"))]
-pub mod tests;
-
 pub use crate::error::ContractError;

contracts/liquidity_hub/fee_collector_integration/.editorconfig

@@ -0,0 +1,11 @@
+root = true
+
+[*]
+indent_style = space
+indent_size = 2
+charset = utf-8
+trim_trailing_whitespace = true
+insert_final_newline = true
+
+[*.rs]
+indent_size = 4

contracts/liquidity_hub/fee_collector_integration/.gitignore

@@ -0,0 +1,19 @@
+# Generated by Cargo
+# will have compiled files and executables
+/target/
+
+artifacts
+
+# These are backup files generated by rustfmt
+**/*.rs.bk
+
+# macOS
+.DS_Store
+
+# IDEs
+.vscode/
+.idea/
+*.iml
+
+# Auto-gen
+.cargo-ok

contracts/liquidity_hub/fee_collector_integration/Cargo.toml

@@ -0,0 +1,42 @@
+[package]
+name = "fee_collector_integration"
+version = "1.1.4"
+authors = ["Kerber0x <[email protected]>"]
+edition.workspace = true
+description = "Integration tests for the fee_collector contract"
+license.workspace = true
+repository.workspace = true
+homepage.workspace = true
+documentation.workspace = true
+publish.workspace = true
+
+exclude = [
+	# Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication.
+	"contract.wasm",
+	"hash.txt",
+]
+
+[lib]
+path = "lib.rs"
+
+[features]
+# for more explicit tests, cargo test --features=backtraces
+
+[dev-dependencies]
+cosmwasm-std.workspace = true
+cw2.workspace = true
+cw20.workspace = true
+cw-multi-test.workspace = true
+white-whale-std.workspace = true
+fee_collector = { path = "../fee_collector" }
+terraswap-router = { path = "../pool-network/terraswap_router" }
+terraswap-factory = { path = "../pool-network/terraswap_factory" }
+terraswap-pair = { path = "../pool-network/terraswap_pair" }
+stableswap-3pool = { path = "../pool-network/stableswap_3pool" }
+terraswap-token = { path = "../pool-network/terraswap_token" }
+fee_distributor = { path = "../fee_distributor" }
+whale-lair = { path = "../whale_lair" }
+vault_factory = { version = "1.0.0", path = "../vault-network/vault_factory" }
+vault = { version = "1.0.0", path = "../vault-network/vault" }
+serde.workspace = true
+thiserror.workspace = true

contracts/liquidity_hub/fee_collector_integration/lib.rs

@@ -0,0 +1,5 @@
+// Cargo requires us to have a target specified in the manifest.
+//
+// This is, by default, scanned at `src/lib.rs` and `src/main.rs`, as well as the `[lib]` and `[[bin]]` sections of a `Cargo.toml`.
+//
+// Since we have none, we must manually create a `lib.rs`.

contracts/liquidity_hub/fee_collector_integration/rustfmt.toml

@@ -0,0 +1,14 @@
+# stable
+newline_style = "Unix"
+hard_tabs = false
+tab_spaces = 4
+
+# unstable... should we require `rustup run nightly cargo fmt` ?
+# or just update the style guide when they are stable?
+#fn_single_line = true
+#format_code_in_doc_comments = true
+#overflow_delimited_expr = true
+#reorder_impl_items = true
+#struct_field_align_threshold = 20
+#struct_lit_single_line = true
+#report_todo = "Always"

contracts/liquidity_hub/fee_collector/src/tests/common_integration.rs → contracts/liquidity_hub/fee_collector_integration/tests/fee_collector/common_integration.rs

@@ -2,7 +2,7 @@ use cosmwasm_std::testing::mock_info;
 use cosmwasm_std::{Addr, Coin, MessageInfo, Uint128};
 use cw_multi_test::{App, AppBuilder, BankKeeper, ContractWrapper, Executor};
 
-use crate::contract::{execute, instantiate, migrate, query, reply};
+use fee_collector::contract::{execute, instantiate, migrate, query, reply};
 
 use super::dummy_contract::create_dummy_flash_loan_contract;