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.

Sign up for GitHub

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

Jump to bottom

Add missing test for orders with nft #1870

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Add missing test for orders with nft #1870

merged 3 commits into from
Jan 22, 2025

Conversation

azarovh
Copy link
Member

@azarovh azarovh commented Jan 21, 2025

Also use PerThousand::denominator function instead of hardcoded 1000

OBorce
OBorce approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@OBorce OBorce left a comment

Choose a reason for hiding this comment

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

Do we have a test that fills the order with ask_amount -1, I guess they will get 0 Tokens/NFT and the next fill order with 1 will get the Token/NFT?

@azarovh
Copy link
Member Author

azarovh commented Jan 22, 2025

Do we have a test that fills the order with ask_amount -1, I guess they will get 0 Tokens/NFT and the next fill order with 1 will get the Token/NFT?

It should be covered by other randomized tests but I added explicit one for nft

@azarovh azarovh merged commit 2ea4a5c into master Jan 22, 2025
18 checks passed
@azarovh azarovh deleted the order_with_nft_test branch January 22, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ImplOfAnImpl ImplOfAnImpl ImplOfAnImpl approved these changes

@OBorce OBorce OBorce approved these changes

@erubboli erubboli Awaiting requested review from erubboli erubboli is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@azarovh @OBorce @ImplOfAnImpl