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
Create Swap Instruction #30
Conversation
|
||
check_accounts(program_id, &market_state, &accounts).unwrap(); | ||
let callback_info = CallBackInfo { | ||
user_account: Pubkey::default(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe can change callbackinfo to use Option instead, but that felt messy (had to update a bunch of places, change CALLBACK_INFO_LEN, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good you didn't do it, there really is no benefit in using an Option
here :
- Requires an extra byte to encode (which means every node in the critbit tree is just that little bit larger => more expensive to deploy new markets)
- The only benefit of using an option is that option::is_some is inherently faster than
Pubkey::eq
, but we actually never perform the expensive comparison between that key andPubkey::default
For now this LGTM, I'll wait for the TS side to be done as well before final review and merge. |
fbdded6
to
661ba30
Compare
todo: