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

F/auction #49

Merged
merged 7 commits into from
Oct 9, 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
7 changes: 7 additions & 0 deletions packages/injective-test-tube/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## 1.13.2-auction - 2024-08-10

### Changed

- Updated to use `[email protected]` which is a manual fix for the Auction protos
Comment on lines +8 to +12
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

Date inconsistency and version clarification needed

The new changelog entry follows the established format and provides clear information about the changes. However, there are two points that need attention:

  1. The date (2024-08-10) is earlier than the date of the previous entry (1.13.2 - 2024-28-08). This creates a chronological inconsistency in the changelog.

  2. The version number (1.13.2-auction) suggests this is a special version based on 1.13.2. It would be helpful to add a brief explanation of why this special version was created and how it relates to the main version line.

Consider the following actions:

  1. Adjust the date to maintain chronological order, or if the date is correct, consider moving this entry after the 1.13.2 entry.
  2. Add a brief note explaining the purpose of this special "auction" version and how it fits into the version history.

Example:

## 1.13.2-auction - 2024-08-10

### Changed

- Updated to use `[email protected]` which is a manual fix for the Auction protos
- Note: This is a special version created to address specific Auction proto issues. It is based on version 1.13.2 but is not part of the main release line.


## 1.13.2 - 2024-28-08

### Changed

- Updated to use [email protected]

## 1.13.0-3 - 2024-14-08
Expand Down
5 changes: 3 additions & 2 deletions packages/injective-test-tube/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@ edition = "2021"
license = "MIT OR Apache-2.0"
name = "injective-test-tube"
repository = "https://github.com/InjectiveLabs/test-tube"
version = "1.13.2"
version = "1.13.2-auction"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

exclude = [ "injective-core", "test_artifacts" ]

[dependencies]
base64 = "0.21.5"
cosmrs = { version = "0.15.0", features = [ "cosmwasm", "rpc" ] }
cosmwasm-schema = { version = "2.1.1" }
cosmwasm-std = { version = "2.1.0", features = [ "abort", "cosmwasm_1_2", "cosmwasm_1_3", "cosmwasm_1_4", "cosmwasm_2_0", "iterator", "stargate" ] }
hex = "0.4.2"
injective-cosmwasm = { version = "0.3.0" }
injective-std = { version = "1.13.0" }
injective-std = { version = "=1.13.2-auction" }
prost = "0.12.3"
serde = "1.0.144"
serde_json = "1.0.85"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ func GenesisStateWithValSet(appInstance *app.InjectiveApp) (app.GenesisState, se
acc := authtypes.NewBaseAccountWithAddress(senderPrivKey.PubKey().Address().Bytes())

//////////////////////
// balances := []banktypes.Balance{}
balance := banktypes.Balance{
Address: acc.GetAddress().String(),
Coins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(100000000000000))),
Expand Down
89 changes: 89 additions & 0 deletions packages/injective-test-tube/src/module/auction.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
use injective_std::types::injective::auction::v1beta1::{
QueryAuctionParamsRequest, QueryAuctionParamsResponse, QueryCurrentAuctionBasketRequest,
QueryCurrentAuctionBasketResponse, QueryLastAuctionResultRequest,
QueryLastAuctionResultResponse, QueryModuleStateRequest, QueryModuleStateResponse,
};
use test_tube_inj::fn_query;

use test_tube_inj::module::Module;
use test_tube_inj::runner::Runner;

pub struct Auction<'a, R: Runner<'a>> {
runner: &'a R,
}

impl<'a, R: Runner<'a>> Module<'a, R> for Auction<'a, R> {
fn new(runner: &'a R) -> Self {
Self { runner }
}
}

impl<'a, R> Auction<'a, R>
where
R: Runner<'a>,
{
fn_query! {
pub query_auction_params ["/injective.auction.v1beta1.Query/AuctionParams"]: QueryAuctionParamsRequest => QueryAuctionParamsResponse
}

fn_query! {
pub query_current_auction_basket ["/injective.auction.v1beta1.Query/CurrentAuctionBasket"]: QueryCurrentAuctionBasketRequest => QueryCurrentAuctionBasketResponse
}

fn_query! {
pub query_module_state ["/injective.auction.v1beta1.Query/ModuleState"]: QueryModuleStateRequest => QueryModuleStateResponse
}

fn_query! {
pub query_last_auction_result ["/injective.auction.v1beta1.Query/LastAuctionResult"]: QueryLastAuctionResultRequest => QueryLastAuctionResultResponse
}
}

