Skip to content

Conversation

@LukasDeco
Copy link
Collaborator

No description provided.

direction,
marketType ?? null,
marketIndex ?? null,
direction ?? null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like we should consider doing a null check inside of getCancelOrdersIx and throw error if any of these are null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hesitate to change the behavior like adding exception throwing if there wasn't any before. I guess the only issue here is they were T | undefined before and the function wanted T | null instead. I think in 99.99% of the time coalescing to null will not change the behavior right?

postOnly: postOnly || null,
bitFlags: bitFlags || null,
policy: policy || null,
policy: policy || undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

how come we are only doing undefined on the policy param and keeping null for the rest? is there something with how this gets converted over to rust and we will get an error there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just what the internal type wanted. Always tricky to know least disruptive way to fix these errors. Are you thinking instead we should change the type instead of the passed value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually I think we should change the type instead...

* fix: many null checks fixed

* fix: prettier

* feat: get active markets helpers

* fix: prettier
@LukasDeco LukasDeco force-pushed the lukas/null-checks-2 branch from 4591301 to 4d49913 Compare January 6, 2026 20:23
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