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

Confidential mint-burn extension #6881

Open
wants to merge 45 commits into
base: master
Choose a base branch
from

Conversation

abcalphabet
Copy link
Contributor

@abcalphabet abcalphabet commented Jun 19, 2024

WIP implementation for the feature proposed in #6879

How to run

Start up a local validator with the bpf compiled from this branch and the zk-token-sdk enabled on it.

Build the cli on the branch and run the following

solana-keygen new -o key.json;

export SPL_TOKEN_CLI=$PWD"/target/debug/spl-token"
export SIGNER_KEYPAIR='key.json'
export SIGNER_PUBKEY="`solana-keygen pubkey $SIGNER_KEYPAIR`"

export TOKEN_ADDR=$($SPL_TOKEN_CLI create-token --program-id TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb \
    --enable-confidential-transfers auto \
    --enable-confidential-mint-burn \
    --decimals 2 --output json \
    --mint-authority $SIGNER_KEYPAIR \
    | jq -r .commandOutput.address)

$SPL_TOKEN_CLI create-account $TOKEN_ADDR \
    --fee-payer $SIGNER_KEYPAIR --owner $SIGNER_KEYPAIR
export ATA_ADDR=$($SPL_TOKEN_CLI accounts $TOKEN_ADDR \
    --owner $SIGNER_KEYPAIR --output json | jq -r .accounts[0].address)
$SPL_TOKEN_CLI configure-confidential-transfer-account --address $ATA_ADDR \
    --fee-payer $SIGNER_KEYPAIR \
    --owner $SIGNER_KEYPAIR

$SPL_TOKEN_CLI mint-confidential-tokens $TOKEN_ADDR 2 \
    --address $ATA_ADDR --fee-payer $SIGNER_KEYPAIR \
    --owner $SIGNER_KEYPAIR
$SPL_TOKEN_CLI confidential-balance $ATA_ADDR --authority $SIGNER_KEYPAIR;
$SPL_TOKEN_CLI apply-pending-balance --address $ATA_ADDR \
    --fee-payer $SIGNER_KEYPAIR --owner $SIGNER_KEYPAIR
$SPL_TOKEN_CLI burn-confidential-tokens $TOKEN_ADDR 1 \
    --address $ATA_ADDR --fee-payer $SIGNER_KEYPAIR \
    --owner $SIGNER_KEYPAIR

Implementation notes

This PR uses the proofs used for the transfers, obviously only proofs for two Ciphertexts (mint_to / burn_from and auditor) are necessary, so there's some inefficiency there as the transfer proofs create a third one on top of that. (Ab)Using these transfer proofs does however mean that the logic can work on it's own without additional changes to the solana-zk-token-sdk. After some discussion on the corresponding github issue, this has been resolved

@mergify mergify bot added the community Community contribution label Jun 19, 2024
@joncinque joncinque requested a review from samkim-crypto June 20, 2024 14:55
@abcalphabet abcalphabet changed the title WIP: Confidential mint-burn extension Confidential mint-burn extension Jul 2, 2024
@abcalphabet abcalphabet marked this pull request as ready for review July 2, 2024 15:49
@samkim-crypto samkim-crypto added the do-not-close Add this tag to exempt a PR / issue from being closed automatically label Jul 10, 2024
@samkim-crypto
Copy link
Contributor

@abcalphabet, there has been some work on refactoring the zk logic in the confidential-transfer submodule. I added the necessary zkp for confidential mint and burn in #6955. Can you rebase and use the logic from the confidential-transfer-proof-extraction module?

@samkim-crypto
Copy link
Contributor

Also, is it okay if you split the PR into

  1. token22 program and program-test
  2. token-client and cli
    components? The program part requires careful attention and it would be easier to focus solely on the program first.

@abcalphabet
Copy link
Contributor Author

Hey @samkim-crypto, will rebase your changes and use that logic. Splitting it into two PRs also makes sense, will get on that as well.

@abcalphabet
Copy link
Contributor Author

@samkim-crypto two questions:

Splitting the PRs

The tests in program-2022-test generally use the token client to call the logic to be tested, so splitting the tests and the client changes would require some major work making only the mint-burn tests work without that. Would a PR with just the program changes and one with the client+cli+tests be okay for you?

Auditability of instructions

I think I forgot to mention this on the issue discussing the feature:

Currently the auditor ciphertexts are only included in the context state accounts which makes using them in practice somewhat complicated since in order to continuously monitor transactions one needs to watch transactions on the token, get the ciphertext pubkey from the instruction, fetch all transactions on the ciphertext pubkey, find the proof instruction and extract the auditor ciphertext from there. This PR includes the auditor ciphertexts in the instruction itself so that token transactions (just mints and burns atm really) can be continuously audited by simply streaming transactions involving the specific token mint.

