Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

Freeze Authorities for Token Swap/AMMs #1515

Closed
wants to merge 8 commits into from
2 changes: 1 addition & 1 deletion token-swap/js/cli/token-swap-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export async function createTokenSwap(): Promise<void> {
const connection = await getConnection();
const payer = await newAccountWithLamports(connection, 1000000000);
owner = await newAccountWithLamports(connection, 1000000000);
const swapPayer = await newAccountWithLamports(connection, 10000000000);
const tokenSwapAccount = new Account();

[authority, nonce] = await PublicKey.findProgramAddress(
Expand Down Expand Up @@ -151,7 +152,6 @@ export async function createTokenSwap(): Promise<void> {
await mintB.mintTo(tokenAccountB, owner, [], currentSwapTokenB);

console.log('creating token swap');
const swapPayer = await newAccountWithLamports(connection, 10000000000);
tokenSwap = await TokenSwap.createTokenSwap(
connection,
swapPayer,
Expand Down
10 changes: 10 additions & 0 deletions token-swap/js/client/token-swap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Layout.publicKey('freezeAuthority'),
BufferLayout.u8('freezeAuthorityBitMask'),
],
);

Expand Down Expand Up @@ -306,6 +309,7 @@ export class TokenSwap {
{pubkey: tokenAccountPool, isSigner: false, isWritable: true},
{pubkey: tokenProgramId, isSigner: false, isWritable: false},
];

const commandDataLayout = BufferLayout.struct([
BufferLayout.u8('instruction'),
BufferLayout.u8('nonce'),
Expand All @@ -319,6 +323,9 @@ export class TokenSwap {
BufferLayout.nu64('hostFeeDenominator'),
BufferLayout.u8('curveType'),
BufferLayout.blob(32, 'curveParameters'),
BufferLayout.u32('freezeAuthorityOption'),
Layout.publicKey('freezeAuthority'),
BufferLayout.u8('freezeAuthorityBitMask'),
]);
let data = Buffer.alloc(1024);
{
Expand All @@ -335,6 +342,9 @@ export class TokenSwap {
hostFeeNumerator,
hostFeeDenominator,
curveType,
freezeAuthorityOption: 0,
freezeAuthority: undefined,
freezeAuthorityBitMask: 0,
Comment on lines +345 to +347
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll probably want to pass these into createInitSwapInstruction function instead of hard-coding

},
data,
);
Expand Down
6 changes: 5 additions & 1 deletion token-swap/program/fuzz/src/native_token_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use spl_token_swap::{

use spl_token::instruction::approve;

use solana_program::{bpf_loader, entrypoint::ProgramResult, pubkey::Pubkey, system_program};
use solana_program::{
bpf_loader, entrypoint::ProgramResult, program_option::COption, pubkey::Pubkey, system_program,
};

pub struct NativeTokenSwap {
pub user_account: NativeAccountData,
Expand Down Expand Up @@ -89,6 +91,8 @@ impl NativeTokenSwap {
nonce,
fees.clone(),
swap_curve.clone(),
COption::None,
0,
)
.unwrap();

Expand Down
1 change: 0 additions & 1 deletion token-swap/program/src/curve/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ impl Default for SwapCurve {
/// Clone takes advantage of pack / unpack to get around the difficulty of
/// cloning dynamic objects.
/// Note that this is only to be used for testing.
#[cfg(any(test, feature = "fuzz"))]
impl Clone for SwapCurve {
fn clone(&self) -> Self {
let mut packed_self = [0u8; Self::LEN];
Expand Down
16 changes: 16 additions & 0 deletions token-swap/program/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,22 @@ pub enum SwapError {
/// The operation cannot be performed on the given curve
#[error("The operation cannot be performed on the given curve")]
UnsupportedCurveOperation,

/// Attempted to access an invalid bit in the freeze authority bitmask
#[error("Attempted to access an invalid bit in the freeze authority bitmask")]
InvalidBitMaskOperation,

/// This action has been frozen by the Freeze Authority
#[error("This action has been frozen by the Freeze Authority")]
FrozenAction,

/// Unauthorized to freeze
#[error("Unauthorized to freeze")]
UnauthorizedToFreeze,

/// Attempted to set bit mask on Swap V1
#[error("Attempted to set bit mask on Swap V1")]
SwapV1UnsupportedAction,
}
impl From<SwapError> for ProgramError {
fn from(e: SwapError) -> Self {
Expand Down
105 changes: 102 additions & 3 deletions token-swap/program/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::error::SwapError;
use solana_program::{
instruction::{AccountMeta, Instruction},
program_error::ProgramError,
program_option::COption,
program_pack::Pack,
pubkey::Pubkey,
};
Expand All @@ -27,6 +28,15 @@ pub struct Initialize {
/// swap curve info for pool, including CurveType and anything
/// else that may be required
pub swap_curve: SwapCurve,
/// The freeze authority of the swap.
pub freeze_authority: COption<Pubkey>,
/// bits, from right to left - 1 disables, 0 enables the actions:
/// 0. process_swap,
/// 1. process_deposit_all_token_types,
/// 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Slight renaming

Suggested change
pub freeze_authority_bit_mask: u8,
pub frozen_operations_bit_mask: u8,

}

/// Swap instruction data
Expand Down Expand Up @@ -92,6 +102,20 @@ pub struct WithdrawSingleTokenTypeExactAmountOut {
pub maximum_pool_token_amount: u64,
}

/// SetFreezeAuthorityBitMask instruction data
#[cfg_attr(feature = "fuzz", derive(Arbitrary))]
#[repr(C)]
#[derive(Clone, Debug, PartialEq)]
pub struct SetFreezeAuthorityBitMask {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub struct SetFreezeAuthorityBitMask {
pub struct SetFrozenOperationsBitMask {

/// bits, from right to left - 1 disables, 0 enables the actions:
/// 0. process_swap,
/// 1. process_deposit_all_token_types,
/// 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub freeze_authority_bit_mask: u8,
pub frozen_operations_bit_mask: u8,

}

/// Instructions supported by the token swap program.
#[repr(C)]
#[derive(Debug, PartialEq)]
Expand All @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't actually passed as an account, no need to have it as a comment

Copy link
Author

Choose a reason for hiding this comment

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

Oops!

/// 8. '[]` Token program id
Initialize(Initialize),

/// Swap the tokens in the pool.
Expand Down Expand Up @@ -187,6 +212,11 @@ pub enum SwapInstruction {
/// 8. `[writable]` Fee account, to receive withdrawal fees
/// 9. '[]` Token program id
WithdrawSingleTokenTypeExactAmountOut(WithdrawSingleTokenTypeExactAmountOut),

/// Update the freeze authority bit mask.
/// 0. `[writable]` Token-swap
/// 1. `[]` Freeze authority (must be signer)
SetFreezeAuthorityBitMask(SetFreezeAuthorityBitMask),
}

impl SwapInstruction {
Expand All @@ -199,11 +229,16 @@ impl SwapInstruction {
if rest.len() >= Fees::LEN {
let (fees, rest) = rest.split_at(Fees::LEN);
let fees = Fees::unpack_unchecked(fees)?;
let swap_curve = SwapCurve::unpack_unchecked(rest)?;
let (swap_curve_data, rest) = rest.split_at(SwapCurve::LEN);
let swap_curve = SwapCurve::unpack_unchecked(swap_curve_data)?;
let (freeze_authority, rest) = Self::unpack_pubkey_option(rest)?;
let freeze_authority_bit_mask = rest[0];
Self::Initialize(Initialize {
nonce,
fees,
swap_curve,
freeze_authority,
freeze_authority_bit_mask,
})
} else {
return Err(SwapError::InvalidInstruction.into());
Expand Down Expand Up @@ -253,6 +288,9 @@ impl SwapInstruction {
maximum_pool_token_amount,
})
}
6 => Self::SetFreezeAuthorityBitMask(SetFreezeAuthorityBitMask {
freeze_authority_bit_mask: rest[0],
}),
_ => return Err(SwapError::InvalidInstruction.into()),
})
}
Expand All @@ -271,6 +309,28 @@ impl SwapInstruction {
}
}

fn unpack_pubkey_option(input: &[u8]) -> Result<(COption<Pubkey>, &[u8]), ProgramError> {
match input.split_first() {
Option::Some((&0, rest)) => Ok((COption::None, rest)),
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with the rest of the code, this should also pack 4 bytes for the optionality instead of just 1

Option::Some((&1, rest)) if rest.len() >= 32 => {
let (key, rest) = rest.split_at(32);
let pk = Pubkey::new(key);
Ok((COption::Some(pk), rest))
}
_ => Err(SwapError::InvalidInstruction.into()),
}
}

fn pack_pubkey_option(value: &COption<Pubkey>, buf: &mut Vec<u8>) {
match *value {
COption::Some(ref key) => {
buf.push(1);
buf.extend_from_slice(&key.to_bytes());
}
COption::None => buf.push(0),
}
}

/// Packs a [SwapInstruction](enum.SwapInstruction.html) into a byte buffer.
pub fn pack(&self) -> Vec<u8> {
let mut buf = Vec::with_capacity(size_of::<Self>());
Expand All @@ -279,6 +339,8 @@ impl SwapInstruction {
nonce,
fees,
swap_curve,
freeze_authority,
freeze_authority_bit_mask,
}) => {
buf.push(0);
buf.push(*nonce);
Expand All @@ -288,6 +350,8 @@ impl SwapInstruction {
let mut swap_curve_slice = [0u8; SwapCurve::LEN];
Pack::pack_into_slice(swap_curve, &mut swap_curve_slice[..]);
buf.extend_from_slice(&swap_curve_slice);
Self::pack_pubkey_option(freeze_authority, &mut buf);
buf.push(*freeze_authority_bit_mask);
}
Self::Swap(Swap {
amount_in,
Expand Down Expand Up @@ -335,6 +399,13 @@ impl SwapInstruction {
buf.extend_from_slice(&destination_token_amount.to_le_bytes());
buf.extend_from_slice(&maximum_pool_token_amount.to_le_bytes());
}

Self::SetFreezeAuthorityBitMask(SetFreezeAuthorityBitMask {
freeze_authority_bit_mask,
}) => {
buf.push(6);
buf.extend_from_slice(&freeze_authority_bit_mask.to_le_bytes());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you can just push here since it's a u8

}
}
buf
}
Expand All @@ -354,11 +425,15 @@ pub fn initialize(
nonce: u8,
fees: Fees,
swap_curve: SwapCurve,
freeze_authority: COption<Pubkey>,
freeze_authority_bit_mask: u8,
) -> Result<Instruction, ProgramError> {
let init_data = SwapInstruction::Initialize(Initialize {
nonce,
fees,
swap_curve,
freeze_authority,
freeze_authority_bit_mask,
});
let data = init_data.pack();

Expand Down Expand Up @@ -569,6 +644,28 @@ pub fn swap(
})
}

/// Creates a set_freeze_authority_bit_mask instruction
/// Creates a 'swap' instruction.
pub fn set_freeze_authority_bit_mask(
program_id: &Pubkey,
swap_pubkey: &Pubkey,
freeze_authority_pubkey: &Pubkey,
instruction: SetFreezeAuthorityBitMask,
) -> Result<Instruction, ProgramError> {
let data = SwapInstruction::SetFreezeAuthorityBitMask(instruction).pack();

let accounts = vec![
AccountMeta::new(*swap_pubkey, false),
AccountMeta::new_readonly(*freeze_authority_pubkey, true),
];

Ok(Instruction {
program_id: *program_id,
accounts,
data,
})
}

/// Unpacks a reference from a bytes buffer.
/// TODO actually pack / unpack instead of relying on normal memory layout.
pub fn unpack<T>(input: &[u8]) -> Result<&T, ProgramError> {
Expand Down Expand Up @@ -618,6 +715,8 @@ mod tests {
nonce,
fees,
swap_curve,
freeze_authority: COption::None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a freeze_authority here to test the new packing?

freeze_authority_bit_mask: 0,
});
let packed = check.pack();
let mut expect = vec![0u8, nonce];
Expand All @@ -631,7 +730,7 @@ mod tests {
expect.extend_from_slice(&host_fee_denominator.to_le_bytes());
expect.push(curve_type as u8);
expect.extend_from_slice(&amp.to_le_bytes());
expect.extend_from_slice(&[0u8; 24]);
expect.extend_from_slice(&[0u8; 26]);
assert_eq!(packed, expect);
let unpacked = SwapInstruction::unpack(&expect).unwrap();
assert_eq!(unpacked, check);
Expand Down
Loading