Skip to content
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

guards #3416

Merged
merged 1 commit into from
Aug 3, 2022
Merged

guards #3416

merged 1 commit into from
Aug 3, 2022

Conversation

aeyakovenko
Copy link
Member

No description provided.

@CriesofCarrots
Copy link
Contributor

I'll merge early, but I want to see the all token tests run here or locally first

@steviez
Copy link
Contributor

steviez commented Aug 3, 2022

Just checked - all other instances of .split_at() in this repo have length checks before calling split_at()

@CriesofCarrots
Copy link
Contributor

twotx CI failure is known and unrelated

@@ -1689,4 +1704,12 @@ mod test {
let unpacked = TokenInstruction::unpack(&expect).unwrap();
assert_eq!(unpacked, check);
}

#[test]
fn test_instruction_unpack_panic() {
Copy link
Member

Choose a reason for hiding this comment

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

As a follow-up, perhaps something more exhaustive?

    #[test]
    fn test_instruction_unpack_panic() {
        for i in 0..255u8 {
            for j in 1..10 {
                let mut data = vec![0;j];
                data[0] = i;
                let _no_panic = TokenInstruction::unpack(&data);
            }
        }
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, on my list already :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am fuzzing it with

    #[test]
    fn test_instruction_unpack_fuzz() {
        for _ in 0..10000000 {
            let r: usize = rand::thread_rng().gen_range(0..10);
            let data: Vec<u8> = (0..r).map(|_| rand::thread_rng().gen()).collect();
            _ = TokenInstruction::unpack(&data);
        }
    }

@CriesofCarrots CriesofCarrots merged commit 22faa05 into solana-labs:master Aug 3, 2022
joncinque added a commit to joncinque/solana-program-library that referenced this pull request Aug 3, 2022
joncinque added a commit that referenced this pull request Aug 3, 2022
* token: Reassign and reallocate accounts on close

* Revert "Refactor unpack and make test more robust (#3417)"

This reverts commit c618de3.

* Revert "check that unpack is tolerant of small sizes (#3416)"

This reverts commit 22faa05.

* Also revert d7f352b
CriesofCarrots pushed a commit to CriesofCarrots/solana-program-library that referenced this pull request Aug 3, 2022
CriesofCarrots added a commit that referenced this pull request Aug 4, 2022
* check that unpack is tolerant of small sizes (#3416)

* Refactor unpack and make test more robust (#3417)

* Refactor hasty fix to match token-2022

* Make test exhaustive

* cargo fmt

Co-authored-by: Michael Vines <mvines@gmail.com>

* Readd proptests without losing unit test, #3421

Co-authored-by: anatoly yakovenko <anatoly@solana.com>
Co-authored-by: Michael Vines <mvines@gmail.com>
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