Does this make sense to you?

If it does make sense I guess putting that logic in a seperate issue / PR adding this across the board (including in the normal confidential-transfer extension) is probably better

@samkim-crypto
Copy link
Contributor

Splitting the PRs

Oh yes, that is completely fine with me. Sorry for the confusion. Let's just isolate the program logic changes.

Auditability of instructions

Yes that is a good point. I think it makes sense to split that logic into a separate issue / PR so that we can add it to the rest of the extensions as well like you suggested (it seems that it would just be the confidential transfer instruction).

Currently, the instructions should contain the addresses of the context/record accounts (https://github.com/solana-labs/solana-program-library/blob/master/token/program-2022/src/extension/confidential_transfer/instruction.rs#L272-L277), so I think one can monitor transactions on the token, look up the associated proof accounts (probably just the equality proof account), and then extract the auditor ciphertext from there.

I wonder if logging can also help here. Every time a confidential transfer, mint, or burn instructions are executed, the program logs (msg!) the associated auditor ciphertext. But let's continue the discussion in a designated issue so that other people can chip in.

@abcalphabet
Copy link
Contributor Author

@samkim-crypto Another question regarding the proof generation logic you added:

When generating the mint proofs there is an assumption that the mint would have a PodAeCiphertext decryptable supply and associated AeKey. How can we guarantee that this decryptable supply is actually always correct in practice? Since only we who have the key can mint it's updated fine on mints, but as any token owner can burn there's no way to update the decryptable supply on burns.

On the issue we had discussed just using a PodElGamalCiphertext for the supply, but that was before the equality proof was added to the mint proofs. We could still use that and decrypt it to get the supply as u64 for the proof, but that obviously has the potential to make the proof data generation quite slow once the supply gets into higher 64-bit territory.

Do you have any ideas on how to properly do this? Or am I just missing something obvious

@samkim-crypto
Copy link
Contributor

Yes that is a good point. I forgot to point this out, but the mint should have a decryptable balance PodAeCiphertext of the total supply. Unlike an ElGamal ciphertext, this ciphertext should be easily decryptable even if the encrypted number is large.

As you suggested though, the decryptable balance cannot always encrypt the correct balance in practice. The important point here is that the an ElGamal ciphertext is hard to decrypt when the encrypted value is large, but it is homomorphic, so it can still be decrypted relatively easily with the help of the decryptable ciphertext. Let's denote the current ElGamal ciphertext encrypting the supply as ct_current.

  1. Decrypt the decryptable ciphertext to find the old (outdated) supply x_old
  2. Encrypt (encode) x_old as an ElGamal ciphertext (e.g. like how it is done for Deposit). Let's denote this ct_old.
  3. Compute ct_old - ct_current using ciphertext arithmetic. Let's denote this ciphertext to be ct_diff.
  4. Decrypting ct_diff will give the burn amount that is not taken into account in the decryptable ciphertext. Let's denote this value x_diff.
  5. Then we know that x_current = x_old - x_diff is the most up to date supply. At this point, the decryptable balance can be updated to encrypt x_current.

The point here is that in step 4, ct_diff will not contain the full supply, but just the burn amounts that happened since the last time decryptable balance was updated. So as long as the decryptable balance is updated relatively frequent, the ElGamal ciphertext that encrypts the supply will not be too expensive to decrypt. Does this make sense?

@abcalphabet
Copy link
Contributor Author

Yup, makes perfect sense. Thanks for the explanation!

@kilogold
Copy link

kilogold commented Dec 5, 2024

Hi @abcalphabet .
Looks like there's some merge conflicts before this can go in.
I'm looking forward to this contribution, and can help finalize once we get into a mergeable state.

@samkim-crypto
Copy link
Contributor

@kilogold This extension is already added to the token program (#7319). We have not yet added this to the token-client yet (and hence why this PR is not yet closed), but that should follow very soon.

@abcalphabet Thanks for the contribution to the token program! I can go ahead and add the relevant components to the token-client (following the commits here) and to the program-2022-tests later this week. If you want to take a shot at the client too, then feel free to let me know!

@abcalphabet
Copy link
Contributor Author

Hey @samkim-crypto, sorry for leaving this one orphaned for a little while... I've fixed all the conflicts, but I'd assume there's still quite a few nits left in there if you have capacity to have a look. I should be free to fix everything up before / during the holidays

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution do-not-close Add this tag to exempt a PR / issue from being closed automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants