-
Notifications
You must be signed in to change notification settings - Fork 2.2k
token-2022: Add Reallocate instruction #2864
token-2022: Add Reallocate instruction #2864
Conversation
d140489
to
6ceea7e
Compare
6ceea7e
to
dd7c557
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this! It looks good to me, but I'll let Michael give the final 👍 since Confidentials need this most. Just the one nit on that unpack_after_realloc
dd7c557
to
7b111a5
Compare
Pull request has been modified.
7b111a5
to
e5a3f77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just the one bit on set_account_type
that we may want to consider if we ever use set_account_type
before doing a check earlier.
Ok(Some(diff)) | ||
/// If AccountType is uninitialized, set it to the BaseState's ACCOUNT_TYPE; | ||
/// if AccountType is already set, check is set correctly for BaseState | ||
pub fn set_account_type<S: BaseState>(input: &mut [u8]) -> Result<(), ProgramError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we only need to consider this for accounts at the moment, we don't need to worry about this case, but I want to get it out there.
In a vacuum, if you pass in a buffer that currently contains just a base mint and call this with S = Account
, it will succeed, correct? type_and_tlv_indices
only checks for the opposite case of passing in a buffer containing a base Account
but call it with S = Mint
. I wonder if just to be totally totally safe, this should check the is_initialized
byte to make sure it's actually an Account
if S = Account
. Doesn't make it too generic though...
Probably doesn't need to get done here, and probably won't become an issue, but this exact case has been in the back of my mind for awhile now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, yeah good point. I originally had let base = S::unpack(base_data)?;
in there, which would protected against that, but deleted for expense reasons. I'm going to merge this as-is, but will file an issue for us to fix in a follow-up.
Some extensions are opt-in for token account owners at any time after the account has been initialized. In these cases, the owner needs the ability to reallocate their token account data to be large enough to hold the new extension. Previous approaches attempted to do this in the initialization logic of the specific extension, but borrows on the token account were messy, and it was inefficient to include additional reallocation-specific accounts (payer and system-program) on every initialization.
This PR adds a separate Reallocate instruction to handle the data resize and rent-exempt-reserve funding for a provided set of extensions.
First and last commits can be ignored for the purposes of discussion