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

feat: add force_void instruction #239

Merged
merged 9 commits into from
Oct 21, 2024

Conversation

obi-ewan
Copy link
Contributor

@obi-ewan obi-ewan commented Oct 16, 2024

Add an instruction which will allow markets to be voided when items still remain in the matching queue.

Tests:

test result: ok. 266 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
Test Suites: 89 passed, 89 total
Tests:       360 passed, 360 total
Snapshots:   0 total
Time:        748.627 s

@obi-ewan obi-ewan marked this pull request as draft October 16, 2024 12:04
@obi-ewan obi-ewan marked this pull request as ready for review October 16, 2024 12:26
Copy link
Contributor

@eoin-betdex eoin-betdex left a comment

Choose a reason for hiding this comment

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

A comment for consideration

Comment on lines 53 to 56
require!(
matching_queue.is_empty(),
CoreError::CloseAccountMarketMatchingQueueNotEmpty
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this what we talked through, and if we get to this point we should be safe? Since you can only close something voided or settled and can only go to those states if the queues are empty?

I had momentarily forgotten that, and was about to suggest that ForceVoidMarket take the matching queue but rather than check that it is empty, just clear it and set len to 0. So we could leave this check here.

I think if we're safe at this point it's OK to remove from here. Though somewhat inconsistent keeping order request len check but not matching queue len check.

From a hotfix pov, I do like the idea of the new instruction just clearing the matching queue, so we don't have code changes outside the new instructions.

Copy link
Contributor Author

@obi-ewan obi-ewan Oct 21, 2024

Choose a reason for hiding this comment

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

I do like the idea of the new instruction just clearing the matching queue, so we don't have code changes outside the new instructions.

Happy to do this, what I have done is safe but yes containing hotfix changes to the new instruction is probably better

@obi-ewan obi-ewan merged commit 2302e29 into MonacoProtocol:hotfix/0.15.3 Oct 21, 2024
@obi-ewan obi-ewan deleted the feat/force-void branch October 21, 2024 14:52
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