Skip to content

fix(chainbase): guard divide-by-zero #6687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Little-Peony wants to merge 8 commits into from
Closed

fix(chainbase): guard divide-by-zero #6687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1f3931f
fix(api): add whitelist rate limiter class to prevent arbitrary class…
Little-Peony Apr 16, 2026
0498357
fix(chainbase): make Snapshot head field volatile in Chainbase
Little-Peony Apr 16, 2026
58798e9
revert(http): remove rate limiter whitelist change
Little-Peony Apr 16, 2026
96347b2
revert(http): restore original RateLimiterServlet implementation
Little-Peony Apr 16, 2026
671fc07
ci: retrigger CI after mirror sync failure
Little-Peony Apr 16, 2026
56fcea7
revert(chainbase): remove volatile from Snapshot head field
Little-Peony Apr 20, 2026
f7c7252
fix(chainbase): fix comment wording and trailing whitespace in Transa…
Little-Peony Apr 20, 2026
42a3345
test(chainbase): add regression tests for newSize==0 in resetAccountU…
Little-Peony Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions chainbase/src/main/java/org/tron/core/db/TransactionTrace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,14 @@ private void resetAccountUsage(AccountCapsule accountCap,
}
long currentSize = accountCap.getWindowSize(ENERGY);
long currentUsage = accountCap.getEnergyUsage();
// Drop the pre consumed frozen energy
// Drop the previously consumed frozen energy
long newArea = currentUsage * currentSize

@waynercheung waynercheung Apr 17, 2026

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.

[QUESTION] newArea still relies on raw long multiplications/subtractions here. I realize this is pre-existing code rather than introduced by this PR, but since this method is already being touched for numeric safety, do we want a follow-up to harden these arithmetic operations with the existing exact-helper pattern as well?

@Sunny6889 Sunny6889 Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good observation. While there's a theoretical overflow risk in newArea = currentUsage * currentSize - (mergedUsage * mergedSize - usage * size), in practice this won't occur in java-tron's business context — both currentUsage (energy usage) and currentSize (window size, capped at WINDOW_SIZE_MS / BLOCK_PRODUCED_INTERVAL = 28800) are tightly bounded by the protocol's energy limits, so the product stays well within long range. But I agree it's worth noting as a pre-existing pattern; happy to track it in a
follow-up if you'd like.

- (mergedUsage * mergedSize - usage * size);
// If area merging happened during suicide, use the current window size
long newSize = mergedSize == currentSize ? size : currentSize;
// Calc new usage by fixed x-axes
long newUsage = max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath());
// A zero window size means no valid time window exists and thus zero usage

@bladehan1 bladehan1 Apr 20, 2026

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.

[NIT] New comment removes the algorithm context from 'fixed x-axes'

The original comment // Calc new usage by fixed x-axes explained the area-based energy accounting model (usage × window = area; new_usage = area / new_window). The replacement comment only explains the zero-guard case, so readers of the non-zero path lose the rationale for newArea / newSize.

The same applies to the equivalent line in resetAccountUsageV2 (line ~320).

Suggestion: Merge both meanings, e.g.

// Calculate new usage from area/window model; zero window means no valid time context and yields zero usage

long newUsage =
newSize == 0 ? 0 : max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath());
// Reset account usage and window size
accountCap.setEnergyUsage(newUsage);
accountCap.setNewWindowSize(ENERGY, newUsage == 0 ? 0L : newSize);
Expand All @@ -312,13 +313,14 @@ private void resetAccountUsageV2(AccountCapsule accountCap,
long currentSize = accountCap.getWindowSize(ENERGY);
long currentSize2 = accountCap.getWindowSizeV2(ENERGY);
long currentUsage = accountCap.getEnergyUsage();
// Drop the pre consumed frozen energy
// Drop the previously consumed frozen energy
long newArea = currentUsage * currentSize - (mergedUsage * mergedSize - usage * size);
// If area merging happened during suicide, use the current window size
long newSize = mergedSize == currentSize ? size : currentSize;
long newSize2 = mergedSize == currentSize ? size2 : currentSize2;
// Calc new usage by fixed x-axes
long newUsage = max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath());
// A zero window size means no valid time window exists and thus zero usage
long newUsage =
newSize == 0 ? 0 : max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath());
// Reset account usage and window size
accountCap.setEnergyUsage(newUsage);
accountCap.setNewWindowSizeV2(ENERGY, newUsage == 0 ? 0L : newSize2);
Expand Down
90 changes: 90 additions & 0 deletions framework/src/test/java/org/tron/core/db/TransactionTraceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.protobuf.Any;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import java.lang.reflect.Method;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -300,6 +301,95 @@ public void testTriggerUseUsage()

}

