Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No mechanism for freezing token accounts #229

Closed
jackcmay opened this issue Aug 6, 2020 · 14 comments · Fixed by #297
Closed

No mechanism for freezing token accounts #229

jackcmay opened this issue Aug 6, 2020 · 14 comments · Fixed by #297
Assignees
Milestone

Comments

@jackcmay
Copy link
Contributor

jackcmay commented Aug 6, 2020

An authority may want the ability to "freeze" specific accounts.

Add a new authority to the mint and a new flag in accounts. The authority is able to toggle that flag. Setting the flag to "frozen" disables the account owner and delegate's authority.

@aeyakovenko
Copy link
Member

Should be settable only once during token initialization, so it’s possible to guarantee that this token instance cannot freeze accounts.

@mvines mvines added this to the Token v1.1 milestone Aug 10, 2020
@mvines
Copy link
Member

mvines commented Aug 10, 2020

The freeze authority needs to optionally be a multisig account

@mvines
Copy link
Member

mvines commented Aug 20, 2020

Updates:

  1. We're like to use the existing mint owner as the freeze authority to avoid changing the mint
  2. There's a strong desire that the bit which indicates that an account is frozen ideally sneaks into the account data without any layout changes, to avoid affecting downstream users

@CriesofCarrots
Copy link
Contributor

1. We're like to use the existing mint owner as the freeze authority to avoid changing the mint

This means that freeze-able tokens will by definition have non-fixed supplies, since a mint owner can mint new tokens. Is this an issue?

@mvines
Copy link
Member

mvines commented Aug 20, 2020

We should try to avoid changing the layout from token 1's account as this will impact the ability of existing users to easily migrate.

Changing

pub is_initialized: bool,

from a bool and enum where "2" == "frozen" would meet this requirement

@mvines
Copy link
Member

mvines commented Aug 20, 2020

This means that freeze-able tokens will by definition have non-fixed supplies, since a mint owner can mint new tokens. Is this an issue?

Thanks, great point. Yeah lets add that separate freeze authority to the mint then after all

@CriesofCarrots
Copy link
Contributor

CriesofCarrots commented Aug 20, 2020

@mvines Is it still okay to expect that the freeze authority will always be the same as the mint owner/multisig, as in Update 1. above? I'm trying to figure out how to handle the accounts passed in with TokenInstruction::InitializeMint, and the options are a bit complicated if we need to support separate owner and freezer.

If it's all good to expect them to be the same, and we need to change the Mint structure anyway, how about bytes for "supply is fixed" and "token is freezable", and stick with a single owner: COption<Pubkey> field? It'll save ~30 bytes.

@mvines
Copy link
Member

mvines commented Aug 20, 2020

I think we want the option of a separate freeze authority with a freeze_authority: COption<Pubkey> to the end of Mint.

While we pass the optional owner in as an account, process_initialize_mint() doesn't really care that it's an account. It just does a:

and we could rework InitializeMint to:

    InitializeMint {
        amount: u64,
        decimals: u8,
        mint_authority: Option<Pubkey>,
        freeze_authority: Option<Pubkey>,
    },

@jackcmay
Copy link
Contributor Author

We could have the owner and freeze authority be the same during mint init and then allow folks to change either in a seperate instruction. We already have a SetOwner instruction and we probably want a SetFreezer instruction anyway.

@mvines
Copy link
Member

mvines commented Aug 20, 2020

We could have the owner and freeze authority be the same during mint init and then allow folks to change either in a seperate instruction

I did consider that but then we do still need a way to toggle freezing in InitializeMint, so we'd still end up adding new fields to InitializeMint

we probably want a SetFreezer instruction anyway.

Yep! Or maybe just replace SetOwner with SetAuthority with an enum input

@jackcmay
Copy link
Contributor Author

Yeah, I wasn't trying to avoid adding the freeze authority to the mint structure because I think that's necessary. I was addressing @CriesofCarrots 's concern about the complicated InitiaizeMint instruction.

@CriesofCarrots
Copy link
Contributor

Should be settable only once during token initialization, so it’s possible to guarantee that this token instance cannot freeze accounts.

@aeyakovenko said this above. I'd say that precludes any kind of SetFreezer or SetAuthority instruction.

@mvines
Copy link
Member

mvines commented Aug 21, 2020

If freezing is disabled on a mint, I agree that it probably shouldn't be possible to enable it later. Like fixed supply. But if freezing is enabled, we do need a Set* to support key rotation of the freezer key

@CriesofCarrots
Copy link
Contributor

Okay, makes sense.
Although I generally prefer the pattern of passing in the authority keys as instruction parameters, in the interest of minimizing the changes between token 1 and 2 and consistency across instructions, I will code out Jack's suggestion for you two to 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants