serviceability-instruction/sdk-rs: carry the IP ownership proof and Ed25519 instruction

[elitegreg]

Resolves #4200. Part of RFC-27 (rfcs/rfc27-ip-verification.md); tracker #4194.

Summary of Changes

• The serviceability program validates an optional IpOwnershipProof as of serviceability: validate IpOwnershipProof via the Ed25519 precompile #4211, but nothing client-side could build a transaction carrying one. This adds the two missing pieces: the native Ed25519SigVerify instruction the program introspects the Instructions sysvar to find, and the Rust SDK plumbing to send it alongside the creation it authorizes.
• New ip_proof module in doublezero-serviceability-instruction: ed25519_verification_instruction lays out the precompile instruction for a proof, and with_ed25519_verification pairs it ahead of the create instruction. The offset layout comes from solana_ed25519_program rather than being written out here — the program rejects any instruction whose offsets name a different instruction or run past the end of its data, so a hand-rolled header is a silent way to build a transaction that can never land.
• The builders keep their signatures. Issue serviceability-instruction/sdk-rs: carry the IP ownership proof and Ed25519 instruction #4200 asks them to take Option<IpOwnershipProof>, but serviceability: validate IpOwnershipProof via the Ed25519 precompile #4211 already put ip_proof in UserCreateArgs / UserCreateSubscribeArgs and made both builders derive the Instructions sysvar append from it. Adding a positional parameter would give the proof two homes that can disagree, so it stays in the args.
• DoubleZeroClient gains send_instructions for a transaction needing more than one instruction. send_transaction is unchanged, so its 223 call sites are untouched.
• CreateUserCommand and CreateSubscribeUserCommand take an optional ip_proof. A shared helper resolves the verifier key from GlobalState.ip_verifier_authority_pk — the same place the program reads it — so a caller cannot pair a proof with the wrong key, and refuses a proof naming a different owner, address, or user type before the transaction is paid for. The epoch window is deliberately left to the program: the ledger's current epoch is its to judge.
• On the --owner override path the proof must name that owner, not the payer. create_user_core binds the proof to the user's effective owner, which differs from the payer on the foundation-allowlist path.
• Passing None produces the pre-RFC-27 transaction byte for byte. Nothing sets ip_proof yet; the CLI obtaining a proof during connect is cli: obtain an IP ownership proof during connect and attach it to user creation #4201.

Transaction size

The issue asks for the headroom at realistic dz_prefix_count, and two tests pin it rather than leaving it to a comment. With a proof attached, CreateUser fits 10 dz_prefix_block accounts and CreateSubscribeUser 8, against 21 and 19 without one — the proof costs about eleven slots: the 111-byte Option<IpOwnershipProof> in the args, a 169-byte Ed25519 instruction, and two more account keys (the Instructions sysvar and the Ed25519 program). Devices carry one or two prefixes, so the margin is large either way, but a future field cannot quietly eat the rest of it without failing these tests.

Diff Breakdown

Category	Files	Lines (+/-)	Net
Core logic	6	+708 / -16	+692
Tests	3	+411 / -6	+405
Fixtures	1	+14 / -2	+12
Scaffolding	4	+13 / -0	+13
Config/build	2	+10 / -3	+7
Docs	1	+4 / -0	+4
Generated	1	+2 / -0	+2
Total	18	+1162 / -27	+1135

The core-logic files carry 507 lines of inline #[cfg(test)] tests of their own, leaving about 200 lines of new logic — two builders, one shared SDK helper, and one trait method.

Key files (click to expand)

• crates/doublezero-serviceability-instruction/src/ip_proof.rs — new module: the Ed25519 precompile instruction for a proof, and the ordered pair
• smartcontract/sdk/rs/src/commands/user/mod.rs — the shared instructions_with_ip_proof: verifier-key lookup plus the three field checks the program also makes, in one place so the two commands cannot drift
• smartcontract/sdk/rs/src/commands/user/create.rs — ip_proof field, and the two-instruction send when one is supplied
• smartcontract/sdk/rs/src/commands/user/create_subscribe.rs — the same, binding the proof to accesspass_payer (the effective owner) rather than the payer
• smartcontract/sdk/rs/src/client.rs — the inner send path now takes a Vec<Instruction>
• smartcontract/sdk/rs/src/doublezeroclient.rs — send_instructions on the trait
• crates/doublezero-serviceability-instruction/src/user.rs — the two transaction-size headroom tests
• smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs — end-to-end acceptance against the real program

Testing Verification

