feat(crypto): shielded transaction API security enhancement#6694
Open
Federico2014 wants to merge 1 commit intotronprotocol:developfrom
Open
feat(crypto): shielded transaction API security enhancement#6694Federico2014 wants to merge 1 commit intotronprotocol:developfrom
Federico2014 wants to merge 1 commit intotronprotocol:developfrom
Conversation
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
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.
What does this PR do?
Enhances the security of shielded transaction APIs by adding overflow protection, tightening default configuration, and cleaning up unused artifacts.
ZenTransactionBuilderandShieldedTRC20ParametersBuilder: replaced raw+=/-=onvalueBalancewithStrictMathWrapper.addExact()/subtractExact(), which throwArithmeticExceptionon overflow. Overflow validation is performed before mutating builder state (spends/receiveslists), ensuring the builder remains consistent if an overflow is detected.createShieldedTransaction,createShieldedTransactionWithoutSpendAuthSig,createShieldedContractParameters, andcreateShieldedContractParametersWithoutAskintry/catch ArithmeticExceptionto convert overflow toZksnarkException("shielded amount overflow")allowShieldedTransactionApifromtruetofalseto reduce exposure by default, since some shielded transaction APIs require passing private keys as parametersconfig.confwith an explicit warning advising users to only invoke these APIs locallysprout-verifying.keyfrom the repository root — the file was not referenced by any codeallowShieldedTransactionApiand properly save/restore the flagWhy are these changes required?
valueBalancewas accumulated via raw+=/-=without overflow checks, allowing crafted inputs to silently wrap aroundlongrange and produce invalid balancessprout-verifying.keywas an unreferenced artifact that added unnecessary repository bloatallowShieldedTransactionApidefault changed fromtruetofalse— nodes that previously relied on the default will find shielded transaction APIs disabled after upgrade. Fix: explicitly setallowShieldedTransactionApi = trueinconfig.conf.This PR has been tested by:
ShieldedTransferActuatorTest,ArgsTest,ShieldedTRC20BuilderTest,CreateSpendAuthSigServletTest,MerkleContainerTestFollow up
No consensus logic or on-chain state transitions affected.
Extra details
Close #6616