Conversation
| /// 0. `[writable]` The native token account to sync with its underlying | ||
| /// lamports. | ||
| /// | ||
| /// * Using Rent sysvar account |
There was a problem hiding this comment.
Not sure if this is the best way to specify an optional account in the docs. There were no other examples, so I followed a pattern similar to the multisig case.
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks good to me overall! Just some small extra bits to consider
| /// 0. `[writable]` The native token account to sync with its underlying | ||
| /// lamports. | ||
| /// | ||
| /// * Using Rent sysvar account |
|
|
||
| let instruction = | ||
| let mut instruction = | ||
| spl_token_interface::instruction::sync_native(&TOKEN_PROGRAM_ID, &source_account_key) |
There was a problem hiding this comment.
Can you add another version of this function that adds the sysvar account? That way we don't have to do anything hacky in the test, and people will just get the new function
| // SAFETY: `remaining` is guaranteed to not be empty. | ||
| let rent_sysvar_info = unsafe { remaining.get_unchecked(0) }; |
There was a problem hiding this comment.
nit: not sure how this impacts performance, but we could change the condition to:
let rent_exempt_reserve = if let Some(rent_sysvar_info) = remaining.first() {
let rent = unsafe { ... }
} else {
Rent::get()?...
}
And that way we can remove one more instance of unsafe code
There was a problem hiding this comment.
Oh, nice, it actually reduced the CUs to 90.
febo
force-pushed
the
febo/sync-native-rent
branch
from
e16dd79 to
2197726
Compare
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great! Can you add the same thing to token-2022?
Yes, I will do that. |
febo
force-pushed
the
febo/sync-native-rent
branch
from
2197726 to
78c3ffa
Compare
2 participants
Problem
PR #132 add the requirement of using the Rent sysvar in
SyncNativeto update the current rent-exempt reserve. This led to an increase in CUs from61to207.Solution
Allow the Rent sysvar account to be provided, which reduces the current CUs from
207to90.Note: This is in draft since it needs p-token: Add compiler hints #136 to land first.