fix: Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #219

Winnode · 2026-01-07T20:52:10Z

[BUG] Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure

Bug Description

Found a panic in the oracle module that can cause chain halt. The pickReferenceDenom function is called from EndBlocker, and contains an unhandled panic(err).

Location: x/oracle/tally.go

params, err := k.Params.Get(ctx)
if err != nil {
    panic(err)
}

The pickReferenceDenom function is called from EndBlocker in x/oracle/abci.go:

referenceDenom, belowThresholdVoteMap := pickReferenceDenom(ctx, k, voteTargets, voteMap)

This is dangerous - panic in a function called from an ABCI method (EndBlocker) will crash all validators on the same block.

Attack Flow

EndBlocker called every block
    → pickReferenceDenom(ctx, k, voteTargets, voteMap)
        → k.Params.Get(ctx) returns error
        → panic(err)
        → ALL VALIDATORS CRASH
        → CHAIN HALT

Conditions that cause Params.Get() to error:

State corruption
Storage issues
Migration failure

Environment

File: x/oracle/tally.go line 28
Function: pickReferenceDenom()
Called from: x/oracle/abci.go EndBlocker

Impact

Severity: Critical

Consensus failure: Yes - all validators crash on the same block
Chain halt: Yes - requires coordinated restart

Similar bugs:

Informal Systems audit Gravity Bridge: Endblocker panics may halt consensus engine althea-net/cosmos-gravity-bridge#348
Sherlock audit Allora (High severity): Kow - Adversary can arbitrarily trigger a chain halt by sending MsgRemove{Delegate}Stake with negative amount sherlock-audit/2024-06-allora-judging#21

Fix

Return error instead of panic:

// Before
params, err := k.Params.Get(ctx)
if err != nil {
    panic(err)
}

// After
params, err := k.Params.Get(ctx)
if err != nil {
    return "", nil, err
}

Full Fix Files

File 1: `x/oracle/tally.go` (MODIFIED)

Change function signature to return error and replace panic with return:

func pickReferenceDenom(ctx sdk.Context, k keeper.Keeper, voteTargets map[string]types.Denom, voteMap map[string]types.ExchangeRateBallot) (string, map[string]types.ExchangeRateBallot, error) {
    // ... existing code ...

    params, err := k.Params.Get(ctx)
    if err != nil {
        return "", nil, err
    }

    // ... existing code ...

    return referenceDenom, belowThresholdVoteMap, nil
}

File 2: `x/oracle/abci.go` (MODIFIED)

Update caller to handle error:

referenceDenom, belowThresholdVoteMap, err := pickReferenceDenom(ctx, k, voteTargets, voteMap)
if err != nil {
    return err
}

File 3: `x/oracle/tally_test.go` (MODIFIED)

Update test to handle new return value:

referenceDenom, belowThresholdVoteMap, _ := pickReferenceDenom(ctx, oracleKeeper, votingTarget, voteMap)

Test Result (After Fix)

=== RUN   TestPickReferenceDenom
--- PASS: TestPickReferenceDenom (0.01s)
PASS
ok      github.com/kiichain/kiichain/v7/x/oracle        0.091s

coderabbitai · 2026-01-07T20:52:22Z

Walkthrough

The PR changes pickReferenceDenom in the oracle module to return an error as a third return value instead of panicking when parameter retrieval fails. tally.go now propagates and returns errors from k.Params.Get(ctx). EndBlocker in abci.go is updated to handle and log the error returned by pickReferenceDenom and to return on error. Tests calling pickReferenceDenom were updated to accept the additional return value (discarding it). CHANGELOG.md was updated with a Fixed entry referencing the change.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

KiiChain/kiichain issue 203 — Implements returning an error from pickReferenceDenom and handling it instead of panicking, matching the issue objective.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: converting an unhandled panic in pickReferenceDenom to proper error handling in the Oracle EndBlocker, preventing potential consensus failure.
Description check	✅ Passed	The description comprehensively explains the bug, its critical impact on chain consensus, the exact location of the issue, and the complete fix implementation across all affected files.

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

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2da8151 and 955caa2.

📒 Files selected for processing (4)

CHANGELOG.md
x/oracle/abci.go
x/oracle/tally.go
x/oracle/tally_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

x/oracle/tally.go
CHANGELOG.md
x/oracle/abci.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

GitHub Check: tests
GitHub Check: Analyze
GitHub Check: test-e2e
GitHub Check: golangci-lint
GitHub Check: CodeQL analysis (go)
GitHub Check: golangci-lint
GitHub Check: Analyze
GitHub Check: test-e2e

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @CHANGELOG.md:
- Line 20: The changelog contains a duplicate entry "Handle error in
pickReferenceDenom instead of panic to prevent consensus failure
[#203](https://github.com/KiiChain/kiichain/issues/203)" present in both the
UNRELEASED section and the v6.0.0 section; remove the duplicate from the
appropriate section: if v6.0.0 has already been released, delete the entry from
the UNRELEASED section, otherwise delete the entry from the v6.0.0 section so
the fix only appears in UNRELEASED; locate the exact line by searching for the
quoted entry text to update the correct section.

In @x/oracle/tally.go:
- Line 22: The call to k.StakingKeeper.TotalBondedTokens(ctx) currently ignores
its returned error, which can yield invalid totalBondedTokens and bad threshold
math; update the code to capture the error (e.g., totalBondedTokens, err :=
k.StakingKeeper.TotalBondedTokens(ctx)), check err, and propagate or handle it
consistently with the PR's error-handling pattern (return the error or wrap it
with context) instead of discarding it.

🧹 Nitpick comments (1)

x/oracle/tally_test.go (1)

100-100: Consider adding a test case for the error path.

Discarding the error with _ is acceptable for this happy-path test since Params is properly set up (lines 45-49). However, consider adding a separate test case that verifies pickReferenceDenom returns an error when k.Params.Get(ctx) fails (e.g., when params are not initialized). This would ensure the new error-handling behavior is covered.

Would you like me to generate a test case for the error scenario?

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67de973 and 36e7df1.

📒 Files selected for processing (4)

CHANGELOG.md
x/oracle/abci.go
x/oracle/tally.go
x/oracle/tally_test.go

🧰 Additional context used

🧬 Code graph analysis (1)

x/oracle/tally.go (4)

x/oracle/keeper/keeper.go (1)

Keeper (22-42)

x/oracle/types/params.pb.go (6)

Denom (113-116)

Denom (119-119)

Denom (120-122)

Params (28-50)

Params (53-53)

Params (54-56)

x/oracle/types/ballot.go (2)

ExchangeRateBallot (51-51)

Claim (13-19)

x/oracle/types/expected_keepers.go (1)

StakingKeeper (17-24)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: tests
GitHub Check: test-e2e
GitHub Check: golangci-lint
GitHub Check: Analyze

🔇 Additional comments (3)

x/oracle/tally.go (2)

26-29: LGTM!

The panic is correctly replaced with proper error propagation. Returning the error allows the caller (EndBlocker) to handle it gracefully, preventing consensus failure.

67-67: LGTM!

The successful path correctly returns nil for the error, maintaining the expected behavior when no errors occur.

x/oracle/abci.go (1)

78-81: LGTM!

The error is properly captured and returned, following the same pattern used elsewhere in EndBlocker (e.g., lines 22-24, 34-36). This ensures that parameter retrieval failures in pickReferenceDenom are gracefully propagated rather than causing a panic.

CHANGELOG.md

x/oracle/tally.go

codecov · 2026-01-07T20:57:39Z

Codecov Report

❌ Patch coverage is 88.23529% with 6 lines in your changes missing coverage. Please review.

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

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR fixes a critical consensus failure vulnerability in the oracle module by replacing an unhandled panic with proper error handling. The panic in pickReferenceDenom was being triggered from the EndBlocker ABCI method, which could cause all validators to crash simultaneously and halt the chain.

Key Changes:

Modified pickReferenceDenom function signature to return an error instead of panicking
Updated the EndBlocker to handle errors from pickReferenceDenom
Updated unit tests to accommodate the new function signature

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
x/oracle/tally.go	Changed `pickReferenceDenom` function signature to return error; replaced `panic(err)` with `return "", nil, err` for proper error propagation
x/oracle/abci.go	Updated `EndBlocker` to handle the error returned by `pickReferenceDenom`
x/oracle/tally_test.go	Updated `TestPickReferenceDenom` to handle the additional error return value from `pickReferenceDenom`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

x/oracle/tally_test.go

- Return error from pickReferenceDenom instead of panic(err) - Handle error in EndBlocker caller with proper logging - Update test to handle new return value - Fixes KiiChain#203

Thaleszh · 2026-01-08T13:49:20Z

Same fix as #220, closing this one

Copilot AI review requested due to automatic review settings January 7, 2026 20:52

Winnode requested a review from jhelison as a code owner January 7, 2026 20:52

Copilot started reviewing on behalf of Winnode January 7, 2026 20:52 View session

Winnode force-pushed the fix/oracle-endblocker-panic-handling branch from f5f872b to 36e7df1 Compare January 7, 2026 20:53

coderabbitai bot reviewed Jan 7, 2026

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

x/oracle/tally.go Outdated Show resolved Hide resolved

Winnode mentioned this pull request Jan 7, 2026

[BUG] Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #203

Closed

Copilot AI reviewed Jan 7, 2026

View reviewed changes

x/oracle/tally_test.go Show resolved Hide resolved

fix: handle error in pickReferenceDenom instead of panic

955caa2

- Return error from pickReferenceDenom instead of panic(err) - Handle error in EndBlocker caller with proper logging - Update test to handle new return value - Fixes KiiChain#203

Winnode force-pushed the fix/oracle-endblocker-panic-handling branch from b9cd73b to 955caa2 Compare January 7, 2026 22:53

Thaleszh closed this Jan 8, 2026

Winnode deleted the fix/oracle-endblocker-panic-handling branch January 8, 2026 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #219

fix: Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #219

Uh oh!

Winnode commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Thaleszh commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #219

fix: Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure #219

Uh oh!

Conversation

Winnode commented Jan 7, 2026

[BUG] Oracle EndBlocker: Unhandled panic in pickReferenceDenom can cause consensus failure

Bug Description

Attack Flow

Environment

Impact

Fix

Full Fix Files

File 1: x/oracle/tally.go (MODIFIED)

File 2: x/oracle/abci.go (MODIFIED)

File 3: x/oracle/tally_test.go (MODIFIED)

Test Result (After Fix)

Uh oh!

coderabbitai bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related issues

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Thaleszh commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

File 1: `x/oracle/tally.go` (MODIFIED)

File 2: `x/oracle/abci.go` (MODIFIED)

File 3: `x/oracle/tally_test.go` (MODIFIED)

coderabbitai bot commented Jan 7, 2026 •

edited

Loading

codecov bot commented Jan 7, 2026 •

edited

Loading