Skip to content

fix(oracle): handle ValAddressFromBech32 error in EndBlocker#241

Merged
Thaleszh merged 2 commits into
KiiChain:mainfrom
ngapaxs:fix/oracle-valaddress-handling
Jan 20, 2026
Merged

fix(oracle): handle ValAddressFromBech32 error in EndBlocker#241
Thaleszh merged 2 commits into
KiiChain:mainfrom
ngapaxs:fix/oracle-valaddress-handling

Conversation

@ngapaxs

@ngapaxs ngapaxs commented Jan 19, 2026

Copy link
Copy Markdown
Contributor

Severity: LOW
File: x/oracle/abci.go
Line: 56
Repository: https://github.com/KiiChain/kiichain
Version: v6.1.0
Commit: 79bdc96
Issue: #234
PR: #240
Auditor: ngapaxs
Date: 2025-01-19

Summary

Found an ignored error in the oracle module's EndBlocker. When parsing validator addresses fails, the code just ignores it with _, which means that validator gets skipped silently.

Affected Code

if validator.IsBonded() {
    valPower := validator.GetConsensusPower(powerReduction)
    operator := validator.GetOperator()
    operatorAddr, _ := sdk.ValAddressFromBech32(operator)  // error ignored here
    claim := types.NewClaim(valPower, 0, 0, false, operatorAddr)
    validatorClaimMap[operator] = claim
}

Root Cause

The error from ValAddressFromBech32 gets ignored with _. If parsing fails, operatorAddr ends up nil and the claim gets an invalid recipient.

Impact

  • Validators that fail to parse just get skipped
  • Their delegation power doesn't count in the vote
  • Voting power ends up wrong (missing some validators)
  • Runs every vote period so this could happen often

Recommended Fix

operatorAddr, err := sdk.ValAddressFromBech32(operator)
if err != nil {
    ctx.Logger().Error("failed to parse operator address", "operator", operator, "error", err)
    continue
}

Just log the error and skip that validator. Don't return error because that could let malicious validators disable the whole module.

Reference

https://github.com/KiiChain/kiichain/blob/v6.1.0/x/oracle/abci.go#L56

@ngapaxs ngapaxs requested a review from jhelison as a code owner January 19, 2026 21:57
@coderabbitai

coderabbitai Bot commented Jan 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

This change updates the EndBlocker in x/oracle/abci.go to handle errors from sdk.ValAddressFromBech32 when parsing validator operator addresses. On parse error, the code now logs the operator and error and skips that validator; on success, it uses the parsed operator address to create and map the claim. A changelog entry describing this fix was also added.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: handling a ValAddressFromBech32 error in the oracle module's EndBlocker function.
Description check ✅ Passed The description is well-related to the changeset, providing context about the ignored error, its impact on validator address parsing, and the implemented fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed Jan 19, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents 
In `@x/oracle/abci.go`:
- Line 63: The syntax error is caused by two statements on one line; split the
call to types.NewClaim and the assignment to validatorClaimMap into separate
statements: create the claim with types.NewClaim(...) and then on the next line
assign it with validatorClaimMap[operator] = claim (referencing types.NewClaim,
validatorClaimMap and operator to locate the code).
🧹 Nitpick comments (1)
x/oracle/abci.go (1)

55-62: Proper error handling added for address parsing.

The error handling approach is correct: logging the failure with structured context (operator and error) and skipping the invalid validator prevents nil addresses from propagating through the claim system. This is preferable to halting the EndBlocker entirely.

Minor nit: Line 56 duplicates the comment from line 55 ("Parse the operator address").

🔧 Remove duplicate comment 
                 operator := validator.GetOperator()                     // Get address to receive coins

-                // Parse the operator address
                 // Parse the operator address
                 operatorAddr, err := sdk.ValAddressFromBech32(operator)

Comment thread x/oracle/abci.go Outdated
@ngapaxs
fix(oracle): handle ValAddressFromBech32 error in EndBlocker
1df0dcb 
- Add proper error handling for validator address parsing
- Log error and skip invalid validators instead of using nil operatorAddr
- Update CHANGELOG.md with fix entry
- Fix import formatting with gci

Fixes KiiChain#234
@ngapaxs
ci: retrigger checks
6373622
@ngapaxs ngapaxs force-pushed the fix/oracle-valaddress-handling branch from 3add895 to 6373622 Compare January 19, 2026 23:44
@codecov

codecov Bot commented Jan 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
x/oracle/abci.go 25.00% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Thaleszh Thaleszh merged commit 2bff014 into KiiChain:main Jan 20, 2026
8 of 9 checks passed
@jhelison jhelison linked an issue Jan 21, 2026 that may be closed by this pull request
[BUG] Ignored error parsing validator address & Error swallowed in RemoveExcessFeeds #234
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@jhelison jhelison Awaiting requested review from jhelison jhelison is a code owner

+1 more reviewer

@Thaleszh Thaleszh Thaleszh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Ignored error parsing validator address & Error swallowed in RemoveExcessFeeds

2 participants

@ngapaxs @Thaleszh