internal CI rehearsal 755 mechanical green (do not review, do not merge) - #5
Open
michael-moffett wants to merge 18 commits into
Open
internal CI rehearsal 755 mechanical green (do not review, do not merge)#5michael-moffett wants to merge 18 commits into
michael-moffett wants to merge 18 commits into
Conversation
surfnet_setTokenAccount can write a confidential balance but nothing could read one back, so a confidential-transfer test had to assert on the credit counter and ciphertext non-emptiness instead of on an amount. surfnet_getConfidentialBalance decrypts the account: the available balance via the AES copy the extension keeps for the owner, the pending balance by decrypting the lo/hi ElGamal ciphertexts and recombining them. surfnet_deriveConfidentialKeys derives the ElGamal and AES keys server-side from the owner's keypair and the token account, so the cheatcodes can be driven with no client-side confidential-transfer crypto at all.
A foreign elgamalSecretKey fails to decrypt a non-zero pending balance, but nothing exercised that: the one test holding a non-zero pending balance only ever used the matching key. The round-trip test's pending assertion is relabelled rather than dropped. The setter writes an all-zero pending ciphertext, which is the identity point and decodes to 0 under any key, so that assertion pins the response shape and says nothing about decryption.
GetConfidentialBalanceResponse carried ts(export, optional_fields), so the generated binding declared available and pending as `field?: T`. Neither field is skipped on serialize, so an unsupplied key comes back as an explicit `"available": null`, not as an absent key. A caller holding one of the two keys - the case the method exists to serve - gets a value the binding says cannot occur. optional_fields is right for the request types next to it, where a client omits what it does not set. It is wrong here. RunbookExecutionStatusReport is the response-side precedent: plain ts(export) with an explicit `bigint | null` on the Option field. pendingBalanceCreditCounter is not an Option and is unaffected.
…read The confidential cheatcodes were covered only by unit tests over fabricated account data, so nothing proved the keys surfnet_deriveConfidentialKeys hands out actually open ciphertexts the Token-2022 program writes. The test derives the owner's keys, funds a configured confidential account, runs a real Deposit and ApplyPendingBalance against the deployed Token-2022 program, and asserts on the decrypted pending and available balances plus the public balance the deposit drew from. Deposit and apply are the confidential-balance movements that carry no zero-knowledge proof. A party-to-party Transfer needs proofs the ZK ElGamal proof program verifies and is out of scope here.
surfnet_deriveConfidentialKeys documented itself as matching what a confidential-transfer client derives from the same keypair. It does not, and the note said the opposite of the truth. Two things differ. The seed is prefixed with solana-conf-bal/v1, which the standardised derivation in solana_zk_sdk::encryption::derivation already applies itself as its HKDF salt, so the prefix applies it a second time; reference clients pass the seed unprefixed. And the derivation reachable from the pinned Token-2022 interface crate is the older new_from_signer path, which that SDK flags in-tree as a non-standard KDF and replaces with HKDF-SHA512. Derived both ways and compared: dropping the prefix alone still does not reproduce the standardised keys, so the seed is left as it is rather than made to look standard while producing different keys. The notes now say what is actually true and point at the dependency that would close the gap.
The doc comment claimed every assertion is on a decrypted amount, so the test fails if the cheatcodes and the program disagree about the ciphertexts. Four of the ten assertions are on decrypted balances; three are on plaintext account fields and three only check that a call succeeded. The second half was wrong in a way that mattered more. A Deposit adds the amount to the commitment and passes the decrypt handle through untouched, so starting from the all-zero pending ciphertext the setter writes, the result opens to the same value under any ElGamal secret key. That assertion pins the response shape and the field plumbing, not the derivation. The available balance round-trips the test's own AES ciphertext, which shows AeKey encrypt and decrypt agree and the cheatcode reads the right field. What binds is the public balance falling by exactly the deposited amount and the pending credit counter moving, both computed by the program. Also records that the account reaches its configured state through surfnet_setTokenAccount rather than an on-chain ConfigureAccount, which is proof-gated and blocked by the same SDK skew as Transfer. Comment only. No assertion and no test logic changed.
… key The pending-balance assertion in test_confidential_balance_deposit_round_trip did not depend on the derived ElGamal key. Deposit adds the amount to the commitment and passes the decrypt handle through untouched, so a deposit onto the all-zero pending ciphertext a configured account starts with yields an identity handle -- and such a ciphertext opens to the same value under any secret key. Seed pending_balance_lo with a real encryption under the derived public key before the deposit, so the ciphertext the deposit lands on carries a non-identity handle, then assert both directions: the derived secret key recovers seed plus deposit exactly, and a random ElGamal secret key recovers nothing. Substituting a stranger's key now fails the test. The doc comment is rewritten to match, and now names which assertions do not bind to the ElGamal key rather than implying they do.
The generated TypeScript was checked in unformatted. formatFiles ran over the per-type files but not over index.ts, and prettier was not a declared dependency of the generator, so whether the output came out formatted depended on what happened to be installed. Declare prettier as a dependency and pass index.ts through formatFiles with the rest. The checked-in output is regenerated, which is where the churn across the generated directory comes from. Fixing this at the generator rather than by hand means it survives the next regeneration. A hand-fix would have looked identical here and then come back on the following change.
surfnet_deriveConfidentialKeys took the owner keypair as an RPC parameter, which teaches callers to put a signing key on the wire. It now takes the two signatures the standard derivation asks the owner to produce, and derives through ElGamalKeypair::new_from_signature and AeKey::new_from_signature. Two signatures rather than one because the SDK domain-separates the two seed messages. What the caller has to sign is documented on the parameters. The seed prefix is dropped. solana_zk_sdk's derivation applies its own HKDF salt, so prefixing applied one a second time and produced keys no reference client would derive. The seed is token_account.as_ref() now, and a test asserts the derived keys are byte-identical to what new_from_signer produces from the same owner and account. Reject the all-zero signature. ElGamalSecretKey::seed_from_signer and AeKey::seed_from_signer both carry that check and the SDK tests that a null signer errors; the signature path went around it, so a defaulted signature would have derived a usable key from nothing. The derived secrets still come back in the response, which the read cheatcode needs in order to decrypt. That is unchanged, and visible on ConfidentialBalanceKeys.
Three round trips through a running surfnet: a deposit, a plaintext transfer_checked followed by a deposit, and a confidential Transfer. The confidential Transfer test is ignored and does not run. Proof generation here resolves solana-zk-sdk 4.0.0 through spl-token-2022-interface and spl-pod, while the proof program the runtime verifies against is 5.0.1, and transcript construction moved layer between those majors. The instruction builds and Token-2022 accepts it; the proof program rejects it with SigmaProof(Equality, AlgebraicRelation). Nothing published aligns the two sides, so the test ships with that reason recorded rather than deleted or weakened into passing. The two that run assert on decrypted balances, and their doc comments name the assertions that do not bind to the derived keys rather than implying they all do. A deposit passes the decrypt handle through untouched, so a deposit onto an all-zero pending ciphertext opens to the same value under any secret key; the pending balance is seeded with a real encryption first so that assertion binds.
Seventeen sites, rewritten rather than character-swapped, so the prose reads the same in a terminal, a diff and a generated .d.ts. The kit bindings are regenerated to match, since these doc comments are their source. Upstream's own em-dashes are left as they are, including the two that reach ConfidentialTransferAccountUpdate.ts from doc comments already on main.
Greptile SummaryThe PR adds confidential-transfer key derivation and balance-decryption cheatcodes, exposes their contracts through the Node SDK, and adds unit and integration coverage.
Confidence Score: 5/5The code changes appear safe to merge because no blocking failure remains, although the PR description independently instructs maintainers not to merge this rehearsal branch. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| crates/core/src/rpc/surfnet_cheatcodes.rs | Registers and implements the two new confidential-transfer RPC methods with input and account-owner validation. |
| crates/core/src/types.rs | Implements signature-based key derivation and confidential available/pending balance decryption. |
| crates/core/src/tests/integration.rs | Adds an end-to-end Token-2022 confidential deposit and apply-pending-balance test across storage configurations. |
| crates/types/src/types.rs | Adds the shared RPC request and response data contracts and extends the canonical cheatcode manifest. |
| crates/sdk-node/surfpool-sdk/kit/types/api.ts | Adds both confidential-transfer methods to the composed Node SDK API. |
| crates/sdk-node/surfpool-sdk/kit/generated/methods.ts | Adds the new RPC names to the generated Node SDK method manifest. |
Sequence Diagram
sequenceDiagram
participant Client
participant RPC as Surfnet RPC
participant SVM
Client->>RPC: deriveConfidentialKeys(signatures)
RPC-->>Client: ElGamal and AES keys
Client->>RPC: getConfidentialBalance(account, keys)
RPC->>SVM: load Token-2022 account
SVM-->>RPC: confidential account data
RPC-->>Client: available, pending, credit counter
Reviews (2): Last reviewed commit: "build(sdk-node): drop the kit-types form..." | Re-trigger Greptile
…ext duplicate test_confidential_balance_transfer_round_trip was #[ignore]d from the day it landed: a party-to-party confidential Transfer needs proofs the ZK ElGamal proof program has to verify, and this harness cannot. A test that never runs asserts nothing, so it goes rather than sit in the file explaining why it is disabled. test_confidential_balance_plaintext_transfer_then_deposit moved value in the clear and then deposited it, which is the deposit round trip the sibling test already covers, with a second owner added. It duplicated that coverage rather than extending it.
… needed spl-token-confidential-transfer-proof-extraction and spl-token-confidential-transfer-proof-generation were pulled in for the ignored transfer test and used nowhere else. With that test gone they are two dependencies the tree carries for no reason.
…-2022 tests It was parked at the end of the file, a long way from test_token2022_metadata_realloc and the rest of the token-2022 suite it belongs to. Move it up next to them; no assertion changes. Its doc comment is also cut back. The long explanation of why the pending balance is seeded with a real ElGamal ciphertext now lives at the seeding site, where a reader hits it in context, and the paragraph contrasting Deposit with a proof-gated Transfer described a test that no longer exists.
derive_confidential_keys carried two paragraphs restating what the RPC method above it already documents, and a third selling the no-client-side-crypto property. Say what the function does and point at the one place the derivation semantics are written down.
GetConfidentialBalanceResponse overrode its ts-rs bindings to "number | bigint", which forces every caller to narrow before doing arithmetic. Dropping the override lets ts-rs map u64 to bigint, which is what the other response types in this crate already emit.
…ings generate-kit-types.js was made to shell out to prettier over everything ts-rs emits. That reformatted thirty-odd binding files this change never touched, so the diff read as a rewrite of the generated directory when three files were actually new. Take the formatter back out and let the generator's own output stand, as it did before. The bindings are regenerated on top of that, which is also where the previous commit's bigint change surfaces.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Internal CI rehearsal only. Not for review, not for merge.
Fires rust.yml, sdk_node.yml and benchmark.yml against 9eba2d8 so the three workflow
conclusions can be read from a real run rather than a local one. Head is byte-identical to
solana-foundation#755 head; base is byte-identical to that PR base, so the diff here
is that PR diff.
Left open and unmerged on purpose: merging would move this fork main off solana-foundation#755 base and break
parity with upstream.
Ticket: T-DIR-MECHGREEN-1A-755-FORK-CI-2026-08-19