Skip to content

Commit

Permalink
refactor: remove circular dependencies (#290)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
kaimen-sano authored Mar 11, 2024
2 parents 4a7a545 + 6850e45 commit afcd1d5
Show file tree
Hide file tree
Showing 70 changed files with 452 additions and 399 deletions.
13 changes: 0 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 11 additions & 11 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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" }

Expand Down
8 changes: 4 additions & 4 deletions contracts/liquidity_hub/epoch-manager/tests/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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!(
Expand Down
6 changes: 3 additions & 3 deletions contracts/liquidity_hub/epoch-manager/tests/epoch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
16 changes: 8 additions & 8 deletions contracts/liquidity_hub/epoch-manager/tests/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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]
Expand All @@ -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", &[]);
Expand Down
2 changes: 1 addition & 1 deletion contracts/liquidity_hub/epoch-manager/tests/instantiate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
19 changes: 3 additions & 16 deletions contracts/liquidity_hub/fee_collector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
4 changes: 0 additions & 4 deletions contracts/liquidity_hub/fee_collector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
4 changes: 0 additions & 4 deletions contracts/liquidity_hub/fee_collector/src/tests/mod.rs

This file was deleted.

11 changes: 11 additions & 0 deletions contracts/liquidity_hub/fee_collector_integration/.editorconfig
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions contracts/liquidity_hub/fee_collector_integration/.gitignore
Original file line number Diff line number Diff line change
@@ -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
42 changes: 42 additions & 0 deletions contracts/liquidity_hub/fee_collector_integration/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[package]
name = "fee_collector_integration"
version = "1.1.4"
authors = ["Kerber0x <kerber0x@protonmail.com>"]
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
5 changes: 5 additions & 0 deletions contracts/liquidity_hub/fee_collector_integration/lib.rs
Original file line number Diff line number Diff line change
@@ -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`.
14 changes: 14 additions & 0 deletions contracts/liquidity_hub/fee_collector_integration/rustfmt.toml
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading

0 comments on commit afcd1d5

Please sign in to comment.