/**
* Regression test for the {@code newSize == 0} guard in {@code resetAccountUsage}.
*
* <p>When {@code mergedSize == currentSize} and {@code size == 0}, {@code newSize} becomes 0.
* Without the guard this would cause an {@link ArithmeticException} (divide by zero).
* The expected outcome is {@code energyUsage = 0} with no exception thrown.
*/
@Test
public void testResetAccountUsageZeroWindowSize() throws Exception {
TriggerSmartContract contract = TriggerSmartContract.newBuilder()
.setContractAddress(contractAddress)
.setOwnerAddress(ownerAddress)
.build();
Transaction transaction = Transaction.newBuilder()
.setRawData(raw.newBuilder()
.addContract(Contract.newBuilder()
.setParameter(Any.pack(contract))
.setType(ContractType.TriggerSmartContract))
.build())
.build();
TransactionTrace trace = new TransactionTrace(
new TransactionCapsule(transaction), StoreFactory.getInstance(), new RuntimeImpl());

AccountCapsule accountCap = new AccountCapsule(Account.newBuilder()
.setAddress(ownerAddress)
.setAccountResource(AccountResource.newBuilder()
.setEnergyUsage(1000L)
.build())
.build());

// A fresh account has EnergyWindowSize=0, so getWindowSize(ENERGY) returns the default
// WINDOW_SIZE_MS / BLOCK_PRODUCED_INTERVAL = 28800.
// Passing mergedSize == currentSize and size == 0 triggers newSize = 0.
final long currentSize = 28800L;

dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(0);
Method method = TransactionTrace.class.getDeclaredMethod(
"resetAccountUsage",
AccountCapsule.class, long.class, long.class, long.class, long.class, long.class);
method.setAccessible(true);
method.invoke(trace, accountCap, 500L, 0L, 500L, currentSize, 0L);

// newSize == 0 → guard returns 0 instead of dividing → energyUsage reset to 0
Assert.assertEquals(0L, accountCap.getEnergyUsage());
}

/**
* Same regression as above but exercises the {@code resetAccountUsageV2} code path,
* which is activated when {@code supportAllowCancelAllUnfreezeV2} is enabled.
*/
@Test
public void testResetAccountUsageV2ZeroWindowSize() throws Exception {
TriggerSmartContract contract = TriggerSmartContract.newBuilder()
.setContractAddress(contractAddress)
.setOwnerAddress(ownerAddress)
.build();
Transaction transaction = Transaction.newBuilder()
.setRawData(raw.newBuilder()
.addContract(Contract.newBuilder()
.setParameter(Any.pack(contract))
.setType(ContractType.TriggerSmartContract))
.build())
.build();
TransactionTrace trace = new TransactionTrace(
new TransactionCapsule(transaction), StoreFactory.getInstance(), new RuntimeImpl());

AccountCapsule accountCap = new AccountCapsule(Account.newBuilder()
.setAddress(ownerAddress)
.setAccountResource(AccountResource.newBuilder()
.setEnergyUsage(1000L)
.build())
.build());

final long currentSize = 28800L;

dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(1);
try {
Method method = TransactionTrace.class.getDeclaredMethod(
"resetAccountUsage",
AccountCapsule.class, long.class, long.class, long.class, long.class, long.class);
method.setAccessible(true);
method.invoke(trace, accountCap, 500L, 0L, 500L, currentSize, 0L);

Assert.assertEquals(0L, accountCap.getEnergyUsage());
} finally {
dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(0);
}
}

private byte[] deployInit(Transaction transaction)
throws VMIllegalException, ContractExeException,
ContractValidateException, BalanceInsufficientException {
Expand Down
Loading