Skip to content

Conversation

@jayantk
Copy link
Contributor

@jayantk jayantk commented Aug 19, 2022

We need this command in order to clean up the situation on pythtest, where we created a bunch of incorrect product and price accounts.

This PR went down a bit of a rabbit hole, as this instruction needs to be tested with the Solana runtime. The existing Python integration tests are pretty hard to work with, so I looked for an alternative. Turns out there's a nice solana TX simulator that you can run directly from Rust, so I built some testing utils around that.

assert_eq!(
size_of::<TimeMachineWrapper>(),
TIME_MACHINE_STRUCT_SIZE.try_into().unwrap(),
TIME_MACHINE_STRUCT_SIZE as usize,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are two ambiguous type parameters for the try_into now. Something that got pulled in with tokio i think 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the right cast anyhow.

Copy link
Contributor

@Reisen Reisen left a comment

Choose a reason for hiding this comment

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

Looks good, only a minor nitpick on the instruction de structure. The test is especially nice.

accounts: &[AccountInfo],
instruction_data: &[u8],
) -> ProgramResult {
let [funding_account, product_account, price_account] = match accounts {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tuples over fixed length array literals + explicit return instead of wrapping/unwrapping an unnecessary Result.

Suggested change
let [funding_account, product_account, price_account] = match accounts {
let (funding_account, product_account, price_account) = match accounts {
[w, x, y] => (w, x, y),
_ => return Err(ProgramError::InvalidArgument),
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm yeah that does seem better. we've been doing it this way in a bunch of places already though, so i'm not going to change this right now.

assert_eq!(
size_of::<TimeMachineWrapper>(),
TIME_MACHINE_STRUCT_SIZE.try_into().unwrap(),
TIME_MACHINE_STRUCT_SIZE as usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the right cast anyhow.

@jayantk jayantk merged commit afd429e into main Aug 22, 2022
@jayantk jayantk deleted the del_price branch August 22, 2022 14:01
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.

4 participants