Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# CHANGELOG

## Unreleased

### Fixed

- Oracle slashing no longer burns coins but instead sends them to community pool
- Slashing overall no longer burn coins, but instead sends them to community pool
- Add denom string length validation (max 128 bytes) to oracle precompile and query server to prevent memory exhaustion via oversized inputs
- Add result limits to oracle list queries (ExchangeRates, Actives, VoteTargets capped at 1000; PriceSnapshotHistory capped at 500) to prevent unbounded iteration
- Fix NewClaim constructor assigning power to Weight field instead of the weight parameter (x/oracle/types/ballot.go)
Expand Down
2 changes: 2 additions & 0 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func NewAppKeeper(
stakingtypes.NewMultiStakingHooks(
appKeepers.DistrKeeper.Hooks(),
appKeepers.SlashingKeeper.Hooks(),
NewSlashHooks(appKeepers.StakingKeeper, appKeepers.BankKeeper, appKeepers.DistrKeeper),
),
)

Expand Down Expand Up @@ -446,6 +447,7 @@ func NewAppKeeper(
appKeepers.AccountKeeper,
appKeepers.BankKeeper,
appKeepers.StakingKeeper,
appKeepers.DistrKeeper,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

Expand Down
117 changes: 117 additions & 0 deletions app/keepers/slash_hooks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package keepers

import (
"context"

"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

tokenfactorytypes "github.com/kiichain/kiichain/v7/x/tokenfactory/types"
)

// slashStakingKeeper is the staking-keeper subset needed by SlashHooks
type slashStakingKeeper interface {
GetValidator(ctx context.Context, addr sdk.ValAddress) (stakingtypes.Validator, error)
BondDenom(ctx context.Context) (string, error)
}

// slashBankKeeper is the bank-keeper subset needed by SlashHooks
type slashBankKeeper interface {
MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error
}

// slashDistrKeeper is the distribution-keeper subset needed by SlashHooks
type slashDistrKeeper interface {
FundCommunityPool(ctx context.Context, amount sdk.Coins, sender sdk.AccAddress) error
}

// SlashHooks implements stakingtypes.StakingHooks
type SlashHooks struct {
stakingKeeper slashStakingKeeper
bankKeeper slashBankKeeper
distrKeeper slashDistrKeeper
}

// NewSlashHooks returns a SlashHooks
func NewSlashHooks(sk slashStakingKeeper, bk slashBankKeeper, dk slashDistrKeeper) SlashHooks {
return SlashHooks{stakingKeeper: sk, bankKeeper: bk, distrKeeper: dk}
}

var _ stakingtypes.StakingHooks = SlashHooks{}

// BeforeValidatorSlashed mints the equivalent of the slash amount into the
// community pool before the tokens are burned
func (h SlashHooks) BeforeValidatorSlashed(ctx context.Context, valAddr sdk.ValAddress, fraction math.LegacyDec) error {
validator, err := h.stakingKeeper.GetValidator(ctx, valAddr)
if err != nil || !validator.Tokens.IsPositive() {
return err
}

// The effectiveFraction passed by slashToPool is tokensToBurn / validator.Tokens,
// so multiplying back recovers tokensToBurn
mintAmount := math.LegacyNewDecFromInt(validator.Tokens).Mul(fraction).TruncateInt()
Comment on lines +53 to +55
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In cosmos-sdk v0.53.6, what exact value does x/staking/keeper/slash.gopass as thefractionargument toStakingHooks.BeforeValidatorSlashed? Please cite the implementation and clarify whether it is the raw slash factor or an effective post-clamp fraction.

💡 Result:

In cosmos-sdk v0.53.6, the x/staking/keeper/slash.go file passes the raw slash factor (the unclamped fraction parameter received by the Slash function) as the fraction argument to StakingHooks.BeforeValidatorSlashed. This occurs before any clamping of the slash amount to ensure the minimum balance is maintained. The hook receives the intended slash fraction prior to applying clamps in the staking keeper's Slash logic.

Citations:


🏁 Script executed:

# Find the BeforeValidatorSlashed hook implementation in the repo
find . -type f -name "*.go" | xargs grep -l "BeforeValidatorSlashed" | head -20

Repository: KiiChain/kiichain

Length of output: 1885


🏁 Script executed:

# Check the slash_hooks.go file to understand the full context
cat -n app/keepers/slash_hooks.go | head -70

Repository: KiiChain/kiichain

Length of output: 1885


🏁 Script executed:

# Search for where slash amounts or tokensToBurn are computed in the staking context
rg -A 5 -B 5 "tokensToBurn|slash.*amount" app/keepers/slash_hooks.go

Repository: KiiChain/kiichain

Length of output: 1885


Reconstruct the exact slash amount instead of deriving it from the current validator tokens.

The fraction parameter is the raw slash factor (before clamping). Multiplying it against validator.Tokens and converting through decimals introduces rounding loss that can under-mint, and delayed evidence execution where validator tokens differ from the infraction-power basis will further drift from the actual amount slashed. Pass or compute the exact tokensToBurn value from the slash execution path instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/keepers/slash_hooks.go` around lines 53 - 55, The mint amount is being
recomputed from validator.Tokens and fraction which causes rounding/drift;
instead use the exact tokensToBurn computed during slashing (or pass it into
this hook) rather than reconstructing it. Update the caller (slashToPool /
whatever invokes this hook) to provide the exact tokensToBurn (sdk.Int) and
change this code path to use that value for mintAmount (replace the
math.LegacyNewDecFromInt(validator.Tokens).Mul(fraction).TruncateInt() logic),
ensuring the hook signature and any callers use the precise tokens burned to
avoid under-minting and drift.

if mintAmount.IsZero() {
return nil
}

bondDenom, err := h.stakingKeeper.BondDenom(ctx)
if err != nil {
return err
}

coins := sdk.NewCoins(sdk.NewCoin(bondDenom, mintAmount))

// Mint into the tokenfactory module account (needs mint permission)
// and immediately forward to the distribution community pool
if err := h.bankKeeper.MintCoins(ctx, tokenfactorytypes.ModuleName, coins); err != nil {
return err
}

tokenfactoryAddr := authtypes.NewModuleAddress(tokenfactorytypes.ModuleName)
return h.distrKeeper.FundCommunityPool(ctx, coins, tokenfactoryAddr)
}
Comment thread
Thaleszh marked this conversation as resolved.

// The remaining hooks are not in use; only BeforeValidatorSlashed is used

func (h SlashHooks) AfterValidatorCreated(_ context.Context, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) BeforeValidatorModified(_ context.Context, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) AfterValidatorRemoved(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) AfterValidatorBonded(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) AfterValidatorBeginUnbonding(_ context.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) BeforeDelegationCreated(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) BeforeDelegationSharesModified(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) BeforeDelegationRemoved(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) AfterDelegationModified(_ context.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
return nil
}

func (h SlashHooks) AfterUnbondingInitiated(_ context.Context, _ uint64) error {
return nil
}
139 changes: 139 additions & 0 deletions app/keepers/slash_hooks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package keepers_test

import (
"testing"
"time"

"github.com/stretchr/testify/require"

tmtypes "github.com/cometbft/cometbft/proto/tendermint/types"

"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

kiichain "github.com/kiichain/kiichain/v7/app"
helpers "github.com/kiichain/kiichain/v7/app/helpers"
)

// createBondedValidator funds a new validator address, creates the validator,
// runs EndBlocker to move it to the bonded set, and registers signing info.
// Returns the val address and consensus address
func createBondedValidator(t *testing.T, app *kiichain.KiichainApp, ctx sdk.Context, stakedAmount math.Int) (sdk.ValAddress, sdk.ConsAddress) {
t.Helper()

privKey := secp256k1.GenPrivKey()
pubKey := privKey.PubKey()
valAddr := sdk.ValAddress(pubKey.Address())
consAddr := sdk.ConsAddress(pubKey.Address())

bondDenom, err := app.StakingKeeper.BondDenom(ctx)
require.NoError(t, err)

selfBond := sdk.NewCoins(sdk.NewCoin(bondDenom, stakedAmount))
require.NoError(t, app.BankKeeper.MintCoins(ctx, "tokenfactory", selfBond))
require.NoError(t, app.BankKeeper.SendCoinsFromModuleToAccount(ctx, "tokenfactory", sdk.AccAddress(valAddr), selfBond))

msgServer := stakingkeeper.NewMsgServerImpl(app.StakingKeeper)
createMsg, err := stakingtypes.NewMsgCreateValidator(
valAddr.String(),
pubKey,
sdk.NewCoin(bondDenom, stakedAmount),
stakingtypes.Description{Moniker: "test-validator"},
stakingtypes.NewCommissionRates(
math.LegacyNewDecWithPrec(5, 2),
math.LegacyNewDecWithPrec(20, 2),
math.LegacyNewDecWithPrec(1, 2),
),
math.OneInt(),
)
require.NoError(t, err)
_, err = msgServer.CreateValidator(ctx, createMsg)
require.NoError(t, err)

_, err = app.StakingKeeper.EndBlocker(ctx)
require.NoError(t, err)

signingInfo := slashingtypes.NewValidatorSigningInfo(
consAddr, ctx.BlockHeight(), 0, time.Unix(0, 0), false, 0,
)
require.NoError(t, app.SlashingKeeper.SetValidatorSigningInfo(ctx, consAddr, signingInfo))

return valAddr, consAddr
}

// TestSlashHooks_SupplyUnchangedAfterSlash verifies that when
// the staking keeper slashes a validator, the total token supply is
// unchanged
func TestSlashHooks_SupplyUnchangedAfterSlash(t *testing.T) {
app := helpers.Setup(t)
ctx := app.NewUncachedContext(true, tmtypes.Header{
Height: 1,
ChainID: "testing",
Time: time.Now().UTC(),
})

bondDenom, err := app.StakingKeeper.BondDenom(ctx)
require.NoError(t, err)

stakedAmount := sdk.TokensFromConsensusPower(100, sdk.DefaultPowerReduction)
valAddr, consAddr := createBondedValidator(t, app, ctx, stakedAmount)

supplyBefore := app.BankKeeper.GetSupply(ctx, bondDenom).Amount

validator, err := app.StakingKeeper.GetValidator(ctx, valAddr)
require.NoError(t, err)
powerReduction := app.StakingKeeper.PowerReduction(ctx)
consensusPower := validator.GetConsensusPower(powerReduction)

slashFactor := math.LegacyNewDecWithPrec(5, 2) // 5%
slashed, err := app.StakingKeeper.Slash(ctx, consAddr, 0, consensusPower, slashFactor)
require.NoError(t, err)
require.True(t, slashed.IsPositive(), "expected a non-zero slash amount")

// Supply must be unchanged: the burn is offset by the hook's mint
require.Equal(t, supplyBefore, app.BankKeeper.GetSupply(ctx, bondDenom).Amount,
"total supply must be unchanged after slash")

// Community pool must have received exactly the slashed amount
feePool, err := app.DistrKeeper.FeePool.Get(ctx)
require.NoError(t, err)
communityPool := feePool.CommunityPool.AmountOf(bondDenom)
require.Equal(t, math.LegacyNewDecFromInt(slashed), communityPool,
"community pool must receive exactly the slash amount")
}

// TestSlashHooks_ZeroSlashFactor verifies no tokens move and supply is
// unchanged when the slash factor is zero
func TestSlashHooks_ZeroSlashFactor(t *testing.T) {
app := helpers.Setup(t)
ctx := app.NewUncachedContext(true, tmtypes.Header{
Height: 1,
ChainID: "testing",
Time: time.Now().UTC(),
})

bondDenom, err := app.StakingKeeper.BondDenom(ctx)
require.NoError(t, err)

stakedAmount := sdk.TokensFromConsensusPower(100, sdk.DefaultPowerReduction)
_, consAddr := createBondedValidator(t, app, ctx, stakedAmount)

supplyBefore := app.BankKeeper.GetSupply(ctx, bondDenom).Amount

validator, err := app.StakingKeeper.GetValidatorByConsAddr(ctx, consAddr)
require.NoError(t, err)
powerReduction := app.StakingKeeper.PowerReduction(ctx)
consensusPower := validator.GetConsensusPower(powerReduction)

slashed, err := app.StakingKeeper.Slash(ctx, consAddr, 0, consensusPower, math.LegacyZeroDec())
require.NoError(t, err)
require.True(t, slashed.IsZero(), "expected zero slash for zero factor")

require.Equal(t, supplyBefore, app.BankKeeper.GetSupply(ctx, bondDenom).Amount,
"supply must be unchanged when slash factor is zero")
}
4 changes: 3 additions & 1 deletion x/oracle/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type Keeper struct {
accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
StakingKeeper types.StakingKeeper
distrKeeper types.DistributionKeeper

// Schema of the module
Schema collections.Schema
Expand All @@ -44,7 +45,7 @@ type Keeper struct {
// NewKeeper creates an oracle Keeper instance
func NewKeeper(cdc codec.BinaryCodec, storeService corestoretypes.KVStoreService,
accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, stakingKeeper types.StakingKeeper,
authority string,
distrKeeper types.DistributionKeeper, authority string,
) Keeper {
// Ensure oracle module account is set
addr := accountKeeper.GetModuleAddress(types.ModuleName)
Expand All @@ -66,6 +67,7 @@ func NewKeeper(cdc codec.BinaryCodec, storeService corestoretypes.KVStoreService
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
StakingKeeper: stakingKeeper,
distrKeeper: distrKeeper,
Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)),
ExchangeRate: collections.NewMap(sb, types.ExchangeRateKey, "exchange_rate", collections.StringKey, codec.CollValue[types.OracleExchangeRate](cdc)),
FeederDelegation: collections.NewMap(sb, types.FeederDelegationKey, "feeder_delegation", sdk.ValAddressKey, collections.StringValue),
Expand Down
59 changes: 54 additions & 5 deletions x/oracle/keeper/slash.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/kiichain/kiichain/v7/x/oracle/types"
Expand All @@ -15,9 +16,6 @@ import (
// SlashAndResetCounters calculate if the validator must be slashed if success votes / total votes
// is lower than MinValidPerWindow param. Then reset the vote penalty info
func (k Keeper) SlashAndResetCounters(ctx sdk.Context) error {
height := ctx.BlockHeight()
distributionHeight := height - sdk.ValidatorUpdateDelay - 1

// Get the module params
params, err := k.Params.Get(ctx)
if err != nil {
Expand Down Expand Up @@ -69,8 +67,8 @@ func (k Keeper) SlashAndResetCounters(ctx sdk.Context) error {
// Calculate consensus power
consensusPower := validator.GetConsensusPower(powerReduction)

// Slash the validator
_, err = k.StakingKeeper.Slash(ctx, consAddr, distributionHeight, consensusPower, slashFraction)
// Redirect the slash amount to the community pool instead of burning
_, err = k.slashToPool(ctx, consAddr, consensusPower, slashFraction)
if err != nil {
k.Logger(ctx).Error("failed to slash validator", "operator", operator.String(), "error", err)
return true, err
Expand Down Expand Up @@ -103,3 +101,54 @@ func (k Keeper) SlashAndResetCounters(ctx sdk.Context) error {
})
return err
}

// slashToPool removes tokens from a validator and sends them to the community pool
// instead of burning, preserving total supply.
func (k Keeper) slashToPool(ctx sdk.Context, consAddr sdk.ConsAddress, consensusPower int64, slashFactor math.LegacyDec) (math.Int, error) {
if slashFactor.IsZero() {
return math.ZeroInt(), nil
}

// Calculate slash amount: same math as the staking module
amount := k.StakingKeeper.TokensFromConsensusPower(ctx, consensusPower)
slashAmount := math.LegacyNewDecFromInt(amount).Mul(slashFactor).TruncateInt()

// Get the concrete validator (needed for RemoveValidatorTokens)
validator, err := k.StakingKeeper.GetValidatorByConsAddr(ctx, consAddr)
if err != nil {
return math.ZeroInt(), err
}

// Clamp slash amount to the validator's available tokens
tokensToBurn := math.MinInt(slashAmount, validator.Tokens)
if tokensToBurn.IsZero() {
return math.ZeroInt(), nil
}

// Remove tokens from the validator's bookkeeping without burning
_, err = k.StakingKeeper.RemoveValidatorTokens(ctx, validator, tokensToBurn)
if err != nil {
return math.ZeroInt(), err
}

// Create the coin (value but with akii)
bondDenom, err := k.StakingKeeper.BondDenom(ctx)
if err != nil {
return math.ZeroInt(), err
}
coins := sdk.NewCoins(sdk.NewCoin(bondDenom, tokensToBurn))
// Send the slashed tokens from the bonded pool to the community pool
bondedPoolAddr := authtypes.NewModuleAddress(stakingtypes.BondedPoolName)
if err := k.distrKeeper.FundCommunityPool(ctx, coins, bondedPoolAddr); err != nil {
return math.ZeroInt(), err
}

k.Logger(ctx).Info(
"oracle slash redirected to community pool",
"validator", consAddr.String(),
"slash_factor", slashFactor.String(),
"amount", tokensToBurn.String(),
)

return tokensToBurn, nil
Comment on lines +107 to +153
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find the oracle keeper slash file
find . -name "slash.go" -path "*/oracle/keeper/*" 2>/dev/null

Repository: KiiChain/kiichain

Length of output: 87


🏁 Script executed:

# Check the file size before reading
wc -l x/oracle/keeper/slash.go 2>/dev/null || find . -name "slash.go" -path "*/oracle*" -exec wc -l {} \;

Repository: KiiChain/kiichain

Length of output: 89


🏁 Script executed:

# Search for where slashToPool or similar functions are used
rg "slashToPool|Slash" --type go -l | head -20

Repository: KiiChain/kiichain

Length of output: 573


🏁 Script executed:

# Look for BeforeValidatorSlashed usage
rg "BeforeValidatorSlashed" --type go -A 2 -B 2

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Search for RemoveValidatorTokens usage to understand the pattern
rg "RemoveValidatorTokens" --type go -l

Repository: KiiChain/kiichain

Length of output: 120


🏁 Script executed:

# Read the complete slash.go file
cat -n x/oracle/keeper/slash.go

Repository: KiiChain/kiichain

Length of output: 6558


🏁 Script executed:

# Search for where slash functions are called to understand the flow
rg "slash\(" --type go x/oracle -A 3 -B 3

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Check the staking keeper interface to understand what Slash does
rg "interface.*StakingKeeper|Slash.*func" --type go -A 5 | head -100

Repository: KiiChain/kiichain

Length of output: 2081


🏁 Script executed:

# Look at the expected_keepers.go to see what methods are expected
cat x/oracle/types/expected_keepers.go

Repository: KiiChain/kiichain

Length of output: 3866


🏁 Script executed:

# Search for any usage of the actual Slash method from staking keeper
rg "\.Slash\(" --type go -B 3 -A 3

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Look for BeforeValidatorSlashed hook registration
rg "BeforeValidatorSlashed|Hooks" --type go x/oracle -A 2 -B 2

Repository: KiiChain/kiichain

Length of output: 429


🏁 Script executed:

# Check if there are any tests for slashToPool to understand the expected behavior
cat -n x/oracle/keeper/slash_test.go 2>/dev/null | head -100

Repository: KiiChain/kiichain

Length of output: 4254


🏁 Script executed:

# Look for staking keeper imports to understand what methods are available
rg "staking" --type go x/oracle/keeper -i -l

Repository: KiiChain/kiichain

Length of output: 298


🏁 Script executed:

# Search for any references to unbonding or redelegation handling
rg "unbonding|Unbonding|redelegation|Redelegation" --type go x/oracle

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Look at the full slash_test.go to understand what's being tested
cat -n x/oracle/keeper/slash_test.go

Repository: KiiChain/kiichain

Length of output: 25621


🏁 Script executed:

# Search for any slash event emission or distribution hooks in oracle keeper
rg "slash|event|Event" --type go x/oracle/keeper/slash.go -i

Repository: KiiChain/kiichain

Length of output: 1716


🏁 Script executed:

# Check if there's any direct usage of staking.Keeper.Slash elsewhere in the codebase
rg "staking\..*Slash|keeper\.Slash" --type go -i

Repository: KiiChain/kiichain

Length of output: 1337


🏁 Script executed:

# Look at the Cosmos SDK documentation by checking what staking.Keeper.Slash method signature should be
# Check if there are any imports or references that show what Slash should do
rg "import.*staking" --type go x/oracle

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Check if there's any infraction height tracking or unbonding handling
rg "unbonding|InfractionHeight|Fraction" --type go x/oracle/keeper

Repository: KiiChain/kiichain

Length of output: 1572


🏁 Script executed:

# Look for distribution slash event emission elsewhere
rg "EventTypeSlash|slash.*event|Event.*Slash" --type go -i

Repository: KiiChain/kiichain

Length of output: 320


🏁 Script executed:

# Check the app keepers configuration to see how distribution hooks are set up
cat -n app/keepers/keepers.go | head -300

Repository: KiiChain/kiichain

Length of output: 13984


🏁 Script executed:

# Check if there are any other slash implementations or references in the staking keeper interface
rg "Slash\(" --type go -B 2 -A 5 | head -80

Repository: KiiChain/kiichain

Length of output: 825


🏁 Script executed:

# Look for any hook registration related to oracle or validation
rg "SetHooks|Hooks\(\)|BeforeValidator" --type go app/keepers/keepers.go -i

Repository: KiiChain/kiichain

Length of output: 255


🏁 Script executed:

# Search for any distribution slash event handling
rg "ValidatorSlashEvent|SlashEvent" --type go

Repository: KiiChain/kiichain

Length of output: 126


🏁 Script executed:

# Search for any actual usage of StakingKeeper.Slash method in the codebase
rg "StakingKeeper\.Slash|\.Slash\(" --type go -B 3 -A 5

Repository: KiiChain/kiichain

Length of output: 791


🏁 Script executed:

# Check the Cosmos SDK version to understand what Slash method does
rg "cosmos-sdk" go.mod | head -5

Repository: KiiChain/kiichain

Length of output: 239


🏁 Script executed:

# Look at the test helper to see the context of distribution hooks
cat -n x/oracle/keeper/test_utils.go | head -100

Repository: KiiChain/kiichain

Length of output: 4397


🏁 Script executed:

# Look for the Cosmos SDK Slash method signature in go.mod dependencies
grep -A 5 "cosmos-sdk" go.mod

Repository: KiiChain/kiichain

Length of output: 876


🏁 Script executed:

# Check if there's any BeforeValidatorSlashed hook in the distribution keeper
rg "BeforeValidatorSlashed" --type go -B 3 -A 3

Repository: KiiChain/kiichain

Length of output: 43


🏁 Script executed:

# Look for any unbonding delegation concepts in oracle
rg "unbond|Unbond" --type go x/oracle

Repository: KiiChain/kiichain

Length of output: 219


🏁 Script executed:

# Examine the test that explicitly mentions previous behavior used StakingKeeper.Slash
sed -n '242,250p' x/oracle/keeper/slash_test.go

Repository: KiiChain/kiichain

Length of output: 549


🌐 Web query:

cosmos-sdk staking keeper Slash method infraction height unbonding

💡 Result:

The Cosmos SDK Staking Keeper's Slash method is used by the Slashing module to penalize validators for infractions like downtime or double-signing. It is called from the slashing keeper's Slash or SlashWithInfractionReason methods. Signature (from pkg.go.dev docs and source references): func (k Keeper) Slash(ctx context.Context, consAddr sdk.ConsAddress, fraction sdkmath.LegacyDec, power int64, ...) error More precisely, the staking Slash method takes parameters including: - validator consensus address - slash fraction (e.g., SlashFractionDowntime or SlashFractionDoubleSign) - validator power at time of infraction - infraction height (int64 block height where infraction occurred) The infraction height is critical for handling unbonding delegations. When slashing: 1. Slash bonded tokens from the validator's pool (BondedPool or NotBondedPool). 2. Iterate over all unbonding delegations (UBDs) from the validator. For each UBD entry: - If the unbonding completion time is after the infraction height + unbonding period (i.e., UBD started after infraction), slash proportionally from InitialBalance. - Mature/completed UBDs (IsMature && !OnHold) are skipped. 3. Similarly slash ongoing redelegations (pseudo-unbonding) that started after the infraction. 4. Burn slashed tokens, reducing total supply. This ensures only stake "at risk" at infraction time is slashed, using historical state via infraction height to filter UBDs/redelegs. Examples: - Liveness slash: distributionHeight = ctx.BlockHeight - sdk.ValidatorUpdateDelay - 1 - Evidence-based (double-sign): exact infraction height from evidence. Source: Official Cosmos SDK GitHub (x/staking/keeper/slash.go), docs.cosmos.network (staking/slashing modules), pkg.go.dev/cosmossdk.io/x/staking, interchain-security ADR on equivocation slashing logic.

Citations:


Use the full staking.Keeper.Slash semantics, only swapping the burn destination to preserve supply.

The helper at lines 107–153 sidesteps the SDK's Slash path in favor of a simplified token removal and community pool redirection. While supply preservation is valuable, this loses critical semantic guarantees:

  • No infraction-height slashing: The SDK's Slash method uses an infraction height to filter which unbonding delegations and redelegations to penalize. Without it, no UBDs/redelegations are slashed at all, allowing validators to move stake into unbonding queues before the slashing window closes and escape the penalty entirely.
  • No hook invocation: The BeforeValidatorSlashed hook is never fired, so x/distribution never records the slash event.
  • Incomplete slash semantics: The effective slash fraction is not calculated, and mature/completed unbonding entries are not properly skipped.

Instead of reimplementing this logic, call StakingKeeper.Slash with the correct infraction height and only replace the final token-burn step with a transfer to the community pool. This preserves both total supply and the full slash semantics that prevent validators from escaping slashing via unbonding or redelegation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@x/oracle/keeper/slash.go` around lines 107 - 153, The current slashToPool
helper bypasses staking.Keeper.Slash and therefore loses infraction-height
filtering, hook invocation, and correct effective-slash semantics; update
slashToPool to call k.StakingKeeper.Slash(ctx, consAddr, infractionHeight,
slashAmountConsensusPower, slashFactor) (using the same infraction height used
by the module when detecting the infraction) to apply all staking semantics,
then intercept/replace the final burn step by transferring the burned amount to
the community pool via k.distrKeeper.FundCommunityPool instead of leaving it
burned; ensure you no longer call RemoveValidatorTokens directly for slashing
(so BeforeValidatorSlashed and UBD/redelegation filtering run) and keep logging
the redirected amount.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Check point 1 of this, it might be wroth

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.

🧩 Analysis chain

Let me check the original implementation to understand what was previously slashed (bonded tokens only vs. UBDs/redelegations) and how significant point 1 is.


🏁 Script executed:

#!/bin/bash
# Check git log for the original slash.go before this PR
git log --oneline x/oracle/keeper/slash.go | head -10

# Check git diff to see the original implementation
git show HEAD~3:x/oracle/keeper/slash.go 2>/dev/null || git show HEAD~2:x/oracle/keeper/slash.go 2>/dev/null || git show HEAD~1:x/oracle/keeper/slash.go 2>/dev/null

Length of output: 5860


🏁 Script executed:

#!/bin/bash
# The PR branch has these commits - let's go back to the base commit before this PR
# Looking at the commit order, the first PR commit is 2bf4a8d
# Let's check the commit just before that
git show 2bf4a8d^:x/oracle/keeper/slash.go 2>/dev/null | head -80

echo "---SEPARATOR---"
# Also check original expected_keepers.go before this PR
git show 2bf4a8d^:x/oracle/types/expected_keepers.go 2>/dev/null

Length of output: 5872


@Thaleszh, yes — point 1 is worth addressing, though the severity depends on your slash fraction. Here's the concrete breakdown:

What the original code did:

distributionHeight := height - sdk.ValidatorUpdateDelay - 1
k.StakingKeeper.Slash(ctx, consAddr, distributionHeight, consensusPower, slashFraction)

The SDK's Slash() not only reduces bonded tokens but also iterates all unbonding delegations and redelegations that began after distributionHeight and slashes them proportionally.

What the new code does:
RemoveValidatorTokens only reduces the validator's currently-bonded stake. Any unbonding queue entries escape the penalty entirely.

The exploit path:

  1. A validator notices their oracle participation is low within the window.
  2. They initiate unbonding of a significant portion of their delegation.
  3. When the slash window closes, slashToPool only sees their reduced bonded stake — the unbonding tokens are not touched.

Practical severity:

  • If SlashFraction is small (e.g., 0.1%), the economic incentive to game this is low.
  • If the slash window is long (days), the exploit is observable but requires deliberate action.
  • The original distributionHeight window was ValidatorUpdateDelay + 1 blocks (~7–8 blocks on most Cosmos chains), so only very recent UBDs were caught anyway — limiting the practical scope.

Suggested fix approach:

The cleanest fix without fully reimplementing Slash() is to also slash UBDs in slashToPool. But since you want to redirect to community pool instead of burn, you need to iterate UBDs manually. A simpler middle ground is to add UBD slashing alongside the bonded slash, redirecting all burned coins to the pool:

// After RemoveValidatorTokens, also slash UBDs started after distributionHeight
distributionHeight := sdk.Context.BlockHeight() - sdk.ValidatorUpdateDelay - 1

// Iterate unbonding delegations from this validator
unbondingDels, err := k.StakingKeeper.GetUnbondingDelegationsFromValidator(ctx, valAddr)
for _, ubd := range unbondingDels {
    for i, entry := range ubd.Entries {
        if entry.IsMature(ctx.BlockTime()) {
            continue // skip already matured entries
        }
        if entry.CreationHeight < distributionHeight {
            continue // only slash entries started after infraction height
        }
        slashAmountUBD := slashFactor.MulInt(entry.InitialBalance).TruncateInt()
        slashAmountUBD = math.MinInt(slashAmountUBD, entry.Balance)
        ubd.Entries[i].Balance = entry.Balance.Sub(slashAmountUBD)
        totalSlashed = totalSlashed.Add(slashAmountUBD)
        // ... accumulate coins and send to community pool
    }
}

Bottom line: If your oracle slash fraction is low and gaming it requires deliberate and observable action by validators, you could defer this. But if you want full parity with original semantics (which protected against this escape hatch), it's worth adding UBD slashing with the funds redirected to the community pool instead of being burned. Would you like me to open a follow-up issue to track this?

}
Loading
Loading