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

[Deepbook] Adding empty return to list_open_orders #14940

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Conversation

healthydeve
Copy link
Contributor

@healthydeve healthydeve commented Nov 20, 2023

Description

Previous list_open_orders function crashes on empty attempting to borrow from a df that does not exist.
image

This PR adds an error check and a test to ensure that this will not fail with error, but instead return an empty vector.

Also this includes an accessor function to usr_open_orders so other contracts can call this to check user positions

Test Plan

How did you test the new or updated feature?

unit test

If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

Copy link

vercel bot commented Nov 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mysten-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 21, 2023 1:38am
sui-core ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 21, 2023 1:38am
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 21, 2023 1:38am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Nov 21, 2023 1:38am
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Nov 21, 2023 1:38am
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Nov 21, 2023 1:38am

Copy link
Contributor

@patrickkuo patrickkuo left a comment

Choose a reason for hiding this comment

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

LGTM

@vercel vercel bot temporarily deployed to Preview – mysten-ui November 21, 2023 01:37 Inactive
@healthydeve healthydeve merged commit 75c7d3f into main Nov 21, 2023
33 checks passed
@healthydeve healthydeve deleted the jlu/filter_db branch November 21, 2023 03:25
tzakian pushed a commit that referenced this pull request Nov 29, 2023
## Description 

Previous `list_open_orders` function crashes on empty attempting to
borrow from a df that does not exist.
<img width="1327" alt="image"
src="https://github.com/MystenLabs/sui/assets/123408603/f1cc8b2b-e9cf-4b76-ab4c-a090bedd0807">

This PR adds an error check and a test to ensure that this will not fail
with error, but instead return an empty vector.

Also this includes an accessor function to usr_open_orders so other
contracts can call this to check user positions


## Test Plan 

How did you test the new or updated feature?

unit test
---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
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.

2 participants