Introduce canonical pointer PDA #374
Conversation
| pub(crate) fn get_wrapped_mint_address_with_seed( | ||
| unwrapped_mint: &Pubkey, | ||
| wrapped_token_program_id: &Pubkey, | ||
| ) -> (Pubkey, u8) { | ||
| get_wrapped_mint_address_with_seed_for_program(unwrapped_mint, wrapped_token_program_id, &id()) | ||
| } | ||
|
|
||
| pub(crate) fn get_wrapped_mint_address_with_seed_for_program( | ||
| unwrapped_mint: &Pubkey, | ||
| wrapped_token_program_id: &Pubkey, | ||
| program_id: &Pubkey, | ||
| ) -> (Pubkey, u8) { | ||
| Pubkey::find_program_address( | ||
| &get_wrapped_mint_seeds(unwrapped_mint, wrapped_token_program_id), | ||
| &id(), | ||
| program_id, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Adding a _for_program() variants to allow for these forks to utilize our package for their address lookups
There was a problem hiding this comment.
Tbh I'd just have them all just take program_id and not support duplicate versions of all of the helpers.
If this is easier for SemVer, though, nbd.
There was a problem hiding this comment.
id() is so deeply embedded into the program+clients that requiring the program_id is a far reaching change. I think I'll keep this forward compatible for now and revisit for a later breaking change if we find lots of folks forking and wanting the simpler name.
buffalojoec
left a comment
buffalojoec
left a comment
There was a problem hiding this comment.
Lgtm! A few nits, take them or leave them!
| pub(crate) fn get_wrapped_mint_address_with_seed( | ||
| unwrapped_mint: &Pubkey, | ||
| wrapped_token_program_id: &Pubkey, | ||
| ) -> (Pubkey, u8) { | ||
| get_wrapped_mint_address_with_seed_for_program(unwrapped_mint, wrapped_token_program_id, &id()) | ||
| } | ||
|
|
||
| pub(crate) fn get_wrapped_mint_address_with_seed_for_program( | ||
| unwrapped_mint: &Pubkey, | ||
| wrapped_token_program_id: &Pubkey, | ||
| program_id: &Pubkey, | ||
| ) -> (Pubkey, u8) { | ||
| Pubkey::find_program_address( | ||
| &get_wrapped_mint_seeds(unwrapped_mint, wrapped_token_program_id), | ||
| &id(), | ||
| program_id, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Tbh I'd just have them all just take program_id and not support duplicate versions of all of the helpers.
If this is easier for SemVer, though, nbd.
program/src/state.rs
Outdated
| /// The mint authority of an unwrapped mint's on-chain signal another token-wrap | ||
| /// deployment is the "canonical" one. |
There was a problem hiding this comment.
nit: this comment is a little bit confusing. Do you think we can clarify it a bit?
| return Err(ProgramError::MissingRequiredSignature); | ||
| } | ||
|
|
||
| let mint_data = unwrapped_mint_info.try_borrow_data()?; |
There was a problem hiding this comment.
Should we be checking the mint's owner here? Maybe it's inconsequential, since the mint address can't be duplicated.
There was a problem hiding this comment.
Sure thing, will add a check+test
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Sorry for the lateness, the implementation looks good! Just a few bits for potential follow-up work
| /// must be: `get_canonical_pointer_address(unwrapped_mint_address)` | ||
| /// 2. `[]` Unwrapped mint | ||
| /// 3. `[]` System program | ||
| SetCanonicalPointer { |
There was a problem hiding this comment.
nit: Take it or leave it, but I find the name CanonicalProgramPointer would be less ambiguous everywhere
| /// fail. | ||
| /// | ||
| /// Accounts expected: | ||
| /// 0. `[s]` Unwrapped mint authority |
There was a problem hiding this comment.
Did we confirm that this is ok from the foundation? If an SPL token mint is using an SPL token multisig as the authority, we can't actually get a signature for it 😅
USDC and USDT both use SPL token multisigs as the mint and freeze authority, so we might need to add SPL multisig support to this instruction, which may require rejiggering the order of accounts here.
Separately, what if we allowed for a signature from either the freeze authority or the mint authority?
There was a problem hiding this comment.
Did we confirm that this is ok from the foundation?
Are you talking about the feature generally? In NY, they specifically requested a way for a mint authority to signal another token-wrap deployment was the canonical one for the mint.
If an SPL token mint is using an SPL token multisig as the authority, we can't actually get a signature for it 😅
Oh shoot. Totally forgot that multisigs cannot directly sign txs. Yes, I'll need to reorder the accounts for that usecase like the other ixs.
Separately, what if we allowed for a signature from either the freeze authority or the mint authority?
Yes, that sounds reasonable too.
Integrating feedback from Foundation
Problem
It's likely that a few high profile mint authorities will not consider the original deployment of token-wrap the one they want folks to use for their mint. They may prefer an alternative configuration (especially for token-2022s) and will fork+deploy their own. These authorities need a way to signal on-chain that a different token wrap deployment is the "canonical" one for their mint.
Solution
This PR adds a
CanonicalPointerPDA which allows mint authorities to set a field with a pubkey to the alternative deployment. One instruction is added to create/update this PDA.Edge cases
If there is no authority, a pointer cannot be created. If one is created and later the mint authority is set to None, the canonical pointer state is frozen as well.