feat(utils): fix audit risk

[0xbigapple]

What does this PR do?

1. Override toString() in PrivateKey to return a redacted representation and add a new toStringWithPrivateKey function.
2. Replace assert to Preconditions.checkArgument/Preconditions.checkNotNull

Why are these changes required?

This PR has been tested by:

• Unit Tests
• Manual Testing

Follow up

Extra details

---

Summary by cubic

Redacts SECP256K1.PrivateKey#toString() to prevent key leaks and adds toStringWithPrivateKey() for explicit access. Replaces assert checks with com.google.common.base.Preconditions and adds stricter null/length validation across ECDH, signature decode, key creation, and verify helpers to fix input handling bugs.

^{Written for commit 34bfbe5. Summary will update on new commits.}

Summary by CodeRabbit

• Security

  • Private key data is now automatically redacted in standard string representations to prevent accidental exposure.

• New Features

  • Added toStringWithPrivateKey() method for accessing the actual private key when explicitly needed.

• Improvements

  • Enhanced input validation and null checks across cryptographic operations and key generation for improved robustness.

[coderabbitai]

📝 Walkthrough

Walkthrough

A single-file update to the SECP256K1 cryptographic utility that replaces runtime assertions with Guava Preconditions for input validation across constructors and factory methods. Additionally implements a security-conscious PrivateKey toString() method that redacts sensitive data, with a new optional method to access the actual private key representation.

Changes

Cohort / File(s)	Summary
SECP256K1 Validation and Security Hardening utils/src/main/java/org/tron/trident/crypto/SECP256K1.java	Replaces runtime assertions with Preconditions checks in verify, signature creation/decoding, key recovery, and ECDH key agreement methods. Adds redacted toString() for PrivateKey and new toStringWithPrivateKey() method for full private key access. Updates constructors and factory methods with consistent null/bounds validation using Preconditions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A cryptographic leap of faith we take,
With Preconditions strong for security's sake,
No secrets revealed in a toString display,
Just safe validation the proper way! 🔐✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'feat(utils): fix audit risk' is vague and does not clearly convey the main changes in the changeset, which include replacing assertions with Preconditions checks and adding a security-conscious toString() for PrivateKey.	Consider revising the title to be more specific and descriptive, such as 'feat(utils): replace assertions with Preconditions and redact PrivateKey.toString()' to better communicate the primary changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
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 docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/audit_risk

📝 Coding Plan

• Generate coding plan for human review comments

---

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.}

[0xbigapple]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cubic-dev-ai]

1 issue found across 1 file

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="utils/src/main/java/org/tron/trident/crypto/SECP256K1.java">

<violation number="1" location="utils/src/main/java/org/tron/trident/crypto/SECP256K1.java:381">
P2: `toStringWithPrivateKey()` returns a `0x`-prefixed string that `PrivateKey.create(String)` immediately rejects, so the new API cannot round-trip its own output.</violation>
</file>

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@utils/src/main/java/org/tron/trident/crypto/SECP256K1.java`:
- Around line 328-331: The create(String hexKey) method in SECP256K1 needs a
null check before calling hexKey.length(); update the method to first validate
hexKey is non-null (e.g., Preconditions.checkNotNull(hexKey, "hexKey cannot be
null")) and then assert the length is 64 with the existing
Preconditions.checkArgument, then proceed to call Bytes32.fromHexString(hexKey)
and return create(...); this keeps behavior consistent with other factory
methods and prevents NullPointerException.
- Around line 607-610: In SECP256K1.decode, add a null-check for the parameter
`bytes` before calling `bytes.size()` (e.g., use
Preconditions.checkNotNull(bytes, "encoded SECP256K1 signature must not be
null")) so a clear error is thrown instead of a NullPointerException, then keep
the existing size check against BYTES_REQUIRED and proceed with signature
decoding; this aligns with other methods' null-checking behavior.
- Around line 159-162: Add explicit null checks for the BigInteger parameters r
and s before calling r.signum() and s.signum() in SECP256K1 (so the NPE is
avoided and error messages are clear): insert Preconditions.checkNotNull(r, "r
must not be null") and Preconditions.checkNotNull(s, "s must not be null") ahead
of the existing Preconditions.checkArgument(r.signum() >= 0, ...) and
Preconditions.checkArgument(s.signum() >= 0, ...), keeping the existing recId
and dataHash checks intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 39c6e071-c25f-4f47-a7e6-384ff1081a04

📥 Commits

Reviewing files that changed from the base of the PR and between 45acc6a and 0e35a76.

📒 Files selected for processing (1)

• utils/src/main/java/org/tron/trident/crypto/SECP256K1.java