#[cfg(test)]
maxrobot marked this conversation as resolved.
Show resolved Hide resolved
mod tests {
use injective_std::types::{
cosmos::base::v1beta1::Coin as BaseCoin,
injective::auction::v1beta1::{
LastAuctionResult, Params, QueryAuctionParamsRequest, QueryLastAuctionResultRequest,
},
};

use crate::{Auction, InjectiveTestApp};
use test_tube_inj::Module;

#[test]
fn auction_integration() {
let app = InjectiveTestApp::new();

let auction = Auction::new(&app);

let response = auction
.query_auction_params(&QueryAuctionParamsRequest {})
.unwrap();
assert_eq!(
response.params,
Some(Params {
auction_period: 604800,
min_next_bid_increment_rate: 2_500_000_000_000_000u128.to_string()
})
);

let response = auction
.query_last_auction_result(&QueryLastAuctionResultRequest {})
.unwrap();
assert!(response.last_auction_result.is_some());

let result = response.last_auction_result.unwrap();
assert_eq!(
result,
LastAuctionResult {
amount: Some(BaseCoin {
denom: "inj".to_string(),
amount: "0".to_string()
}),
winner: "".to_string(),
round: 0u64,
}
);
}
Comment on lines +54 to +88
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

Add tests for all query methods in the Auction module

Currently, the test module includes tests for query_auction_params and query_last_auction_result functions. To ensure comprehensive test coverage and verify the functionality of all query methods, consider adding tests for the query_current_auction_basket and query_module_state functions.

Would you like assistance in writing these additional tests or opening a GitHub issue to track this task?

}
2 changes: 2 additions & 0 deletions packages/injective-test-tube/src/module/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod auction;
mod authz;
mod bank;
mod exchange;
Expand All @@ -12,6 +13,7 @@ mod wasmx;
pub use test_tube_inj::macros;
pub use test_tube_inj::module::Module;

pub use auction::Auction;
pub use authz::Authz;
pub use bank::Bank;
pub use exchange::Exchange;
Expand Down
2 changes: 0 additions & 2 deletions packages/injective-test-tube/src/module/oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@ mod tests {
)
.unwrap();

println!("{:#?}", res);

let proposal_id = res
.events
.iter()
Expand Down
15 changes: 7 additions & 8 deletions packages/test-tube/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ version = "2.0.1"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
base64 = "0.21.5"
cosmrs = { version = "0.15.0", features = [ "cosmwasm", "rpc" ] }
cosmwasm-std = { version = "2.1.0", features = [ "abort", "cosmwasm_1_2", "cosmwasm_1_3", "cosmwasm_1_4", "cosmwasm_2_0", "iterator", "stargate" ] }
prost = "0.12.4"
serde = "1.0.144"
serde_json = "1.0.85"
tendermint-proto = "0.32.0"
thiserror = "1.0.34"
base64 = "0.21.5"
cosmrs = { version = "0.20.0", features = [ "cosmwasm", "rpc" ] }
cosmwasm-std = { version = "2.1.0", features = [ "abort", "cosmwasm_1_2", "cosmwasm_1_3", "cosmwasm_1_4", "cosmwasm_2_0", "iterator", "stargate" ] }
prost = { version = "0.13.3", default-features = false, features = [ "prost-derive" ] }
serde = "1.0.144"
serde_json = "1.0.85"
thiserror = "1.0.34"

