Skip to content

feat(crypto): shielded transaction API security enhancement#6694

Open
Federico2014 wants to merge 1 commit intotronprotocol:developfrom
Federico2014:fix/shielded-api-security
Open

feat(crypto): shielded transaction API security enhancement#6694
Federico2014 wants to merge 1 commit intotronprotocol:developfrom
Federico2014:fix/shielded-api-security

Conversation

@Federico2014
Copy link
Copy Markdown
Collaborator

What does this PR do?

Enhances the security of shielded transaction APIs by adding overflow protection, tightening default configuration, and cleaning up unused artifacts.

  1. Added integer overflow protection in ZenTransactionBuilder and ShieldedTRC20ParametersBuilder: replaced raw +=/-= on valueBalance with StrictMathWrapper.addExact()/subtractExact(), which throw ArithmeticException on overflow. Overflow validation is performed before mutating builder state (spends/receives lists), ensuring the builder remains consistent if an overflow is detected.
  2. Wrapped createShieldedTransaction, createShieldedTransactionWithoutSpendAuthSig, createShieldedContractParameters, and createShieldedContractParametersWithoutAsk in try/catch ArithmeticException to convert overflow to ZksnarkException("shielded amount overflow")
  3. Changed the default value of allowShieldedTransactionApi from true to false to reduce exposure by default, since some shielded transaction APIs require passing private keys as parameters
  4. Updated config.conf with an explicit warning advising users to only invoke these APIs locally
  5. Removed sprout-verifying.key from the repository root — the file was not referenced by any code
  6. Updated test fixtures to explicitly enable allowShieldedTransactionApi and properly save/restore the flag

Why are these changes required?

  • valueBalance was accumulated via raw +=/-= without overflow checks, allowing crafted inputs to silently wrap around long range and produce invalid balances
  • Shielded transaction APIs accept private keys as parameters — enabling them by default on public-facing nodes risks private key leakage
  • sprout-verifying.key was an unreferenced artifact that added unnecessary repository bloat

⚠️ Breaking Changes:

  • allowShieldedTransactionApi default changed from true to false — nodes that previously relied on the default will find shielded transaction APIs disabled after upgrade. Fix: explicitly set allowShieldedTransactionApi = true in config.conf.

This PR has been tested by:

  • Unit Tests — ShieldedTransferActuatorTest, ArgsTest, ShieldedTRC20BuilderTest, CreateSpendAuthSigServletTest, MerkleContainerTest

Follow up

No consensus logic or on-chain state transitions affected.

Extra details
Close #6616

@github-actions github-actions bot requested a review from 3for April 18, 2026 02:29
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Shielded transaction API security enhancement

2 participants