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

Added tests for OCEX and LMP Pallet #895

Merged
merged 17 commits into from
Feb 21, 2024
Merged

Added tests for OCEX and LMP Pallet #895

merged 17 commits into from
Feb 21, 2024

Conversation

zktony
Copy link
Contributor

@zktony zktony commented Jan 9, 2024

Describe your changes

Added tests for OCEX and LP Pallet.

@zktony zktony changed the title Added tests for OCEX and LP Pallet Added tests for OCEX and LMP Pallet Jan 9, 2024
@Gauthamastro Gauthamastro added the A0-PleaseReview Pull request needs code review. label Jan 10, 2024
Copy link
Member

@Gauthamastro Gauthamastro left a comment

Choose a reason for hiding this comment

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

Existing unit tests of validator logic should not be ignored. The rest we can review, @zktony

pallets/liquidity-mining/src/lib.rs Show resolved Hide resolved
pallets/liquidity-mining/src/lib.rs Show resolved Hide resolved
pallets/liquidity-mining/src/lib.rs Show resolved Hide resolved
pallets/ocex/src/tests.rs Outdated Show resolved Hide resolved
@@ -163,9 +163,10 @@ impl<T: Config> LiquidityMiningCrowdSourcePallet<T::AccountId> for Pallet<T> {
.to_u128()
.ok_or(Error::<T>::ConversionError)?
.saturated_into();
//FIXME: What are we doing with base_freed and quote_freed?
Copy link
Contributor

Choose a reason for hiding this comment

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

When it should be discussed and fixed? in scope of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be fixed in @Gauthamastro PR

&pool_config.pool_id,
&lp,
quote_amt_to_claim,
Preservation::Expendable,
)?;
// TODO: Emit events
// TODO: Emit events (Ask @frontend team about this)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be fixed in @Gauthamastro PR

primitives/orderbook/src/lib.rs Outdated Show resolved Hide resolved
pallets/ocex/src/tests.rs Outdated Show resolved Hide resolved
pallets/ocex/src/tests.rs Outdated Show resolved Hide resolved
pallets/liquidity-mining/src/tests.rs Outdated Show resolved Hide resolved
pallets/liquidity-mining/src/tests.rs Outdated Show resolved Hide resolved
pallets/liquidity-mining/src/tests.rs Outdated Show resolved Hide resolved
pallets/liquidity-mining/src/tests.rs Outdated Show resolved Hide resolved
pallets/liquidity-mining/src/tests.rs Outdated Show resolved Hide resolved
Gauthamastro and others added 4 commits January 24, 2024 17:41
# Conflicts:
#	Cargo.lock
#	pallets/liquidity-mining/Cargo.toml
#	primitives/orderbook/src/lib.rs
@Gauthamastro Gauthamastro added A0-PleaseReview Pull request needs code review. and removed A0-PleaseReview Pull request needs code review. labels Jan 25, 2024
@Gauthamastro Gauthamastro added A0-PleaseReview Pull request needs code review. and removed A0-PleaseReview Pull request needs code review. labels Jan 26, 2024
@Gauthamastro Gauthamastro added A0-PleaseReview Pull request needs code review. and removed A0-PleaseReview Pull request needs code review. labels Feb 21, 2024
@zktony zktony merged commit 66e0da1 into gj/lmp Feb 21, 2024
@zktony zktony deleted the zk-test-lmp branch February 21, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-PleaseReview Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants