Skip to content

Conversation

@victorchukwuemeka
Copy link

The TryFrom implementation for TokenInstruction was missing the
UnwrapLamports = 45 variant, causing
test_token_instruction_from_u8_exhaustive to fail.

This PR adds the missing variant to keep the conversion logic in sync with
the enum definition.

@joncinque joncinque requested a review from febo January 8, 2026 13:08
Copy link
Contributor

@joncinque joncinque 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 to me, but let's get an approval from @febo

@febo
Copy link
Contributor

febo commented Jan 8, 2026

@victorchukwuemeka Looks good – I think we need to remove the extra blank spaces that were added so CI passes.

use {crate::error::TokenError, pinocchio::program_error::ProgramError};


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not needed.

Comment on lines 540 to 541


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not needed.

AccountOwner,
/// Authority to close a token account
CloseAccount,

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not needed.

assert_eq!(
TokenInstruction::from_repr(variant_u8),
Some(TokenInstruction::try_from(variant_u8).unwrap())

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not needed.

Comment on lines 607 to 608


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not needed.

@victorchukwuemeka
Copy link
Author

okay i;m clearing the spaces now

@victorchukwuemeka victorchukwuemeka force-pushed the fix/pinocchio-instruction-variant branch 2 times, most recently from 2600e50 to ead864c Compare January 8, 2026 14:24
@victorchukwuemeka
Copy link
Author

@febo i have cleaned the spaces

@febo
Copy link
Contributor

febo commented Jan 8, 2026

@febo i have cleaned the spaces

There are a couple ones left.

}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Remove the extra spaces.

Copy link
Author

Choose a reason for hiding this comment

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

done

@victorchukwuemeka victorchukwuemeka force-pushed the fix/pinocchio-instruction-variant branch from ead864c to 851ffe1 Compare January 8, 2026 15:59
match value {
// SAFETY: `value` is guaranteed to be in the range of the enum variants.
0..=24 | 38 | 255 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
0..=24 | 38 | 45 | 255 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Last nit for CI.

Suggested change
0..=24 | 38 | 45 | 255 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
0..=24 | 38 | 45 | 255 => {
Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) })
}

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