Freeze Authorities for Token Swap/AMMs #1515
Conversation
…ot in love with the hack.
… save freeze authority via cloning. I like this situation better - no extra enum and hackiness required.
| @@ -83,6 +83,9 @@ export const TokenSwapLayout: typeof BufferLayout.Structure = BufferLayout.struc | |||
| Layout.uint64('hostFeeDenominator'), | |||
| BufferLayout.u8('curveType'), | |||
| BufferLayout.blob(32, 'curveParameters'), | |||
| BufferLayout.u32('freezeAuthorityOption'), | |||
There was a problem hiding this comment.
There's a mixup of using a u8 in some parts for the Some/None COption and a u32 in the instructions. It looks like spl-token uses a u32, but I have no idea why. @CriesofCarrots I think you took care of this -- why does spl-token use a u32 for the option part of COption?
There was a problem hiding this comment.
Yeah I'm not sure why he uses it. I just copied his pattern. The instructions seem to expect a u8 though, while the storage uses a u32.
| freezeAuthorityOption: 0, | ||
| freezeAuthority: undefined, | ||
| freezeAuthorityBitMask: 0, |
There was a problem hiding this comment.
You'll probably want to pass these into createInitSwapInstruction function instead of hard-coding
| /// 2. process_withdraw_all_token_types, | ||
| /// 3. process_deposit_single_token_type_exact_amount_in, | ||
| /// 4. process_withdraw_single_token_type_exact_amount_out, | ||
| pub freeze_authority_bit_mask: u8, |
There was a problem hiding this comment.
Slight renaming
| pub freeze_authority_bit_mask: u8, | |
| pub frozen_operations_bit_mask: u8, |
| #[cfg_attr(feature = "fuzz", derive(Arbitrary))] | ||
| #[repr(C)] | ||
| #[derive(Clone, Debug, PartialEq)] | ||
| pub struct SetFreezeAuthorityBitMask { |
There was a problem hiding this comment.
| pub struct SetFreezeAuthorityBitMask { | |
| pub struct SetFrozenOperationsBitMask { |
| /// 2. process_withdraw_all_token_types, | ||
| /// 3. process_deposit_single_token_type_exact_amount_in, | ||
| /// 4. process_withdraw_single_token_type_exact_amount_out, | ||
| pub freeze_authority_bit_mask: u8, |
There was a problem hiding this comment.
| pub freeze_authority_bit_mask: u8, | |
| pub frozen_operations_bit_mask: u8, |
| @@ -345,6 +391,7 @@ impl Processor { | |||
| return Err(ProgramError::IncorrectProgramId); | |||
| } | |||
| let token_swap = SwapVersion::unpack(&swap_info.data.borrow())?; | |||
| Self::check_allowed_to_use(token_swap.as_ref(), 0)?; | |||
There was a problem hiding this comment.
take it or leave it, but a better alternative to hard-coding these bit numbers would be to use a true bitmask library. bitflags seems to be a pretty good one https://docs.rs/bitflags/1.2.1/bitflags/
| let bitmask: u8 = token_swap.freeze_authority_bit_mask(); | ||
| match Self::get_bit_at(bitmask, bit_position) { | ||
| Ok(frozen) => { | ||
| if frozen { |
There was a problem hiding this comment.
A newish best practice is to have a msg! explaining what's happening before throwing an error, which makes debugging from the explorer much easier. Swap isn't doing this everywhere of course, but this new code should 😄
| /// Processes a [SetFreezeAuthorityBitMask](enum.Instruction.html). | ||
| pub fn process_set_freeze_authority_bit_mask( | ||
| _: &Pubkey, | ||
| freeze_authority_bit_mask: u8, |
There was a problem hiding this comment.
Setting a raw bitmask is fine, but be sure there's an easy library to do it from the outside!
|
|
||
| Self::check_allowed_to_freeze(token_swap.as_ref(), freeze_authority_info)?; | ||
|
|
||
| let clone = SwapVersion::SwapV2(SwapV2 { |
There was a problem hiding this comment.
This can become brittle once we add a v3 -- could you deserialize it and set the new bitmask through a function? The v1 swap can error out for it
| SwapError::UnauthorizedToFreeze => { | ||
| msg!("Error: Unauthorized to freeze") | ||
| } | ||
| SwapError::SwapV1UnsupportedAction => { |
There was a problem hiding this comment.
I might be missing something, but it doesn't seem like this is ever used
| @@ -107,7 +131,8 @@ pub enum SwapInstruction { | |||
| /// Must be empty, not owned by swap authority | |||
| /// 6. `[writable]` Pool Token Account to deposit the initial pool token | |||
| /// supply. Must be empty, not owned by swap authority. | |||
| /// 7. '[]` Token program id | |||
| /// 7. '[]` Freeze authority. Optional. | |||
There was a problem hiding this comment.
This isn't actually passed as an account, no need to have it as a comment
|
I was thinking about this some more, and you'll likely need an instruction to set the freeze authority as well, which takes in |
github-actions
bot
added
the
stale
As part of @bartosz-lipinski 's NFT project, this introduces Freeze Authorities for the Token Swap project and a bitmask that covers a toggle on-off switch for each of the actions available in the program. A freeze authority can be introduced on creation of a swap and then that signer (single sig only) can toggle on/off the bits with another action.
Couples with project-serum/oyster-swap#39