[dev-dependencies]
cw1-whitelist = "0.15.0"
Expand Down
3 changes: 0 additions & 3 deletions packages/test-tube/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,6 @@ extern "C" {
extern "C" {
pub fn IncreaseTime(envId: GoUint64, seconds: GoInt64);
}
extern "C" {
pub fn Execute(envId: GoUint64, base64ReqDeliverTx: GoString) -> *mut ::std::os::raw::c_char;
}
extern "C" {
pub fn Query(
envId: GoUint64,
Expand Down
91 changes: 61 additions & 30 deletions packages/test-tube/src/runner/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use cosmrs::crypto::secp256k1::SigningKey;
use cosmrs::proto::tendermint::v0_38::abci::ResponseFinalizeBlock;
use cosmrs::tx;
use cosmrs::tx::{Fee, SignerInfo};
use cosmwasm_std::Coin;
use cosmwasm_std::{Coin, Timestamp};
use prost::Message;

use crate::account::{Account, FeeSetting, SigningAccount};
use crate::bindings::{
AccountNumber, AccountSequence, FinalizeBlock, GetBlockHeight, GetBlockTime, GetParamSet,
GetValidatorAddress, GetValidatorPrivateKey, IncreaseTime, InitAccount, InitTestEnv, Query,
Simulate,
AccountNumber, AccountSequence, CleanUp, FinalizeBlock, GetBlockHeight, GetBlockTime,
GetParamSet, GetValidatorAddress, GetValidatorPrivateKey, IncreaseTime, InitAccount,
InitTestEnv, Query, Simulate,
};
use crate::redefine_as_go_string;
use crate::runner::error::{DecodeError, EncodeError, RunnerError};
Expand Down Expand Up @@ -96,16 +96,14 @@ impl BaseApp {
.map_err(DecodeError::Utf8Error)?
.to_string();

println!("pkey: {:?}", pkey);

let secp256k1_priv = BASE64_STANDARD
.decode(pkey)
.map_err(DecodeError::Base64DecodeError)?;

let signing_key = SigningKey::from_slice(&secp256k1_priv).unwrap();

let validator = SigningAccount::new(
"inj".to_string(),
self.address_prefix.clone(),
signing_key,
FeeSetting::Auto {
gas_price: Coin::new(INJECTIVE_MIN_GAS_PRICE, denom),
Expand All @@ -116,6 +114,27 @@ impl BaseApp {
Ok(validator)
}

pub fn get_chain_id(&self) -> &str {
&self.chain_id
}

pub fn get_account_sequence(&self, address: &str) -> u64 {
redefine_as_go_string!(address);
unsafe { AccountSequence(self.id, address) }
}

pub fn get_account_number(&self, address: &str) -> u64 {
redefine_as_go_string!(address);
unsafe { AccountNumber(self.id, address) }
}

/// Get the current block time
pub fn get_block_timestamp(&self) -> Timestamp {
let result = unsafe { GetBlockTime(self.id) };

Timestamp::from_nanos(result as u64)
}
Comment on lines +131 to +136
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

Handle potential negative values in get_block_timestamp

The GetBlockTime(self.id) function returns an i64, which may be negative. Casting a negative i64 to u64 can lead to incorrect timestamp values due to integer underflow. Consider checking for negative values before casting to u64 to ensure correctness.

Apply the following diff to handle potential negative block times:

 pub fn get_block_timestamp(&self) -> Timestamp {
     let result = unsafe { GetBlockTime(self.id) };
+    if result < 0 {
+        // Handle negative block time appropriately
+        // For example, return an error or a default Timestamp
+        panic!("Block time is negative");
+    }
     Timestamp::from_nanos(result as u64)
 }
📝 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
/// Get the current block time
pub fn get_block_timestamp(&self) -> Timestamp {
let result = unsafe { GetBlockTime(self.id) };
Timestamp::from_nanos(result as u64)
}
/// Get the current block time
pub fn get_block_timestamp(&self) -> Timestamp {
let result = unsafe { GetBlockTime(self.id) };
if result < 0 {
// Handle negative block time appropriately
// For example, return an error or a default Timestamp
panic!("Block time is negative");
}
Timestamp::from_nanos(result as u64)
}


/// Get the current block time
pub fn get_block_time_nanos(&self) -> i64 {
unsafe { GetBlockTime(self.id) }
Expand Down Expand Up @@ -187,23 +206,23 @@ impl BaseApp {
redefine_as_go_string!(addr);

let seq = unsafe { AccountSequence(self.id, addr) };

let account_number = unsafe { AccountNumber(self.id, addr) };

let signer_info = SignerInfo::single_direct(Some(signer.public_key()), seq);

let chain_id = self
.chain_id
.parse()
.expect("parse const str of chain id should never fail");

let auth_info = signer_info.auth_info(fee);
let sign_doc = tx::SignDoc::new(
&tx_body,
&auth_info,
&(self
.chain_id
.parse()
.expect("parse const str of chain id should never fail")),
account_number,
)
.map_err(|e| match e.downcast::<prost::EncodeError>() {
Ok(encode_err) => EncodeError::ProtoEncodeError(encode_err),
Err(e) => panic!("expect `prost::EncodeError` but got {:?}", e),
})?;
let sign_doc =
tx::SignDoc::new(&tx_body, &auth_info, &chain_id, account_number).map_err(|e| {
match e.downcast::<prost::EncodeError>() {
Ok(encode_err) => EncodeError::ProtoEncodeError(encode_err),
Err(e) => panic!("expect `prost::EncodeError` but got {:?}", e),
}
})?;

let tx_raw = sign_doc.sign(signer.signing_key()).unwrap();

Expand All @@ -224,15 +243,7 @@ impl BaseApp {
where
I: IntoIterator<Item = cosmrs::Any>,
{
let zero_fee = Fee::from_amount_and_gas(
cosmrs::Coin {
denom: self.fee_denom.parse().unwrap(),
amount: INJECTIVE_MIN_GAS_PRICE,
},
0u64,
);

let tx = self.create_signed_tx(msgs, signer, zero_fee)?;
let tx = self.create_signed_tx(msgs, signer, self.default_simulation_fee())?;
let base64_tx_bytes = BASE64_STANDARD.encode(tx);

redefine_as_go_string!(base64_tx_bytes);
Expand All @@ -246,6 +257,17 @@ impl BaseApp {
.map_err(RunnerError::DecodeError)
}
}

pub fn default_simulation_fee(&self) -> Fee {
Fee::from_amount_and_gas(
cosmrs::Coin {
denom: self.fee_denom.parse().unwrap(),
amount: INJECTIVE_MIN_GAS_PRICE,
},
0u64,
)
}
Comment on lines +261 to +269
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

Handle potential parsing errors when parsing fee_denom

In the default_simulation_fee method, using self.fee_denom.parse().unwrap() can cause a panic if fee_denom cannot be parsed into the expected type. To enhance robustness, consider handling the parsing error gracefully.

Apply this diff to handle the parsing error:

 pub fn default_simulation_fee(&self) -> Fee {
     Fee::from_amount_and_gas(
         cosmrs::Coin {
-            denom: self.fee_denom.parse().unwrap(),
+            denom: match self.fee_denom.parse() {
+                Ok(denom) => denom,
+                Err(e) => {
+                    // Handle the error, possibly by returning a Result or a default value
+                    panic!("Invalid fee_denom '{}': {}", self.fee_denom, e);
+                }
+            },
             amount: INJECTIVE_MIN_GAS_PRICE,
         },
         0u64,
     )
 }

Committable suggestion was skipped due to low confidence.


fn estimate_fee<I>(&self, msgs: I, signer: &SigningAccount) -> RunnerResult<Fee>
where
I: IntoIterator<Item = cosmrs::Any>,
Expand Down Expand Up @@ -290,6 +312,15 @@ impl BaseApp {
}
}

/// Cleanup the test environment when the app is dropped.
impl Drop for BaseApp {
fn drop(&mut self) {
unsafe {
CleanUp(self.id);
}
}
}

impl<'a> Runner<'a> for BaseApp {
fn execute_multiple<M, R>(
&self,
Expand Down
8 changes: 4 additions & 4 deletions packages/test-tube/src/runner/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ where
.into_iter()
.map(|a| -> Result<Attribute, Utf8Error> {
Ok(Attribute {
key: std::str::from_utf8(a.key.as_ref())?.to_string(),
value: std::str::from_utf8(a.value.as_ref())?.to_string(),
key: std::str::from_utf8(a.key_bytes())?.to_string(),
value: std::str::from_utf8(a.value_bytes())?.to_string(),
})
})
.collect::<Result<Vec<Attribute>, Utf8Error>>()?,
Expand Down Expand Up @@ -102,8 +102,8 @@ where
.into_iter()
.map(|a| -> Result<Attribute, Utf8Error> {
Ok(Attribute {
key: a.key.to_string(),
value: a.value.to_string(),
key: std::str::from_utf8(a.key_bytes())?.to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

why not import fuction ?

value: std::str::from_utf8(a.value_bytes())?.to_string(),
})
})
.collect::<Result<Vec<Attribute>, Utf8Error>>()?,
Expand Down
Loading