• test_builder_user_creation_with_ip_proof runs both builders against the in-process program with require-ip-ownership-proof set and a real verifier key in global state, so each creation only lands if the builder-assembled Ed25519 instruction actually validates — the acceptance criterion. Both users end Activated, and the multicast one carries its subscription. This is the first test in the repo that submits a precompile instruction produced by production code rather than a test helper.
• The pre-existing test_builder_create_subscribe_user is the None case end to end, and builder-level tests assert that a None proof leaves both account lists byte-identical to the pre-RFC-27 layout.
• Ed25519 layout is asserted field by field against the offsets the program's own parser reads: one signature, all three instruction indices at the u16::MAX sentinel, and the key / signature / message slices at the declared offsets. A separate test bends the proof's epoch and asserts the covered message moves with it, so the builder can never sign for a message the program will not reconstruct.
• SDK command tests cover the proof path on both commands and each local rejection: unset verifier key, a proof naming another payer, a proof for another address, a proof for another user type, and — on the --owner path — a proof bound to the payer instead of the owner.
• Transaction-size tests assert the exact maximum dz_prefix_count with and without a proof, for both instructions.
• user_ip_proof_test (34), doublezero-daemon-cli (180), and doublezero-serviceability-cli (419) all still pass.
• make generate-fixtures produces no .bin / .json change, confirming the no-proof wire shape is untouched.

[juan-malbeclabs]

Reviewed the RFC-27 client side. The mechanism is correct: I re-derived the Ed25519 precompile layout against solana-ed25519-program-3.0.0 (DATA_START=16, key→sig→msg order, the u16::MAX sentinel) and it agrees with check_ed25519_instruction's reconstruction; the sysvar scan is position-independent and bounds-checked; the epoch window reads correctly at epoch 0; and proof_owner lines up with create_user_core's effective_owner on both paths (CreateUser passes owner_override: None, so the payer is right; CreateSubscribeUser derives it from args.owner, so accesspass_payer is right). No wrong conditions, off-by-ones, dropped errors, or broken callers.

Two notes, both on the local pre-send gate this PR introduces, inline.

Unrelated to the diff but newly relevant: client.rs:244 maps InstructionError::Custom(n) to DoubleZeroError::from(n) and discards the instruction index. That was safe while every instruction was serviceability's — now that transactions carry a precompile instruction whose PrecompileError lives in the same Custom space, InvalidSignature (2) would print as InvalidExchangePubkey. Unreachable while skip_preflight is hardcoded true, so a comment or a cheap index guard is enough.

Verified locally: cargo test -p doublezero-serviceability-instruction (81 pass), cargo test -p doublezero_sdk (189 pass, including all 7 new RFC-27 command tests), rfc26_builders_test test_builder_user_creation_with_ip_proof, and cargo check --workspace --all-targets (only the pre-existing, unrelated doublezero-geolocation::entrypoint test-target failures).

[juan-malbeclabs]

The helper resolves the verifier key and builds the Ed25519 instruction with it, but never checks proof.signature against that key — and send_transaction_inner sends with skip_preflight: true (client.rs:200).

So if the verifier rotates its key and GlobalState.ip_verifier_authority_pk is updated while a client still holds a proof signed by the old one (same for any truncated or corrupted proof), all three field checks pass, the Ed25519 instruction is built with the new key over the old signature, and agave-precompiles rejects the transaction during precompile verification — which the RPC only runs when !skip_preflight. The transaction is dropped by the leader and never lands, so there is no TransactionError for parse_transaction_error to find: the caller blocks in send_and_confirm_transaction_with_spinner_and_config until blockhash expiry and gets "unable to confirm transaction", with no program logs and no DoubleZeroError.

Every other failure in this path lands onchain with a named error and logs; this is the first that can fail invisibly, which is exactly what the doc comment above says the local checks are here to avoid. The helper already holds both the proof and the key, and doublezero_ip_proof::verify(proof, &verifier) exists behind the crate's signer feature — so this is a one-line check plus features = ["signer"] on the sdk/rs dependency (safe for the BPF build: cargo build-sbf doesn't pull sdk/rs into the program's graph, and the program already enables the same feature in dev-dependencies).

Worth noting the test fixtures encode the current behavior — both proof_for helpers build a [5u8; 64] signature against a Pubkey::new_unique() verifier and their doc comments say the signature is never checked client-side — so the two happy-path tests would need real keypairs and doublezero_ip_proof::sign, the shape rfc26_builders_test.rs already uses. The mismatched-field tests bail before the signature check, so they are unaffected.

[elitegreg]

Agreed, and fixed in f0226ab. instructions_with_ip_proof now calls doublezero_ip_proof::verify(proof, &verifier) right after the GlobalState lookup and before building the Ed25519 instruction, so a proof signed by a rotated (or corrupted) key fails locally with a named error instead of the invisible confirmation timeout. sdk/rs enables the crate's signer feature for the verification half only — the comment on the dependency says so, and the BPF build is unaffected as you noted.

