fix(security): bind burn cipher nonce to nullifier

[Federico2014]

What does this PR do?

Hardens shielded TRC-20 burn encryption by binding the burn AEAD nonce to the spend nullifier, storing the nonce in the burn record, and making the burn-record version explicit in the trailing reserved bytes.

Why are these changes required?

The original implementation encrypted every burn payload under the same long-lived ovk with a static all-zero ChaCha20-Poly1305 nonce. Reusing (key, nonce) pairs breaks the AEAD security assumptions and leaks keystream relationships across burns.

This PR gives each burn a nonce derived from its nullifier and records enough metadata on chain for scanners to distinguish legacy and upgraded burn records without relying on ambiguous zero-nonce heuristics.

Design

Nullifier-bound nonce with domain separation

nonce = SHA3("Ztron_BurnNonce" || nf)[:12]

The fixed domain tag prevents collisions with any other SHA3-of-nullifier use. Since the nullifier is unique per spend, the resulting (ovk, nonce) pair is also unique per burn.

96-byte burn record

encryptBurnMessageByOvk returns a 96-byte record that occupies the same calldata / log slot as the previous cipher(80) || zero-padding(16) layout:

record (96B) = cipher(80) || nonce(12) || reserved/version(4)

Current version marker:

v2 marker = 0x00000001

Explicit legacy/v2 discrimination for scanners

decryptBurnMessageByOvk(ovk, cipher, nonceFromLog, reservedFromLog, nf) now treats the trailing 4 reserved bytes as an explicit version marker:

• reserved = 0x00000000 and nonce = 0x000...000 -> legacy v1 path
• reserved = 0x00000001 -> v2 path; the scanner must verify nonce == SHA3(domain || nf)[:12]
• any other reserved value -> reject

This removes the previous ambiguity where an all-zero nonce could be interpreted both as a valid new record and as the legacy sentinel.

Encryption moved into the builder

BURN encryption now runs inside ShieldedTRC20ParametersBuilder.build() after generateSpendProof(...), once the nullifier is available. Callers only need to provide ovk; if omitted, the builder falls back to spend.expsk.getOvk().

Compatibility

Dimension	Impact
Consensus / hard fork	None — wallet / scanner / trigger-input only, no protocol-state transition
On-chain burn log layout	Still 160B total (toAddress 32 + value 32 + burn record 96)
New burn record semantics	cipher(80) + nonce(12) + reserved/version(4) replaces cipher(80) + zero padding(16) for newly generated burns
Legacy burns already on chain	Still scannable via the explicit legacy branch (reserved=0 && nonce=0)
getTriggerInputForShieldedTRC20Contract	Breaking change for legacy callers: 80-byte burn ciphers are now deprecated and rejected; callers must send the new 96-byte burn record
gRPC / HTTP / JSON-RPC shape	Field names are unchanged, but acceptable burn payload semantics are narrower because legacy 80-byte burn ciphers are rejected
Config / DB schema	None
Third-party SDKs	SDKs that still build or submit 80-byte burn ciphers must be updated to emit the 96-byte burn record

Key changes

File	Change
NoteEncryption.java	Adds BURN_CIPHER_RECORD_SIZE=96, explicit reserved/version marker handling, nonce derivation from nullifier, and strict v1/v2 burn-record decryption rules
ShieldedTRC20ParametersBuilder.java	Moves BURN encryption into the builder after spend proof generation so the nullifier is available; emits the 96-byte burn record
Wallet.java	Parses burn logs as cipher(80)+nonce(12)+reserved/version(4); scanner now uses explicit version markers; 80-byte trigger-input burn ciphers throw a clear deprecation/rejection error
BurnCipherTest.java	Covers 96-byte record structure, reserved/version marker behavior, v2 nf-required decryption, unknown marker rejection, and malformed input handling
NoteEncDecryTest.java	Covers legacy zero-nonce compatibility, 96-byte trigger-input acceptance, 80-byte trigger-input rejection, burn-log round trip, burn-log truncation, missing-nf rejection, and same-tx cursor pairing

Testing

• ./gradlew framework:test --tests "org.tron.core.zen.note.BurnCipherTest"
• ./gradlew framework:test --tests "org.tron.core.zksnark.NoteEncDecryTest"
• ./gradlew framework:test --tests org.tron.core.zksnark.NoteEncDecryTest.testGetTriggerInputBurn80ByteCipherRejected

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7d2bb7a4-c8b2-4af6-afeb-6bd8db0555e6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Implements v2 burn-message encryption for shielded TRC-20 using a nonce derived from the note nullifier (nf), updates encrypt/decrypt signatures to accept nf and a nonce-from-log, integrates OVK into the parameters builder, propagates pending nullifiers during Wallet log scanning, and adds comprehensive unit tests.

