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

Closed
operagxsasha wants to merge 6 commits into from
Closed

chore: add safety guard for negative cycle index #6527

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b733257
chore: add safety guard for negative cycle index
operagxsasha Jan 26, 2026
c9e2299
chore: move guard to method level
operagxsasha Mar 2, 2026
3e334c7
chore: fix return type for beginCycle guard
operagxsasha Mar 2, 2026
dad308a
style: fix checkstyle LineLength in ShieldedReceiveTest
operagxsasha Mar 2, 2026
5b426c6
Merge branch 'develop' into patch-3
operagxsasha Mar 2, 2026
8077531
chore: remove redundant beginCycle guard
operagxsasha Mar 2, 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
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
Copy Markdown
Collaborator

@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
Copy Markdown
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
Copy Markdown
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;
Copy link
Copy Markdown
Contributor

@warku123 warku123 Mar 2, 2026

Choose a reason for hiding this comment

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

BTW, the indentation looks off this line. Please fix to match project style (4 spaces).

}

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