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

feat(00-hello-world-counter): implement the classic, counter contract for testing #155

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Cargo.lock

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

Binary file modified artifacts/broker_bank.wasm
Binary file not shown.
Binary file modified artifacts/broker_staking.wasm
Binary file not shown.
26 changes: 14 additions & 12 deletions artifacts/checksums.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
e3d3422db32e4e6ebe0247a22f6be733ccf6b637bfeb4ff8ffcd8122a68d7853 broker_bank.wasm
40288f4cda1e48740269ab6425ca234b9b99419c165aaf1d410fa37c40843e49 broker_staking.wasm
7b674c043971df4c0a362ab8f6142680afeae32f4be88fc6a5d1e8c0e7dab680 broker_bank.wasm
b544cf24505320138bcd467ab73e8c2a206fe926113e61cf2caeb604d2e25b25 broker_staking.wasm
382c05baf544f2886de849933ecf59e8bc3bcdcdd552d5a63537bd6d63f2ecf1 controller.wasm
ed4a89ae4669b22863fcabd18e3bd7e40d039899f863a07eb0449eed059a898e core_token_vesting_v2.wasm
b56a880d4c67d9f353f549b502256f73159f89b50aa6dae683948e117efa4792 cw3_flex_multisig.wasm
1ecff403bbf3b5fcedccb5de76a0ef5f1fdbcc5f60890e3388f5425584899f0b incentives.wasm
dc89ed88f1c69bf63fc284492b7bf6935e3a85da2945067d70f71f08c01df60d lockup.wasm
222eac4e17c7ddffbecde0b196bc06ed1e458b8578ab25ed200a6fd7db4e5eda nibi_stargate.wasm
ef5b4de76526713e3531c3b9bbc4620b5d61599c4a0e8605365ebb0f1d7ee2ac nibi_stargate_perp.wasm
0074489ff40c8ecbd766f7140b32d288dcaf7302ba630d452f79e7d292ea57ef nusd_valuator.wasm
955592d08017aa41f3c9ba3883153d6de024e8c7a3a79aa3b664a241ec1e7a19 pricefeed.wasm
89e3236c932a73575bf39da532bcb93f8e4a5f4a3a7f3836e43f8970993eb809 shifter.wasm
8d982ca2d679ea8d44f825fe91a3d4e0cb92150b12e4684497eee9e76991d247 token_vesting.wasm
77efd174302c8d4622c1ce855b652aeefd3695064caaba90582bac89213a3187 core_token_vesting_v2.wasm
6b447bd36b84ec14c602c92ff5a2efbec6b9970ea984ee43e4829570c0dc75e3 cw3_fixed_multisig.wasm
8999a6e88c168045833de9a5f77529856ece75a14556aedd3a79c7eda60a3f6a cw3_flex_multisig.wasm
aede52ea6c6e1df652d77da770111e12a606e7718ac54d07a792c509bbb1081e hello_world_counter.wasm
55aef2add88da435dd5bf11aa29eb51edab909a03515e701f771b2869ca43fe1 incentives.wasm
cbd8a9774b2da7d9aceeec555bc44cd20252a05a0ad50b4968a5041df06df8b3 lockup.wasm
32ccda53511b00dffa0bd5b95ef11165a03c955545aac6d2ecc21830e9f610b2 nibi_stargate.wasm
74fc402b1b31336838fdc814f89f17a7ffe4b019422097b38d92e92fd61d6c64 nibi_stargate_perp.wasm
6de11082ba049deaf4702329195f065e555f51a7fbb7d102443e94e75beee1da nusd_valuator.wasm
83d4b903c7aed4f19f8b762ef6bccf286de68ac71fd6e0149fb46274afda7379 pricefeed.wasm
3c9725f2171571f17f921da0bbb60606838d2ad1861f2790d820f79d819036c5 shifter.wasm
584685070441058ff7185e9143113703e09a53852b29795aaaf2e9902eab1ceb token_vesting.wasm
Binary file modified artifacts/core_token_vesting_v2.wasm
Binary file not shown.
Binary file added artifacts/cw3_fixed_multisig.wasm
Binary file not shown.
Binary file modified artifacts/cw3_flex_multisig.wasm
Binary file not shown.
Binary file added artifacts/hello_world_counter.wasm
Binary file not shown.
Binary file modified artifacts/incentives.wasm
Binary file not shown.
Binary file modified artifacts/lockup.wasm
Binary file not shown.
Binary file modified artifacts/nibi_stargate.wasm
Binary file not shown.
Binary file modified artifacts/nibi_stargate_perp.wasm
Binary file not shown.
Binary file modified artifacts/nusd_valuator.wasm
Binary file not shown.
Binary file modified artifacts/pricefeed.wasm
Binary file not shown.
Binary file modified artifacts/shifter.wasm
Binary file not shown.
Binary file modified artifacts/token_vesting.wasm
Binary file not shown.
9 changes: 9 additions & 0 deletions contracts/00-hello-world-counter/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[alias]
wasm = "build --release --lib --target wasm32-unknown-unknown"
wasm-debug = "build --lib --target wasm32-unknown-unknown"
schema = "run --bin schema"