The check is ordered after the field comparisons deliberately: a mismatched payer/IP/user_type keeps naming its own cause rather than surfacing as a generic signature failure.

Both proof_for helpers now build a real Keypair and call doublezero_ip_proof::sign, and the happy-path tests pass verifier.pubkey() to create_test_client_with_ip_verifier. New test test_commands_user_create_with_ip_proof_rejects_a_rotated_verifier_key covers exactly the rotation case. The mismatched-field tests are unchanged in behavior, as you predicted.

[juan-malbeclabs]

This mirrors the program's payer/client_ip/user_type comparisons but leaves out its is_supported_version(proof.version) check (ip_proof.rs:125).

During a v2 rollout, where the verification service starts issuing v2 proofs before a given ledger's program is upgraded, signed_message() builds v2 bytes, the precompile verifies them fine, the transaction lands, and the program rejects it with IpProofVersionUnsupported — after the fee is paid. That's the exact outcome the doc comment above gives as the reason for doing the other three checks locally, so this reads as an oversight rather than a decision (unlike the epoch window, which is deliberately excluded and correctly so).

[elitegreg]

Oversight, not a decision — fixed in f0226ab. is_supported_version(proof.version) now runs first in instructions_with_ip_proof, matching the program's order: every comparison after it is about a message this client cannot reconstruct for a version it does not know. The doc comment lists IpProofVersionUnsupported alongside the other three, and the epoch window keeps its separate paragraph explaining why it stays the program's call. Covered by test_commands_user_create_with_ip_proof_rejects_unsupported_version.

Your third point (the Custom(n) mapping at client.rs:244) is also addressed in the same commit, with a guard rather than a comment: the mapping through DoubleZeroError now only applies when transaction.message.program_id(index) is the serviceability program, so a precompile failure is reported as the runtime described it instead of as an unrelated serviceability error.

One more thing, from rebasing onto current main. #4120 rewrote CreateSubscribeUserCommand around batched groups and added a transaction-size guard that measured [cu_limit, heap_frame, ix] — which under-counts by roughly 300 bytes once RFC-27 attaches the Ed25519 instruction and two account keys. The instruction list is now assembled before that measurement, so the group cap is judged against the transaction that actually goes on the wire. expect_create_lookups gained an accesspass_owner parameter so the --owner tests can reuse it.

[juan-malbeclabs]

Re-reviewed at f0226ab4. Both inline notes and the Custom(n) point from my review body are resolved, and I re-checked the rebase delta on top of #4120.

Signature check (sdk/rs/src/commands/user/mod.rs:84) — verify(proof, &verifier) now runs after the GlobalState lookup and before the Ed25519 instruction is built, so a proof signed by a rotated key fails locally with a named error instead of the invisible confirmation timeout. The signer feature is enabled on the sdk/rs dependency only; the crate's default build stays borsh + solana-program, so the BPF graph is unaffected. Both proof_for helpers now sign for real, which means the happy-path tests exercise a signature that actually verifies rather than [5u8; 64], and test_commands_user_create_with_ip_proof_rejects_a_rotated_verifier_key covers the rotation case directly.

Version check (mod.rs:49) — is_supported_version runs first, matching the program's order in ip_proof.rs:125, and the epoch window is still correctly left to the program with the doc comment explaining why.

Custom(n) mapping (client.rs:250) — the guard on transaction.message.program_id(index) is the better fix over the comment I suggested. It reads the legacy message the send path actually builds, an out-of-range index falls through to the raw error, and it is strictly more correct for any non-serviceability instruction, not just the precompile.

Transaction-size guard (create_subscribe.rs:154-180) — assembling the instruction list before the measurement is right. The Ed25519 instruction carries ~170 bytes of data plus the sysvar and precompile account keys, which the old [cu_limit, heap_frame, ix] message missed entirely; the group cap is now judged against what goes on the wire. The accesspass_owner parameter on expect_create_lookups is a faithful adaptation — the --owner test seeds the pass under the owner and signs the proof for the owner, which is what create_user_core's effective_owner resolves to on that path.

ip_proof.rs in the instruction crate is byte-identical to what I reviewed pre-rebase; the only other delta is the test-only &[] for the new extra_mgroup_pks parameter.

Two non-blocking notes, neither worth another round:

• match instructions.len() { 1 => ..., _ => ... } in create_subscribe.rs would read more directly as a match on self.ip_proof, since that is what actually decides the shape.
• Every caller still passes ip_proof: None, so RFC-27 is not reachable end-to-end yet. Expected — the issuing side lands separately — just flagging that nothing exercises this outside tests today.

Approving.