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

0.7.0 Release #255

Open
wants to merge 354 commits into
base: main
Choose a base branch
from
Open

0.7.0 Release #255

wants to merge 354 commits into from

Conversation

0xKitsune
Copy link
Collaborator

Ref #235

@0xKitsune 0xKitsune marked this pull request as ready for review December 29, 2024 19:25
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
function liquidity() external view returns (uint128);
}

// /**

Choose a reason for hiding this comment

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

should this code be removed now?

/// @title Pool state that can change
/// @notice These methods compose the pool's state, and can change with any frequency including multiple times
/// per transaction
interface IUniswapV3PoolState {

Choose a reason for hiding this comment

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

dedupe interface?

}
}

pub fn q64_to_float(num: u128) -> Result<f64, AMMError> {

Choose a reason for hiding this comment

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

dedupe with utils in src/amms/float.rs?

self.state.keys().cloned().collect()
}

/// Calculates a f64 representation of base token price in the AMM. This is a "tax inclusive" spot approximation.

Choose a reason for hiding this comment

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

It appears that the other pools don't include fees in this method. I think either approach is ok, but should probably be consistent and specified in the trait doc.


current_state.amount_calculated -= I256::from_raw(step.amount_out);

// TODO: adjust for fee protocol

Choose a reason for hiding this comment

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

Does protocol fee matter for us? My understanding is that comes out of the fees already paid.

src/amms/uniswap_v3/mod.rs Show resolved Hide resolved
.cloned()
.map(|amm| {
let pool_address = amm.address();
// TODO FIXME: Need to update this when we have balancer/v3 support

Choose a reason for hiding this comment

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

seems unfinished

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants