Skip to content

chore: add safety guard for negative cycle index #6527

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

Open
operagxsasha wants to merge 1 commit into
base: develop
Choose a base branch
from
+4 −0
Open

chore: add safety guard for negative cycle index #6527

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions chainbase/src/main/java/org/tron/core/service/MortgageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ private long computeReward(long beginCycle, long endCycle, AccountCapsule accoun
}
if (beginCycle < endCycle) {
for (Pair<byte[], Long> vote : srAddresses) {
if (beginCycle == 0) {
Copy link
Contributor

@bladehan1 bladehan1 Jan 26, 2026

Choose a reason for hiding this comment

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

Image beginCycle will be start with 1

Copy link
Contributor Author

@operagxsasha operagxsasha Jan 27, 2026

Choose a reason for hiding this comment

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

Correct, this prevents beginCycle from being 0

Copy link
Contributor

@vividctrlalt vividctrlalt Feb 28, 2026

Choose a reason for hiding this comment

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

I agree with @bladehan1's observation. After tracing the call sites of computeReward(beginCycle, endCycle, accountCapsule):

  1. In withdrawReward() (line 94): beginCycle = delegationStore.getBeginCycle(address), and the delegation cycle starts from 1.
  2. Before reaching line 215, beginCycle has already been validated (beginCycle > currentCycle returns early at line 98-100), and may have been incremented (beginCycle += 1 at line 116).
  3. In queryReward() (line 142): same pattern — beginCycle is loaded from store and validated before use.

So beginCycle == 0 cannot occur in normal execution flow. Additionally, even as a defensive check, using continue inside the vote loop is incorrect — it skips individual votes rather than handling the invalid cycle at the method level. A proper guard would be placed before the for-loop, e.g., if (beginCycle <= 0) return 0;.

Also noting that the indentation of the added code is inconsistent with the surrounding code style.

continue;
}

byte[] srAddress = vote.getKey();
BigInteger beginVi = delegationStore.getWitnessVi(beginCycle - 1, srAddress);
BigInteger endVi = delegationStore.getWitnessVi(endCycle - 1, srAddress);
Expand Down
Loading