[Federico2014]

What does this PR do?

Harden shielded proof generation, parameter encoding, note scanning, and initialization:

• Reject both native-accepted identity encodings of spend authorization keys. Revalidate mutable spend/output proof parameters immediately before native calls, check output diversifiers, and validate ZIP32 seed bounds and input/output buffer lengths.
• Check spend-authorization and binding-signature return values before attaching signatures or returning results. Preserve proving-context cleanup on failure, and add Wallet guards for reported nullifier failures.
• Validate description counts and fixed-size fields before encoding mint, transfer, and burn TRC20 trigger inputs, including proofs, commitments, signatures, and ciphertexts. Preserve the existing Wallet checks for external signature counts and burn nullifiers.
• Convert malformed burn-record hex into a ContractValidateException with a fixed client-facing message instead of a generic HTTP internal-server-error message. Preserve 0x prefixes, odd-length zero padding, and existing burn-record length rejection behavior.
• Skip authenticated note plaintexts with unsupported headers or negative values in both decryption paths, allowing native shielded and TRC20 IVK/OVK scans to continue and return valid notes.
• Stop ZK-SNARK initialization with ZCASH_INIT when parameter-file copying fails, attempt to remove the partial file, and avoid calling native initialization with an incomplete file.

Why are these changes required?

Identity spend keys can terminate the node during native proof generation. Ignored signature failures can publish invalid output, malformed ABI fields can shift encoded arguments, and sender-controlled malformed note plaintexts can interrupt an entire scan range. Parameter-copy failures previously allowed initialization to continue with a missing or incomplete file.

The changes reject these failures at the relevant Java boundaries while preserving valid transaction construction and scan results. Nullifier failure propagation still requires the SDK/native follow-up below.

This PR has been tested by:

• Local ./gradlew clean build -x test passed on macOS with the x86_64 / JDK 8 runtime.
• All 22 selected tests passed across WalletShieldedDescriptionValidationTest, UtilProcessErrorTest, and the five NoteEncDecryTest.testGetTriggerInputBurn* cases.
• All 12 tests in ShieldedNativeValidationTest passed. The new case sets byte 30 of an otherwise identity-like encoding for both sign bits; changing the loop bound from i < 31 to i < 30 was verified to fail this test. The correct bound was restored.
• Regression coverage includes signature failures and context cleanup, mocked nullifier failures, malformed-note scanning, parameter-copy failure/retry, ABI field lengths and encoding compatibility, malformed burn hex, and consensus rejection of an invalid binding signature.
• Required framework/plugins Checkstyle, Math usage, and diff whitespace checks passed.
• For head 74cdf6b32b, PR Check, Math usage, and single-node smoke tests passed. The PR Build matrix was still running when this description was updated.

Follow up

• Publish the SDK/JNI/Rust changes, upgrade the dependency, and connect librustzcashSaplingComputeNfChecked. The current Java wrapper still returns true; the mocked Wallet tests verify error branches, not end-to-end native failure propagation.
• Replace ZIP32 native decoding panics with controlled errors and add correctly sized invalid-key regression cases. Java buffer checks do not validate serialized key contents.
• Allocate parameter files atomically with Files.createTempFile, preferably in a node-private temporary directory, while preserving cleanup and error propagation.

Extra details

The changes are confined to shielded API construction, scanning, and initialization across chainbase/framework. Dependencies, consensus verification rules, and public Note.decode behavior are unchanged. ZIP32 validation preserves empty/prefix seeds and unsigned hardened child indices; valid TRC20 ABI encoding is preserved.

[3for]

[NIT / Hardening] Validate the diversifier in librustzcashSaplingOutputProof() before invoking native code.

Both existing builders already reject invalid diversifiers before invoking the output-proof wrapper:

• ZenTransactionBuilder.generateOutputProof() checks cm() and the result of encrypt() before calling the wrapper.
• ShieldedTRC20ParametersBuilder.generateOutputProof() performs the equivalent checks.

The encryption path rejects invalid diversifiers through Note.encrypt() and NoteEncryption.fromDiversifier(), which returns an empty result when librustzcashSaplingKaDerivepublic() rejects the diversifier.

However, JLibrustzcash.librustzcashSaplingOutputProof() forwards parameters directly to native code without independently validating the diversifier.

Calling params.valid() alone would be insufficient, since OutputProofParams.valid() currently checks only that d is non-null and exactly 11 bytes long. The existing librustzcashCheckDiversifier() can provide the required semantic validation.

Consider adding parameter validation and an explicit diversifier check at the wrapper boundary, along with a regression test confirming that an invalid diversifier is rejected before the native output-proof call.

This is a defense-in-depth improvement for future direct callers; the two existing builder paths are already protected.

[3for]

[NIT / SDK & Native Hardening] Handle invalid serialized ZIP32 keys without native panics.

The new params.valid() checks validate buffer lengths but do not validate the serialized extended-key contents:

• librustzcashZip32XskDerive(): Zip32XskDeriveParams.valid() checks the 169-byte buffer lengths, but native decoding of xsk_parent still uses ExtendedSpendingKey::read(...).expect(...).
• librustzcashZip32XfvkAddress(): Zip32XfvkAddressParams.valid() checks buffer lengths, but native decoding of xfvk still uses ExtendedFullViewingKey::read(...).expect(...).

Consequently, correctly sized buffers containing invalid serialized keys can still trigger a native panic during decoding.

I found no production callers of these wrappers in the current java-tron tree, so this can be tracked as a follow-up rather than blocking this PR.

Before accepting untrusted extended keys through these interfaces, please replace the native expect() calls with controlled error handling and propagate failures safely through JNI and the SDK. The derive API currently returns void, so its failure-reporting mechanism also needs to be addressed.

Please add regression tests using correctly sized but invalid serialized XSK/XFVK values, verifying that decoding failures are handled without native panics.

The absence of production callers in java-tron does not establish whether other SDK consumers expose these functions to untrusted inputs.

[Federico2014]

@3for Agreed. The length checks do not validate serialized key contents, so this remains incomplete, as noted in the PR description.

I'll address it in a separate SDK/native follow-up: replace both decoding expect() calls with controlled error handling, propagate failures through JNI and Java—including an explicit failure contract for XSK derivation—and add regression tests using correctly sized but invalid XSK/XFVK inputs. Other SDK consumers also need to be considered, even though these wrappers currently have no production callers in java-tron.

[3for]

[SHOULD] Native nullifier failures still cannot reach this check.

JLibrustzcash.librustzcashComputeNf() calls the SDK and unconditionally returns true. The SDK Java method returns void, and the JNI bridge discards Rust’s boolean return value.

Rust can return false before writing the output buffer. The Java-side ComputeNfParams.valid() checks lengths and non-negative values, but does not cover all native semantic checks. Consequently, createShieldNullifier() can still return the untouched output buffer as a successful result.

The new null check in isSpend() has the same limitation: Note.nullifier() returns null only when that wrapper returns false.

Please preserve an explicit success/failure status through Rust → JNI → SDK Java → JLibrustzcash, or provide an equivalent reliable error contract. Add regression coverage across that boundary: the current failure tests mock JLibrustzcash itself, so they exercise the Wallet branches without detecting the status loss. An all-zero output check should not substitute for the success status.

[Federico2014]

@3for Agreed. The current checks do not complete the fix: JNI discards the native return value, the SDK method returns void, and librustzcashComputeNf() always returns true. The mocked tests only verify the Wallet failure branches.

As noted in the PR description, this remains incomplete until the SDK/JNI/native changes are published and the dependency is upgraded. I'll wire librustzcashSaplingComputeNfChecked through the Java wrapper and add regression coverage exercising a real native failure through JNI and the SDK, verifying that it cannot produce a successful nullifier result. An all-zero output check will not substitute for an explicit failure status.

[3for]

[NIT / Hardening] Stop ZK-SNARK initialization when parameter file copying fails.

In ZksnarkInitService.getParamsFile(), an IOException during parameter file copying is logged, but the method still returns the destination path.

As a result, initialization can proceed with a missing or incomplete parameter file. The native implementation uses expect() when opening and deserializing parameter files, so this can potentially turn a recoverable Java initialization error into a native panic or process abort.

Please stop initialization when copying fails, clean up any partially written file, and return the destination path only after a successful copy. As parameter initialization runs during normal node startup, including when shielded transaction APIs are disabled.

Please also add a regression test verifying that a copy failure prevents the native initialization call.

This can be addressed entirely within java-tron as a non-blocking follow-up.

[Federico2014]

@3for I've implemented both hardening changes:

• librustzcashSaplingOutputProof() now revalidates parameters and checks the diversifier before invoking native output-proof generation. Regression tests verify that invalid inputs never reach that call and valid inputs preserve the native return value.
• getParamsFile() now stops initialization with a ZCASH_INIT error when copying fails and attempts to remove the partial file. Regression tests cover both spend and output parameter-copy failures, verify that native initialization is not called, and confirm that initialization can be retried.

The targeted tests, Checkstyle, Math usage scan, and diff checks passed.

[3for]

[NIT] Could we allocate these parameter files atomically with Files.createTempFile, preferably in a node-private temporary directory, instead of constructing timestamp-based paths? Please preserve the new cleanup and error propagation behavior. The native hash check validates parameter contents, but does not establish ownership of the destination used by the earlier copy operation.

[Federico2014]

Thanks for the suggestion. We will keep this hotfix scoped and address it in a follow-up — switching to Files.createTempFile (or a node-private temp directory) while preserving the cleanup and error propagation added here.

[3for]

[NIT] Decoding the burn record at Wallet.java:4546 happens before the argument-validation wrapper. Malformed hex therefore escapes as a DecoderException and is rendered as an internal-server-error message by the HTTP endpoint. Consider converting this decoding failure into a ContractValidateException with a fixed message and adding a malformed-hex regression test. This behavior predates the PR and need not block it.

[waynercheung]

LGTM at d25a731b5a. Verified on aarch64 / JDK 17 with the real native library:

• Both identity ak encodings and invalid output diversifiers are rejected before native proof generation. Consensus verification is unchanged.
• The new ABI parameter validation rejects malformed field lengths before encoding, while valid calldata remains unchanged. Signature-source selection and the existing Wallet burn validations are preserved.
• Related shielded tests and Checkstyle pass; 29/30 mutants killed (see inline for the surviving mutant).

[SHOULD] Please update the PR description to include the output-proof and ABI validation and current verification results, and link tracking issues for the deferred SDK/JNI and temporary-file hardening work.