# OLDER:
# wasm = "build --release --target wasm32-unknown-unknown"
# wasm-debug = "build --target wasm32-unknown-unknown"
# schema = "run --example schema"
31 changes: 31 additions & 0 deletions contracts/00-hello-world-counter/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
[package]
name = "hello-world-counter"
version = "0.1.0"
edition = "2021"
homepage = { workspace = true }
repository = { workspace = true }

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
crate-type = ["cdylib", "rlib"]

[features]
# features.library: Use the library feature to disable all
# instantiate/execute/query exports. This is necessary use this as a dependency
# for another smart contract crate.
library = []

[dependencies]
cosmwasm-std = { workspace = true }
cosmwasm-schema = { workspace = true }
cw-storage-plus = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true }
thiserror = { workspace = true }
nibiru-std = { workspace = true }
cw2 = { workspace = true }
serde_json = { workspace = true }
anyhow = { workspace = true }
Comment on lines +18 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Dependency Definition Issue: Missing in Workspace Root

The dependencies homepage and repository are not defined in the workspace root Cargo.toml. Please ensure these dependencies are correctly added to maintain consistency across the project.

  • homepage
  • repository
🔗 Analysis chain

LGTM: Dependencies are well-structured and appropriate.

The dependencies list includes all necessary libraries for CosmWasm smart contract development in the Nibiru ecosystem. Using workspace = true for all dependencies ensures version consistency across the project.

To ensure all dependencies are correctly defined in the workspace, please run the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all dependencies are defined in the workspace root Cargo.toml

# Test: Check if all dependencies are defined in the workspace
grep -E '^\s*[a-zA-Z0-9_-]+ = \{ workspace = true \}' contracts/00-hello-world-counter/Cargo.toml | cut -d'=' -f1 | while read -r dep; do
    if ! grep -q "$dep" Cargo.toml; then
        echo "Warning: $dep is not defined in the workspace root Cargo.toml"
    fi
done

Length of output: 715


Script:

#!/bin/bash
# Description: Verify that all dependencies in contracts/00-hello-world-counter/Cargo.toml are defined in the workspace root Cargo.toml

# Extract dependency names from the project Cargo.toml
DEPENDENCIES=$(grep -E '^\s*[a-zA-Z0-9_-]+ = \{ workspace = true \}' contracts/00-hello-world-counter/Cargo.toml | cut -d'=' -f1 | tr -d ' ')

# Iterate through each dependency and check its presence in the workspace root Cargo.toml
for dep in $DEPENDENCIES; do
    if ! grep -q "^$dep\s*=" Cargo.toml; then
        echo "Warning: Dependency '$dep' is not defined in the workspace root Cargo.toml"
    fi
done

Length of output: 1476


[dev-dependencies]
easy-addr = { workspace = true }
4 changes: 4 additions & 0 deletions contracts/00-hello-world-counter/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# contracts/00-hello-world-counter

The classic "counter" smart contract. It serves as a hello-world example on
how to implement execute messages and queries in Wasm.
185 changes: 185 additions & 0 deletions contracts/00-hello-world-counter/src/contract.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
use cosmwasm_std::{
to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Response,
};
use cw2::set_contract_version;

use crate::{
msg::{ExecuteMsg, InstantiateMsg, QueryMsg},
state::{State, STATE},
};

type ContractError = anyhow::Error;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider defining a custom ContractError enum for better error handling.

Currently, ContractError is defined as anyhow::Error. Using a custom ContractError enum derived from thiserror::Error can provide more precise error handling and clearer error messages. This approach aligns with CosmWasm best practices and improves maintainability.


