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

token-2022: Add Pausable extension #7562

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Conversation

joncinque
Copy link
Contributor

@joncinque joncinque commented Dec 5, 2024

Problem

Users want a more Ethereum-style token experience by being able to "pause" their token, similar to the "Pausable" interface. When a mint is paused, tokens cannot be minted, burned, or transferred.

Summary of changes

Add the extension and some tests. It covers the following interactions:

  • mint / mint-checked
  • burn / burn-checked
  • transfer / transfer-checked / transfer-with-fee
  • confidential transfer / confidential transfer with fee
  • confidential mint / confidential burn
  • confidential deposit / confidential withdraw

Unfortunately, the confidential mint / burn extension doesn't have testing, so I couldn't get a full end-to-end test for it.

Also note that it's still possible to:

  • move withheld tokens
  • initialize token accounts
  • close token accounts
  • set authority
  • freeze / thaw
  • approve / revoke
  • almost every other bit of extension management

cc @gitteri @jnwng

#### Problem

Users want a more Ethereum-style token experience by being able to
"pause" their token, similar to the "Pausable" interface. When a mint is
paused, tokens cannot be minted, burned, or transferred.

#### Summary of changes

Add the extension and some tests. It covers the following interactions:

* mint / mint-checked
* burn / burn-checked
* transfer / transfer-checked / transfer-with-fee
* confidential transfer / confidential transfer with fee
* confidential mint / confidential burn

Unfortunately, the confidential mint / burn extension doesn't have
testing, so I couldn't get a full end-to-end test for it.

Also note that it's still possible to:

* move withheld tokens
* initialize token accounts
* close token accounts
* set authority
* freeze / thaw
* approve / revoke
* almost every other bit of extension management
Copy link
Contributor

@samkim-crypto samkim-crypto left a comment

Choose a reason for hiding this comment

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

Looks very good! I could only find very minor things/comments.

For the confidential mint/burn, I am planning on just adding the token-client code myself, so I'll just go ahead and add the suitable tests for the pausable extension then when I add the other tests for confidential mint/burn.

token/program-2022/src/extension/pausable/processor.rs Outdated Show resolved Hide resolved
token/program-2022/src/error.rs Outdated Show resolved Hide resolved
@@ -397,6 +398,12 @@ fn process_deposit(
let mint_data = &mint_info.data.borrow_mut();
let mint = PodStateWithExtensions::<PodMint>::unpack(mint_data)?;

if let Ok(extension) = mint.get_extension::<PausableConfig>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

So technically, deposit and withdraw in the confidential transfer extension is neither minting, burning, or transferring of tokens. Seems like it might be worth adding a comment here and here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, sorry, I forgot to mention this in the PR description. Do you think that makes sense? If the mint is paused, should people be able to move tokens between their confidential and non-confidential balance? I was thinking yes. I'll update the comment and also add tests

cc @gitteri @jnwng in case they want to chime in

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it makes sense either way. Way back when we first started out on token22, we were able to deposit and withdraw funds to other accounts, so deposit/withdraw were another way to transfer funds. We have since restricted deposit/withdraw to work within same accounts only, but some intuition from that time remains, which makes me feel that pausing deposits/withdraws makes sense. Given this, the current changes looks good to me!

@joncinque
Copy link
Contributor Author

Ok this should be ready for another look

Copy link
Contributor

@samkim-crypto samkim-crypto left a comment

Choose a reason for hiding this comment

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

Things look good to me assuming that we stick with pausing deposits and withdraws!

@joncinque
Copy link
Contributor Author

I'll land this as is, and we can always remove it later

@joncinque joncinque merged commit 612a301 into solana-labs:master Dec 9, 2024
35 checks passed
@joncinque joncinque deleted the pause branch December 9, 2024 12:19
@Mohammed-Alanazisa

This comment was marked as spam.

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 this pull request may close these issues.

3 participants