Changes

Shielded TRC-20 V2 Burn-Message Implementation

Layer / File(s)	Summary
Data Contracts & Constants framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java, framework/src/main/java/org/tron/core/Wallet.java	Adds BURN_CIPHER_LEN, BURN_NONCE_LEN, BURN_CIPHER_RECORD_SIZE; introduces SHIELDED_TRC20_LOG_TOPICS_NOTE_SPENT.
Core Encryption & Decryption framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java	Adds v2 encryptBurnMessageByOvk requiring nf and producing a 96-byte record (cipher+nonce+reserved); replaces decryptBurnMessageByOvk to accept nonceFromLog and nf with zero-nonce legacy fallback; introduces deriveNonceFromNf and isAllZero.
Builder Integration framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java	Adds optional ovk field/setter; BURN build derives nf and calls encryptBurnMessageByOvk(..., nf), storing the returned record in burnCiphertext; serialization now embeds the actual record and adjusts padding.
Wallet Integration & Orchestration framework/src/main/java/org/tron/core/Wallet.java	Sets OVK on builder; classifies NoteSpent as logType = 5; scan flow captures pendingNf from NoteSpent logs and passes it into subsequent burn-log decoding (logType = 4), validates decrypted address-prefix, clears pendingNf, and enforces record-length validation.
Test Coverage framework/src/test/java/org/tron/core/zen/note/BurnCipherTest.java, framework/src/test/java/org/tron/core/zksnark/NoteEncDecryTest.java	Adds tests validating burn-cipher constants/formatting, encrypt→decrypt round-trips, nonce determinism and per-nf uniqueness, rejection on wrong nf or tampered nonce, v1 zero-nonce compatibility, record formatting expectations, input validation, and trigger/log handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A rabbit hops through cipher's dance,
With nullifiers seeding nonce by chance,
V2 records tuck the nonce inside,
Old zero-nonce paths still glide—
Burn messages now stride in stride.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'fix(security): bind burn cipher nonce to nullifier' directly and clearly describes the main security improvement—binding the AEAD nonce to the nullifier. This is the primary change throughout the codebase.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/burn-nonce-nf-binding

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java">

<violation number="1" location="framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java:372">
P2: Treating `null` as all-zero silently falls back to insecure v1 decryption. If a caller fails to extract the nonce from the log and passes `null`, the nonce verification is bypassed entirely — undermining the replay protection this fix adds. Consider returning `false` for null (or throwing) so that a missing nonce is never silently accepted as a v1 record.</violation>
</file>

<file name="framework/src/test/java/org/tron/core/zen/note/BurnCipherV2Test.java">

<violation number="1" location="framework/src/test/java/org/tron/core/zen/note/BurnCipherV2Test.java:86">
P2: This test should assert rejection for the bad address prefix. As written, it asserts decryption succeeds and only checks the raw byte value, so it would still pass if invalid prefixes were accepted.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java`:
- Around line 298-310: Validate transparentToAddress before copying in the OVK
burn-encrypt path: inside the block guarded by ovk != null (the code that
creates addr21 and calls Encryption.encryptBurnMessageByOvk), check that
transparentToAddress is non-null and has length >= 20 (same guard as
ArrayUtils.isEmpty(transparentToAddress) used elsewhere); if it fails, throw a
clear ZksnarkException (or skip encryption explicitly) so a meaningful error is
raised instead of letting System.arraycopy throw
NullPointerException/ArrayIndexOutOfBoundsException. Ensure the check is placed
before creating addr21 or calling Encryption.encryptBurnMessageByOvk so the
error context remains tied to build()/ovk handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29c7f520-6cb7-4e57-a0ea-02eeddd7b8ae

📥 Commits

Reviewing files that changed from the base of the PR and between 62d1943 and 9b2c9ae.

📒 Files selected for processing (4)

• framework/src/main/java/org/tron/core/Wallet.java
• framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java
• framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java
• framework/src/test/java/org/tron/core/zen/note/BurnCipherTest.java

🚧 Files skipped from review as they are similar to previous changes (1)

• framework/src/main/java/org/tron/core/Wallet.java