Skip to content

Swap-out solana-zk-sdk for solana-zk-elgamal-proof-interface and solana-zk-sdk-pod#1090

Open
samkim-crypto wants to merge 4 commits intosolana-program:mainfrom
samkim-crypto:swap-zk-sdk
Open

Swap-out solana-zk-sdk for solana-zk-elgamal-proof-interface and solana-zk-sdk-pod#1090
samkim-crypto wants to merge 4 commits intosolana-program:mainfrom
samkim-crypto:swap-zk-sdk

Conversation

@samkim-crypto
Copy link
Copy Markdown
Contributor

@samkim-crypto samkim-crypto commented Apr 8, 2026

Summary of Changes

487e83c: I swapped out solana-zk-sdk for solana-zk-elgamal-proof-interface and solana-zk-sdk-pod in token-2022 interface and program. The solana-zk-sdk is needed in some places like proof-generation. For these places, I bumped the version to v6.0.0. This commit should be pretty straightforward and mechanical, just re-organizing the imports. The exception is replacing the OptionalNonZeroElGamalPubkey with MaybeNull<PodElGamalPubkey>.

0fb0bed: I bumped agave crates like solana-test-validator and solana-program-test to v4-beta while downgrading some of the solana-sdk dependencies to resolve some dependency conflicts. I divided this commit in smaller commits in #1106.

e814132: in 487e83c, I made a mistake in comparing MaybeNull ElGamalPubkeys, so I fixed it here.

76e9728: Addressed #1116 (comment).

@samkim-crypto samkim-crypto force-pushed the swap-zk-sdk branch 3 times, most recently from 351682b to 87f0130 Compare April 16, 2026 00:55
Copy link
Copy Markdown
Member

@grod220 grod220 left a comment

Choose a reason for hiding this comment

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

Thanks for this work! Unraveling dependencies can be tricky. Just have some cleanup comments.

Comment thread clients/cli/Cargo.toml
Comment on lines +33 to +34
solana-zk-sdk = "6.0.0"
solana-zk-sdk-pod = "0.1.1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you put these in alphabetical order?

Comment thread clients/cli/Cargo.toml

[dev-dependencies]
solana-nonce = "3.2.0"
solana-nonce = "3.1.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Think we can keep v3.2 here?

Comment on lines +70 to +85
// solana_zk_sdk::{
// encryption::{
// auth_encryption::AeKey,
// elgamal::{ElGamalCiphertext, ElGamalKeypair, ElGamalPubkey, ElGamalSecretKey},
// pod::{
// auth_encryption::PodAeCiphertext,
// elgamal::{PodElGamalCiphertext, PodElGamalPubkey},
// },
// },
// zk_elgamal_proof_program::{
// self,
// instruction::{close_context_state, ContextStateInfo},
// proof_data::*,
// state::ProofContextState,
// },
// },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: remove

solana-address = "2.6.0"
solana-banks-client = { version = "3.0.0", optional = true }
solana-banks-interface = { version = "3.0.0", optional = true }
solana-address = "2.5.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: v2.6 here

solana-cli-output = { version = "3.1.0", features = ["agave-unstable-api"], optional = true }
solana-hash = "4.3.0"
solana-cli-output = { version = "4.0.0-beta.7", features = ["agave-unstable-api"], optional = true }
solana-hash = "4.2.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe some of these version regressions are rebase artifacts? Perhaps you can do another pass to catch these in the rest of the PR.

[package]
name = "spl-token-confidential-transfer-proof-extraction"
version = "0.5.1"
version = "0.6.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't bump these ourselves right? Think the publish github action is responsible for that. See a few examples in this PR of this.

Comment on lines +88 to +95
// test_transfer_with_fee_proof_validity(65535, 65535, 5, 10);
// test_transfer_with_fee_proof_validity(65535, 65535, 5, 1);

test_transfer_with_fee_proof_validity(65536, 65536, 5, 10);
test_transfer_with_fee_proof_validity(65536, 65536, 5, 1);

test_transfer_with_fee_proof_validity(281474976710655, 281474976710655, 5, 10); // 2^48 - 1
test_transfer_with_fee_proof_validity(281474976710655, 281474976710655, 5, 1);
// test_transfer_with_fee_proof_validity(65536, 65536, 5, 10);
// test_transfer_with_fee_proof_validity(65536, 65536, 5, 1);
//
// test_transfer_with_fee_proof_validity(281474976710655, 281474976710655, 5, 10); // 2^48 - 1
// test_transfer_with_fee_proof_validity(281474976710655, 281474976710655, 5, 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably can re-enable?

Copy link
Copy Markdown
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 overall! I agree with all of Gabe's comments

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