Solana CT: SDK Sprint 1 — confidential mint + proof-account tx group - #9384
Solana CT: SDK Sprint 1 — confidential mint + proof-account tx group#9384bhavidhingra with Copilot wants to merge 1 commit into
Conversation
|
@copilot are you working on it? |
Yes, I’m still on it. I’ve addressed the prior review feedback in the latest commits and will keep an eye out for any additional comments. |
ff0c936 to
2349023
Compare
Review: Solana CT SDK Sprint 1 — confidential mint + proof-account tx groupReviewed against Linear TDD CHALO-1090 requirements. Findings below. ✅ What's good
🔴 Issues found1. CRITICAL — Builder produces ONE transaction, not 5-6 separate transactionsThis is the most important architectural issue per the TDD. The TDD explicitly states:
for (const instruction of this._instructionsData) {
tx.add(...solInstructionFactory(instruction));
}This puts all 6+ instructions into one This will not work on-chain for two reasons:
Recommendation: 2. CRITICAL — Proof instruction offsets are wrong (positive instead of negative)
equalityProofInstructionOffset: params.equalityProofInstructionOffset ?? 5,
ciphertextValidityProofInstructionOffset: params.ciphertextValidityProofInstructionOffset ?? 4,
rangeProofInstructionOffset: params.rangeProofInstructionOffset ?? 3,In the instruction order (indices 0-5): The offsets are relative to the confidentialMint instruction (signed i8, negative = look backward). The correct values are:
The code defaults to 3. MAJOR — Record account owner defaults to a wallet address instead of ZK_ELGAMAL_PROOF_PROGRAM_ID
const recordOwner = params.recordAccountOwnerAddress || params.contextStateAuthorityAddress;Then Similarly, programId: new PublicKey(recordAccountOwnerAddress || ZK_ELGAMAL_PROOF_PROGRAM_ID),When Fix: 4. MAJOR —
|
c863ba0 to
06edfad
Compare
…group - Fix proof instruction offsets: -3, -2, -1 (signed i8 relative to confidentialMint) instead of positive 5, 4, 3. Encoded as u8 two's complement: 253, 254, 255 - Record account owner defaults to ZK_ELGAMAL_PROOF_PROGRAM_ID instead of contextStateAuthorityAddress (wallet) - Add lamports parameter to CreateRecordAccount; calculate rent-exempt default: (space + 128) * 6960 * 2 - Fix VerifyRangeProof: only proof account key (no context state) — VerifyBatchedRangeProofU128 does not use context state - Fix closeRecordAccount: use ZK_ELGAMAL_PROOF_PROGRAM_ID instead of TOKEN_2022_PROGRAM_ID (record owned by zk program) - Remove confidential instructions from VALID_SYSTEM_INSTRUCTION_TYPES (they are Token-2022/zk-proof program, not System program) - Add instruction-level test assertions for coverage target Remaining (follow-up): - Critical 1: multi-tx architecture (builder produces single tx, TDD requires 5-6 separate txs for 1232B limit) - Major 5: ConfigureAccount missing ElGamal pubkey (32B) + AES key (16B) Ticket: CHALO-1090
06edfad to
b383cf2
Compare
Adds Token-2022 confidential transfer transaction construction in
sdk-coin-solusing the proof-account approach.ConfigureConfidentialTransferAccount,ConfidentialMint,CreateRecordAccount,WriteRecordData,VerifyEqualityProof,VerifyValidityProof,VerifyRangeProof,CloseRecordAccount,CloseContextState.solInstructionFactory.ts.create-record → write → verify ×3 → confidentialMint) as one logical transaction, with optional cleanup closes.TransactionBuilderFactory.getConfidentialMintBuilder()and exported fromindex.ts.CustomInstruction.ConfigureConfidentialTransferAccountlayout, full proof-group ordering, multi-chunk record writes, and raw tx serialization.