Skip to content

Conversation

guibescos
Copy link
Contributor

@guibescos guibescos commented Aug 4, 2022

Add price instruction in Rust.

@guibescos guibescos marked this pull request as ready for review August 5, 2022 14:13
Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

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

lgtm, will approve once you merge/rebase

) -> OracleResult {
if instruction_data.len() < size_of::<cmd_add_price_t>() {
return Err(ProgramError::InvalidArgument);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should put this check into load

if product_data.magic_ != PC_MAGIC
|| product_data.ver_ != cmd_args.ver_
|| product_data.type_ != PC_ACCTYPE_PRODUCT
|| price_data.magic_ != 0
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing i've noticed from doing this translation exercise is that the check for whether an account is uninitialized differs in all of the instructions. init_mapping checks both the magic and ver fields, but add_account only checks the magic. I think it should be fine for us to standardize on one of these and encode that in validate_fresh_account.

.next_
.k1_
.copy_from_slice(&next_mapping.key.to_bytes());
pubkey_assign(cur_mapping.next_, &next_mapping.key.to_bytes());
Copy link
Contributor

Choose a reason for hiding this comment

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

🙇

jayantk
jayantk previously approved these changes Aug 5, 2022
Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

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

nice

@guibescos guibescos merged commit 608d2ed into main Aug 5, 2022
@guibescos guibescos deleted the add-price-rust branch August 5, 2022 19:04
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