#[cfg_attr(not(feature = "library"), cosmwasm_std::entry_point)]
pub fn instantiate(
deps: DepsMut,
_env: Env,
info: MessageInfo,
msg: InstantiateMsg,
) -> Result<Response, ContractError> {
set_contract_version(
deps.storage,
format!("nibiru-wasm/contracts/{CONTRACT_NAME}"),
CONTRACT_VERSION,
)?;

STATE.save(
deps.storage,
&State {
count: msg.count,
owner: info.sender.clone(),
},
)?;
Ok(Response::default())
}

#[cfg_attr(not(feature = "library"), cosmwasm_std::entry_point)]
pub fn execute(
deps: DepsMut,
_env: Env,
info: MessageInfo,
msg: ExecuteMsg,
) -> Result<Response, ContractError> {
match msg {
ExecuteMsg::Increment {} => {
STATE.update(
deps.storage,
|mut state| -> Result<_, anyhow::Error> {
state.count += 1;
Ok(state)
},
)?;
Ok(Response::default())
}
ExecuteMsg::Reset { count } => {
STATE.update(
deps.storage,
|mut state| -> Result<_, anyhow::Error> {
let owner = state.owner.clone();
if info.sender != owner {
return Err(anyhow::anyhow!(
"Unauthorized: only the owner ({owner}) can use reset",
));
Comment on lines +60 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid exposing the owner's address in error messages.

Including the owner's address in the error message could lead to unwanted disclosure of sensitive information. It's recommended to provide a generic error message without revealing the owner's identity.

Apply this diff to modify the error message:

 return Err(anyhow::anyhow!(
-    "Unauthorized: only the owner ({owner}) can use reset",
+    "Unauthorized: only the owner can use reset",
));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return Err(anyhow::anyhow!(
"Unauthorized: only the owner ({owner}) can use reset",
));
return Err(anyhow::anyhow!(
"Unauthorized: only the owner can use reset",
));

}
state.count = count;
Ok(state)
},
)?;
Ok(Response::default())
}
}
}

#[cfg_attr(not(feature = "library"), cosmwasm_std::entry_point)]
pub fn query(
deps: Deps,
_env: Env,
msg: QueryMsg,
) -> Result<Binary, ContractError> {
match msg {
QueryMsg::Count {} => {
let state = STATE.load(deps.storage)?;
Ok(to_json_binary(&state)?)
}
Comment on lines +81 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Limit the data exposed in query responses.

The QueryMsg::Count {} handler returns the entire State, which includes the owner's address. To enhance security and privacy, consider returning only the count value, unless there's a specific need to expose the owner's address.

Refactor the query handler to return only the count:

-    let state = STATE.load(deps.storage)?;
-    Ok(to_json_binary(&state)?)
+    let count = STATE.load(deps.storage)?.count;
+    Ok(to_json_binary(&count)?)

Also, update the corresponding response type in your msg module to reflect the new return structure.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let state = STATE.load(deps.storage)?;
Ok(to_json_binary(&state)?)
}
let count = STATE.load(deps.storage)?.count;
Ok(to_json_binary(&count)?)
}

}
}

pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME");
pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");

#[cfg(test)]
pub mod tests {

use easy_addr;
use nibiru_std::errors::TestResult;

use crate::{
contract::{execute, query},
msg::{ExecuteMsg, QueryMsg},
state::State,
tutil::{mock_info_for_sender, setup_contract, TEST_OWNER},
};

struct TestCaseExec<'a> {
exec_msg: ExecuteMsg,
sender: &'a str,
err: Option<&'a str>,
start_count: i64,
want_count_after: i64,
}

/// Test that all owner-gated execute calls fail when the tx sender is not
/// the smart contract owner.
#[test]
pub fn test_exec() -> TestResult {
let not_owner = easy_addr::addr!("not-owner");

let test_cases: Vec<TestCaseExec> = vec![
TestCaseExec {
sender: not_owner,
exec_msg: ExecuteMsg::Increment {},
err: None,
start_count: 0,
want_count_after: 1,
},
TestCaseExec {
sender: not_owner,
exec_msg: ExecuteMsg::Increment {},
err: None,
start_count: -70,
want_count_after: -69,
},
TestCaseExec {
sender: TEST_OWNER,
exec_msg: ExecuteMsg::Reset { count: 25 },
err: None,
start_count: std::i64::MAX,
want_count_after: 25,
},
TestCaseExec {
sender: TEST_OWNER,
exec_msg: ExecuteMsg::Reset { count: -25 },
err: None,
start_count: 0,
want_count_after: -25,
},
TestCaseExec {
sender: not_owner,
exec_msg: ExecuteMsg::Reset { count: 25 },
err: Some("Unauthorized: only the owner"),
start_count: 0, // unused
want_count_after: 0, // unused
},
];

for tc in &test_cases {
// instantiate smart contract from the owner
let (mut deps, env, _info) = setup_contract(tc.start_count)?;

// send the exec msg and it should fail.
let info = mock_info_for_sender(tc.sender);
let res =
execute(deps.as_mut(), env.clone(), info, tc.exec_msg.clone());

if let Some(want_err) = tc.err {
let err = res.expect_err("err should be defined");
let is_contained = err.to_string().contains(want_err);
assert!(is_contained, "got error {}", err);
Comment on lines +165 to +167
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling in tests by matching error types instead of messages.

In the tests, errors are being matched by comparing error messages as strings, which can be fragile and may break if error messages change. Defining a custom ContractError enum with specific variants allows for more robust error handling and testing.

Consider updating your error handling approach:

  1. Define a custom ContractError enum with variants for different error cases.
  2. Implement thiserror::Error for the ContractError enum.
  3. In your contract logic, return specific ContractError variants.
  4. In tests, match against these variants instead of comparing error message strings.

continue;
}

let res = res?;
assert_eq!(res.messages.len(), 0);

let query_req = QueryMsg::Count {};
let binary = query(deps.as_ref(), env, query_req)?;
let query_resp: State = cosmwasm_std::from_json(binary)?;

let state = query_resp;
let got_count_after = state.count;
assert_eq!(got_count_after, tc.want_count_after);
assert_eq!(state.owner.as_str(), TEST_OWNER);
}
Ok(())
}
}
5 changes: 5 additions & 0 deletions contracts/00-hello-world-counter/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub mod contract;
pub mod msg;
pub mod state;
#[cfg(test)]
pub mod tutil;
21 changes: 21 additions & 0 deletions contracts/00-hello-world-counter/src/msg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use crate::state;
use cosmwasm_schema::cw_serde;

#[cw_serde]
pub enum ExecuteMsg {
Increment {}, // Increase count by 1
Reset { count: i64 }, // Reset to any i64 value
}

#[cw_serde]
#[derive(cosmwasm_schema::QueryResponses)]
pub enum QueryMsg {
// Count returns the JSON-encoded state
#[returns(state::State)]
Count {},
}

#[cw_serde]
pub struct InstantiateMsg {
pub count: i64,
}
11 changes: 11 additions & 0 deletions contracts/00-hello-world-counter/src/state.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use cosmwasm_schema::cw_serde;
use cosmwasm_std::Addr;
use cw_storage_plus::Item;

pub const STATE: Item<State> = Item::new("state");

#[cw_serde]
pub struct State {
pub count: i64,
pub owner: Addr,
}
47 changes: 47 additions & 0 deletions contracts/00-hello-world-counter/src/tutil.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//! tutil.rs: Test helpers for the contract
#![cfg(not(target_arch = "wasm32"))]

use cosmwasm_std::{Env, MessageInfo, OwnedDeps};

#[cfg(not(target_arch = "wasm32"))]
use cosmwasm_std::testing::{
mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage,
};

use crate::{contract::instantiate, msg::InstantiateMsg};

pub const TEST_OWNER: &str = easy_addr::addr!("owner");

pub fn setup_contract(
count: i64,
) -> anyhow::Result<(
OwnedDeps<MockStorage, MockApi, MockQuerier>,
Env,
MessageInfo,
)> {
let mut deps = mock_dependencies();
let env = mock_env();
let info = mock_info(TEST_OWNER, &[]);
let msg = InstantiateMsg { count };
let res = instantiate(deps.as_mut(), env.clone(), info.clone(), msg)?;
assert_eq!(0, res.messages.len());
Ok((deps, env, info))
}

pub fn setup_contract_defaults() -> anyhow::Result<(
OwnedDeps<MockStorage, MockApi, MockQuerier>,
Env,
MessageInfo,
)> {
setup_contract(0)
}

pub fn mock_info_for_sender(sender: &str) -> MessageInfo {
mock_info(sender, &[])
}

pub fn mock_env_height(height: u64) -> Env {
let mut env = mock_env();
env.block.height = height;
env
}
2 changes: 1 addition & 1 deletion contracts/broker-bank/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "broker-bank"
version = "0.1.0"
edition = "2021"
homepage = { workspace = true }
repository = "https://github.com/NibiruChain/nibiru-wasm"
repository = { workspace = true }

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
Expand Down