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

[token-cli] Add confidential transfer mint commands #5335

Merged

Conversation

samkim-crypto
Copy link
Contributor

@samkim-crypto samkim-crypto commented Sep 24, 2023

Problem

There are no cli support for confidential transfers yet.

Summary of changes

Updated/added cli commands regarding the mint. The create-token with --enable-confidential-transfers flag should already be supported.

f616231: Updated the display command to support the confidential transfer mint extension. Unfortunately, the solana-account-decoder is still using the older version of token-2022 before the confidential transfer and confidential transfer fee extensions were split, which means we can't use the UiConfidentialTransferMint type from solana-account-decoder. I hard-coded a temporary UiConfidentialTransferMint type in output.rs to account for this. This part can be removed in the next monorepo upgrade.

09ad0d9: ElGamal keys are supported in solana-clap-v3-utils, but an upgrade to clap-v3 for token-cli is blocked until 1.17 due to a new panicking behavior that was introduced with clap-v3 (solana-labs/solana#33184). So I added temporary functions to read ElGamal keypairs and pubkeys to unblock work on the token-cli. These functions only support either reading from ElGamal keypair files or reading form base64-encoded pubkey string. Once we upgrade to solana-clap-v3-utils, we can support a more variety of ways to take ElGamal arguments.

I didn't write tests for these hard-coded functions since they will eventually be replaced anyway, but I can add them in if suggested.

7e52fc5: Added update-confidential-transfer-settings, which allows updating the confidential transfer mint configuration.

To try out the new commands:

> cargo run -- create-token --enable-confidential-transfers auto -p TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb

Address:  [MINT-ADDRESS]
Decimals:  9

... the auditing feature is set to none by default
> cargo run -- display [MINT-ADDRESS]
... will display the confidential transfer mint settings

> cargo run -- update-confidential-transfer-settings [MINT-ADDRESS] manual 5DcMEnLGCFYe2qChv9uexI8tuM6sDJlrbe+tMeIf+UA=
... will update the confidential transfer mint settings to manual approve and auditor key

> cargo run --display [MINT-ADDRESS]
... verify that the update was done correctly

To generate an arbitrary ElGamal public key, I just did

let elgamal_keypair = ElGamalKeypair::new_rand();
println!("{}", elgamal_keypair.pubkey());
// this prints base64 encoding of ElGamal public key like `5DcMEnLGCFYe2qChv9uexI8tuM6sDJlrbe+tMeIf+UA=`

Once the issues in solana-clap-v3-utils are resolved and we publish solana-zk-keygen, then we can use that instead to generate ElGamal keypairs/pubkeys.

@samkim-crypto samkim-crypto added the WIP Work in progress label Sep 24, 2023
@samkim-crypto samkim-crypto force-pushed the token-cli/ct-mint-commands branch from 9d42d1b to 7e52fc5 Compare September 24, 2023 19:51
@samkim-crypto samkim-crypto removed the WIP Work in progress label Sep 25, 2023
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great! Mostly tiny nits

@@ -288,6 +297,7 @@ pub(crate) struct CliMint {
pub(crate) epoch: u64,
#[serde(flatten)]
pub(crate) mint: UiMint,
pub(crate) ui_confidential_transfer_extension: Option<UiConfidentialTransferExtension>,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you put a note to remove it here too?

{
auditor_pubkey
} else {
"audits are not enabled"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we just say "disabled" 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.

Yep, good idea!

Comment on lines 754 to 796
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub(crate) enum UiConfidentialTransferExtension {
ConfidentialTransferMint(UiConfidentialTransferMint),
// TODO: add `ConfidentialTransferAccount`
}

#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub(crate) struct UiConfidentialTransferMint {
pub authority: Option<String>,
pub auto_approve_new_accounts: bool,
pub auditor_encryption_pubkey: Option<String>,
}

pub(crate) fn has_confidential_transfer(data: &[u8]) -> Option<UiConfidentialTransferExtension> {
if let Ok(mint) = StateWithExtensions::<Mint>::unpack(data) {
if let Some(confidential_transfer_mint) =
mint.get_extension::<ConfidentialTransferMint>().ok()
{
let authority: Option<Pubkey> = confidential_transfer_mint.authority.into();
let auditor_encryption_pubkey: Option<ElGamalPubkey> =
confidential_transfer_mint.auditor_elgamal_pubkey.into();
return Some(UiConfidentialTransferExtension::ConfidentialTransferMint(
UiConfidentialTransferMint {
authority: authority.map(|pubkey| pubkey.to_string()),
auto_approve_new_accounts: confidential_transfer_mint
.auto_approve_new_accounts
.into(),
auditor_encryption_pubkey: auditor_encryption_pubkey
.map(|pubkey| pubkey.to_string()),
},
));
}
}
None
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should all get removed too, right? If so, please put a comment about it

Comment on lines 30 to 43
spl-token = { version = "4.0", path = "../program", features = [
"no-entrypoint",
] }
spl-token-2022 = { version = "0.8", path = "../program-2022", features = [
"no-entrypoint",
] }
spl-token-client = { version = "0.6", path = "../client" }
spl-token-metadata-interface = { version = "0.2", path = "../../token-metadata/interface" }
spl-associated-token-account = { version = "2.0", path = "../../associated-token-account/program", features = [
"no-entrypoint",
] }
spl-memo = { version = "4.0.0", path = "../../memo/program", features = [
"no-entrypoint",
] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Did your formatter pick these up? I haven't seen this before!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I recently updated my neovim config (I now use lazyvim) to enable lsp-server with taplo and it automatically validates cargo files as in an IDE, which is pretty cool. I also enabled auto-format on save as well. I guess should have filtered these format changes since it is orthogonal to this PR, sorry 😅.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment here too to remove and use the proper parsers from clap-v3-utils?

token/cli/src/main.rs Outdated Show resolved Hide resolved
.long("approve-policy")
.value_name("APPROVE_POLICY")
.takes_value(true)
.index(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than setting these by index, which can be annoying if you just want to update one parameter, how about making them optional? That is, unless sign-only is present, in which case you'll always need it

.long("auditor-pubkey")
.value_name("AUDITOR_PUBKEY")
.takes_value(true)
.index(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, how about making it optional? Then you only set the specified params. Unless sign-only is present, of course

token/cli/src/main.rs Show resolved Hide resolved
token/cli/src/main.rs Outdated Show resolved Hide resolved
@samkim-crypto samkim-crypto force-pushed the token-cli/ct-mint-commands branch from 7ed0096 to dd18895 Compare September 27, 2023 04:32
joncinque
joncinque previously approved these changes Sep 27, 2023
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just one last nit, but otherwise looks great!

token/cli/src/main.rs Outdated Show resolved Hide resolved
token/cli/src/main.rs Outdated Show resolved Hide resolved
token/cli/src/main.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed joncinque’s stale review September 27, 2023 16:05

Pull request has been modified.

@samkim-crypto samkim-crypto merged commit 08302f7 into solana-labs:master Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants