Skip to content

pinocchio-interface: include UnwrapLamports (45) in TokenInstruction::try_from - #118

Merged
joncinque merged 3 commits into
solana-program:mainfrom
victorchukwuemeka:fix/pinocchio-instruction-variant
Jan 9, 2026
Merged

pinocchio-interface: include UnwrapLamports (45) in TokenInstruction::try_from#118
joncinque merged 3 commits into
solana-program:mainfrom
victorchukwuemeka:fix/pinocchio-instruction-variant

Conversation

@victorchukwuemeka

Copy link
Copy Markdown
Contributor

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 feboJanuary 8, 2026 13:08

@joncinquejoncinque left a comment

Copy link
Copy Markdown
Contributor

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

febo commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
ContributorAuthor

okay i;m clearing the spaces now

@victorchukwuemeka
victorchukwuemekaforce-pushed the fix/pinocchio-instruction-variant branch 2 times, most recently from 2600e50 to ead864cCompareJanuary 8, 2026 14:24
@victorchukwuemeka

Copy link
Copy Markdown
ContributorAuthor

@febo i have cleaned the spaces

@febo

febo commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

@febo i have cleaned the spaces

There are a couple ones left.

}
}

Copy link
Copy Markdown
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
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done

@victorchukwuemeka
victorchukwuemekaforce-pushed the fix/pinocchio-instruction-variant branch from ead864c to 851ffe1CompareJanuary 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
Copy Markdown
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)})
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@febo done

@victorchukwuemeka
victorchukwuemekaforce-pushed the fix/pinocchio-instruction-variant branch from 87e83b9 to 682522eCompareJanuary 9, 2026 10:10
febo
febo approved these changes Jan 9, 2026

@febofebo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good – thanks!

@joncinquejoncinque changed the title Fix: include UnwrapLamports (45) in TokenInstruction::try_frompinocchio-interface: include UnwrapLamports (45) in TokenInstruction::try_fromJan 9, 2026
@joncinque
joncinque merged commit 4f76dd7 into solana-program:mainJan 9, 2026
17 checks passed
Sign up for freeto 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

@victorchukwuemeka@febo@joncinque