-
Notifications
You must be signed in to change notification settings - Fork 7
Add confidential transfer ext by default #167
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
Conversation
07eb41b
to
67ee807
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the motivation here?
977cd17
to
33b418b
Compare
A follow up after #141. Arcium reached out for getting confidential transfer support on the wrapped mints. This had also been something brought up by the foundation in the past. After discussing with a few folks there, they felt it was something worthwhile to include by default. It essentially makes privacy a first-class citizen in the program as it would have out-of-the-box support. So in general, this is an effort to further promote this feature and its use in the ecosystem. |
Does it make sense to be opt-in or opt-out, rather than automatic? I'm sure there are some teams who probably really don't want to enable confidentiality. |
A few things come to mind:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This enables the option to use confidential transfers, but doesn't force its use. Users will be able to use their transparent balance all the same.
- It is indeed a opinionated design though. Shipping it by default essentially says privacy should be the baseline. The escape hatch would be a fork.
It doesn't force its use but there's nothing you can do to stop its use. That's my point.
If I'm a protocol that cannot allow confidential transfers, I am barred from using the token wrap protocol because of this change. I would not make this automatic without an opt-out feature.
What does this mean exactly? If a protocol cannot allow confidential transfers, the protocol can apply this policy on its own token accounts, by not enabling confidential transfers. But it shouldn't care about how other people use the token. Do you have any examples where this would be a problem?
If there's an opt-out, then anyone can grief the protocol by just creating all of the mints with or without the extension. |
Varying extension configurations for the wrapped mint is not something that we can reasonably allow for in the program. There are too many combinations and differing initialization params---which would prevent a sane PDA derivation scheme. A different wrapped mint config requires a fork. So that essentially leaves us to choose an opinionated default. The motivation to include confidential transfer extension is to 1) serve requested usecases of folks wanting to use this program and 2) generally promote privacy as a normalized-standard. |
How about I'm an organization that issues United Arab Emirates Dirham stablecoins, and the government has strict requirements about transparency. Imagine they outright ban confidential token transfers altogether. If the mint supports confidential transfers, that means anyone can use that feature if they create a token account to do so.
Wouldn't you also consider it griefing if I wrapped your mint - which does not allow confidential transfer - and could suddenly transfer it confidentially? I could just build an entire confidential transfer protocol that allows people to wrap their Dirham stables and transfer them confidentially. (The irony is not lost on me that most of us would applaud such a protocol, however I find it hard to believe Anza wants to be responsible for enabling this in their program. Someone can easily fork and do this themselves, if they are okay with the risks involved)
That's true, and I would say if you wish to enable confidential transfers on a mint that does not currently have that extension, then by this definition (which applies to all other extensions) you should fork.
I don't believe this is our call to make if the original mint has not included this extension. We are essentially saying you cannot wrap tokens without enabling confidentiality, which is over-stepping. This program should only include |
This has come up as an issue in the past, especially around transfer fees. With this program as is, someone can just wrap a token that has transfer fees, and avoid paying them. If an issuer doesn't like a particular version of a wrapped token, then they can just freeze the escrow holding all the tokens. Either way, we should get more opinions on this before moving forward. |
Giving my two cents here since we were one of the teams that initially asked for this: If each time someone wants to wrap an existing token confidentially they need to fork this program this creates a huge mess with 100s if not 1000s of basically the same program floating around, some of which will inevitably include some kind of attack on unsuspecting people wrapping their token. If we assume this consolidates in one standardized forked wrap program that everyone uses at some point, it makes a lot of sense for this program to fall under the Anza parasol as a) this is a low-level primitive centered around SPL-token and Token2022, both of which are for the most part maintained by Anza atm (similar situation as wrapped SOL in my mind) and b) there already exists the token-wrap program whose purpose is to wrap tokens anyhow. Re the concerns around wrapping tokens people don't want to be confidential, I see it the same way Jon said - in fact, an issuer could even create the escrow with this program & then immediately freeze it if they want to be extra clear that they don't want this for their token. |
9965623
to
19cfdf9
Compare
Post discussions on token-wrap direction, updated this PR to make use of the new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some tiny nits, the rest looks great!
clients/cli/src/create_mint.rs
Outdated
// For token-2022, confidential transfers extension added by default | ||
let extensions = if args.wrapped_token_program == spl_token_2022::id() { | ||
vec![ExtensionType::ConfidentialTransferMint] | ||
} else { | ||
vec![] | ||
}; | ||
let mint_size = ExtensionType::try_calculate_account_len::<Mint>(&extensions)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: how about calling the mint customizer directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The challenge is that the customizer trait requires access to accounts. It's sort of designed with only the program use-case in mind.
fn get_token_2022_mint_space(
unwrapped_mint_account: &AccountInfo,
all_accounts: &[AccountInfo],
)
It assumes that account arguments could potentially be used to calculate space. For the other methods, accounts are definitely needed. But perhaps it's not really necessary for calculating space? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right my bad, I forgot because none of the implementations were using the accounts 😅
Do you know of use-cases where the accounts would be needed? The only possibility I could think of was metadata, but that's a variable-length extension, and all variable-length extensions are initialized after the mint is initialized, so we wouldn't need it here to calculate the space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it's reasonable to update the interface and we can later add to it as more is actually needed. We can probably change all methods to associated functions too (versus methods) as we aren't using any struct state.
19cfdf9
to
33f2358
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!
If the caller is creating a wrapped mint under the token-2022 program, the token wrap program will now add the
ConfidentialTransfer
extension by default.