Skip to content

fix(rewards): ValidateGenesis checks denom consistency for inactive schedules #356

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
amathxbt wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(rewards): ValidateGenesis checks denom consistency for inactive schedules #356

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
25 changes: 14 additions & 11 deletions x/rewards/types/release_schedule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@ func InitialReleaseSchedule() ReleaseSchedule {

// ValidateGenesis validates the release schedule for a genesis state
func (rr ReleaseSchedule) ValidateGenesis() error {
// Validate denom consistency between TotalAmount and ReleasedAmount regardless
// of active state. An inactive schedule with mismatched denoms can corrupt
// state the moment it is activated.
if !rr.TotalAmount.IsZero() && !rr.ReleasedAmount.IsZero() {

@coderabbitai coderabbitai Bot Jul 6, 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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Denom-mismatch check is skipped entirely when TotalAmount is zero.

The gate !rr.TotalAmount.IsZero() && !rr.ReleasedAmount.IsZero() means a schedule with a zero TotalAmount but a non-zero ReleasedAmount (mismatched denom or exceeding total) bypasses this validation entirely, since the && requires both sides non-zero. Given the PR's stated goal — catching mismatched denoms on inactive schedules before governance activation — this is a residual gap: an inactive schedule could have TotalAmount = 0uatom and ReleasedAmount = 100akii, which would still pass genesis validation.

Consider validating denom whenever ReleasedAmount is non-zero (regardless of TotalAmount's zero-ness), since a non-zero released amount with no matching total is inherently invalid.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/rewards/types/release_schedule.go` at line 26, The denom-mismatch
validation in the release schedule check is being skipped whenever TotalAmount
is zero, so a non-zero ReleasedAmount can still bypass the inactive-schedule
validation. Update the conditional in the release schedule logic to validate
denom whenever ReleasedAmount is non-zero, regardless of TotalAmount, and keep
the check localized to the existing release schedule validation path in the
relevant release_schedule helper.

if rr.ReleasedAmount.Denom != rr.TotalAmount.Denom {
return fmt.Errorf("released amount denom %s doesn't match total amount denom %s",
rr.ReleasedAmount.Denom, rr.TotalAmount.Denom)
}
if rr.ReleasedAmount.Amount.GT(rr.TotalAmount.Amount) {
return fmt.Errorf("released amount %s cannot be greater than total amount %s",
rr.ReleasedAmount.String(), rr.TotalAmount.String())
}
}

Comment on lines +23 to +36

@coderabbitai coderabbitai Bot Jul 6, 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.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== go.mod excerpt =="
sed -n '1,220p' go.mod | sed -n '/github.com\/cosmos\/cosmos-sdk/p;/github.com\/cosmos\/cosmossdk.io/p'

echo
echo "== release_schedule.go =="
cat -n x/rewards/types/release_schedule.go | sed -n '1,220p'

echo
echo "== release_schedule_test.go mentions =="
rg -n 'invalid total amount|active with zero end time|ReleasedAmount: sdk\.Coin\{\}|ValidateGenesis|IsZero\(' x/rewards/types/release_schedule_test.go x/rewards/types -n -A3 -B3

echo
echo "== module cache sdk coin/int source candidates =="
go env GOMODCACHE 2>/dev/null || true
fd -a 'coin.go|int.go' "$(go env GOMODCACHE 2>/dev/null)/github.com" 2>/dev/null | rg 'cosmos-sdk/.*/math/(coin\.go|int\.go)$|cosmossdk\.io/.*/math/(coin\.go|int\.go)$' || true

Repository: KiiChain/kiichain

Length of output: 12972

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Resolve the exact SDK version used by the module and inspect the source files if present.
SDK_VER=$(awk '/github.com\/cosmos\/cosmos-sdk/ {print $2; exit} /cosmossdk\.io\/core/ {print $2; exit}' go.mod || true)
echo "SDK version token: ${SDK_VER:-<none>}"

GOMODCACHE=$(go env GOMODCACHE)
echo "GOMODCACHE=$GOMODCACHE"

echo
echo "== find cosmos-sdk math sources =="
find "$GOMODCACHE" -path '*github.com/cosmos/cosmos-sdk*' -path '*/math/int.go' -o -path '*/types/coin.go' 2>/dev/null | sed -n '1,40p'

echo
echo "== inspect any matching int.go / coin.go files =="
for f in $(find "$GOMODCACHE" -path '*github.com/cosmos/cosmos-sdk*' \( -path '*/math/int.go' -o -path '*/types/coin.go' \) 2>/dev/null | sed -n '1,4p'); do
  echo "--- $f ---"
  cat -n "$f" | sed -n '1,220p' | rg -n 'IsZero|Sign|nil|type Int|type Coin|Amount'
done

Repository: KiiChain/kiichain

Length of output: 255

🌐 Web query:

github.com/cosmos/cosmos-sdk v0.53.6 math.Int IsZero Sign nil pointer Coin.IsZero source

💡 Result:

In Cosmos SDK v0.53.6, a math.Int with a nil internal big.Int can cause a nil pointer panic when methods such as IsZero, IsNegative, or others are called, because these methods attempt to dereference the internal value without checking if it is nil [1]. This issue arises primarily when an unmarshaling process (such as decoding malformed protobuf data) creates a math.Int where the internal big.Int is not properly initialized [1]. While standard SDK operations typically ensure that Coin amounts are initialized, malformed or crafted inputs that bypass these checks can result in a Coin struct with a nil Amount. Key technical details: 1. Panic Mechanism: Methods like IsZero and IsNegative dereference the internal big.Int [1]. If that pointer is nil, the Go runtime triggers a panic [1]. 2. Coin.IsZero: The implementation of Coin.IsZero in v0.53.6 calls Amount.IsZero directly [2]. If Amount is nil, this leads to the aforementioned panic. 3. Mitigation: The Cosmos SDK has introduced several guards over time, such as adding Coin.IsNil checks [2][3] and implementing validation logic in Coin.Validate to ensure the Amount is not nil before it is processed [3][4]. 4. Recent Fixes: Ongoing efforts, such as PR #26536, aim to prevent nil internal pointers during unmarshaling by ensuring empty inputs are initialized to a zero-valued big.Int rather than a nil pointer [1]. Developers should use the IsNil method provided on the Coin struct to safely verify if an amount is initialized before performing operations [2][3].

Citations:

🌐 Web query:

github.com/cosmos/cosmos-sdk v0.53.6 sdk.Coin IsNil method source

💡 Result:

In the Cosmos SDK version v0.53.6, the IsNil method for the sdk.Coin type is defined in the types/coin.go file [1]. The method implementation is as follows: func (coin Coin) IsNil bool { return coin.Amount.BigInt == nil } This method returns true if the coin amount is nil, and false otherwise [1][2]. It functions by checking if the underlying BigInt representation of the Coin's amount is nil [1].

Citations:

Guard IsZero() behind IsNil() checks — sdk.Coin{} can make IsZero() panic in v0.53.6, so this block needs to skip uninitialized coins first. Use rr.TotalAmount.IsNil() / rr.ReleasedAmount.IsNil() here; the invalid total amount and active with zero end time cases hit this path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/rewards/types/release_schedule.go` around lines 23 - 36, The validation in
the release schedule checks currently calls IsZero on TotalAmount and
ReleasedAmount without first guarding against nil coins, which can panic in this
SDK version. Update the validation in the release schedule type to check
rr.TotalAmount.IsNil() and rr.ReleasedAmount.IsNil() before any IsZero or
denom/amount comparisons, and skip or handle uninitialized coins accordingly in
this block so the invalid total amount and active with zero end time paths
remain safe.

// Some validations just make sense if active
if rr.Active {
// Validate TotalAmount
Expand All @@ -37,17 +51,6 @@ func (rr ReleaseSchedule) ValidateGenesis() error {
if err := rr.ReleasedAmount.Validate(); err != nil {
return fmt.Errorf("invalid released amount: %w", err)
}

// Check ReleasedAmount doesn't exceed TotalAmount
if rr.ReleasedAmount.Denom != rr.TotalAmount.Denom {
return fmt.Errorf("released amount denom %s doesn't match total amount denom %s",
rr.ReleasedAmount.Denom, rr.TotalAmount.Denom)
}

if rr.ReleasedAmount.Amount.GT(rr.TotalAmount.Amount) {
return fmt.Errorf("released amount %s cannot be greater than total amount %s",
rr.ReleasedAmount.String(), rr.TotalAmount.String())
}
}
}
return nil
Expand Down
Loading