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

Add Mint() tests #135

Merged
merged 51 commits into from
Nov 1, 2023
Merged

Add Mint() tests #135

merged 51 commits into from
Nov 1, 2023

Conversation

uri-99
Copy link
Collaborator

@uri-99 uri-99 commented Oct 10, 2023

Resolves Issue #131 , adds 24 new tests, along with 8 incomplete tests that require the implementation of the burn() function to be completed

@uri-99 uri-99 marked this pull request as ready for review October 17, 2023 17:41
@uri-99
Copy link
Collaborator Author

uri-99 commented Oct 17, 2023

Mint Tests unavailable to finish because of missing functions ( mainly burn() ):
test_protocol_fees_accum
test_positions_protected
test_unallow_poke_on_uninit_pos
test_remove_liquidityGross
test_clear_tick_upper
test_remove
test_clear_tick_lower
test_below_remove
test_burn
test_clear_tick_unused

Copy link
Contributor

@dpinones dpinones left a comment

Choose a reason for hiding this comment

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

Great job!!! I left you some small comments.

Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Show resolved Hide resolved
crates/yas_core/src/libraries/tick.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Outdated Show resolved Hide resolved
Copy link
Contributor

@dubzn dubzn left a comment

Choose a reason for hiding this comment

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

Congrats for the excellent job. There are still some things to correct, but they are minor issues. 🚀

crates/yas_core/src/contracts/yas_pool.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/contracts/yas_pool.cairo Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@uri-99 uri-99 mentioned this pull request Oct 23, 2023
Copy link
Collaborator

@rcatalan98 rcatalan98 left a comment

Choose a reason for hiding this comment

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

Great job! I left a few minor comments and questions. But overall the PR looks great!

@rcatalan98
Copy link
Collaborator

Please also update the branch before merging

Copy link
Collaborator

@rcatalan98 rcatalan98 left a comment

Choose a reason for hiding this comment

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

Great work! 🚀

Copy link
Contributor

@dubzn dubzn left a comment

Choose a reason for hiding this comment

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

Just minimal changes, almost there!

@uri-99 uri-99 requested a review from dubzn November 1, 2023 02:09
Copy link
Contributor

@dubzn dubzn left a comment

Choose a reason for hiding this comment

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

🫡 great job

@dpinones dpinones merged commit 54a3633 into main Nov 1, 2023
4 checks passed
@dpinones dpinones deleted the tests/mint branch November 1, 2023 21:45
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.

None yet

4 participants