Skip to content

feat(vm): implement TIP-7883 ModExp gas cost increase#6654

Open
yanghang8612 wants to merge 2 commits intotronprotocol:developfrom
yanghang8612:implement-tip-7883
Open

feat(vm): implement TIP-7883 ModExp gas cost increase#6654
yanghang8612 wants to merge 2 commits intotronprotocol:developfrom
yanghang8612:implement-tip-7883

Conversation

@yanghang8612
Copy link
Copy Markdown
Collaborator

Summary

Implement TIP-7883 ModExp precompile repricing, gated behind the existing allowTvmOsaka flag.

Changes from the current pricing formula:

  • Minimum energy: raised to 500
  • Divisor removed: no longer divides by GQUAD_DIVISOR, tripling general cost
  • Exponent >32 bytes multiplier: doubled from 8 to 16
  • Multiplication complexity: floor of 16 for ≤32 bytes, 2 * ceil(maxLen/8)² for >32 bytes

Related: tronprotocol/tips#837

Comment on lines +739 to +748
BigInteger energy = BigInteger.valueOf(multComplexity)
.multiply(BigInteger.valueOf(iterCount));

BigInteger minEnergy = BigInteger.valueOf(500);
if (isLessThan(energy, minEnergy)) {
return 500L;
}

return isLessThan(energy, BigInteger.valueOf(Long.MAX_VALUE)) ? energy.longValueExact()
: Long.MAX_VALUE;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good defensive coding — using BigInteger for the intermediate multiplication avoids any potential overflow with large baseLen/modLen values (up to 1024 bytes), and the Long.MAX_VALUE cap ensures the result is always safe to return as a long. Well handled.

*/
private long getMultComplexityTIP7883(int baseLen, int modLen) {
long maxLength = max(baseLen, modLen, VMConfig.disableJavaLangMath());
long words = (maxLength + 7) / 8; // ceil(maxLength / 8)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment // ceil(maxLength / 8) is a nice touch — it makes the intent of (maxLength + 7) / 8 immediately clear without needing to mentally decode the arithmetic. The structure also maps 1:1 to the spec's pseudocode, which makes auditing straightforward.

@halibobo1205 halibobo1205 added the topic:vm VM, smart contract label Apr 9, 2026
Copy link
Copy Markdown
Collaborator

@CodeNinjaEvan CodeNinjaEvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if (maxLength <= 32) {
return 16;
}
return 2 * words * words;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Suggestion: use overflow-safe arithmetic for long-term review burden reduction

2 * words * words uses plain long multiplication. Proving it won't overflow requires tracing the full input chain: parseLen()intValueSafe() returns int → max words = 268,435,456 → 2 * words² = 1.44×10¹⁷ < Long.MAX_VALUE. This reasoning is correct today, but not self-evident — if parseLen() return type or UPPER_BOUND ever changes, the safety assumption silently breaks.

Using overflow-safe arithmetic makes the code self-evidently safe with zero external reasoning required, reducing the review burden for future readers:

return Math.multiplyExact(2, Math.multiplyExact(words, words));

This is a long-term maintainability suggestion, not a current correctness issue.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for flagging this. Applied in e512e21 — used StrictMathWrapper.multiplyExact instead of Math.multiplyExact to stay consistent with the cross-platform-determinism convention this repo follows (the Maths javadoc explicitly redirects new code to StrictMathWrapper). Same fail-fast guarantee, and reviewers no longer need to trace parseLen() → int → UPPER_BOUND to convince themselves the multiplication is safe.

* Minimal complexity of 16; doubled complexity for base/modulus > 32 bytes.
*/
private long getMultComplexityTIP7883(int baseLen, int modLen) {
long maxLength = max(baseLen, modLen, VMConfig.disableJavaLangMath());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD]getMultComplexityTIP7883 and getIterationCountTIP7883 are new methods with no legacy constraints. They should use StrictMathWrapper directly instead of the deprecated Maths.max(a, b, boolean) pattern.

Suggested changes:

// getMultComplexityTIP7883
- long maxLength = max(baseLen, modLen, VMConfig.disableJavaLangMath());
+ long maxLength = StrictMathWrapper.max(baseLen, modLen);

// avoid silent overflow on 2 * words * words
 - return 2 * words * words;
 + return StrictMathWrapper.multiplyExact(2L, StrictMathWrapper.multiplyExact(words, words));
 
 // getIterationCountTIP7883
 - return max(iterCount, 1, VMConfig.disableJavaLangMath());
 + return StrictMathWrapper.max(iterCount, 1L);

Reasons:

  1. Maths is @Deprecated — new code should not add more callers.
  2. 2 * words * words relies on manual reasoning about value ranges for overflow safety. Using multiplyExact makes this explicit and fail-fast.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three suggestions applied in e512e21getMultComplexityTIP7883 / getIterationCountTIP7883 now call StrictMathWrapper.max directly, and the 2 * words * words product is wrapped in StrictMathWrapper.multiplyExact so the overflow argument no longer depends on parseLen() returning int. Agreed that new code shouldn't keep adding callers to the deprecated Maths helper.

Raise the floor to 500, switch the >32-byte branch to 2*words²,
and bump the long-exponent multiplier to 16. Drop the OSAKA gate's
config-file plumbing — the on-chain proposal is the only switch.
Verify the new floor, the doubled-formula branch at the 33-byte
boundary, and that pre-OSAKA pricing is preserved when the gate
is off.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:vm VM, smart contract

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants