Skip to content

Conversation

jayantk
Copy link
Contributor

@jayantk jayantk commented Aug 22, 2022

This is again needed to clean up the pythtest situation, where we created extra product/price accounts.

Copy link
Contributor

@guibescos guibescos left a comment

Choose a reason for hiding this comment

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

I would add a test that deletes the last element of the array just for extra security. Otherwise LGTM.

let product_data = load_checked::<pc_prod_t>(product_account, cmd_args.ver_)?;

// This assertion is just to make the subtractions below simpler
pyth_assert(mapping_data.num_ >= 1, 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.

Rather than assert, can we used .checked_sub on the subtractions below? We can (and should) enable clippy’s arithmetic lint which will be safer. We should really be using checked arithmetic for everything on-chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call on the arithmetic checks. will enable in a separate pr.

@jayantk jayantk merged commit 5baf3c3 into main Aug 22, 2022
@jayantk jayantk deleted the del_product2 branch August 22, 2022 20:02
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