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

Remove cw20 support #9

Closed
wants to merge 4 commits into from
Closed

Conversation

lubkoll
Copy link

@lubkoll lubkoll commented Jun 24, 2024

No description provided.

#[error("{0}")]
Overflow(#[from] OverflowError),

#[error("{0}")]
AdminError(#[from] AdminError),

#[error("{0}")]
Asset(#[from] AssetError),
Copy link
Member

Choose a reason for hiding this comment

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

This is unused

Copy link
Author

Choose a reason for hiding this comment

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

it is used in some asset checks in contract.rs. not sure if we really want these, but kept them for now.

#[derive(Error, Debug)]
pub enum ContractError {
#[error("{0}")]
Std(#[from] StdError),

#[error("{0}")]
CwDexError(#[from] CwDexError),

#[error("{0}")]
Overflow(#[from] OverflowError),

#[error("{0}")]
AdminError(#[from] AdminError),
Copy link
Member

Choose a reason for hiding this comment

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

This is unused. We are using an equivalent from cw-controllers

#[error("Caller is not admin")]
    NotAdmin {},

Copy link
Author

Choose a reason for hiding this comment

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

yes, that was for supporting the AdminError from cw-controllers. Changed it to use the mars-owner instead (same thing, but a bit nicer implementation)

#[derive(Error, Debug)]
pub enum ContractError {
#[error("{0}")]
Std(#[from] StdError),

#[error("{0}")]
CwDexError(#[from] CwDexError),

#[error("{0}")]
Overflow(#[from] OverflowError),
Copy link
Member

Choose a reason for hiding this comment

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

Unused

Copy link
Author

Choose a reason for hiding this comment

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

removed in the PR to the quasar repo

#[error("{0}")]
Overflow(#[from] OverflowError),

#[error("{0}")]
AdminError(#[from] AdminError),

#[error("{0}")]
Asset(#[from] AssetError),

#[error("Incorrect amount of native token sent. You don't need to pass in offer_amount if using native tokens.")]
IncorrectNativeAmountSent,
Copy link
Member

Choose a reason for hiding this comment

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

Unused, also the below ones to this line

Copy link
Author

Choose a reason for hiding this comment

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

removed in the PR to the quasar repo

}

#[cw_serde]
pub struct BestPathForPairResponse {
/// the operations that will be executed to perform the swap
pub operations: crate::operations::SwapOperationsList,
pub operations: Vec<SwapAmountInRoute>,
/// the amount of tokens that are expected to be received after the swap
pub return_amount: Uint128,
}

#[cw_serde]
#[derive(QueryResponses)]
pub enum QueryMsg {
Copy link
Member

@magiodev magiodev Jun 25, 2024

Choose a reason for hiding this comment

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

I see what you are aiming for here. Looks good to me in this PR, we can finish that query list on PR #8

Copy link
Author

Choose a reason for hiding this comment

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

yes, we might still want to introduce an alias or our own type (as the SwapAmountInRoute is a bit weird). Just kept it simple for now

Copy link
Author

Choose a reason for hiding this comment

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

we might want to replace this with our own type, not sure yet. The SwapAmountInRoute is a bit annoying as it renames the pool id from pool_id to poolID during serialization/deserialization and parses u64 as string..


/// As an MVP we hardcode paths for each tuple of assets (offer, ask).
/// In a future version we want to find the path that produces the highest
/// number of ask assets, but this will take some time to implement.
/// To support multiple paths between the same asset, we add an id field we increment per asset of the same
/// path
pub const PATHS: Map<(AssetInfoKey, AssetInfoKey, u64), SwapOperationsList> = Map::new("paths");
pub const PATHS: Map<(String, String, u64), Vec<SwapAmountInRoute>> = Map::new("paths");
Copy link
Member

Choose a reason for hiding this comment

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

This should be changed as we said on PR 8 then, removing the unnecessary u64 as we are not excluding using it on the BestPathsForPair query which has been disabled.

Copy link
Author

Choose a reason for hiding this comment

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

I changed it to using Vec<Vec<SwapAmountInRoute>> in the other PR. I'd still like to keep the option to have multiple paths, as we need that later and you already wrote almost all of the required code. I only need to find out how to simulate the swap in one query, then I can enable the commented out code again

@lubkoll lubkoll closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants