Conversation
This ratio is dynamically changing, with a minimum and maximum value depending on the number of subnets Formula: subnet_reward_ratio = min( max_ratio, base + k * log(1 + subnet_count) ) Base: Initial subnet reward ratio (e.g. 0.10) k: Growth rate coefficient (e.g. 0.1) Max_ratio: Maximum subnet reward ratio (e.g. 0.5)
…e in the bank module
…e in the bank module
…e in the bank module
…e in the bank module
Created the MintAlphaTokens function to call the mint method of the AlphaToken contract Added calls in three locations: validator dividend, incentive reward, and owner share
WalkthroughAdds three new modules (event, blockinflation, stakework) with full keepers, types, protos, ABIs, CLI and tests; wires an EventKeeper into the EVM keeper for post-commit log dispatch; implements block-emission, coinbase/alpha minting, epoch engine, genesis/init scripts, and an ante vesting refactor. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Tx
participant EVM as EVM Keeper
participant App as App Commit
participant Event as EventKeeper
participant Store as Event State
User->>EVM: Submit DeliverTx
EVM-->>EVM: Execute VM → receipt + logs
EVM->>App: PostTxProcessing / Commit
App->>Event: Dispatch receipt logs (post-commit)
Event-->>Event: Decode logs using embedded ABIs
Event->>Store: Persist subnets/neurons/stakes/weights/prices/emissions
sequenceDiagram
participant Begin as BeginBlock
participant BI as BlockInflationKeeper
participant Event as EventKeeper
participant SW as StakeworkKeeper
participant EVM as ERC20/EVM
Begin->>BI: BeginBlocker()
BI-->>BI: CalculateBlockEmission()
BI->>Event: Determine subnets to emit to / update pending
BI->>BI: RunCoinbase(blockEmission)
BI->>Event: ApplySubnetRewards / AddToPendingEmission
alt epoch due for netuid
BI->>SW: RunEpoch(netuid, emission)
SW-->>BI: EpochResult (dividends/incentives)
BI->>EVM: MintAlphaTokens(recipient, amount)
else not due
BI-->>BI: Increment blocks-since-last-step
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 80
🔭 Outside diff range comments (6)
x/evm/keeper/keeper.go (1)
326-334: Use a pointer receiver for GetNonce to avoid copying Keeper and for receiver consistency.Most methods use pointer receivers; switching GetNonce to a value receiver copies the Keeper (large struct) per call and is inconsistent with the rest of the API. This can be a hot path for RPC queries.
Apply this diff:
-// GetNonce returns the sequence number of an account, returns 0 if not exists. -func (k Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 { +// GetNonce returns the sequence number of an account, returns 0 if not exists. +func (k *Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 {x/stakework/types/interfaces.go (1)
24-25: Remove stray trailing token that will break compilationThere is a dangling
25at the end of the file. This will not compile.-} -25 +}init_validators.sh (4)
101-105: Inconsistent denoms: stop minting “gas” to genesis accounts after switching evm_denom to “ahetu”You changed evm_denom to ahetu (Line 111) but still mint both ahetu and gas to each account here. This doubles supply across denoms and is likely unintended.
- hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM},${GENESIS_BALANCE}gas" \ + hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM}" \ --keyring-backend="${KEYRING}" \ --home "${HOME_PREFIX}"
199-199: Minimum gas price denom should match the staking/bank denomAfter switching to ahetu, setting minimum-gas-prices to “0.0001gas” is inconsistent.
- sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001gas"/g' "$APP_TOML" + sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001ahetu"/g' "$APP_TOML"
151-155: Bank supply updates are brittle: don’t assume array ordering; update per-denomIndexing supply[0] and supply[1] assumes ordering and duplicates totals across denoms. Compute and set amounts for the ahetu entry by matching on .denom to avoid double-minting and ordering issues. Also, bc is required; add a dependency check.
+# Ensure bc is installed +command -v bc >/dev/null 2>&1 || { echo >&2 "bc not installed."; exit 1; } @@ -total_supply=$(echo "$NUM_VALIDATORS * $GENESIS_BALANCE + $amount_to_claim" | bc) -jq -r --arg total_supply "$total_supply" '.app_state["bank"]["supply"][0]["amount"]=$total_supply' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" -jq -r --arg total_supply "$total_supply" '.app_state["bank"]["supply"][1]["amount"]=$total_supply' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" +total_supply=$(echo "$NUM_VALIDATORS * $GENESIS_BALANCE + $amount_to_claim" | bc) +# Update only the ahetu supply entry +jq -r --arg total_supply "$total_supply" ' + .app_state["bank"]["supply"] |= + (map(if .denom == "ahetu" then .amount = $total_supply else . end)) +' "$GENESIS" >"$TMP_GENESIS" && mv "$GENESIS" "$GENESIS.bak" && mv "$TMP_GENESIS" "$GENESIS"
141-141: Inconsistent denomination: 'gas' is injected while evm_denom is set to "ahetu"init_validators.sh sets evm_denom = "ahetu" but still injects "gas" coins into genesis and config — pick one denom and make all scripts/configs consistent.
Files/locations that need attention:
- init_validators.sh
- line ~102: hetud add-genesis-account adds
${GENESIS_BALANCE}gas(remove if denom is ahetu).- line 141: jq adds a {"denom":"gas",...} entry to bank.balances (remove).
- line ~199: sed sets minimum-gas-prices = "0.0001gas" (update to ahetu).
- Audit other scripts/tests for mixed usage and align:
- local_node.sh (uses evm_denom="gas" and adds gas balances)
- tests/e2e/init-node.sh (evm_denom="gas" and similar bank additions)
- any start/rsync commands using
--minimum-gas-prices=...gasSuggested minimal fixes (examples):
Replace the claims balance jq line:
-jq -r --arg amount_to_claim "$amount_to_claim" '.app_state["bank"]["balances"] += [{"address":"hetu15cvq3ljql6utxseh0zau9m8ve2j8erz89c94rj","coins":[{"denom":"ahetu", "amount":$amount_to_claim}, {"denom":"gas", "amount":$amount_to_claim}]}]' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" +jq -r --arg amount_to_claim "$amount_to_claim" '.app_state["bank"]["balances"] += [{"address":"hetu15cvq3ljql6utxseh0zau9m8ve2j8erz89c94rj","coins":[{"denom":"ahetu", "amount":$amount_to_claim}]}]' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS"Also remove
,${GENESIS_BALANCE}gasfrom add-genesis-account invocations:-hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM},${GENESIS_BALANCE}gas" \ +hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM}" \And update minimum-gas-prices substitution:
-sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001gas"/g' "$APP_TOML" +sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001ahetu"/g' "$APP_TOML"If "gas" must remain for backward compatibility, state that explicitly and revert/set evm_denom to "gas" (and update all references), otherwise remove all "gas" bank/min-gas uses and standardize on "ahetu".
🧹 Nitpick comments (59)
app/ante/cosmos/vesting.go (3)
119-123: Add denom sanity-check against bond denomWhile MsgDelegate.ValidateBasic() should enforce denom, adding this guard here prevents silent logic skew if interfaces change or upstreams are bypassed. It also improves error locality.
Apply this diff:
// Get bond denomination bondDenom, err := vdd.sk.BondDenom(ctx) if err != nil { return err } + + // Sanity-check the delegation denom against the chain bond denom + if delegateMsg.Amount.Denom != bondDenom { + return errorsmod.Wrapf( + errortypes.ErrInvalidRequest, + "invalid denom for delegation: expected %s, got %s", + bondDenom, delegateMsg.Amount.Denom, + ) + }
125-133: Edge case: “No vested coins” error path is good; consider tightening message for observabilityThe error is correct and useful. Minor nit: including denom and block time can aid debugging in ops.
Apply this (optional) tweak:
- return errorsmod.Wrap( - vestingtypes.ErrInsufficientVestedCoins, - "account has no vested coins", - ) + return errorsmod.Wrapf( + vestingtypes.ErrInsufficientVestedCoins, + "account has no vested coins at %s for denom %s", + ctx.BlockTime().UTC().Format(time.RFC3339), bondDenom, + )Note: would require
timeimport if not already present.
117-146: Tests needed: multi-delegation and authz-exec scenariosGiven the adjusted semantics, add tests to prevent regressions:
- Delegating multiple times: ensure the second delegation cannot exceed (vested - delegatedFree).
- Authz execution path: inner MsgDelegate for a clawback account enforces the same rules.
- Denom mismatch returns ErrInvalidRequest (if you add the suggested guard).
I can draft table-driven tests for these cases against the ante decorator. Want me to push a test file scaffold?
x/event/types/utils.go (3)
7-7: Clarify doc comments: base, clamping, and error semanticsMake the comments explicit about base-10 parsing, clamping behavior for subtraction, and that errors are returned on invalid inputs. This reduces misuse across modules dealing with token amounts.
If adopting error returns, the diffs above already update comments. If retaining string-only signatures, minimally adjust docs:
-// AddBigIntString adds two big integer strings +// AddBigIntString adds two base-10 big integer strings and returns the decimal string result. +// Invalid inputs are treated as "0" (not recommended for financial logic). -// SubBigIntString subtracts two big integer strings +// SubBigIntString subtracts b from a for base-10 big integer strings and clamps negatives to "0". +// Invalid inputs are treated as "0" (not recommended for financial logic).Also applies to: 15-15
1-26: Consider using Cosmos SDK math.Int for non-negative token amountsIf these helpers are exclusively for Alpha token amounts (non-negative domain), using cosmossdk.io/math.Int (or sdkmath.Int in older SDKs) provides built-in non-negativity and parsing helpers, reducing error surface and clarifying intent. You can still expose string I/O via String() and SetString equivalents.
Example:
import sdkmath "cosmossdk.io/math" func AddAmountStrings(a, b string) (string, error) { ai, ok := sdkmath.NewIntFromString(a); if !ok { return "", fmt.Errorf("invalid a") } bi, ok := sdkmath.NewIntFromString(b); if !ok { return "", fmt.Errorf("invalid b") } return ai.Add(bi).String(), nil }
1-26: Add focused unit tests for parsing, large values, and clampingThese helpers sit on critical reward/inflation paths. Tests should cover valid/invalid inputs, huge numbers, and a<b clamping.
Proposed test skeleton (place in x/event/types/utils_test.go):
package types import "testing" func TestAddBigIntString(t *testing.T) { got, err := AddBigIntString("1", "2") if err != nil || got != "3" { t.Fatalf("want 3, got %q, err=%v", got, err) } // Large numbers a := "99999999999999999999999999999999999999" b := "1" got, err = AddBigIntString(a, b) if err != nil || got != "100000000000000000000000000000000000000" { t.Fatalf("want 10^38, got %q, err=%v", got, err) } // Invalid input if _, err = AddBigIntString("abc", "1"); err == nil { t.Fatalf("expected error for invalid input") } } func TestSubBigIntString(t *testing.T) { got, err := SubBigIntString("10", "3") if err != nil || got != "7" { t.Fatalf("want 7, got %q, err=%v", got, err) } // Clamp got, err = SubBigIntString("3", "5") if err != nil || got != "0" { t.Fatalf("want 0 clamp, got %q, err=%v", got, err) } // Invalid input if _, err = SubBigIntString("xyz", "5"); err == nil { t.Fatalf("expected error for invalid input") } }If you prefer keeping string-only signatures, I can provide an alternate test suite accordingly.
x/stakework/types/module.go (1)
3-4: Use Go-style doc comments and align identifiers with module conventionsThe exported constant lacks a name-prefixed doc comment, and only ModuleName is defined here while other modules (e.g., event) expose StoreKey/RouterKey/MemStoreKey. For consistency and to simplify wiring, consider adding them now.
Apply this diff to expand and document the identifiers:
package types -// Provides basic identifiers for the module, used by other modules and systems -const ModuleName = "stakework" +// ModuleName defines the module name. +const ModuleName = "stakework" + +// StoreKey defines the primary module store key. +const StoreKey = ModuleName + +// RouterKey defines the module's message routing key (if the module handles messages). +const RouterKey = ModuleName + +// MemStoreKey defines the in-memory store key. +const MemStoreKey = "mem_stakework"x/event/types/keys.go (1)
3-15: Consider adding a KeyPrefix helper to avoid hardcoding []byte conversions everywhereThis is the idiomatic spot to provide a small helper to construct KV prefixes, which improves readability and consistency across the module.
Apply this diff to add the helper:
const ( // ModuleName defines the module name ModuleName = "event" // StoreKey defines the primary module store key StoreKey = ModuleName // RouterKey defines the module's message routing key RouterKey = ModuleName // MemStoreKey defines the in-memory store key MemStoreKey = "mem_event" ) + +// KeyPrefix returns a KVStore prefix for the provided key. +func KeyPrefix(p string) []byte { return []byte(p) }x/event/types/stake.go (1)
3-14: Document exported structs and clarify amount units/encodingPublic types should carry brief doc comments. Also, Amount is a string; please document its units and encoding (e.g., integer string in smallest Alpha unit) to avoid ambiguity when marshaling/unmarshaling and doing arithmetic.
Apply this diff to add documentation:
package types -type ValidatorStake struct { +// ValidatorStake represents a validator's stake on a given subnet. +// Amount is a string-encoded integer in Alpha's smallest unit (no decimals). +type ValidatorStake struct { Netuid uint16 `json:"netuid"` Validator string `json:"validator"` Amount string `json:"amount"` } -type Delegation struct { +// Delegation represents a staker's delegated stake to a validator on a subnet. +// Amount is a string-encoded integer in Alpha's smallest unit (no decimals). +type Delegation struct { Netuid uint16 `json:"netuid"` Validator string `json:"validator"` Staker string `json:"staker"` Amount string `json:"amount"` }If consumers perform arithmetic or comparisons on Amount, consider switching to a fixed-precision type (e.g., cosmossdk.io/math.Int or sdk.Int) to avoid parse errors and ensure safety. Example:
// Alternative (outside this diff) if adopting typed amounts: import sdkmath "cosmossdk.io/math" type ValidatorStake struct { Netuid uint16 `json:"netuid"` Validator string `json:"validator"` Amount sdkmath.Int `json:"amount"` }x/event/abi/SubnetManager.full.json (1)
1809-1812: Large embedded bytecode artifact: keep out of runtime embeds and consider relocatingThis full artifact includes deployment bytecode and significantly increases the binary/text footprint. If your runtime code only needs the ABI, ensure that .full.json files are not embedded in the production binary.
Recommendations:
- Move full artifacts into a subdirectory (e.g., x/event/abi/full/). If your //go:embed uses abi/*.json it will exclude subdirectories by default, keeping them out of the binary.
- Keep runtime embeds constrained to minimal ABI files (no bytecode).
- Add a brief README in x/event/abi/full describing usage (deployment/tests only).
If your embed currently glob-includes all *.json, adjust it (outside this diff). Example:
// In x/event/abi/abi.go (outside this file) //go:embed abi/*.json var FS embed.FS // Move SubnetManager.full.json to abi/full/SubnetManager.full.json so it isn't matched by abi/*.json.You can verify what's embedded by listing the FS at startup in a debug build or by running a scripted search in the repo for the go:embed declaration.
x/event/abi/StakingSelfNative.json (1)
76-88: Payable stake() ABI — confirm callers enforce nonzero value & denom mappingChecked the repo: stake() is payable in x/event/abi/StakingSelfNative.json; the manager contract enforces a minimumStake, but I found no Go-side call sites that pack/call stake (callers appear to be external clients/tests).
Files to review:
- x/event/abi/StakingSelfNative.json — stake() stateMutability: "payable"
- contracts/manager_contracts/contracts/staking.sol — contains require(msg.value >= minimumStake) and updates stakes
- app/app.go:678 — StakingSelfABI is loaded (abi.JSON)
- contracts/manager_contracts/test/staking.js — tests call stake({ value: stakeAmount })
- x/event/keeper/keeper.go:542 — logs "stake", event.Stake.String()
Action: If this repo sends stake() transactions, add an explicit pre-send check that the native coin amount and denom map to a nonzero tx.Value (and document the minimum stake/denom mapping for external callers). Otherwise confirm external callers adhere to these checks.
x/event/types/subnet.go (2)
18-19: Avoid magic numbers for Mechanism; define a typed enumMechanism is uint8 with comment “0=stable, 1=dynamic”. Replace with a typed enum to improve readability and prevent invalid values.
Here’s a small addition outside this hunk you can place near the top of the file:
type Mechanism uint8 const ( MechanismStable Mechanism = 0 MechanismDynamic Mechanism = 1 )And update the field:
- Mechanism uint8 `json:"mechanism" yaml:"mechanism"` + Mechanism Mechanism `json:"mechanism" yaml:"mechanism"`
85-107: MarshalJSON/UnmarshalJSON are no-ops; remove to reduce maintenanceThese methods just defer to default behavior via an alias without adding behavior. They’re redundant and can be safely removed.
Apply this diff to drop the dead code:
-// MarshalJSON implements json.Marshaler -func (s Subnet) MarshalJSON() ([]byte, error) { - type Alias Subnet - return json.Marshal(&struct { - *Alias - }{ - Alias: (*Alias)(&s), - }) -} - -// UnmarshalJSON implements json.Unmarshaler -func (s *Subnet) UnmarshalJSON(data []byte) error { - type Alias Subnet - aux := &struct { - *Alias - }{ - Alias: (*Alias)(s), - } - if err := json.Unmarshal(data, &aux); err != nil { - return err - } - return nil -} +// Default JSON marshalling is sufficient for Subnet; custom methods not required.x/event/abi/AlphaToken.full.json (2)
582-585: Consider shipping ABI-only (drop bytecode) unless you truly deploy from this artifactIncluding full bytecode in-repo increases churn and footprint, and risks accidental drift from the actually deployed contract. If the node never deploys AlphaToken and only needs the ABI for log parsing/calls, prefer an ABI-only artifact to keep provenance clean.
Apply this diff if you decide to keep only the ABI:
], - "bytecode": "0x60c0604052346103ef57...<omitted>...0033", - "networks": {} + "networks": {} }If bytecode is required elsewhere (e.g., CLI deploy tool), keep it—but ensure it’s cryptographically pinned to a source commit and compiler settings to avoid mismatches.
1-585: Add artifact provenance metadata (compiler, settings, source commit) for reproducibilityThis full artifact lacks metadata (e.g., compiler version, settings, source hash). Adding a small companion metadata JSON or README entry improves traceability for audits and future upgrades.
I can draft a short metadata schema and validation script on request.
x/blockinflation/keeper/abci.go (1)
17-19: Use structured error logging and include block height for contextPass the error object (not the string) and include the block height for easier debugging/correlation across nodes.
- k.Logger(ctx).Error("failed to mint and allocate block inflation", "error", err.Error()) + k.Logger(ctx).Error("failed to mint and allocate block inflation", "err", err, "height", ctx.BlockHeight())validator.json (1)
7-7: Use “security-contact” key for broad Cosmos CLI compatibilityCosmos create-validator JSON commonly uses “security-contact”. Using “security” may be ignored by tooling.
- "security": "validator's (optional) security contact email", + "security-contact": "validator's (optional) security contact email",x/blockinflation/types/subnet_reward.go (1)
9-14: Doc inconsistency: special-case for subnetCount=0 isn’t reflectedThe code returns 0 when subnetCount=0, but the doc-comment suggests base + k*log(1 + n). If zero subnets should produce zero reward, update the comment to avoid confusion. If not, remove the special-case.
Option A (update comment to match behavior):
-// CalculateSubnetRewardRatio calculates the subnet reward ratio based on the formula: -// subnet_reward_ratio = min( max_ratio, base + k * log(1 + subnet_count) ) +// CalculateSubnetRewardRatio calculates the subnet reward ratio: +// - If subnetCount == 0, returns 0. +// - Otherwise: subnet_reward_ratio = min(max_ratio, base + k * log(1 + subnet_count)).Option B (change behavior to always apply the formula):
- if subnetCount == 0 { - return math.LegacyZeroDec() - } + // Note: when subnetCount == 0 => log(1) == 0, so ratio == base, still clamped by max_ratio.Would you like me to align callers and tests based on the chosen behavior?
x/blockinflation/types/params_test.go (2)
14-19: Assert the constructor’s second return value instead of discarding itNewIntFromString returns a second value (bool or error depending on version). Don’t ignore it—assert it to catch malformed constants early.
If it returns (Int, bool):
- expectedTotalSupply, _ := math.NewIntFromString("21000000000000000000000000") + expectedTotalSupply, ok := math.NewIntFromString("21000000000000000000000000") + require.True(t, ok, "invalid total supply literal") @@ - expectedBlockEmission, _ := math.NewIntFromString("1000000000000000000") + expectedBlockEmission, ok := math.NewIntFromString("1000000000000000000") + require.True(t, ok, "invalid block emission literal")If it returns (Int, error):
- expectedTotalSupply, _ := math.NewIntFromString("21000000000000000000000000") + expectedTotalSupply, err := math.NewIntFromString("21000000000000000000000000") + require.NoError(t, err) @@ - expectedBlockEmission, _ := math.NewIntFromString("1000000000000000000") + expectedBlockEmission, err := math.NewIntFromString("1000000000000000000") + require.NoError(t, err)
21-31: Consider asserting additional default params to prevent silent driftAdd checks for SubnetRewardBase, SubnetRewardK, SubnetRewardMaxRatio, SubnetMovingAlpha, and SubnetOwnerCut so param changes don’t slip in unnoticed.
I can push a follow-up test block covering those values if you’d like, using LegacyDec equality via String().
x/blockinflation/keeper/alpha_token.go (4)
38-41: Parse ABI once; avoid per-call overhead and parse failures at runtimeParsing the ABI on every call is unnecessary and adds failure surface. Cache the parsed ABI (e.g., via
sync.Once) and reuse it.Apply within this function:
- // 4. Get the AlphaToken ABI - alphaTokenABI, err := abi.JSON(strings.NewReader(string(eventabi.AlphaTokenABI))) - if err != nil { - return fmt.Errorf("failed to parse AlphaToken ABI: %w", err) - } + // 4. Get the AlphaToken ABI (cached) + alphaTokenABI, err := getAlphaTokenABI() + if err != nil { + return fmt.Errorf("failed to load AlphaToken ABI: %w", err) + }Add this helper in the same package (outside the selected range):
package keeper import ( "bytes" "sync" "github.com/ethereum/go-ethereum/accounts/abi" eventabi "github.com/hetu-project/hetu/v1/x/event/abi" ) var ( alphaABIOnce sync.Once alphaABIVal abi.ABI alphaABIValErr error ) func getAlphaTokenABI() (abi.ABI, error) { alphaABIOnce.Do(func() { alphaABIVal, alphaABIValErr = abi.JSON(bytes.NewReader(eventabi.AlphaTokenABI)) }) return alphaABIVal, alphaABIValErr }
49-51: Confirm unit semantics and consider widening the amount typeAmount is
uint64but minted ERC-20 tokens typically use 18 decimals, and emissions can exceed 2^64 in aggregate over time. Either:
- Clarify that
amountis in base units and guaranteed to fit in uint64, or- Introduce a big-int overload and have this helper delegate to it.
Example (outside the selected range):
// New helper func (k Keeper) MintAlphaTokensBig(ctx sdk.Context, netuid uint16, recipient string, amount *big.Int) error { /* same body, skip the SetUint64 */ } // Keep the current API, delegate to big-int func (k Keeper) MintAlphaTokens(ctx sdk.Context, netuid uint16, recipient string, amount uint64) error { return k.MintAlphaTokensBig(ctx, netuid, recipient, new(big.Int).SetUint64(amount)) }Also decide whether zero-amount mints should be rejected; if yes, add a guard before the call.
53-64: Consider surfacing/logging the tx hash or EVM call resultIf
CallEVMreturns a receipt/tx hash, include it in logs to aid observability and postmortems. Today the result is discarded with_.If the interface returns a type with
HashorTxHash, extend the log:res, err := k.erc20Keeper.CallEVM(/*...*/) if err != nil { /*...*/ } k.Logger(ctx).Info("Minted alpha tokens", "tx_hash", res.TxHash.Hex(), /* ... */)
17-76: Prefer Cosmos SDK error wrapping utilities for consistencyFor consistency with the rest of the codebase and better error introspection, consider
errorsmod.Wrap/Wrapf(or the project’s errors package) instead of plainfmt.Errorf.I can provide a patch switching to
cosmossdk.io/errorsonce we confirm the preferred errors package in this repo.x/stakework/keeper/keeper.go (1)
14-19: Fix comment and align naming with module"Keeper simplified yuma keeper" looks like a leftover. Update to reflect the stakework module.
-// Keeper simplified yuma keeper +// Keeper is the stakework module keeperx/blockinflation/types/emission.go (1)
7-18: Document units/semantics of amounts to prevent ambiguityAll numeric fields are
math.Int, but it’s unclear if they’re base units (10^decimals), token units, per-block, per-epoch, or cumulative. Add doc comments to avoid misinterpretation downstream and in CLI output.-// SubnetEmissionData represents emission data for a subnet +// SubnetEmissionData represents emission data for a subnet. +// All amounts are in base units (e.g., 10^decimals) unless otherwise specified. +// Fields represent per-block emissions for the current block unless noted as cumulative. type SubnetEmissionData struct {README.md (1)
108-113: Optional: Add punctuation and parallel phrasing for bulletsMinor readability polish: end each bullet with a period and keep phrasing parallel.
x/blockinflation/keeper/genesis.go (2)
15-20: Reduce noisy logging; use consistent levels and remove “DEBUG” prefixesKeep a single Info summary at the end and move detailed key-value state to Debug logs without "DEBUG " prefixes.
Apply this diff:
- // Add debug information - k.Logger(ctx).Info("DEBUG InitGenesis: Setting params", "params", data.Params) + // Debug: Params being set + k.Logger(ctx).Debug("InitGenesis: setting params", "params", data.Params) // Verify parameters were set correctly params := k.GetParams(ctx) - k.Logger(ctx).Info("DEBUG InitGenesis: Retrieved params", "params", params) + k.Logger(ctx).Debug("InitGenesis: retrieved params", "params", params) @@ - k.Logger(ctx).Debug("DEBUG InitGenesis Params", "params", data.Params) - k.Logger(ctx).Info("initialized blockinflation genesis state", + k.Logger(ctx).Debug("InitGenesis: params", "params", data.Params) + k.Logger(ctx).Info("blockinflation: initialized genesis state", "total_issuance", data.TotalIssuance.String(), "total_burned", data.TotalBurned.String(), "pending_subnet_rewards", data.PendingSubnetRewards.String(), )Also applies to: 35-41
44-52: Minor: construct and return genesis directlyUsing DefaultGenesisState then overriding fields allocates unnecessarily. Construct directly for clarity.
Apply this diff:
-func (k Keeper) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) *blockinflationtypes.GenesisState { - genesis := blockinflationtypes.DefaultGenesisState() - genesis.Params = k.GetParams(ctx) - genesis.TotalIssuance = k.GetTotalIssuance(ctx) - genesis.TotalBurned = k.GetTotalBurned(ctx) - genesis.PendingSubnetRewards = k.GetPendingSubnetRewards(ctx) - - return genesis -} +func (k Keeper) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) *blockinflationtypes.GenesisState { + return &blockinflationtypes.GenesisState{ + Params: k.GetParams(ctx), + TotalIssuance: k.GetTotalIssuance(ctx), + TotalBurned: k.GetTotalBurned(ctx), + PendingSubnetRewards: k.GetPendingSubnetRewards(ctx), + } +}x/event/types/genesis.go (1)
6-12: Optional: add yaml tags for genesis parity with CLI/export toolingIf your toolchain relies on YAML (e.g.,
initscaffolding, docs), add yaml tags mirroring json tags to keep outputs consistent.x/event/abi/abi.go (1)
3-3: Import “embed” (not as a blank import) for clarity
go:embedworks with a normal import of "embed". Using a blank import is unusual here and may confuse readers.Apply this diff:
-import _ "embed" +import "embed"x/blockinflation/types/codec.go (1)
18-27: Placeholders: register concrete types when addedAs messages/types are introduced (e.g., keeper msgs, events), wire them into RegisterLegacyAminoCodec and RegisterInterfaces.
x/stakework/README.md (2)
7-15: Minor editorial pass for clarity and parallelism in the feature list.Tighten phrasing for consistency.
Apply this diff:
-- **Epoch Algorithm**: Complete implementation of Bittensor epoch calculation logic -**Weight Management**: Handles weight allocation and updates between validators -**Consensus Calculation**: Consensus mechanism based on weighted median -**Reward Distribution**: Distributes network rewards based on consensus results -**Bonds System**: Implements exponential moving average of historical weights -**Dynamic Alpha**: Supports dynamically adjustable EMA parameters +- **Epoch Algorithm**: Full implementation of Bittensor’s epoch calculation +- **Weight Management**: Allocation and updates between validators +- **Consensus Calculation**: Weighted-median–based consensus +- **Reward Distribution**: Rewards allocated from consensus results +- **Bonds System**: Exponential moving average (EMA) of historical weights +- **Dynamic Alpha**: Dynamically adjustable EMA parameters
35-51: Zero-sum normalization behavior is documented; consider noting why zeros are returned.Returning all zeros when sum == 0 is fine; add a one-liner to clarify that this preserves a neutral state without introducing bias.
init.sh (1)
9-11: Remove or use LOGLEVEL; currently unusedShellcheck flags LOGLEVEL as unused. Either use it or drop it to avoid confusion.
-LOGLEVEL="info" -LOGLEVEL2="debug" +LOGLEVEL="debug" @@ -hetud start --pruning=nothing $TRACE --log_level "$LOGLEVEL2" --minimum-gas-prices=0.0001ahetu --home "$HETUD_HOME" --chain-id "$CHAINID" +hetud start --pruning=nothing $TRACE --log_level "$LOGLEVEL" --minimum-gas-prices=0.0001ahetu --home "$HETUD_HOME" --chain-id "$CHAINID"If you need both, keep both but use LOGLEVEL somewhere meaningful (e.g., for ABCI logs) and rename for clarity.
x/blockinflation/keeper/inflation.go (2)
101-106: Replace ad-hoc test logs with consistent structured logsMessages like “Test whether the subnet was successfully obtained” in production code are noisy. Prefer debug logs with clear context or remove.
-k.Logger(ctx).Info("Test whether the subnet was successfully obtained", - "subnetCount", subnetCount, -) +k.Logger(ctx).Debug("fetched subnet count", "subnet_count", subnetCount)Apply similarly to the subnet reward ratio log below.
100-112: Verify EventKeeper provides a cheap subnet count
len(k.eventKeeper.GetAllSubnetNetuids(ctx))allocates the full slice just to count. If counts are used frequently in BeginBlock, consider EventKeeper.SubnetCount(ctx) to avoid allocations and iterating.If no such method exists, I can add it to the expected keeper and wire a KV-length scan in the event module for O(1) count.
x/blockinflation/keeper/rewards.go (1)
37-43: Potential precision loss in TAO reward calculation.Converting to
LegacyNewDecFromIntthen back toTruncateIntmay lead to precision loss. Consider using integer arithmetic throughout if possible, or document the acceptable precision loss.- var taoIn math.Int - if totalMovingPrices.IsZero() { - taoIn = math.ZeroInt() - } else { - taoInRatio := movingPrice.Quo(totalMovingPrices) - taoIn = math.LegacyNewDecFromInt(blockEmission).Mul(taoInRatio).TruncateInt() - } + var taoIn math.Int + if totalMovingPrices.IsZero() { + taoIn = math.ZeroInt() + } else { + // Use higher precision calculation to minimize rounding errors + taoInRatio := movingPrice.QuoRoundUp(totalMovingPrices) + taoIn = math.LegacyNewDecFromInt(blockEmission).Mul(taoInRatio).RoundInt() + }app/app.go (3)
609-614: Remove commented-out code instead of keeping it.The commented-out epochs keeper initialization code should be removed entirely rather than left as comments. Use version control history to track removed code.
- // epochsKeeper := epochskeeper.NewKeeper(appCodec, keys[epochstypes.StoreKey]) - // app.EpochsKeeper = *epochsKeeper.SetHooks( - // epochskeeper.NewMultiEpochHooks( - // app.InflationKeeper.Hooks(), - // ), - // )
674-715: Improve error handling consistency for ABI parsing.The ABI parsing uses
panicfor failures, which is appropriate during initialization. However, the error messages could be more descriptive by including the actual error details.subnetRegistryABI, err := abi.JSON(strings.NewReader(string(eventabi.SubnetRegistryABI))) if err != nil { - panic(fmt.Sprintf("failed to parse SubnetRegistry ABI: %v", err)) + panic(fmt.Sprintf("failed to parse SubnetRegistry ABI: %v\nABI content length: %d", err, len(eventabi.SubnetRegistryABI))) }
717-733: Remove debug print statement.The commented debug print statement should be removed from production code.
subspace := app.GetSubspace(blockinflationtypes.ModuleName) - // fmt.Printf("DEBUG: blockinflation subspace is zero? %v, hasKeyTable? %v\n", reflect.ValueOf(subspace).IsZero(), subspace.HasKeyTable()) app.BlockInflationKeeper = blockinflationkeeper.NewKeeper(x/blockinflation/types/query.go (1)
45-55: Consider implementing actual gRPC functionality.The placeholder implementations currently return unimplemented errors. This is acceptable for initial scaffolding, but actual implementations will be needed for the module to be functional.
Would you like me to help implement the actual gRPC query handlers that connect to the keeper methods?
init_validators.sh (2)
12-15: Validate exact number of validator IPs to avoid silent misconfigurationsRequiring “at least” 5 IPs allows accidental extra args. Prefer exact count to prevent miswiring.
-if [ "$#" -lt 5 ]; then +if [ "$#" -ne 5 ]; then echo "Usage: $0 <validator0_ip> <validator1_ip> <validator2_ip> <validator3_ip> <validator4_ip>" echo "Example: $0 1.2.3.4 5.6.7.8 9.10.11.12 13.14.15.16 17.18.19.20" exit 1 fi
80-81: Hardcoding remote user “root” is risky and environment-specificParameterize SSH/rsync user (e.g., SSH_USER env var, default “root” or detect) to avoid forcing privileged operations and to work across environments.
- ssh root@${TARGET_IP} "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true" + SSH_USER="${SSH_USER:-root}" + ssh "${SSH_USER}@${TARGET_IP}" "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true" @@ - ssh root@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" + SSH_USER="${SSH_USER:-root}" + ssh "${SSH_USER}@${TARGET_IP}" "rm -rf ${HOME_PREFIX}${i}" @@ - rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/" + rsync -av "${HOME_PREFIX}${i}/" "${SSH_USER}@${TARGET_IP}:${HOME_PREFIX}${i}/"Also applies to: 296-300
x/blockinflation/keeper/coinbase.go (1)
322-336: getStakeMap: handle invalid amounts and consider returning math.Intnew(big.Int).SetString may fail; the code ignores it and treats as zero. At least log when parsing fails. Also, stakeMap isn’t used downstream—either use it in weighting or remove to avoid confusion.
- if s.Validator == acc { - amount, _ := new(big.Int).SetString(s.Amount, 10) - stakeMap[acc] += amount.Uint64() - } + if s.Validator == acc { + if amount, ok := new(big.Int).SetString(s.Amount, 10); ok { + stakeMap[acc] += amount.Uint64() + } else { + k.Logger(ctx).Error("invalid stake amount string", "netuid", netuid, "validator", acc, "amount", s.Amount) + } + }If the intent is to weight dividends by stake, incorporate stakeMap into the dividend allocation calculation.
x/event/abi/NeuronManager.full.json (1)
1-716: Verify ABI-bytecode sync and license/solc metadata before embeddingFull artifacts are useful for deployment, but stale bytecode vs ABI can cause on-chain call failures. Confirm:
- bytecode corresponds to this ABI (same commit/solc version),
- expected compiler version (0.8.28?) matches “solc” metadata in bytecode,
- no redundant duplication with NeuronManager.json if only the ABI is used at runtime.
If not required for runtime, consider keeping only the ABI to reduce repo bloat.
x/blockinflation/types/params.go (2)
34-37: Units in comments still reference RAO; update to aHETU for consistency.Docs still say RAO and 1,000,000,000 RAO while defaults use 10^18 precision for aHETU. Update to avoid confusion.
Apply this doc-only diff:
- // TotalSupply defines the maximum total supply (21,000,000,000,000,000 RAO) + // TotalSupply defines the maximum total supply (21,000,000 aHETU with 10^18 precision) @@ - // DefaultBlockEmission defines the default block emission (1,000,000,000 RAO) + // DefaultBlockEmission defines the default block emission (1 aHETU per block, 10^18 aHETU)
185-194: Consider bounding SubnetRewardK to [0,1].Other ratio params are clamped to [0,1], but K is only constrained to non-negative. If K>1 is not intended, add a ≤1 guard for consistency with the spec.
Would you like me to submit a patch adding v.GT(math.LegacyOneDec()) validation, and adjust tests accordingly?
x/blockinflation/keeper/keeper.go (1)
146-167: Return an error from AddToPendingSubnetRewards on invalid denom.Currently logs and returns silently. Callers can’t detect failure. Consider returning an error to propagate handling, or at least increment a metric.
If you’d like, I can prepare a follow-up patch to change the signature to (sdk.Context, sdk.Coin) error and adjust call sites.
x/event/module.go (2)
52-55: Consider registering interfaces/legacy amino for types.Unlike blockinflation, Event’s AppModuleBasic leaves RegisterInterfaces/RegisterLegacyAminoCodec empty. If the module defines protobuf interfaces or requires legacy Amino for CLI, hook them up via types.RegisterInterfaces / types.RegisterLegacyAminoCodec.
I can wire these up if you confirm the event/types packages to register.
167-173: Remove unused simulation scaffolding or align signature.WeightedOperations and RegisterStoreDecoder are non-standard here and return []interface{}. If simulation isn’t used, consider removing for clarity; otherwise align to the SDK’s simulation types.
x/blockinflation/module.go (1)
67-71: Add gRPC gateway routes and tx commands when ready.Stubs are fine for now. When APIs stabilize, wire the query/tx services and gateway routes for full CLI/REST access.
Also applies to: 72-80
x/blockinflation/types/expected_keepers.go (2)
46-49: Consider deriving halvingBlocks from stored subnet params to avoid duplicated config inputsUpdateMovingPrice accepts halvingBlocks even though Subnet has EMAPriceHalvingBlocks. Prefer sourcing from state to minimize call-site inconsistencies.
72-73: Validator stake amounts as strings may add parsing overheadGetAllValidatorStakesByNetuid returns []eventtypes.ValidatorStake where Amount is a string. If BlockInflation requires math.Int, consider exposing a math.Int accessor to avoid repeated parsing.
x/event/abi/GlobalStaking.full.json (1)
1-12: Large full artifact inclusion—consider slimming or gating in dev/test buildsIncluding full bytecode artifacts grows repo size. If only the ABI is needed at runtime, consider:
- Keeping .json (ABI-only) for runtime embedding.
- Gating .full.json (bytecode) behind build tags or moving to testdata/ if only used in tests.
Also applies to: 575-578
x/event/abi/SubnetManager.json (2)
46-75: Indexed string in NetworkConfigUpdated will be topic-hashedparamName is indexed as a string—Solidity indexes string topics via keccak256, so consumers can’t recover the raw string from topics alone. Ensure indexers also read event data for the clear-text or pre-hash the param when filtering.
377-388: DuplicategetNextNetuidandnextNetuidaccessors — please verify intended API surfaceI confirmed both accessors appear in generated ABI artifacts (one is likely an explicit function, the other an autogenerated public var getter). This increases the public API surface; please confirm consumers aren’t unintentionally using both.
Files/locations found:
- x/event/abi/SubnetManager.json — getNextNetuid (line 378), nextNetuid (line 931)
- x/event/abi/SubnetManager.full.json — getNextNetuid (380), nextNetuid (933)
- x/event/abi/all-contracts.json — getNextNetuid (380), nextNetuid (933)
- x/event/abi/contracts-localhost.json — getNextNetuid (1841), nextNetuid (2394)
- x/event/abi/contracts-customchain.json — getNextNetuid (1841), nextNetuid (2394)
- x/event/abi/SubnetRegistry.json — nextNetuid (line 47) (only the var getter present)
If the duplication is unintended, remove/rename the explicit accessor in source and regenerate ABIs; otherwise, document the overlap so callers know which to use.
x/event/keeper/keeper.go (2)
718-745: Defensive programming against nil store is goodThe
GetAllSubnetsfunction properly checks for nil storeKey and KVStore, which is excellent defensive programming. However, consider whether returning an empty slice might mask initialization issues.Consider panicking or returning an error to make initialization issues more visible during development:
func (k Keeper) GetAllSubnets(ctx sdk.Context) []types.Subnet { // Add debug information if k.storeKey == nil { - k.Logger(ctx).Error("GetAllSubnets: storeKey is nil") - return []types.Subnet{} + panic("GetAllSubnets: storeKey is nil - keeper not properly initialized") }
694-696: Helper function could be more robustThe
uint16ToBytesfunction uses a simple byte conversion that could be replaced with a more standard approach.Use standard library encoding for better maintainability:
+import "encoding/binary" + func uint16ToBytes(u uint16) []byte { - return []byte{byte(u >> 8), byte(u)} + b := make([]byte, 2) + binary.BigEndian.PutUint16(b, u) + return b }
There was a problem hiding this comment.
Actionable comments posted: 82
🔭 Outside diff range comments (6)
app/ante/cosmos/vesting.go (1)
85-115: Consider extending validation to MsgCreateValidator (initial self-bond).Currently only MsgDelegate is validated. MsgCreateValidator includes an initial self-delegation (Value) and DelegatorAddress; without validating it, an account can create a validator using unvested coins and bypass this ante.
I can provide a small refactor to handle both MsgDelegate and MsgCreateValidator by extracting a common “delegator address + amount” from staking messages and reusing the same validation. Do you want me to draft that patch?
README.md (1)
126-131: Correct repo links and unify docs repo namingThe PR/Issues links should target hetu-chain, and the docs repo naming should be consistent with the earlier "docs" link.
-- Cloning code repo and opening a [PR](https://github.com/hetu-project/hetu/pulls). -- Submitting feature requests or [bugs](https://github.com/hetu-project/hetu/issues). -- Improving our product or contribution [documentation](https://github.com/hetu-project/hetu-docs). +- Cloning code repo and opening a [PR](https://github.com/hetu-project/hetu-chain/pulls). +- Submitting feature requests or [bugs](https://github.com/hetu-project/hetu-chain/issues). +- Improving our product or contribution [documentation](https://github.com/hetu-project/docs). @@ -For additional instructions, standards and style guides, please refer to the [Contributing](https://github.com/hetu-project/hetu-docs) document. +For additional instructions, standards and style guides, please refer to the [Contributing](https://github.com/hetu-project/docs) document.x/evm/keeper/keeper.go (1)
325-334: Use a pointer receiver for GetNonce to avoid copying the Keeper and to keep method-set consistencyValue receiver here copies the Keeper (a large struct) and may cause interface satisfaction or method-set surprises. Most keeper methods use pointer receivers for state access and uniformity.
Apply this diff:
-// GetNonce returns the sequence number of an account, returns 0 if not exists. -func (k Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 { +// GetNonce returns the sequence number of an account, returns 0 if not exists. +func (k *Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 {init_validators.sh (2)
199-199: Inconsistent min gas denom with evm_denom/ahetuYou set evm_denom to ahetu (Line 111) but minimum-gas-prices remains in “gas”. This will cause fee denomination mismatches on mempool checks and can make TXs fail to be accepted.
-sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001gas"/g' "$APP_TOML" +sed -i.bak 's/^minimum-gas-prices *=.*/minimum-gas-prices = "0.0001ahetu"/g' "$APP_TOML"
153-154: Brittle bank.supply updates by fixed indexes; update by denom insteadHard-coding [0] and [1] assumes a particular ordering/length of the supply array and can silently corrupt genesis. Update by matching denom.
-jq -r --arg total_supply "$total_supply" '.app_state["bank"]["supply"][0]["amount"]=$total_supply' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" -jq -r --arg total_supply "$total_supply" '.app_state["bank"]["supply"][1]["amount"]=$total_supply' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" +jq -r --arg denom "$DENOM" --arg total_supply "$total_supply" \ + '.app_state["bank"]["supply"] |= (map(if .denom == $denom then .amount = $total_supply else . end))' \ + "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS"If you also keep a “gas” balance around, apply a second line for denom "gas". Otherwise, consider removing “gas” entirely for consistency.
app/app.go (1)
769-777: Event module implemented but not registered in ModuleManager — add itx/event implements module.AppModule (it provides InitGenesis/ExportGenesis) and app.EventKeeper is created and wired as the EVM event handler, but the event module is not included in app.mm. Registering it ensures its genesis is applied and the module basics are wired.
Files to update:
- app/app.go — add the event module to app.mm (insert after erc20.NewAppModule).
- app/app.go — add import for the event package.
Suggested diff:
@@ - erc20.NewAppModule(app.Erc20Keeper, app.AccountKeeper, - app.GetSubspace(erc20types.ModuleName)), + erc20.NewAppModule(app.Erc20Keeper, app.AccountKeeper, + app.GetSubspace(erc20types.ModuleName)), + event.NewAppModule(appCodec, *app.EventKeeper), @@ import ( @@ - eventkeeper "github.com/hetu-project/hetu/v1/x/event/keeper" + eventkeeper "github.com/hetu-project/hetu/v1/x/event/keeper" + event "github.com/hetu-project/hetu/v1/x/event" )If the omission was intentional, please explain how the event module's genesis and any query/service surfaces are expected to be exposed externally.
🧹 Nitpick comments (83)
x/event/types/utils.go (4)
7-7: Clarify doc comments to document base-10 and invalid-input behaviorDocstrings should reflect base-10 parsing and how invalid inputs are handled (now defaulting to 0), and that subtraction clamps to "0".
-// AddBigIntString adds two big integer strings +// AddBigIntString adds two base-10 integer strings a and b and returns the sum as a base-10 string. +// Invalid inputs are treated as 0. @@ -// SubBigIntString subtracts two big integer strings +// SubBigIntString subtracts b from a for base-10 integer strings and returns the result as a base-10 string. +// If a < b, the result is clamped to "0". Invalid inputs are treated as 0.Also applies to: 15-15
3-5: Optionally trim inputs to be more forgivingIf these helpers may receive inputs with surrounding whitespace, trim before parsing to reduce surprises.
Apply this diff and add the import:
import ( "math/big" + "strings" ) @@ - aInt, ok := new(big.Int).SetString(a, 10) + aInt, ok := new(big.Int).SetString(strings.TrimSpace(a), 10) @@ - bInt, ok := new(big.Int).SetString(b, 10) + bInt, ok := new(big.Int).SetString(strings.TrimSpace(b), 10)Also applies to: 9-10, 17-18
1-25: Consider using Cosmos SDK math types for consistencyWithin a Cosmos SDK module, sdk-int types provide safer, canonical behavior and better integration with the rest of the stack.
Alternative using cosmossdk.io/math (sdkmath):
- Parse: x, ok := sdkmath.NewIntFromString(a); y, ok := sdkmath.NewIntFromString(b)
- Add/Sub: x.Add(y), x.Sub(y)
- String: .String()
This avoids direct math/big usage and gives you typed invariants aligned with coin amounts across modules.
1-25: Add table-driven tests to lock behaviorEdge cases need tests: invalid strings, large values, leading zeros, negative inputs, and subtraction underflow.
I can add x/event/types/utils_test.go with cases like:
- ("0", "0") → "0"
- ("1", "2") add → "3"; sub → "0" (clamped)
- ("999999999999999999999999", "1") add → "1000000000000000000000000"
- ("abc", "5") add → "5"; sub → "0"
- ("-5", "3") add → "-2" (if allowed); sub → "0" (given clamp)
Do you want me to open a follow-up PR with these tests?x/stakework/types/module.go (2)
4-4: Add GoDoc-style comment for exported identifier.Exported constants should have comments that start with the identifier name to satisfy linters and maintain consistency.
Apply this diff:
-// Provides basic identifiers for the module, used by other modules and systems -const ModuleName = "stakework" +// ModuleName is the canonical identifier for the stakework module and is used by other modules and systems. +const ModuleName = "stakework"
4-4: Consider adding StoreKey/RouterKey/MemStoreKey alongside ModuleName.Following Cosmos SDK conventions, defining StoreKey, RouterKey, and MemStoreKey here avoids duplication and improves consistency across the module wiring.
Example:
-const ModuleName = "stakework" +const ( + // ModuleName is the canonical identifier for the stakework module. + ModuleName = "stakework" + // StoreKey is the KVStore key for the stakework module. + StoreKey = ModuleName + // RouterKey is the message route for the stakework module. + RouterKey = ModuleName + // MemStoreKey is the in-memory store key for the stakework module. + MemStoreKey = "mem_" + ModuleName +)x/stakework/abci.go (2)
16-18: Return nil instead of an empty slice.Idiomatic Go for “no updates” is nil to avoid an unnecessary allocation.
Apply this diff:
-func EndBlock(ctx sdk.Context) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} +func EndBlock(ctx sdk.Context) []abci.ValidatorUpdate { + return nil +}
8-15: Improve doc comment to GoDoc style and remove redundant block comment.Use a single GoDoc-style comment that starts with EndBlock to satisfy linters.
Apply this diff:
-// EndBlock runs at the end of each block -/* -This file implements the Cosmos SDK's ABCI (Application Blockchain Interface) interface: -Purpose: -EndBlock: Called at the end of each block, used to execute logic at block end -Currently a simplified implementation, only returns an empty validator update list -Can add cleanup work, state updates, etc. at block end here -*/ +// EndBlock runs at the end of each block. This placeholder currently returns no validator updates. +// Future end-of-block logic (e.g., cleanup, state updates) can be added here or in AppModule.EndBlock.x/event/types/stake.go (2)
9-14: Naming collision risk with “Delegation”; clarify intent or rename to avoid confusion with x/staking Delegation.Because Cosmos SDK also has a Delegation type, cross-module imports may become ambiguous. Consider a more specific name (e.g., SubnetDelegation/EventDelegation).
Also apply the same amount-type suggestion as above, and document address formats (staker vs validator, bech32 HRPs).
3-14: Add basic validation helpers to prevent invalid state from entering the system.Introduce ValidateBasic methods for both structs.
Example (outside this hunk):
func (v ValidatorStake) ValidateBasic() error { if v.Validator == "" { return fmt.Errorf("validator address required") } if v.Amount.IsZero() || v.Amount.IsNegative() { return fmt.Errorf("amount must be positive") } return nil } func (d Delegation) ValidateBasic() error { if d.Validator == "" || d.Staker == "" { return fmt.Errorf("validator and staker addresses required") } if d.Amount.IsZero() || d.Amount.IsNegative() { return fmt.Errorf("amount must be positive") } return nil }Note: if you keep string amounts, these validators should parse amounts and enforce a denom.
x/event/types/weight.go (1)
3-7: Clarify units and normalization for weight values.Document whether weights are normalized (e.g., sum to 1e18), bounded, and whether 0 is allowed. This helps downstream modules (e.g., BlockInflation) interpret values correctly.
Add a brief comment above the struct explaining invariants and expected key format (bech32 addresses).
validator.json (3)
2-2: Gitleaks false positive: ed25519 public key is not a secret.The “pubkey.key” here is a public key, not a private key or API token. Consider adding an allowlist rule for this path/pattern to avoid noisy CI failures.
If you want, I can draft a gitleaks allowlist snippet for validator.json and for ed25519 public-key patterns.
13-13: Add trailing newline and remove trailing whitespace.Some linters/tools expect a newline at EOF and no trailing spaces.
Apply this diff:
-} +}
3-12: Optional: sanity-check that min-self-delegation ≤ amount and address HRPs are correct.
- min-self-delegation equals amount; that’s OK but leaves no headroom.
- Ensure validator/staker address HRPs align with chain config (e.g., hetuvaloper for operator keys if used elsewhere).
Want a quick script to validate HRPs and fields via the CLI template you’re using?
x/event/types/params.go (4)
9-9: Fix misleading comment for max_allowed_uids"max_allowed_uids" governs the max UIDs (per subnet), not "subnet count". Adjust the comment to avoid confusion for operators and tooling.
- "max_allowed_uids": "4096", // Default maximum allowed subnet count + "max_allowed_uids": "4096", // Default maximum allowed UIDs per subnet
20-20: Tighten wording for weights_set_rate_limit commentThe parenthetical reference is unnecessary and potentially confusing. Keep the comment concise and aligned with the key name.
- "weights_set_rate_limit": "1000", // Default weight setting rate limit (corresponds to weights_rate_limit) + "weights_set_rate_limit": "1000", // Default weights set rate limit
32-36: Disambiguate alpha/alpha_high/alpha_low vs new alpha/delta parametersYou now have alpha_enabled/alpha_high/alpha_low and also alpha/delta/alpha_sigmoid_steepness. Make sure downstream consumers don’t conflate these and that names are consistent with Stakework terminology. At minimum, document their distinct roles here.
Would you like me to add clarifying comments for these three Stakework epoch params to prevent confusion?
5-37: Consider key constants and a typed view to prevent drift and typosUsing raw strings for keys risks misspellings and cross-module drift. Consider introducing key constants and (optionally) a typed struct view that can be parsed from this map, centralizing validation and unit semantics.
Example approach (sketch, can implement if you want):
- Define const keys and a small parser that converts string values into typed fields with validation.
- Maintain DefaultParamsMap() as the wire-defaults for EVM events, but provide a helper DefaultTypedParams() for local consumers.
x/event/abi/Weights.json (1)
70-86: Name getter inputs for readability and tooling convenienceInput names in the weights getter are empty. While optional, meaningful names improve generated bindings and developer ergonomics.
"inputs": [ { "internalType": "uint16", - "name": "", + "name": "netuid", "type": "uint16" }, { "internalType": "address", - "name": "", + "name": "validator", "type": "address" }, { "internalType": "address", - "name": "", + "name": "dest", "type": "address" } ],x/blockinflation/types/codec.go (2)
8-16: Initialize and register interfaces once; share the registry across ModuleCdcBest practice: construct a single interface registry, register interfaces in init, and reuse it for ModuleCdc. This avoids missing registrations if/when interfaces are added later.
var ( - amino = codec.NewLegacyAmino() - // ModuleCdc references the global blockinflation module codec. Note, the codec should - // ONLY be used in certain instances of tests and for JSON encoding. - ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) - - // AminoCdc is a amino codec created to support amino JSON compatible msgs. - AminoCdc = codec.NewAminoCodec(amino) + amino = codec.NewLegacyAmino() + // interfaceRegistry is shared between ModuleCdc and RegisterInterfaces. + interfaceRegistry = codectypes.NewInterfaceRegistry() + // ModuleCdc references the global blockinflation module codec. Note, the codec should + // ONLY be used in certain instances of tests and for JSON encoding. + ModuleCdc = codec.NewProtoCodec(interfaceRegistry) + // AminoCdc is an amino codec created to support amino JSON compatible msgs. + AminoCdc = codec.NewAminoCodec(amino) ) + +// Ensure interface and amino registrations happen on module import. +func init() { + RegisterInterfaces(interfaceRegistry) + RegisterLegacyAminoCodec(amino) +}
18-27: Leave a targeted TODO for future registrationsThe no-op RegisterInterfaces/RegisterLegacyAminoCodec are fine initially—add a TODO to update when msg/query types arrive to prevent accidental omissions later.
x/blockinflation/types/emission.go (1)
7-18: Add short field docs and ensure zero-safety when constructingTwo small improvements:
- Add one-liners describing “In” vs “Out” semantics and units (Alpha/Tao native denomination, per-block vs per-epoch).
- Ensure constructors (where you build this struct) initialize math.Int with math.ZeroInt() to avoid nil-int surprises if any fields are left unset.
If you’d like, I can add a small helper like NewSubnetEmissionData() and update keeper call sites accordingly.
README.md (1)
57-65: Add checksum verification for tarballsSupply a SHA256 verification step to improve supply-chain hygiene.
# Download the binary for your platform wget https://github.com/hetu-project/hetu-chain/releases/download/v0.x.x/hetud-v0.x.x-linux-amd64.tar.gz +wget https://github.com/hetu-project/hetu-chain/releases/download/v0.x.x/hetud-v0.x.x-linux-amd64.tar.gz.sha256 + +# Verify checksum +sha256sum -c hetud-v0.x.x-linux-amd64.tar.gz.sha256 # Extract the binary tar -xzf hetud-v0.x.x-linux-amd64.tar.gzx/event/abi/SubnetAMM.full.json (1)
982-984: Avoid committing heavyweight bytecode in full ABIs to reduce repository bloat.The .full.json includes full bytecode, which inflates the repo and increases load times. For runtime decoding, the minimal ABI (without bytecode) is sufficient.
Apply a policy:
- Embed and use only the minimal ABI JSONs at runtime.
- Keep “full” artifacts in a release/artifacts directory excluded from embedding and optional in source control, or distribute via package registry.
- Update x/event/abi/abi.go to embed only *.json (not *.full.json).
x/stakework/README.md (5)
23-28: Exported/unexported method naming inconsistency (Go).Example shows shouldRunEpoch with a lowercase s, while other references and public usage imply an exported method (ShouldRunEpoch).
Apply this diff for consistency in the README:
-func (k Keeper) shouldRunEpoch(ctx sdk.Context, netuid uint16, tempo uint64) bool { +func (k Keeper) ShouldRunEpoch(ctx sdk.Context, netuid uint16, tempo uint64) bool {
35-51: Float equality check is brittle; use an epsilon tolerance.sum == 0 is fragile with floats and may lead to unexpected behavior.
Update the example to use an epsilon:
-func (k Keeper) normalize(values []float64) []float64 { +func (k Keeper) normalize(values []float64) []float64 { sum := 0.0 for _, v := range values { sum += v } - - if sum == 0 { + + const eps = 1e-12 + if math.Abs(sum) < eps { return make([]float64, len(values)) }And add:
+import "math"
137-139: Naming consistency in examples (exported getters).Examples use getSubnetValidators and calculateActive with lowercase initial letters; if these are public APIs, capitalize them.
-validators := k.getSubnetValidators(ctx, netuid) +validators := k.GetSubnetValidators(ctx, netuid)-active := k.calculateActive(ctx, netuid, validators, params) +active := k.CalculateActive(ctx, netuid, validators, params)Also applies to: 143-145
147-151: Add a determinism note in README to set expectations for contributors.Given the consensus-sensitive nature, explicitly warn against non-deterministic constructs (floats, maps without ordering, nondeterministic iteration).
Proposed addition:
+## Determinism Note + +Stakework logic executed within the Cosmos state machine must be fully deterministic. +Avoid float64 and platform-dependent behavior in on-chain code; prefer sdk.Dec or fixed-point math. +When iterating maps or sets, ensure stable ordering.
9-15: Minor grammar/punctuation improvements for bullets.Hyphenated phrases in bullets can be tightened for clarity (e.g., remove repeated “Consensus” and ensure parallel structure).
I can push a small PR to smooth the bullet phrasing if you’d like.
Also applies to: 77-83, 148-151
x/event/abi/SubnetAMMFactory.full.json (2)
16-51: Revisit which three fields are indexed in PoolCreated for optimal filtering.You currently index netuid, hetuToken, alphaToken. Since only three indexed params are allowed, consider whether indexing the pool address (emitted as the log address or as a param) would better serve off-chain consumers. Many indexers filter by the created pool address or netuid.
Options:
- Keep netuid indexed and replace one token with pool as indexed, if queries by pool are more common.
- Document the indexing strategy so indexers know to use the log address for pool (since the pool is also the emitting address for its events).
274-274: Trim full artifact bytecode from version control/embedding to reduce size.As with other full ABIs, prefer shipping minimal runtime ABIs and relocating compiled bytecode to a release-only artifacts directory.
If you want, I can send a patch that:
- moves *.full.json under artifacts/,
- updates go:embed patterns to include only *.json,
- and adjusts any unit tests that load ABIs.
x/event/abi/AlphaToken.json (1)
489-500: Optional: consider EIP-2612 permit for UX and gas savings.Permit reduces approve friction and improves integrator UX.
If in scope, add permit (EIP-2612) to AlphaToken and extend the ABI.
Also applies to: 444-454
x/blockinflation/types/params.go (4)
65-81: Fix mismatched doc for SubnetRewardMaxRatio default (0.90 vs comment 0.50)The comment says 0.50 but the code sets 0.90.
math.LegacyNewDecWithPrec(10, 2), // Default SubnetRewardK (0.10) - math.LegacyNewDecWithPrec(90, 2), // Default SubnetRewardMaxRatio (0.50) + math.LegacyNewDecWithPrec(90, 2), // Default SubnetRewardMaxRatio (0.90) math.LegacyNewDecWithPrec(3, 6), // Default SubnetMovingAlpha (0.000003)
28-48: Update units in comments (RAO → aHETU) for consistency with defaultsField comments still reference RAO while defaults/docstrings have switched to aHETU with 18 decimals.
// TotalSupply defines the maximum total supply (21,000,000,000,000,000 RAO) + // TotalSupply defines the maximum total supply (21,000,000,000,000,000,000,000,000 aHETU; 21M HETU with 18 decimals) TotalSupply math.Int `json:"total_supply" yaml:"total_supply"` - // DefaultBlockEmission defines the default block emission (1,000,000,000 RAO) + // DefaultBlockEmission defines the default block emission (1,000,000,000,000,000,000 aHETU; 1 HETU per block) DefaultBlockEmission math.Int `json:"default_block_emission" yaml:"default_block_emission"`
23-27: Add compile-time assertion for ParamSet implementationEnsures compile-time safety if ParamSet interface changes upstream.
// ParamKeyTable returns the parameter key table. func ParamKeyTable() paramstypes.KeyTable { return paramstypes.NewKeyTable().RegisterParamSet(&Params{}) } + +// enforce interface compliance +var _ paramstypes.ParamSet = (*Params)(nil)
185-194: Consider upper bound for SubnetRewardK (optional)Depending on how K feeds into your reward curve, a very large K might destabilize the ratio before the MaxRatio clamp. If appropriate, clamp K to a sane upper bound in validation or document expected ranges.
If you want, I can scan usages of SubnetRewardK in the keeper/math to recommend a concrete bound.
x/event/abi/WHETU.json (1)
171-233: Permit/EIP-2612 domain events present – confirm handler readinessDomain separator and EIP712DomainChanged support look good. Confirm the event module updates any cached domain state on EIP712DomainChanged to avoid signature validation issues after forks/chainId changes.
x/event/abi/WHETU.full.json (1)
1-610: Keep only what you use: drop bytecode if not needed by the applicationIf runtime only needs the ABI for decoding (common in keeper/event pipelines), consider excluding the bytecode to reduce repo weight and drift risk. If deployment tooling in-repo consumes this file, keeping the bytecode is fine.
Would you like me to generate a trimmed artifact (ABI-only) and wire the loader to prefer it?
x/event/abi/SubnetAMM.json (1)
117-141: Non-indexed reserves/price events are fine; consider indexing if frequent querying by value is requiredReservesUpdated and PriceUpdated have no indexed args (typical). If you anticipate heavy on-chain log scanning by price/reserves, consider indexing one field — but note there’s a trade-off with topic limits and filtering semantics. Optional.
x/stakework/keeper/keeper.go (1)
3-12: Drop fmt import; avoid unnecessary dependency
fmtis only used in Logger; string concatenation is sufficient.Apply this diff:
-import ( - "fmt" - - "cosmossdk.io/log" +import ( + "cosmossdk.io/log" storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types"And update Logger:
-func (k Keeper) Logger(ctx sdk.Context) log.Logger { - return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName)) -} +func (k Keeper) Logger(ctx sdk.Context) log.Logger { + return ctx.Logger().With("module", "x/"+types.ModuleName) +}x/event/abi/SubnetRegistry.json (1)
1-141: ABI looks consistent; verify contract parity and naming consistencyThe ABI schema is valid (indexed address/uint16 topics are fine). Minor nit: function parameter is named
burnAmountwhile storage getter/event useburnedTao. Consider aligning names to reduce confusion across code and docs.Please also verify this ABI matches the deployed SubnetRegistry bytecode to avoid event decode failures at runtime (topic/order/field types must match exactly).
x/blockinflation/types/keys.go (1)
20-29: Consider prefixing for future state expansionIf you plan to store per-subnet/per-denom data under this module, add typed key prefixes now (e.g., SubnetIssuancePrefix, RewardQueuePrefix) to avoid rekeying later.
x/blockinflation/types/query.go (1)
15-17: Proto struct tags are misleading without generated code
protobuf:"..."tags imply generated types, but this is a hand-written type. Keeping these tags is confusing and provides no value.type QueryParamsResponse struct { // Params defines the parameters of the module. - Params Params `protobuf:"bytes,1,opt,name=params,proto3" json:"params"` + Params Params `json:"params"` }x/blockinflation/keeper/alpha_token.go (2)
19-24: Prefer Cosmos SDK error types for classification and better client UXReturning
fmt.Errorfloses error codes. Usesdkerrors(orcosmossdk.io/errors) with well-known codes (e.g., ErrNotFound, ErrInvalidAddress) so clients can react programmatically.Example:
return errorsmod.Wrapf(sdkerrors.ErrNotFound, "subnet not found: %d", netuid) return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid alpha token address: %s", alphaTokenAddress)Additional changes needed outside the selected lines:
- Import
errorsmod "cosmossdk.io/errors"andsdkerrors "github.com/cosmos/cosmos-sdk/types/errors".Also applies to: 25-30
68-73: Log fields are good; consider adding tx hash or EVM call identifier if availableIf
CallEVMreturns a tx hash or execution identifier, include it in logs to aid tracing across EVM/Cosmos boundaries.x/event/abi/StakingSelfNative.json (1)
1-136: ABI LGTM; aligns with self-stake flowsEvents and function signatures look coherent for self-staking. Inputs/outputs and mutability are consistent with expectations.
If practical, consider naming the return of
getStake(e.g.,stakeAmount) for readability in downstream ABI bindings, though not required by the EVM.x/blockinflation/keeper/genesis.go (3)
21-35: Nil checks on Amounts are redundant and may hide denom/validation issues
DefaultGenesisState()sets zero coins;Amount.IsNil()will be false for zero. These guards are effectively always true for valid coins and do not enforce denom checks or positivity. If the intent is to conditionally set state only when provided, consider explicit denom validation instead.Example refactor:
- Drop the
IsNil()guards and always set, or- Validate
data.TotalIssuance.Denom == params.MintDenom(same for others) before setting to prevent cross-denom contamination.I can draft a denom-validation snippet if desired.
15-20: Use consistent log levels; avoid “DEBUG” prefix in Info logsYou mix
InfoandDebugwith “DEBUG” prefixes. PreferDebugfor verbose state dumps and keepInfofor high-level events to reduce log noise in production.- k.Logger(ctx).Info("DEBUG InitGenesis: Setting params", "params", data.Params) + k.Logger(ctx).Debug("InitGenesis: Setting params", "params", data.Params) - k.Logger(ctx).Info("DEBUG InitGenesis: Retrieved params", "params", params) + k.Logger(ctx).Debug("InitGenesis: Retrieved params", "params", params) - k.Logger(ctx).Debug("DEBUG InitGenesis Params", "params", data.Params) + k.Logger(ctx).Debug("InitGenesis Params", "params", data.Params)Also applies to: 35-40
10-13: cdc parameter is currently unusedIf you don’t need
cdchere, consider removing it or using_ codec.JSONCodecto avoid lints. Many modules keep it for future-proofing; if you retain it, suppress linters accordingly.x/event/types/genesis.go (2)
29-32: Return empty slices instead of nil for safer iterationNil slices can be fine in Go, but emitting empty arrays in JSON/YAML improves UX and avoids surprises in clients and tests that iterate without nil checks.
func DefaultGenesisState() *GenesisState { - return NewGenesisState(nil, nil, nil, nil) + return NewGenesisState([]Subnet{}, []ValidatorStake{}, []Delegation{}, []ValidatorWeight{}) }
6-12: Add yaml tags for consistency with other genesis typesOther module types carry both
jsonandyamltags to support dual encodings in tooling.type GenesisState struct { - Subnets []Subnet `json:"subnets"` - ValidatorStakes []ValidatorStake `json:"validator_stakes"` - Delegations []Delegation `json:"delegations"` - ValidatorWeights []ValidatorWeight `json:"validator_weights"` + Subnets []Subnet `json:"subnets" yaml:"subnets"` + ValidatorStakes []ValidatorStake `json:"validator_stakes" yaml:"validator_stakes"` + Delegations []Delegation `json:"delegations" yaml:"delegations"` + ValidatorWeights []ValidatorWeight `json:"validator_weights" yaml:"validator_weights"` }x/evm/keeper/keeper.go (1)
85-87: Consider returning an error from HandleEvmLogs for better failure propagationProcessing EVM logs can fail (ABI decode, storage writes). Returning an error allows callers (e.g., state transition) to decide whether to abort or log. Right now, failures must be handled via side effects or panics.
Apply this diff to make the interface return an error (follow-up changes will be required at call sites):
-type EventHandler interface { - HandleEvmLogs(ctx sdk.Context, logs []ethtypes.Log) -} +type EventHandler interface { + HandleEvmLogs(ctx sdk.Context, logs []ethtypes.Log) error +}init.sh (3)
1-2: Harden script execution: fail fast on errors and unset varsAdd strict mode to reduce flakiness and avoid partial state on failure.
Apply this diff:
-#!/bin/bash +#!/bin/bash +set -euo pipefail
9-10: Remove or use the unused LOGLEVEL variable (ShellCheck SC2034)LOGLEVEL is set but never used; prefer removing to avoid confusion.
Apply this diff:
-LOGLEVEL="info" -LOGLEVEL2="debug" +LOGLEVEL2="debug"
77-81: Pass --home consistently to CLI commandsBe explicit and consistent to avoid surprises if HETUD_HOME changes.
Apply this diff:
-hetud collect-gentxs +hetud collect-gentxs --home "$HETUD_HOME" # Validate genesis file -hetud validate-genesis +hetud validate-genesis --home "$HETUD_HOME"x/stakework/types/interfaces.go (1)
21-23: Nit: remove “New:” and “optional” phrasing from commentsKeep comments timeless and neutral. Consumers can infer optional usage from context.
Apply this diff:
- // New: Neuron info interfaces (optional, for future optimization) + // Neuron info interfacesinit_validators.sh (5)
102-104: Genesis account uses “gas” despite evm_denom switchYou add balances with both ahetu and gas here. Given evm_denom is “ahetu”, continuing to mint and include “gas” may create inconsistent supply and UX confusion unless you have a legacy reason to keep it. If not needed, drop the gas component.
Proposed change (only if you’re deprecating “gas”):
-hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM},${GENESIS_BALANCE}gas" \ +hetud add-genesis-account "${KEYS[$i]}" "${GENESIS_BALANCE}${DENOM}" \Would you like me to propagate the “no gas” approach across Lines 141, 153–154 as well?
141-141: Claims module bank balance still mints “gas”Same denom inconsistency as above: consider removing “gas” if not intentionally supported.
-jq -r --arg amount_to_claim "$amount_to_claim" '.app_state["bank"]["balances"] += [{"address":"hetu15cvq3ljql6utxseh0zau9m8ve2j8erz89c94rj","coins":[{"denom":"ahetu", "amount":$amount_to_claim}, {"denom":"gas", "amount":$amount_to_claim}]}]' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS" +jq -r --arg amount_to_claim "$amount_to_claim" '.app_state["bank"]["balances"] += [{"address":"hetu15cvq3ljql6utxseh0zau9m8ve2j8erz89c94rj","coins":[{"denom":"ahetu", "amount":$amount_to_claim}]}]' "$GENESIS" >"$TMP_GENESIS" && mv "$TMP_GENESIS" "$GENESIS"
79-81: Hard-coded root SSH user; make it configurableHard-coding root@ is risky. Use an overridable SSH_USER with sensible default; this also avoids editing these lines for different environments.
Within the header (near the dependency checks), add:
SSH_USER=${SSH_USER:-root}Then change:
-ssh root@${TARGET_IP} "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true" +ssh "${SSH_USER}"@"${TARGET_IP}" "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true"Apply the same to rsync below.
295-300: Parameterize SSH user for rsync as wellKeep SSH user consistent and configurable as above.
-ssh root@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" +ssh "${SSH_USER}"@"${TARGET_IP}" "rm -rf ${HOME_PREFIX}${i}" @@ -rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/" +rsync -av "${HOME_PREFIX}${i}/" "${SSH_USER}"@"${TARGET_IP}:${HOME_PREFIX}${i}/"
17-21: Check all required dependencies (bc, rsync, ssh) up frontYou use bc, rsync, and ssh later but only validate jq. Add checks to fail early with a clear message.
Add after the jq check:
command -v bc >/dev/null 2>&1 || { echo >&2 "bc not installed. sudo apt-get install bc"; exit 1; } command -v rsync >/dev/null 2>&1 || { echo >&2 "rsync not installed. sudo apt-get install rsync"; exit 1; } command -v ssh >/dev/null 2>&1 || { echo >&2 "ssh client not installed. Install OpenSSH client."; exit 1; }x/blockinflation/keeper/inflation.go (3)
65-75: Adjust debug fields to match new computation (remove log_result/multiplier)The previous debug fields relied on removed variables; keep logs consistent.
k.Logger(ctx).Debug("calculated block emission (high-precision)", "total_issuance", totalIssuance.String(), "total_supply", params.TotalSupply.String(), "ratio", ratio.String(), "log_arg", logArg.String(), - "log_result", fmt.Sprintf("%.6f", logResult), - "floored_log", flooredLogInt, - "multiplier", fmt.Sprintf("%.6f", multiplier), - "emission_percentage", blockEmissionPercentage.String(), + "k", k, + "emission_percentage", blockEmissionPercentage.String(), "block_emission", blockEmissionInt.String(), )
3-10: Drop stdmath import if unused after refactorAfter removing float usage, stdmath becomes unused; clean it up to avoid compile warnings.
-import ( - "fmt" - stdmath "math" +import ( + "fmt"
103-116: Polish log messages and levels“Test whether the subnet was successfully obtained” and “…reward ratio…” are noisy and not actionable. Switch to Debug and use standard phrasing.
- k.Logger(ctx).Info("Test whether the subnet was successfully obtained", - "subnetCount", subnetCount, - ) + k.Logger(ctx).Debug("discovered subnets", "count", subnetCount) @@ - k.Logger(ctx).Info("Test whether the subnet reward ratio was successfully obtained", - "subnetRewardRatio", subnetRewardRatio.String(), - ) + k.Logger(ctx).Debug("subnet reward ratio", "value", subnetRewardRatio.String())x/event/abi/AlphaToken.full.json (1)
1-585: Large artifact committed (ABI+bytecode). Confirm necessity and usage pathThis is large and includes bytecode. If the runtime only needs the ABI for EVM calls, prefer the ABI-only artifact to reduce repo bloat. If you need bytecode for deployments/tests, consider:
- Placing full artifacts under testdata/ or
- Using go:embed to include artifacts at build time without expanding source repo size.
I can help refactor the loading code to prefer ABI-only at runtime and keep full artifacts in test scope.
x/blockinflation/keeper/rewards.go (2)
21-29: Guard against empty/zero moving-price aggregatesYou handle totalMovingPrices.IsZero() when computing taoIn later—good. Consider short-circuiting if subnetsToEmitTo is empty to avoid extra logging and map allocations, returning early.
150-159: Redundant read-modify-write could be replaced by delta accountingYou first add full AlphaOut in ApplySubnetRewards then subtract ownerCut here by Get+Set. If eventKeeper supports “AddSubnetAlphaOutDelta(ctx, netuid, delta)”, prefer a delta to reduce store I/O. If not, this is fine for now.
x/blockinflation/client/cli/query.go (5)
3-11: Add strconv import for robust parsing and prefer clientCtx printing for uniform UXYou parse netuid with fmt.Sscanf and print with fmt.Printf/Println in several commands. Use strconv for parsing and clientCtx.PrintString for output where available.
import ( "fmt" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/spf13/cobra" "github.com/hetu-project/hetu/v1/x/blockinflation/types" + "strconv" )
93-99: Use clientCtx.PrintString for CLI consistencyStick to clientCtx for output to respect –output flags and formatting.
- // Print subnet reward specific parameters - fmt.Printf("Subnet Reward Base: %s\n", res.Params.SubnetRewardBase.String()) - fmt.Printf("Subnet Reward K: %s\n", res.Params.SubnetRewardK.String()) - fmt.Printf("Subnet Reward Max Ratio: %s\n", res.Params.SubnetRewardMaxRatio.String()) - - return nil + return clientCtx.PrintString( + fmt.Sprintf("Subnet Reward Base: %s\n", res.Params.SubnetRewardBase.String())+ + fmt.Sprintf("Subnet Reward K: %s\n", res.Params.SubnetRewardK.String())+ + fmt.Sprintf("Subnet Reward Max Ratio: %s\n", res.Params.SubnetRewardMaxRatio.String()), + )
143-147: Prefer strconv.ParseUint for netuid parsing with input validationMore explicit error handling and numeric bounds for uint16.
- var netuid uint16 - if _, err := fmt.Sscanf(args[0], "%d", &netuid); err != nil { - return fmt.Errorf("invalid netuid: %s", args[0]) - } + val, err := strconv.ParseUint(args[0], 10, 16) + if err != nil { + return fmt.Errorf("invalid netuid: %s", args[0]) + } + netuid := uint16(val)Apply the same parsing refactor to Lines 219–222 and 245–248.
121-128: Ensure String() is used if PendingSubnetRewards is a non-string typeIf PendingSubnetRewards is sdk.Coin or math.Int, print its String() to avoid Go printing structure data by default.
- return clientCtx.PrintString(fmt.Sprintf("Pending Subnet Rewards: %s\n", res.PendingSubnetRewards)) + return clientCtx.PrintString(fmt.Sprintf("Pending Subnet Rewards: %s\n", res.PendingSubnetRewards.String()))
224-228: Use cmd.Println/Printf or clientCtx.PrintString for output (avoid fmt directly)For placeholders, using cmd.Printf/cmd.Println integrates better with Cobra; or use clientCtx.PrintString if you want consistent behavior across commands.
- fmt.Printf("Subnet price query for netuid %d - gRPC service not yet implemented\n", netuid) - fmt.Println("This will show alpha_price, moving_price, subnet_tao, subnet_alpha_in, subnet_alpha_out, and volume") + cmd.Printf("Subnet price query for netuid %d - gRPC service not yet implemented\n", netuid) + cmd.Println("This will show alpha_price, moving_price, subnet_tao, subnet_alpha_in, subnet_alpha_out, and volume")Repeat for other placeholder commands for consistency.
x/blockinflation/keeper/inflation_test.go (1)
156-156: Typo in comment contradicts expected valueLine 155 comment says "7th halving, 1.5625% reward" but Line 156 shows it's actually testing 0.78125% reward (which would be 2^-7 = 0.0078125).
Fix the comment to match the actual test expectation:
-{"Cycle 7", 0.9921875, 0.015625, "7th halving, 1.5625% reward"}, // In actual calculation, 0.9921875 * 1000000 = 992187, rounded down to 992187 +{"Cycle 7", 0.9921875, 0.0078125, "7th halving, 0.78125% reward"},x/blockinflation/keeper/coinbase.go (2)
56-59: Use of float64 for financial calculationsUsing float64 for logarithmic calculations in financial contexts can lead to precision issues. Consider documenting why float64 is acceptable here or using a higher precision approach.
The conversion to float64 for log2 calculation is a potential source of precision loss. While the comment indicates this is the only place needing float64, consider:
- Adding a detailed comment explaining why the precision loss is acceptable
- Implementing bounds checking to ensure the float64 conversion doesn't lose significant digits
- Consider using a decimal library that supports logarithmic operations if available
298-300: TODO comment should be addressedThere's a TODO comment about implementing epoch-based emission draining that should be tracked.
The TODO at Line 299 indicates missing functionality for epoch-based emission draining. Would you like me to create an issue to track this implementation or provide a skeleton implementation?
x/event/abi/NeuronManager.full.json (1)
1-716: Consider splitting ABI and bytecode into separate filesHaving both ABI and bytecode in the same JSON file makes it harder to manage and review. The bytecode is rarely needed during normal operations and significantly increases file size.
Consider:
- Keep only the ABI in this file for runtime use
- Store bytecode separately (e.g.,
NeuronManager.bytecode.json) for deployment scripts- This separation would improve:
- File readability and reviewability
- Load times when only ABI is needed
- Version control efficiency (ABI changes are easier to track without bytecode noise)
x/blockinflation/types/expected_keepers.go (1)
16-23: Avoid duplicating subnet models across packagesSubnetInfo partially overlaps with eventtypes.Subnet (x/event/types/subnet.go). Divergence here will create drift and unnecessary conversions. Prefer consuming eventtypes.Subnet directly everywhere or clearly document why this trimmed shape is required and where it is used.
Would you like me to refactor call sites to use eventtypes.Subnet and remove this duplicate type if unused?
x/event/abi/SubnetManager.full.json (1)
1-1812: Large bytecode artifact checked-in: confirm necessity and sync with interface ABIThis full artifact includes bytecode and is sizable. If the repo only needs the ABI for decoding/logging, consider keeping only SubnetManager.json to reduce bloat and avoid drift. If tests need bytecode, gate it under testdata or add a CI check to ensure SubnetManager.json and SubnetManager.full.json stay in sync.
I can add a small CI script to diff the ABI portions and fail on mismatch.
x/event/types/subnet.go (2)
85-94: Remove redundant JSON marshal/unmarshal methods for SubnetThese methods do not add any transformation or validation beyond the default behavior and increase surface area for maintenance. Let the standard encoding/json behavior handle this.
-// MarshalJSON implements json.Marshaler -func (s Subnet) MarshalJSON() ([]byte, error) { - type Alias Subnet - return json.Marshal(&struct { - *Alias - }{ - Alias: (*Alias)(&s), - }) -} - -// UnmarshalJSON implements json.Unmarshaler -func (s *Subnet) UnmarshalJSON(data []byte) error { - type Alias Subnet - aux := &struct { - *Alias - }{ - Alias: (*Alias)(s), - } - if err := json.Unmarshal(data, &aux); err != nil { - return err - } - return nil -}Also applies to: 95-107
9-20: Define typed constants for Subnet.Mechanism instead of magic numbersReplace raw 0/1 usage with named constants for clarity and to avoid misuse across modules.
Add this near the top of the file:
// Mechanism values for Subnet.Mechanism const ( MechanismStable uint8 = 0 MechanismDynamic uint8 = 1 )And use these in call sites (e.g., Mechanism: MechanismDynamic).
x/event/keeper/keeper.go (3)
918-929: Clarify whether root subnet (netuid=0) should be includedCurrent implementation includes root subnet; previous code suggests excluding it. This affects downstream flows that iterate all subnets (e.g., emissions). Verify intent and adjust.
If the root should be excluded:
func (k Keeper) GetAllSubnetNetuids(ctx sdk.Context) []uint16 { subnets := k.GetAllSubnets(ctx) var netuids []uint16 for _, subnet := range subnets { - // if subnet.Netuid != 0 { // Filter out root subnet - // netuids = append(netuids, subnet.Netuid) - // } - netuids = append(netuids, subnet.Netuid) + if subnet.Netuid != 0 { // Filter out root subnet + netuids = append(netuids, subnet.Netuid) + } } return netuids }
1195-1225: Alpha price when AlphaIn is zero returns 0 — confirm economicsReturning 0 may feed into moving average and suppress price for long periods. Consider returning 1.0 (neutral) or a configured floor to avoid divide-by-zero semantics leaking into price.
Would you like a small change to return 1.0 when AlphaIn is zero to stabilize early epochs?
700-706: Handle marshal errors when persisting stateIgnoring json.Marshal errors can hide data corruption. At minimum, log and return to prevent writing nil/partial bytes.
func (k Keeper) SetSubnet(ctx sdk.Context, subnet types.Subnet) { store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte("subnet:")) - bz, _ := json.Marshal(subnet) - store.Set(uint16ToBytes(subnet.Netuid), bz) + bz, err := json.Marshal(subnet) + if err != nil { + k.Logger(ctx).Error("SetSubnet: marshal failed", "netuid", subnet.Netuid, "error", err) + return + } + store.Set(uint16ToBytes(subnet.Netuid), bz) }
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (14)
init_validators.sh (2)
12-16: Require exactly 5 IPs (or derive from args) to avoid misconfigurationCurrent check allows any count ≥ 5 while NUM_VALIDATORS is fixed to 5. Tighten to exactly 5, or better, derive NUM_VALIDATORS from args.
Apply minimal fix:
-if [ "$#" -lt 5 ]; then +if [ "$#" -ne 5 ]; then echo "Usage: $0 <validator0_ip> <validator1_ip> <validator2_ip> <validator3_ip> <validator4_ip>" echo "Example: $0 1.2.3.4 5.6.7.8 9.10.11.12 13.14.15.16 17.18.19.20" exit 1 fiAlternative (more flexible): use all arguments and set NUM_VALIDATORS dynamically (see suggestion on Lines 26–29).
26-29: Quote array assignment and derive NUM_VALIDATORS from args (SC2206)Unquoted array expansion risks word-splitting and ignores extra arguments; also NUM_VALIDATORS is hardcoded.
Apply this refactor:
-# NUM_VALIDATORS=7 - -NUM_VALIDATORS=5 -# Store validator IPs in array -# declare -a VALIDATOR_IPS=($1 $2 $3 $4 $5 $6 $7) -declare -a VALIDATOR_IPS=($1 $2 $3 $4 $5) +# NUM_VALIDATORS=7 +# Store validator IPs in array +# declare -a VALIDATOR_IPS=("$1" "$2" "$3" "$4" "$5" "$6" "$7") +declare -a VALIDATOR_IPS=("$@") +NUM_VALIDATORS=${#VALIDATOR_IPS[@]}This satisfies ShellCheck (SC2206) and keeps the script consistent if you later change the validator count.
x/event/types/subnet.go (2)
11-13: Unify naming with SubnetInfo and use strong numeric types for amountsPrefer consistent names and types across Subnet and SubnetInfo to avoid brittle mappings and parsing overhead. Amount-like fields should be math.Int; address-like fields should remain strings.
Apply this change:
- LockAmount → LockedAmount (math.Int)
- BurnedTao → BurnedAmount (math.Int)
- Pool → AmmPool (string), matching SubnetInfo
- LockAmount string `json:"lock_amount" yaml:"lock_amount"` - BurnedTao string `json:"burned_tao" yaml:"burned_tao"` - Pool string `json:"pool" yaml:"pool"` + LockedAmount math.Int `json:"locked_amount" yaml:"locked_amount"` + BurnedAmount math.Int `json:"burned_amount" yaml:"burned_amount"` + AmmPool string `json:"amm_pool" yaml:"amm_pool"`If wire-compatibility forces strings, add typed accessors that parse once (e.g., LockedAmountInt(), BurnedAmountInt()) and centralize validation.
95-100: Extend SubnetEmissionData to match BlockInflation schema (avoid type drift)BlockInflation tracks additional emission components (owner cut, root dividends, subnet_* splits). Storing a truncated shape here invites lossy conversions and drift.
Consider extending the struct for forward compatibility:
type SubnetEmissionData struct { TaoInEmission math.Int `json:"tao_in_emission" yaml:"tao_in_emission"` // TAO input emission AlphaInEmission math.Int `json:"alpha_in_emission" yaml:"alpha_in_emission"` // Alpha input emission AlphaOutEmission math.Int `json:"alpha_out_emission" yaml:"alpha_out_emission"` // Alpha output emission + // Extended fields (align with blockinflation/types/emission.go) + OwnerCut math.Int `json:"owner_cut" yaml:"owner_cut"` + RootDivs math.Int `json:"root_divs" yaml:"root_divs"` + SubnetAlphaInEmission math.Int `json:"subnet_alpha_in_emission" yaml:"subnet_alpha_in_emission"` + SubnetAlphaOutEmission math.Int `json:"subnet_alpha_out_emission" yaml:"subnet_alpha_out_emission"` + SubnetTaoInEmission math.Int `json:"subnet_tao_in_emission" yaml:"subnet_tao_in_emission"` }To verify the exact field names in this branch of the repo, run:
#!/bin/bash # Inspect emission types to align field names precisely. rg -n -C2 -g '!**/vendor/**' -P '(?s)^\s*type\s+\w*Emission\w*\s+struct\s*\{.*?\}' rg -n -C2 -g '!**/vendor/**' -P '\b(owner_cut|root_divs|subnet_alpha_in_emission|subnet_alpha_out_emission|subnet_tao_in_emission)\b'x/blockinflation/keeper/alpha_token.go (4)
40-44: Avoid stringly-typed params; trim and validate AlphaToken address (prefer typed field if available)You’re reading a free-form string from subnet.Params without trimming or format validation. The Event module in this PR reportedly introduced a typed AlphaToken field on SubnetInfo — prefer that if available to avoid magic strings. At minimum, trim and validate with common.IsHexAddress.
Apply this hardening:
- // 2. Check if the subnet has an AlphaToken address in its params - alphaTokenAddress, ok := subnet.Params["alpha_token"] - if !ok || alphaTokenAddress == "" { - return fmt.Errorf("subnet has no alpha token address in params: %d", netuid) - } + // 2. Resolve and validate the subnet's AlphaToken address + alphaTokenAddress, ok := subnet.Params["alpha_token"] + if !ok { + return fmt.Errorf("subnet has no alpha token address in params: %d", netuid) + } + alphaTokenAddress = strings.TrimSpace(alphaTokenAddress) + if alphaTokenAddress == "" || !common.IsHexAddress(alphaTokenAddress) { + return fmt.Errorf("invalid or missing alpha token address in subnet params: %d", netuid) + }Additional import required outside this hunk:
import ( "bytes" "fmt" "math/big" "sync" + "strings"
46-51: Harden AlphaToken address validation (IsHexAddress + zero-address check)common.HexToAddress will coerce malformed inputs; validate format first and reject zero address explicitly.
- // 3. Parse the AlphaToken address - alphaTokenAddr := common.HexToAddress(alphaTokenAddress) - if (alphaTokenAddr == common.Address{}) { - return fmt.Errorf("invalid alpha token address: %s", alphaTokenAddress) - } + // 3. Validate and parse the AlphaToken address + if !common.IsHexAddress(alphaTokenAddress) { + return fmt.Errorf("invalid alpha token address: %s", alphaTokenAddress) + } + alphaTokenAddr := common.HexToAddress(alphaTokenAddress) + if alphaTokenAddr == (common.Address{}) { + return fmt.Errorf("alpha token address cannot be the zero address: %s", alphaTokenAddress) + }
58-63: Validate recipient with IsHexAddress and reject zero addressAvoid accepting short/malformed hex inputs and the zero address as a recipient.
- // 5. Convert recipient address to Ethereum address - recipientAddr := common.HexToAddress(recipient) - if (recipientAddr == common.Address{}) { - return fmt.Errorf("invalid recipient address: %s", recipient) - } + // 5. Validate and convert recipient address + if !common.IsHexAddress(recipient) { + return fmt.Errorf("invalid recipient address: %s", recipient) + } + recipientAddr := common.HexToAddress(recipient) + if recipientAddr == (common.Address{}) { + return fmt.Errorf("recipient address cannot be the zero address: %s", recipient) + }
32-36: Prefer big.Int (or sdk.Int) for amount to avoid overflow and unit ambiguityMint amounts can exceed uint64 and ERC20 units are big integers (often 18 decimals). Consider changing the API to accept *big.Int (or sdk.Int) and pass it through.
Example refactor sketch (cross-cutting, update call sites accordingly):
// Prefer this signature: func (k Keeper) MintAlphaTokens(ctx sdk.Context, netuid uint16, recipient string, amount *big.Int) error { // ... // Remove SetUint64 and pass amount directly: // ... _, err = k.erc20Keeper.CallEVM( ctx, alphaTokenABI, common.BytesToAddress(moduleAddress.Bytes()), alphaTokenAddr, true, // commit "mint", recipientAddr, amount, ) // ... }If you must keep uint64, clearly document the unit and enforce max bounds at call sites.
Optionally locate call sites to scope the change:
#!/bin/bash # Find all MintAlphaTokens usages to estimate refactor blast radius rg -nP --glob '!**/vendor/**' --glob '!**/build/**' '\bMintAlphaTokens\s*\(' -C2Also applies to: 64-66
x/blockinflation/keeper/coinbase.go (6)
56-69: Avoid float Pow and precision loss in emission; clamp exponent and use integer divisionThe float path with stdmath.Pow and LegacyNewDecWithPrec can overflow and lose precision. Compute 2^n via bit shifts and use Dec.QuoInt64. Also guard NaN/Inf.
- // Convert to float64 for log2 calculation (this is the only place we need float64) - logArgFloat := logArg.MustFloat64() - logResult := stdmath.Log2(logArgFloat) - - // Floor the log result - flooredLog := stdmath.Floor(logResult) - flooredLogInt := int64(flooredLog) - - // Calculate 2^flooredLog - multiplier := stdmath.Pow(2.0, float64(flooredLogInt)) - - // Calculate block emission percentage: 1 / multiplier - blockEmissionPercentage := math.LegacyOneDec().Quo(math.LegacyNewDecWithPrec(int64(multiplier*1000), 3)) + // Convert to float64 for log2 with clamping to avoid overflow/NaN + logArgFloat := logArg.MustFloat64() + log2 := stdmath.Log2(logArgFloat) + if stdmath.IsInf(log2, 0) || stdmath.IsNaN(log2) { + return math.ZeroInt(), nil + } + // Floor and clamp exponent into a safe range + exp := int64(stdmath.Floor(log2)) + if exp < 0 { + exp = 0 + } + if exp > 60 { + exp = 60 + } + denom := int64(1) << exp + + // Calculate block emission percentage: 1 / (2^exp) + blockEmissionPercentage := math.LegacyNewDec(1).QuoInt64(denom) @@ - "log_result", fmt.Sprintf("%.6f", logResult), - "floored_log", flooredLogInt, - "multiplier", fmt.Sprintf("%.6f", multiplier), + "log_result", fmt.Sprintf("%.6f", log2), + "floored_log", exp, + "multiplier", fmt.Sprintf("%d", denom), "emission_percentage", blockEmissionPercentage.String(),Also applies to: 82-85
199-205: Guard uint64 conversions; clamp epoch input and derive budgets from the clamped valuependingAlpha can exceed uint64. Clamp the epoch input and compute validator allocation from that clamped budget to avoid silent overflow.
// Run epoch consensus - epochResult, err := k.stakeworkKeeper.RunEpoch(ctx, netuid, pendingAlpha.Uint64()) + // Guard against overflow to uint64 for epoch input + emissionForEpoch := pendingAlpha + if !pendingAlpha.IsUint64() { + k.Logger(ctx).Error("pending emission exceeds uint64, clamping for epoch input", + "netuid", netuid, "pending_alpha", pendingAlpha.String()) + emissionForEpoch = math.NewIntFromUint64(^uint64(0)) + } + epochResult, err := k.stakeworkKeeper.RunEpoch(ctx, netuid, emissionForEpoch.Uint64()) if err != nil { k.Logger(ctx).Error("RunEpoch failed", "netuid", netuid, "error", err) continue } @@ // Calculate validator-allocatable alpha - var pendingValidatorAlpha uint64 - if incentiveSum != 0 { - pendingValidatorAlpha = pendingAlpha.Uint64() / 2 - } else { - pendingValidatorAlpha = pendingAlpha.Uint64() - } + var pendingValidatorAlpha uint64 + epochBudget := emissionForEpoch.Uint64() + if incentiveSum != 0 { + pendingValidatorAlpha = epochBudget / 2 + } else { + pendingValidatorAlpha = epochBudget + }Also applies to: 212-219
224-227: Dividend allocation: remove float, ensure deterministic rounding; comment mismatchCurrent code uses float shares and says “weight by subnet stake” but doesn’t use stakeMap. Either fix the comment to reflect epochResult-based allocation or actually weight by stakes. Also switch to pure integer math and distribute any remainder deterministically.
- // Calculate dividends + // Calculate dividends (proportional to epochResult.Dividend) dividends := map[string]uint64{} for i, addr := range epochResult.Accounts { dividends[addr] = epochResult.Dividend[i] } @@ - // Dividend allocation (no parent-child relationship, direct allocation, weight by subnet stake) - alphaDividends := map[string]uint64{} - for addr, alphaDiv := range dividends { - var share float64 - if totalAlphaDivs > 0 { - share = float64(alphaDiv) / float64(totalAlphaDivs) - } - alphaDividends[addr] = uint64(float64(pendingValidatorAlpha) * share) - } + // Dividend allocation (no parent-child relationship). Use integer math and deterministic remainder handling. + alphaDividends := map[string]uint64{} + if totalAlphaDivs > 0 && pendingValidatorAlpha > 0 { + var allocated uint64 + // First pass: floor division + for addr, d := range dividends { + alloc := (uint64(d) * pendingValidatorAlpha) / totalAlphaDivs + alphaDividends[addr] = alloc + allocated += alloc + } + // Second pass: distribute remainder deterministically by address order + if allocated < pendingValidatorAlpha { + remainder := pendingValidatorAlpha - allocated + addrs := make([]string, 0, len(dividends)) + for a := range dividends { + addrs = append(addrs, a) + } + sort.Strings(addrs) + for i := uint64(0); i < remainder && i < uint64(len(addrs)); i++ { + alphaDividends[addrs[i]]++ + } + } + }Ensure the comment aligns with intent: is the allocation supposed to be proportional to epochResult.Dividend (as implemented) or weighted by stakeMap? If the latter, we should incorporate stakeMap weights.
Also applies to: 238-247
249-277: Normalize incentives to fit the incentive budget; avoid minting beyond epoch budgetIf incentiveSum > (epochBudget - pendingValidatorAlpha), this can overshoot the budget. Scale proportionally and distribute any remainder.
- // Incentive allocation - incentives := map[string]uint64{} - for i, addr := range epochResult.Accounts { - incentives[addr] = epochResult.Incentive[i] - } + // Incentive allocation normalized to the incentive budget + incentives := map[string]uint64{} + incentiveBudget := epochBudget - pendingValidatorAlpha + if incentiveSum > 0 && incentiveBudget > 0 { + var allocated uint64 + for i, addr := range epochResult.Accounts { + alloc := (epochResult.Incentive[i] * incentiveBudget) / incentiveSum + incentives[addr] = alloc + allocated += alloc + } + // Distribute any rounding remainder deterministically + if allocated < incentiveBudget { + remainder := incentiveBudget - allocated + for i := uint64(0); i < remainder && i < uint64(len(epochResult.Accounts)); i++ { + incentives[epochResult.Accounts[i]]++ + } + } + } k.Logger(ctx).Debug("Incentives", "netuid", netuid, "incentives", incentives)
278-288: Guard owner address and avoid uint64 overflow when minting owner cutsubnet.Owner may be empty and ownerCut may exceed uint64. Guard both before minting.
- if ownerCut.IsPositive() { + if ownerCut.IsPositive() && subnet.Owner != "" { k.Logger(ctx).Info("Owner cut distributed", "netuid", netuid, "amount", ownerCut.String()) - if err := k.MintAlphaTokens(ctx, netuid, subnet.Owner, ownerCut.Uint64()); err != nil { + var amt uint64 + if ownerCut.IsUint64() { + amt = ownerCut.Uint64() + } else { + k.Logger(ctx).Error("Owner cut exceeds uint64, clamping to MaxUint64", "netuid", netuid, "amount", ownerCut.String()) + amt = ^uint64(0) + } + if err := k.MintAlphaTokens(ctx, netuid, subnet.Owner, amt); err != nil { k.Logger(ctx).Error("Failed to mint alpha tokens for subnet owner", "netuid", netuid, "owner", subnet.Owner, - "amount", ownerCut.Uint64(), + "amount", amt, "error", err, ) } }
328-335: Validate stake parsing and prevent uint64 overflowParsing big.Int to uint64 can overflow silently. Validate and skip/handle overflow.
- if amount, ok := new(big.Int).SetString(s.Amount, 10); ok { - stakeMap[acc] += amount.Uint64() + if amount, ok := new(big.Int).SetString(s.Amount, 10); ok { + if amount.IsUint64() { + stakeMap[acc] += amount.Uint64() + } else { + k.Logger(ctx).Error("stake amount overflow for validator", + "netuid", netuid, + "validator", acc, + "amount", s.Amount, + ) + // skip overflowed amount + } } else { k.Logger(ctx).Error("invalid stake amount string", "netuid", netuid, "validator", acc, "amount", s.Amount) }
🧹 Nitpick comments (13)
init_validators.sh (4)
79-81: Hard-coded SSH user “root”; make it configurable and verify accessRoot SSH may be disabled in some environments. Parameterize user and confirm access.
Edit these lines to use a configurable user:
- ssh root@${TARGET_IP} "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true" + ssh "${REMOTE_USER}@${TARGET_IP}" "pkill hetud || true; rm -rf \"${HOME_PREFIX}\" \"${HOME_PREFIX}\"* 2>/dev/null || true"And define REMOTE_USER near the top (outside this hunk):
# SSH user for remote operations REMOTE_USER="${REMOTE_USER:-root}"Please verify that passwordless SSH is set up for $REMOTE_USER on all target hosts.
141-141: Check units for claim amount vs total supplyamount_to_claim is “10000” ahetu, which is 1e-14 hetu given 1 hetu = 1e18 ahetu. Confirm this is intended; otherwise scale amount_to_claim accordingly and keep total_supply consistent.
155-161: Make bank.supply update resilient if ahetu entry is missingCurrent jq assumes an existing “ahetu” entry. If missing, map(...) will leave supply unchanged. Prefer a conditional that updates or appends.
Proposed replacement:
-jq -r --arg total_supply "$total_supply" ' - .app_state["bank"]["supply"] |= - (map(if .denom == "ahetu" then .amount = $total_supply else . end)) -' "$GENESIS" >"$TMP_GENESIS" && mv "$GENESIS" "$GENESIS.bak" && mv "$TMP_GENESIS" "$GENESIS" +jq -r --arg total_supply "$total_supply" ' + .app_state["bank"]["supply"] |= + (if (map(.denom) | index("ahetu")) != null + then map(if .denom == "ahetu" then .amount = $total_supply else . end) + else . + [{"denom":"ahetu","amount":$total_supply}] + end) +' "$GENESIS" >"$TMP_GENESIS" && mv "$GENESIS" "$GENESIS.bak" && mv "$TMP_GENESIS" "$GENESIS"
301-306: Parameterize SSH user for rsync, consistent with cleanup sectionMirror the SSH user configuration used earlier to avoid root assumptions.
Apply:
- ssh root@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" + ssh "${REMOTE_USER}@${TARGET_IP}" "rm -rf ${HOME_PREFIX}${i}" @@ - rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/" + rsync -av "${HOME_PREFIX}${i}/" "${REMOTE_USER}@${TARGET_IP}:${HOME_PREFIX}${i}/"And add (once) near the top if not already:
REMOTE_USER="${REMOTE_USER:-root}"x/event/types/subnet.go (9)
35-35: Avoid strings for economic params; use math.Int/LegacyDec for safety and consistencyThese are economic values and thresholds. Strings increase parsing friction and risk silent failures.
- BaseNeuronCost: token amount → math.Int
- ValidatorThreshold, NeuronThreshold: fractional thresholds → math.LegacyDec
- BaseNeuronCost string `json:"base_neuron_cost" yaml:"base_neuron_cost"` + BaseNeuronCost math.Int `json:"base_neuron_cost" yaml:"base_neuron_cost"` @@ - ValidatorThreshold string `json:"validator_threshold" yaml:"validator_threshold"` - NeuronThreshold string `json:"neuron_threshold" yaml:"neuron_threshold"` + ValidatorThreshold math.LegacyDec `json:"validator_threshold" yaml:"validator_threshold"` + NeuronThreshold math.LegacyDec `json:"neuron_threshold" yaml:"neuron_threshold"`Also applies to: 46-47
27-31: Clarify semantics for MaxValidators vs MaxAllowedValidatorsNames are very similar and can confuse operators and devs. If they differ (network hard cap vs per-subnet governance cap), add explicit comments or rename to reduce ambiguity (e.g., MaxNetworkValidators vs MaxAllowedValidators).
16-16: Replace magic numbers for Mechanism with a typed enumIntroduce a small enum to improve readability and validation.
- Mechanism uint8 `json:"mechanism" yaml:"mechanism"` // Subnet mechanism (0=stable, 1=dynamic) + Mechanism SubnetMechanism `json:"mechanism" yaml:"mechanism"` // Subnet mechanismAdd alongside the types (outside this hunk):
// SubnetMechanism enumerates subnet pricing mechanisms. type SubnetMechanism uint8 const ( MechanismStable SubnetMechanism = 0 MechanismDynamic SubnetMechanism = 1 )
78-80: Constrain port fields to uint16 (0–65535)Ports cannot exceed 65535; using uint16 expresses domain constraints and prevents invalid values.
- AxonPort uint32 `json:"axon_port" yaml:"axon_port"` + AxonPort uint16 `json:"axon_port" yaml:"axon_port"` @@ - PrometheusPort uint32 `json:"prometheus_port" yaml:"prometheus_port"` + PrometheusPort uint16 `json:"prometheus_port" yaml:"prometheus_port"`
83-83: Remove non-actionable commentThe note about default JSON marshalling adds noise without value. Recommend removing.
-// Default JSON marshalling is sufficient for Subnet; custom methods not required.
14-14: Avoid untyped Params; prefer a typed SubnetHyperparams (or document intended use)A free-form map risks key drift and inconsistent interpretation. If these are the same “hyperparameters,” store them as SubnetHyperparams to leverage compile-time checks and consistent tags.
- Params map[string]string `json:"params" yaml:"params"` + Hyperparams SubnetHyperparams `json:"hyperparams" yaml:"hyperparams"`If the map is strictly for dynamic, contract-sourced key/values, document its expected keys and ranges, or consider a dedicated type with validation.
74-74: Use math.Int for NeuronInfo.StakeStake is a token amount; strings force repetitive parsing and error handling at call sites.
- Stake string `json:"stake" yaml:"stake"` + Stake math.Int `json:"stake" yaml:"stake"`
10-10: Document and validate address fields (Owner, AlphaToken, AmmPool)
- Owner likely bech32 (Cosmos) address; AlphaToken and AmmPool look like EVM addresses.
- Add brief doc comments clarifying expected formats, and validate at ingestion (keeper) to fail fast on malformed inputs.
Optionally, add ValidateBasic methods on these structs or centralize checks in the keeper using bech32 and hex-address validators.
Also applies to: 54-55
85-93: Consider documenting units and update cadence for price/volume fields
- MovingPrice, AlphaPrice (LegacyDec): specify whether these are in TAO per ALPHA, ALPHA per TAO, or normalized ratio.
- SubnetTAO/AlphaIn/AlphaOut/Volume (math.Int): specify units (e.g., RAO) and whether Volume is per-epoch, per-day, or cumulative.
Clear units prevent downstream misinterpretation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
init.sh(1 hunks)init_validators.sh(11 hunks)x/blockinflation/keeper/abci.go(1 hunks)x/blockinflation/keeper/alpha_token.go(1 hunks)x/blockinflation/keeper/coinbase.go(1 hunks)x/blockinflation/keeper/genesis.go(1 hunks)x/event/types/stake.go(1 hunks)x/event/types/subnet.go(1 hunks)x/event/types/utils.go(1 hunks)x/evm/keeper/keeper.go(2 hunks)x/stakework/types/module.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- x/event/types/utils.go
- x/blockinflation/keeper/abci.go
- x/event/types/stake.go
- x/evm/keeper/keeper.go
- init.sh
- x/blockinflation/keeper/genesis.go
- x/stakework/types/module.go
🧰 Additional context used
🧬 Code Graph Analysis (3)
x/blockinflation/keeper/alpha_token.go (3)
x/event/abi/abi.go (1)
AlphaTokenABI(29-29)x/blockinflation/keeper/keeper.go (1)
Keeper(18-29)x/stakework/types/module.go (1)
ModuleName(4-4)
x/blockinflation/keeper/coinbase.go (4)
x/blockinflation/keeper/keeper.go (1)
Keeper(18-29)x/stakework/types/epoch.go (1)
ParseEpochParams(70-172)x/blockinflation/types/params.go (1)
Params(29-48)x/blockinflation/types/emission.go (1)
SubnetEmissionData(8-18)
x/event/types/subnet.go (1)
x/blockinflation/types/params.go (1)
Params(29-48)
🪛 Shellcheck (0.10.0)
init_validators.sh
[warning] 29-29: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 29-29: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 29-29: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 29-29: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
[warning] 29-29: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (8)
init_validators.sh (7)
101-104: LGTM: genesis account funding uses only ahetu denom nowSwitching add-genesis-account to a single base denom aligns with the denom migration.
111-111: LGTM: evm denom set to ahetuAligns EVM gas denomination with chain base denom.
114-121: Validate new blockinflation/event genesis schema against module expectationsThe added fields look reasonable, but ensure they match the modules’ GenesisState schemas and types (strings vs ints/decimals).
Since you already run validate-genesis later, that will catch schema mismatches. If needed, I can generate a jq-based probe to assert presence/types of each added field.
151-153: LGTM: explicit bc dependency checkGood guard to prevent runtime failure during supply calculation.
205-205: LGTM: minimum-gas-prices updated to ahetuMatches denom migration and avoids mismatch during node start.
227-227: Longer timeout_commit (12s) — confirm intended block timeThis will materially increase time-to-finality relative to the 200ms propose. Ensure it aligns with target block times and network conditions.
281-283: LGTM: persistent_peers built and injected with safe sed delimiterPipe delimiter avoids collisions; echo helps with debugging peer config.
x/blockinflation/keeper/alpha_token.go (1)
24-30: ABI caching with sync.Once looks goodThe lazy, once-only parse of the AlphaToken ABI is clean and thread-safe. Returning the cached value/error is correct.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (26)
x/stakework/module.go (1)
114-118: Previous epoch-triggering concern resolved by delegating to blockinflationYou’ve removed checkAndRunEpochs and delegated epoch execution to coinbase in blockinflation, addressing prior feedback to avoid double-triggering and per-block invocations.
x/event/keeper/keeper.go (10)
424-429: Rho/Kappa formatting and scaling look correct nowUsing strconv.FormatFloat with the 1e4 scale for rho and integer formatting for kappa preserves precision and aligns with defaults.
515-529: Avoid partial activation updates; validate subnet existence before mutating both recordsIf Subnet isn’t found, updating SubnetInfo alone leaves state inconsistent. Validate Subnet exists first; then update both.
-// Update subnet activation status -subnet, found := k.GetSubnet(ctx, event.Netuid) -if found { - subnet.FirstEmissionBlock = event.BlockNumber.Uint64() - k.SetSubnet(ctx, subnet) -} - -// Update detailed subnet information -subnetInfo, found := k.GetSubnetInfo(ctx, event.Netuid) -if found { - subnetInfo.IsActive = true - subnetInfo.ActivatedAt = event.Timestamp.Uint64() - subnetInfo.ActivatedBlock = event.BlockNumber.Uint64() - k.SetSubnetInfo(ctx, subnetInfo) -} +// Validate subnet exists before updating +subnet, ok := k.GetSubnet(ctx, event.Netuid) +if !ok { + k.Logger(ctx).Error("Subnet not found for activation", "netuid", event.Netuid) + return +} +subnet.FirstEmissionBlock = event.BlockNumber.Uint64() +k.SetSubnet(ctx, subnet) + +// Update detailed subnet information +subnetInfo, ok := k.GetSubnetInfo(ctx, event.Netuid) +if !ok { + k.Logger(ctx).Error("SubnetInfo not found for activation", "netuid", event.Netuid) + return +} +subnetInfo.IsActive = true +subnetInfo.ActivatedAt = event.Timestamp.Uint64() +subnetInfo.ActivatedBlock = event.BlockNumber.Uint64() +k.SetSubnetInfo(ctx, subnetInfo)
924-935: Avoid double iteration in GetAllValidatorStakesAmountCompute delegations in the same pass to reduce redundant scans and ensure consistency with the filtered validator list.
-func (k Keeper) GetAllValidatorStakesAmount(ctx sdk.Context, netuid uint16) map[string]string { - result := make(map[string]string) - for _, stake := range k.GetAllValidatorStakesByNetuid(ctx, netuid) { - result[stake.Validator] = stake.Amount - } - for _, stake := range k.GetAllValidatorStakesByNetuid(ctx, netuid) { - for _, deleg := range k.GetDelegationsByValidator(ctx, netuid, stake.Validator) { - result[stake.Validator] = types.AddBigIntString(result[stake.Validator], deleg.Amount) - } - } - return result -} +func (k Keeper) GetAllValidatorStakesAmount(ctx sdk.Context, netuid uint16) map[string]string { + result := make(map[string]string) + stakes := k.GetAllValidatorStakesByNetuid(ctx, netuid) + for _, stake := range stakes { + total := stake.Amount + for _, deleg := range k.GetDelegationsByValidator(ctx, netuid, stake.Validator) { + total = types.AddBigIntString(total, deleg.Amount) + } + result[stake.Validator] = total + } + return result +}
1286-1347: Add input validation to UpdateMovingPrice (movingAlpha range and halvingBlocks > 0)Prevents degenerate cases and logs clear errors.
func (k Keeper) UpdateMovingPrice(ctx sdk.Context, netuid uint16, movingAlpha math.LegacyDec, halvingBlocks uint64) { + // Validate inputs + if movingAlpha.IsNegative() || movingAlpha.GT(math.LegacyNewDec(1)) { + k.Logger(ctx).Error("invalid movingAlpha value", "netuid", netuid, "movingAlpha", movingAlpha.String()) + return + } + if halvingBlocks == 0 { + k.Logger(ctx).Error("halvingBlocks cannot be zero", "netuid", netuid) + return + }
264-272: Use the found flag; netuid==0 is a valid subnet (root) and should not imply “not found”Rely on the boolean return from GetValidatorStake to avoid misclassifying root-subnet stakes.
- stake, _ := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) - if stake.Netuid == 0 { + stake, found := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) + if !found { stake = types.ValidatorStake{Netuid: event.Netuid, Validator: event.Validator.Hex(), Amount: "0"} } stake.Amount = types.AddBigIntString(stake.Amount, event.Amount.String()) k.SetValidatorStake(ctx, stake)
289-295: Same root-subnet bug on self-unstake: use found flag, not Netuid==0This early-return will incorrectly drop updates for netuid=0.
- stake, _ := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) - if stake.Netuid == 0 { + stake, found := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) + if !found { return } stake.Amount = types.SubBigIntString(stake.Amount, event.Amount.String()) k.SetValidatorStake(ctx, stake)
309-315: Delegated stake: use found flag to distinguish absence from root-subnetAvoid treating netuid=0 as “not found”.
- deleg, _ := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) - if deleg.Netuid == 0 { + deleg, found := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) + if !found { deleg = types.Delegation{Netuid: event.Netuid, Validator: event.Validator.Hex(), Staker: event.Staker.Hex(), Amount: "0"} } deleg.Amount = types.AddBigIntString(deleg.Amount, event.Amount.String()) k.SetDelegation(ctx, deleg)
329-335: Delegated unstake: use found flag, not Netuid==0, to decide existenceSame root-subnet misclassification here.
- deleg, _ := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) - if deleg.Netuid == 0 { + deleg, found := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) + if !found { return } deleg.Amount = types.SubBigIntString(deleg.Amount, event.Amount.String()) k.SetDelegation(ctx, deleg)
804-832: Ensure validators-only set; current code includes all active neuronsThis can incorrectly distribute emissions to non-validators. Filter by IsValidator.
for _, neuron := range activeNeurons { - stake := types.ValidatorStake{ - Netuid: neuron.Netuid, - Validator: neuron.Account, - Amount: neuron.Stake, - } - stakes = append(stakes, stake) + if !neuron.IsValidator { + continue + } + stakes = append(stakes, types.ValidatorStake{ + Netuid: neuron.Netuid, + Validator: neuron.Account, + Amount: neuron.Stake, + }) }
28-50: Remove deprecated global topic constants; use Keeper.k.topicXXX (ABI-derived) onlyQuick verification: a repo-wide search for the deprecated constant names returned only the declaration block inside x/event/keeper/keeper.go (15 matches total). Keeper already initializes and uses k.topicXXX from the ABIs, so removing the globals is safe and prevents future drift.
Files to change:
- x/event/keeper/keeper.go — remove the deprecated global topic constants block (the "// ----------- Event topic constants -----------" var (...) block).
Apply this diff to remove the block:
-// ----------- Event topic constants ----------- -// Deprecated: Use Keeper instance fields (k.topicXXX) instead of these global constants -var ( - // Legacy events remain unchanged (backward compatibility) - SubnetRegisteredTopic = crypto.Keccak256Hash([]byte("SubnetRegistered(address,uint16,uint256,uint256,address,string)")).Hex() - StakedSelfTopic = crypto.Keccak256Hash([]byte("Staked(uint16,address,uint256)")).Hex() - UnstakedSelfTopic = crypto.Keccak256Hash([]byte("Unstaked(uint16,address,uint256)")).Hex() - StakedDelegatedTopic = crypto.Keccak256Hash([]byte("Staked(uint16,address,address,uint256)")).Hex() - UnstakedDelegatedTopic = crypto.Keccak256Hash([]byte("Unstaked(uint16,address,address,uint256)")).Hex() - WeightsSetTopic = crypto.Keccak256Hash([]byte("WeightsSet(uint16,address,(address,uint256)[])")).Hex() - - // New event topics - NetworkRegisteredTopic = crypto.Keccak256Hash([]byte("NetworkRegistered(uint16,address,address,address,uint256,uint256,uint256,string,(uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint256,uint64,uint16,uint16,uint64,bool,bool,uint64,uint64,uint256,uint256))")).Hex() - SubnetActivatedTopic = crypto.Keccak256Hash([]byte("SubnetActivated(uint16,address,uint256,uint256)")).Hex() - NetworkConfigUpdatedTopic = crypto.Keccak256Hash([]byte("NetworkConfigUpdated(string,uint256,uint256,address)")).Hex() - NeuronRegisteredTopic = crypto.Keccak256Hash([]byte("NeuronRegistered(uint16,address,uint256,bool,bool,string,uint32,string,uint32,uint256)")).Hex() - NeuronDeregisteredTopic = crypto.Keccak256Hash([]byte("NeuronDeregistered(uint16,address,uint256)")).Hex() - NeuronStakeChangedTopic = crypto.Keccak256Hash([]byte("StakeAllocationChanged(uint16,address,uint256,uint256,uint256)")).Hex() - ServiceUpdatedTopic = crypto.Keccak256Hash([]byte("ServiceUpdated(uint16,address,string,uint32,string,uint32,uint256)")).Hex() - SubnetAllocationChangedTopic = crypto.Keccak256Hash([]byte("SubnetAllocationChanged(address,uint16,uint256,uint256)")).Hex() - DeallocatedFromSubnetTopic = crypto.Keccak256Hash([]byte("DeallocatedFromSubnet(address,uint16,uint256)")).Hex() -)x/stakework/types/epoch.go (2)
9-18: Document and enforce length invariants across EpochResult slices.The
EpochResultstruct contains multiple slices (Accounts, Emission, Dividend, Incentive, Bonds, Consensus) that should maintain consistent lengths, but this invariant isn't enforced. This could lead to silent mismatches and runtime errors.Add a validation method to enforce these invariants:
+// Validate checks if EpochResult slices have consistent lengths +func (e EpochResult) Validate() error { + n := len(e.Accounts) + if len(e.Emission) != n { + return fmt.Errorf("epoch result length mismatch: accounts=%d emission=%d", n, len(e.Emission)) + } + if len(e.Dividend) != n { + return fmt.Errorf("epoch result length mismatch: accounts=%d dividend=%d", n, len(e.Dividend)) + } + if len(e.Incentive) != n { + return fmt.Errorf("epoch result length mismatch: accounts=%d incentive=%d", n, len(e.Incentive)) + } + if len(e.Consensus) != n { + return fmt.Errorf("epoch result length mismatch: accounts=%d consensus=%d", n, len(e.Consensus)) + } + if len(e.Bonds) != n { + return fmt.Errorf("epoch result bonds length mismatch: accounts=%d bonds=%d", n, len(e.Bonds)) + } + // Check that each bonds row has the expected length + for i, row := range e.Bonds { + if len(row) != n { + return fmt.Errorf("epoch result bonds[%d] length mismatch: expected=%d actual=%d", i, n, len(row)) + } + } + return nil +}
270-276: Avoid float64 for stake amounts — use an exact representation.The
ValidatorInfo.Stakefield usesfloat64, which will lose precision for large monetary amounts. This is particularly critical in blockchain systems where precision is paramount.Consider using an exact representation:
type ValidatorInfo struct { Address string `json:"address"` - Stake float64 `json:"stake"` + Stake string `json:"stake"` // bigint string to avoid precision loss Weights []uint64 `json:"weights"` Active bool `json:"active"` }This change would require updates to consumers of this type, particularly in
x/stakework/keeper/epoch.go.x/stakework/README.md (2)
128-132: Update emission variable name to match the correct denomination.The example uses
ahetuEmissionwhich is now correct and consistent with the module's denomination. The code example properly reflects the expected usage.
58-71: Critical: float64 usage in consensus-critical code poses determinism risk.The module extensively uses
float64andmath.*functions in consensus-critical code, which can lead to non-determinism across different platforms and architectures. This is a serious issue for blockchain consensus.Locations requiring migration to
sdk.Decor fixed-point arithmetic:
- All type definitions using
float64in the code examples- The
normalize,weightedMedianCol, andcomputeBondsfunction examples- Any actual implementations that use these floating-point operations
Let me verify the extent of float64 usage in the actual implementation:
#!/bin/bash # Check for float64 usage in stakework module implementation files echo "=== Checking float64 usage in stakework module ===" rg -n "float64|math\.(Exp|Pow|Log)" x/stakework/keeper/ --type go | head -20 # Check if sdk.Dec is already being used anywhere echo -e "\n=== Checking for sdk.Dec usage ===" rg -n "sdk\.Dec|math\.LegacyDec" x/stakework/ --type go | head -10x/blockinflation/keeper/coinbase.go (5)
56-75: Critical: Alpha emission calculation uses float64 operations risking overflow and precision loss.The current implementation converts to
float64for logarithmic calculations, which can overflow whenratioapproaches 1 and lose precision. The use ofmath.Pow(2.0, float64(flooredLogInt))followed by the conversion trickmath.LegacyNewDecWithPrec(int64(multiplier*1000), 3)is particularly problematic as it will overflow for modest exponents.Apply a safer approach with clamped exponents:
- // Convert to float64 for log2 calculation (this is the only place we need float64) - logArgFloat := logArg.MustFloat64() - logResult := stdmath.Log2(logArgFloat) - - // Floor the log result - flooredLog := stdmath.Floor(logResult) - flooredLogInt := int64(flooredLog) - - // Calculate 2^flooredLog - multiplier := stdmath.Pow(2.0, float64(flooredLogInt)) - - // Calculate block emission percentage: 1 / multiplier - blockEmissionPercentage := math.LegacyOneDec().Quo(math.LegacyNewDecWithPrec(int64(multiplier*1000), 3)) + // Convert to float64 for log2 with safety checks + logArgFloat := logArg.MustFloat64() + if stdmath.IsInf(logArgFloat, 0) || stdmath.IsNaN(logArgFloat) { + return math.ZeroInt(), nil + } + logResult := stdmath.Log2(logArgFloat) + if stdmath.IsInf(logResult, 0) || stdmath.IsNaN(logResult) { + return math.ZeroInt(), nil + } + + // Floor and clamp exponent to safe range [0, 60] + exp := int64(stdmath.Floor(logResult)) + if exp < 0 { + exp = 0 + } + if exp > 60 { + exp = 60 + } + + // Calculate block emission percentage: 1 / 2^exp using integer arithmetic + denom := int64(1) << exp + blockEmissionPercentage := math.LegacyNewDec(1).QuoInt64(denom)
204-223: Unsafe uint64 conversions can overflow silently.Converting
math.Intvalues touint64can cause silent truncation when the values exceedMaxUint64. This is particularly dangerous for monetary amounts.Add overflow checks before conversion:
+ // Check for overflow before converting to uint64 + if !pendingAlpha.IsUint64() { + k.Logger(ctx).Error("pending emission exceeds uint64 range", + "netuid", netuid, + "pending_alpha", pendingAlpha.String()) + // Skip this epoch or handle appropriately + continue + } + // Run epoch consensus epochResult, err := k.stakeworkKeeper.RunEpoch(ctx, netuid, pendingAlpha.Uint64())Consider updating the
RunEpochinterface to acceptmath.Intdirectly to avoid these conversions entirely.
242-251: Dividend allocation uses float64 introducing rounding errors and non-determinism.Using
float64for dividend calculations introduces precision loss and potential non-determinism. Implement pure integer arithmetic with remainder distribution:- // Dividend allocation (no parent-child relationship, direct allocation, weight by subnet stake) - alphaDividends := map[string]uint64{} - for addr, alphaDiv := range dividends { - var share float64 - if totalAlphaDivs > 0 { - share = float64(alphaDiv) / float64(totalAlphaDivs) - } - alphaDividends[addr] = uint64(float64(pendingValidatorAlpha) * share) - } + // Dividend allocation using integer arithmetic to avoid float precision issues + alphaDividends := map[string]uint64{} + if totalAlphaDivs > 0 && pendingValidatorAlpha > 0 { + var allocated uint64 + // First pass: integer division + for addr, d := range dividends { + alloc := (uint64(d) * pendingValidatorAlpha) / uint64(totalAlphaDivs) + alphaDividends[addr] = alloc + allocated += alloc + } + // Second pass: distribute remainder deterministically + if allocated < pendingValidatorAlpha { + remainder := pendingValidatorAlpha - allocated + // Sort addresses for deterministic distribution + addrs := make([]string, 0, len(dividends)) + for a := range dividends { + addrs = append(addrs, a) + } + sort.Strings(addrs) + // Distribute remainder one unit at a time + for i := uint64(0); i < remainder && i < uint64(len(addrs)); i++ { + alphaDividends[addrs[i]]++ + } + } + }
282-292: Guard owner minting against empty address and overflow.The code mints tokens to
subnet.Ownerwithout validating the address is non-empty, and convertsmath.Inttouint64without overflow checking.- if ownerCut.IsPositive() { + if ownerCut.IsPositive() && subnet.Owner != "" { k.Logger(ctx).Info("Owner cut distributed", "netuid", netuid, "amount", ownerCut.String()) - if err := k.MintAlphaTokens(ctx, netuid, subnet.Owner, ownerCut.Uint64()); err != nil { + // Check for overflow before conversion + if !ownerCut.IsUint64() { + k.Logger(ctx).Error("Owner cut exceeds uint64 range", + "netuid", netuid, + "owner", subnet.Owner, + "amount", ownerCut.String()) + // Continue processing other subnets + continue + } + if err := k.MintAlphaTokens(ctx, netuid, subnet.Owner, ownerCut.Uint64()); err != nil {
329-340: Potential integer overflow in stake amount parsing.The conversion from string to
big.Inttouint64can overflow if the stake amount exceedsMaxUint64. Also, the parsing error is not properly handled.for _, s := range stakes { if s.Validator == acc { - if amount, ok := new(big.Int).SetString(s.Amount, 10); ok { - stakeMap[acc] += amount.Uint64() - } else { - k.Logger(ctx).Error("invalid stake amount string", - "netuid", netuid, - "validator", acc, - "amount", s.Amount) - } + amount, ok := new(big.Int).SetString(s.Amount, 10) + if !ok { + k.Logger(ctx).Error("Failed to parse stake amount", + "netuid", netuid, + "validator", acc, + "amount", s.Amount) + continue + } + // Check for overflow before converting to uint64 + if !amount.IsUint64() { + k.Logger(ctx).Error("Stake amount exceeds uint64 range", + "netuid", netuid, + "validator", acc, + "amount", s.Amount) + continue + } + stakeMap[acc] += amount.Uint64() } }Consider accumulating stakes using
big.Intthroughout to avoid overflow entirely.x/stakework/keeper/epoch.go (6)
547-549: Minimize precision loss in emission distribution.The current implementation converts each weighted component to
uint64separately before addition, truncating fractional parts twice. This can lose up to 2 units of emission.- // emission[i] = uint64(normIncentive[i]*float64(raoEmission)) + uint64(normDividends[i]*float64(raoEmission)) - emission[i] = uint64((normIncentive[i] + normDividends[i]) * float64(raoEmission)) + // Add the normalized values first, then multiply and round to minimize precision loss + combinedWeight := normIncentive[i] + normDividends[i] + emission[i] = uint64(math.Round(combinedWeight * float64(raoEmission)))Note: The code already has the improved version commented out, which is good!
241-243: Add error handling for string to big.Int conversion.The conversion ignores the boolean return value from
SetString(). If the conversion fails, the code continues with an invalidbig.Int, potentially causing incorrect calculations.- amount, _ := new(big.Int).SetString(stake.Amount, 10) + amount, ok := new(big.Int).SetString(stake.Amount, 10) + if !ok { + k.Logger(ctx).Error("Failed to parse stake amount", + "validator", stake.Validator, + "amount", stake.Amount) + continue + } stakeFloat := new(big.Float).SetInt(amount) stakeValue, _ := stakeFloat.Float64()Additionally, consider keeping the stake as
big.IntorstringinValidatorInfoto avoid precision loss.
284-295: TODO: Implement actual validator activity checking.The
calculateActivefunction contains a TODO comment and doesn't properly check validator activity. The current implementation relies on a stubgetLastActiveBlockthat always returns the current block height, making all validators appear active.Based on the available EventKeeper interface, implement proper activity checking:
func (k Keeper) calculateActive(ctx sdk.Context, netuid uint16, validators []types.ValidatorInfo, params types.EpochParams) []bool { active := make([]bool, len(validators)) currentBlock := uint64(ctx.BlockHeight()) + // Get active neurons from event module + activeNeurons := k.eventKeeper.GetActiveNeuronInfosByNetuid(ctx, netuid) + activeSet := make(map[string]struct{}, len(activeNeurons)) + for _, n := range activeNeurons { + activeSet[n.Account] = struct{}{} + } + for i, validator := range validators { - // Check if the validator has activity within the active cutoff time - // This needs to get the last active time from the event module - // For now, using a simple logic: all validators are set to active - // TODO: Implement actual active status check logic - lastActiveBlock := k.getLastActiveBlock(ctx, netuid, validator.Address) - if currentBlock-lastActiveBlock <= params.ActivityCutoff { + // Check if validator is in the active set + if _, ok := activeSet[validator.Address]; ok { active[i] = true - } else { - active[i] = false + continue + } + + // Fallback: check immunity period for new validators + if neuron, found := k.eventKeeper.GetNeuronInfo(ctx, netuid, validator.Address); found { + if params.ImmunityPeriod > 0 && currentBlock-neuron.RegistrationBlock <= params.ImmunityPeriod { + active[i] = true + continue + } } + active[i] = false } return active }Would you like me to help create an issue to track this implementation or provide the complete fix?
301-305: Stub implementation makes activity checking meaningless.The
getLastActiveBlockfunction is a stub that always returns the current block height, defeating the purpose of activity checking. This function should either be properly implemented or removed if using the suggestedGetActiveNeuronInfosByNetuidapproach above.If this function is kept, it should query actual validator activity:
func (k Keeper) getLastActiveBlock(ctx sdk.Context, netuid uint16, validator string) uint64 { - // TODO: Get the last active time for the validator from the event module - // For now, return the current block, indicating all validators are active - return uint64(ctx.BlockHeight()) + // Get neuron info to determine last activity + if neuron, found := k.eventKeeper.GetNeuronInfo(ctx, netuid, validator); found { + // Use registration block as a proxy for activity + // Note: LastUpdate is a Unix timestamp, not a block height + return neuron.RegistrationBlock + } + // Return 0 if no neuron info found (validator will be marked inactive) + return 0 }
345-351: Fix incorrect bounds check in weight matrix construction.The condition
if i < len(validators[i].Weights)is incorrect - it compares the row indexiwith the length of the weights array for that validator, which doesn't make sense. This could cause weights to be skipped incorrectly.for i := 0; i < n; i++ { weights[i] = make([]float64, n) - if i < len(validators[i].Weights) { - for j := 0; j < n; j++ { - if j < len(validators[i].Weights) { - weights[i][j] = float64(validators[i].Weights[j]) - } + for j := 0; j < n; j++ { + if j < len(validators[i].Weights) { + weights[i][j] = float64(validators[i].Weights[j]) } } }
570-575: String-based float storage is inefficient and error-prone.Storing float values as strings in the KV store is inefficient and can lead to precision issues. Use binary encoding instead:
// Read bonds value from storage bz := store.Get([]byte(key)) if bz != nil { - // Convert string back to float64 - if bondStr := string(bz); bondStr != "" { - if bondValue, err := strconv.ParseFloat(bondStr, 64); err == nil { - bonds[i][j] = bondValue - } - } + // Use binary encoding for efficiency + if len(bz) == 8 { + bonds[i][j] = math.Float64frombits(binary.BigEndian.Uint64(bz)) + } }Also update
saveBondsaccordingly:- // Store float64 as string - bondStr := fmt.Sprintf("%.10f", bondValue) - store.Set([]byte(key), []byte(bondStr)) + // Store float64 as binary for efficiency + bz := make([]byte, 8) + binary.BigEndian.PutUint64(bz, math.Float64bits(bondValue)) + store.Set([]byte(key), bz)Don't forget to add the
encoding/binaryimport.
🧹 Nitpick comments (11)
x/stakework/module.go (6)
21-24: Interface assertions likely won’t compile without the signature fixesWith the current method signatures, AppModule won’t satisfy github.com/cosmos/cosmos-sdk/types/module.AppModule. Fixing InitGenesis and BeginBlock as suggested will make these assertions valid. Alternatively, if you intend to target only the new cosmossdk.io/core/appmodule flow, drop the legacy assertion.
Two options:
- Preferred: keep both assertions and fix signatures (see other diffs).
- Alternative: remove the legacy assertion:
var ( - _ module.AppModule = AppModule{} _ module.AppModuleBasic = AppModuleBasic{} _ appmodule.AppModule = AppModule{} )
80-86: Rename unused parameter to avoid linter noisecdc is unused. If intentional (common in module scaffolding), rename it to _.
-func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { +func NewAppModule(_ codec.Codec, keeper keeper.Keeper) AppModule {
128-132: Good: fail fast in ProvideModule instead of returning a zero KeeperReturning an error instead of a zero-value Keeper resolves the earlier panic risk. Next step is wiring real dependencies (cdc, store key, event keeper) via depinject.
If you want, I can sketch a depinject.In struct and a NewKeeper call pattern for this module.
140-144: Duplicate ExportGenesis helper with inconsistent return type; remove or alignThis package-level ExportGenesis returns interface{} and nil, while AppModule.ExportGenesis returns json.RawMessage. Keeping both with different types is confusing.
Option A (remove dead helper):
-// ExportGenesis exports the genesis state -func ExportGenesis(ctx sdk.Context, k keeper.Keeper) interface{} { - // Simplified export, return empty state - return nil -}Option B (align types with AppModule and reuse):
-func ExportGenesis(ctx sdk.Context, k keeper.Keeper) interface{} { - // Simplified export, return empty state - return nil -} +func ExportGenesis(ctx sdk.Context, k keeper.Keeper) json.RawMessage { + // Simplified export, return empty state + return json.RawMessage(`{}`) +}
50-54: Genesis validation is a no-op; add minimal schema checksEven a thin module benefits from basic validation (e.g., tempo/epoch length, non-negative params) to fail fast on bad chains.
I can add a simple types.GenesisState with Validate() and wire it here if you provide the intended fields.
29-33: Use a central ModuleName constant for consistencyPrefer referencing types.ModuleName to avoid string drift.
-func (AppModuleBasic) Name() string { - return "stakework" -} +func (AppModuleBasic) Name() string { + return types.ModuleName +}Note: requires importing x/stakework/types and defining ModuleName there if not already present.
x/event/keeper/keeper.go (5)
228-233: Handle invalid Param JSON gracefully in SubnetRegisteredCurrently, JSON unmarshal errors are ignored. Log and proceed with defaults to avoid silent misconfigurations.
-params := types.DefaultParamsMap() // Default parameters map implemented in types -userParams := map[string]string{} -_ = json.Unmarshal([]byte(event.Param), &userParams) -for k, v := range userParams { - params[k] = v -} +params := types.DefaultParamsMap() +var userParams map[string]string +if err := json.Unmarshal([]byte(event.Param), &userParams); err != nil { + k.Logger(ctx).Warn("Invalid Param JSON in SubnetRegistered event; using defaults", "netuid", event.Netuid, "error", err) +} else { + for k, v := range userParams { + params[k] = v + } +}
351-356: Guard against weight overflow when casting big.Int to uint64A 256-bit EVM value can overflow uint64 silently via Uint64(). Clamp or reject oversized weights and log a warning.
-weights := make(map[string]uint64) -for _, w := range event.Weights { - weights[w.Dest.Hex()] = w.Weight.Uint64() -} +weights := make(map[string]uint64) +for _, w := range event.Weights { + if w.Weight.BitLen() > 64 { + k.Logger(ctx).Warn("Weight too large; clamping to uint64 max", "netuid", event.Netuid, "validator", event.Validator.Hex(), "dest", w.Dest.Hex(), "weight", w.Weight.String()) + weights[w.Dest.Hex()] = ^uint64(0) + } else { + weights[w.Dest.Hex()] = w.Weight.Uint64() + } +}
955-966: Fix mismatch between comment and behavior for GetAllSubnetNetuidsThe comment says “excluding root subnet 0” but the implementation includes it. Align the comment (or re-enable the filter if exclusion is intended).
Option A (update comment):
-// GetAllSubnetNetuids returns all subnet netuids (excluding root subnet 0) +// GetAllSubnetNetuids returns all subnet netuids (including root subnet 0)Option B (restore exclusion):
- for _, subnet := range subnets { - // if subnet.Netuid != 0 { // Filter out root subnet - // netuids = append(netuids, subnet.Netuid) - // } - netuids = append(netuids, subnet.Netuid) - } + for _, subnet := range subnets { + if subnet.Netuid != 0 { // Filter out root subnet + netuids = append(netuids, subnet.Netuid) + } + }
969-981: Remove stale commented-out filter or document intended inclusion of rootThe inline comment suggests filtering root but code includes it. Clean up the commented code to prevent confusion.
- // if subnet.Netuid != 0 && subnet.FirstEmissionBlock > 0 { // Filter out root subnet and subnets without first emission block set - // netuids = append(netuids, subnet.Netuid) - // } - if subnet.FirstEmissionBlock > 0 { // Filter out root subnet and subnets without first emission block set + if subnet.FirstEmissionBlock > 0 { netuids = append(netuids, subnet.Netuid) }
739-742: Handle json.Marshal errors when persisting Subnet (applies to other Set methods too)*Marshalling errors are ignored. Log and fail fast to avoid writing invalid state. Consider applying this pattern to other Set* methods for consistency.
func (k Keeper) SetSubnet(ctx sdk.Context, subnet types.Subnet) { store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte("subnet:")) - bz, _ := json.Marshal(subnet) + bz, err := json.Marshal(subnet) + if err != nil { + k.Logger(ctx).Error("failed to marshal subnet", "netuid", subnet.Netuid, "error", err) + return + } store.Set(uint16ToBytes(subnet.Netuid), bz) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
init.sh(1 hunks)x/blockinflation/keeper/coinbase.go(1 hunks)x/event/keeper/keeper.go(1 hunks)x/stakework/README.md(1 hunks)x/stakework/keeper/epoch.go(1 hunks)x/stakework/module.go(1 hunks)x/stakework/types/epoch.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- init.sh
🧰 Additional context used
🧬 Code Graph Analysis (4)
x/stakework/keeper/epoch.go (3)
x/event/keeper/keeper.go (1)
Keeper(52-83)x/stakework/keeper/keeper.go (1)
Keeper(15-19)x/stakework/types/epoch.go (4)
EpochResult(10-18)ParseEpochParams(118-268)ValidatorInfo(271-276)EpochParams(21-47)
x/stakework/module.go (3)
x/event/module.go (15)
AppModule(80-83)AppModule(96-96)AppModule(167-167)AppModule(170-170)AppModuleBasic(40-42)AppModuleBasic(48-50)AppModuleBasic(52-52)AppModuleBasic(53-53)AppModuleBasic(55-61)AppModuleBasic(63-69)AppModuleBasic(71-71)AppModuleBasic(72-72)AppModuleBasic(73-73)AppModuleBasic(74-74)NewAppModule(85-90)x/blockinflation/types/codec.go (2)
RegisterLegacyAminoCodec(20-22)RegisterInterfaces(25-27)x/stakework/keeper/keeper.go (1)
Keeper(15-19)
x/blockinflation/keeper/coinbase.go (5)
x/blockinflation/keeper/keeper.go (1)
Keeper(18-29)x/stakework/types/epoch.go (1)
ParseEpochParams(118-268)x/blockinflation/types/params.go (1)
Params(29-48)x/event/types/subnet.go (1)
SubnetEmissionData(96-100)x/blockinflation/types/emission.go (1)
SubnetEmissionData(8-18)
x/event/keeper/keeper.go (8)
testutil/network/network.go (2)
Logger(225-228)Validator(196-220)x/event/types/params.go (1)
DefaultParamsMap(5-37)x/event/types/subnet.go (3)
Subnet(8-18)SubnetInfo(51-65)SubnetEmissionData(96-100)x/blockinflation/types/params.go (1)
Params(29-48)x/event/types/stake.go (2)
ValidatorStake(5-9)Delegation(13-18)x/blockinflation/types/expected_keepers.go (1)
SubnetInfo(17-23)x/event/types/weight.go (1)
ValidatorWeight(3-7)x/blockinflation/types/emission.go (1)
SubnetEmissionData(8-18)
🪛 LanguageTool
x/stakework/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...ion of Bittensor epoch calculation logic - Weight Management: Handles weight allo...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...llocation and updates between validators - Consensus Calculation: Consensus mecha...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ensus mechanism based on weighted median - Reward Distribution: Distributes netwo...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...twork rewards based on consensus results - Bonds System: Implements exponential m...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ial moving average of historical weights - Dynamic Alpha: Supports dynamically ad...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...appa**: Majority threshold (default 0.5) - alpha: EMA parameter (default 0.1-0.9)...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...alpha**: EMA parameter (default 0.1-0.9) - delta: Weight clipping range (default ...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...a**: Weight clipping range (default 1.0) - tempo: Epoch execution frequency - **r...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...) - tempo: Epoch execution frequency - rho: Incentive parameter - **liquid_al...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...frequency - rho: Incentive parameter - liquid_alpha_enabled: Whether to enabl...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...le**: Provides subnet and validator data - Bank Module: Handles token transfers a...
(QB_NEW_EN)
[grammar] ~149-~149: There might be a mistake here.
Context: ...dles token transfers and balance queries - Staking Module: Gets staking-related i...
(QB_NEW_EN)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (2)
x/event/keeper/keeper.go (2)
85-126: Good: Topic IDs computed from ABIs in NewKeeperDeriving topic IDs from ABIs removes brittleness and aligns with deployed contracts.
144-196: Good: Dispatcher uses ABI-derived topicsSwitching on k.topicXXX ensures consistency with ABI-derived IDs. Logging is informative.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (12)
x/stakework/module.go (3)
3-18: Blocking: Missing abci import; fix method signatures to satisfy Cosmos SDK module interfacesBeginBlock and InitGenesis signatures don’t match the legacy Cosmos SDK module.AppModule interface, and you’re asserting conformance to it. Add the CometBFT abci types import and update both signatures. This was flagged earlier and remains unresolved.
Apply:
import ( "encoding/json" "fmt" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" + abci "github.com/cometbft/cometbft/abci/types" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/spf13/cobra" "github.com/hetu-project/hetu/v1/x/stakework/keeper" )
98-103: InitGenesis must return []abci.ValidatorUpdate (not []interface{})The current signature won’t satisfy module.AppModule and will fail the compile-time assertion. Return []abci.ValidatorUpdate instead. Previously flagged; still pending.
-// InitGenesis initializes the genesis state -func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, bz json.RawMessage) []interface{} { +// InitGenesis initializes the genesis state +func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, bz json.RawMessage) []abci.ValidatorUpdate { // Simplified initialization, mainly setting basic state InitGenesis(ctx, am.keeper) - return []interface{}{} + return []abci.ValidatorUpdate{} }
114-117: BeginBlock signature should use abci.RequestBeginBlockUsing interface{} breaks the module.AppModule contract. Switch to abci.RequestBeginBlock. Previously flagged; still pending.
-// BeginBlock executes at the beginning of each block -func (am AppModule) BeginBlock(ctx sdk.Context, _ interface{}) { - -} +// BeginBlock executes at the beginning of each block +func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { + // Epoch execution delegated to blockinflation's coinbase flow. +}x/blockinflation/keeper/alpha_token.go (1)
48-52: Address validation hardening looks solid (trim + IsHexAddress + zero-check)This resolves previous feedback about HexToAddress coercion and zero-address handling for both token and recipient addresses. Good defensive checks.
Also applies to: 60-68
x/blockinflation/types/subnet_reward.go (1)
13-15: Zero-subnet case should return base, not zeroThe documented formula yields base when subnetCount == 0. Returning zero contradicts the formula and will underpay subnets at genesis/early stages.
Apply this diff:
- if subnetCount == 0 { - return math.LegacyZeroDec() - } + if subnetCount == 0 { + // log(1 + 0) = 0 => ratio = base + return params.SubnetRewardBase + }x/blockinflation/keeper/genesis.go (1)
21-23: Call Params.Validate() in InitGenesisParams.Validate() is implemented at x/blockinflation/types/params.go — invoke it from InitGenesis to centralize validation and remove the manual mint-denom check.
- File to change:
- x/blockinflation/keeper/genesis.go
Suggested change (replace the manual check + TODO with a Validate call):
- // Validate parameters - if data.Params.MintDenom == "" { - panic("blockinflation: params.mint_denom must not be empty") - } - // TODO: If Params exposes a Validate() or ValidateBasic(), call it here. - // if err := data.Params.Validate(); err != nil { panic(fmt.Errorf("invalid blockinflation params: %w", err)) } + // Validate parameters + if err := data.Params.Validate(); err != nil { + panic(fmt.Errorf("invalid blockinflation params: %w", err)) + }x/event/keeper/keeper.go (6)
819-831: Avoid fallback to stake store; separate “historical” from “active” to prevent masking inconsistenciesThe fallback to the stake store when no active neurons are found can hide data inconsistencies between neuron info and stake storage.
- // If no active neurons found, fallback to original stake data query - if len(stakes) == 0 { - store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte("stake:")) - prefixKey := uint16ToBytes(netuid) - iterator := storetypes.KVStorePrefixIterator(store, prefixKey) - defer iterator.Close() - for ; iterator.Valid(); iterator.Next() { - var stake types.ValidatorStake - _ = json.Unmarshal(iterator.Value(), &stake) - stakes = append(stakes, stake) - } - } + // Return only active validator stakes; expose historical stakes via a dedicated method.And add this method elsewhere in the file:
// GetAllHistoricalValidatorStakesByNetuid returns all stakes including inactive validators func (k Keeper) GetAllHistoricalValidatorStakesByNetuid(ctx sdk.Context, netuid uint16) []types.ValidatorStake { store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte("stake:")) prefixKey := uint16ToBytes(netuid) it := storetypes.KVStorePrefixIterator(store, prefixKey) defer it.Close() var stakes []types.ValidatorStake for ; it.Valid(); it.Next() { var stake types.ValidatorStake _ = json.Unmarshal(it.Value(), &stake) stakes = append(stakes, stake) } return stakes }
1287-1347: Add input validation to UpdateMovingPrice (alpha bounds, halvingBlocks > 0)Out-of-range movingAlpha or halvingBlocks==0 can yield invalid math or degenerate behavior.
func (k Keeper) UpdateMovingPrice(ctx sdk.Context, netuid uint16, movingAlpha math.LegacyDec, halvingBlocks uint64) { + // Validate inputs + if movingAlpha.IsNegative() || movingAlpha.GT(math.LegacyNewDec(1)) { + k.Logger(ctx).Error("UpdateMovingPrice: movingAlpha out of [0,1]", "netuid", netuid, "movingAlpha", movingAlpha) + return + } + if halvingBlocks == 0 { + k.Logger(ctx).Error("UpdateMovingPrice: halvingBlocks cannot be zero", "netuid", netuid) + return + }
264-271: Use the found flag; netuid==0 is valid and shouldn’t signal “not found”Current logic treats netuid==0 as missing, breaking root-subnet updates.
- stake, _ := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) - if stake.Netuid == 0 { + stake, found := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) + if !found { stake = types.ValidatorStake{Netuid: event.Netuid, Validator: event.Validator.Hex(), Amount: "0"} } stake.Amount = types.AddBigIntString(stake.Amount, event.Amount.String()) k.SetValidatorStake(ctx, stake)
289-295: Use the found flag for self-unstakeAvoid misclassifying valid netuid=0 stakes as absent.
- stake, _ := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) - if stake.Netuid == 0 { + stake, found := k.GetValidatorStake(ctx, event.Netuid, event.Validator.Hex()) + if !found { return } stake.Amount = types.SubBigIntString(stake.Amount, event.Amount.String()) k.SetValidatorStake(ctx, stake)
309-315: Use the found flag for delegated stake updatesNetuid==0 check is ambiguous; rely on the boolean.
- deleg, _ := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) - if deleg.Netuid == 0 { + deleg, found := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) + if !found { deleg = types.Delegation{Netuid: event.Netuid, Validator: event.Validator.Hex(), Staker: event.Staker.Hex(), Amount: "0"} } deleg.Amount = types.AddBigIntString(deleg.Amount, event.Amount.String()) k.SetDelegation(ctx, deleg)
329-335: Use the found flag for delegated unstakeSame ambiguity; netuid==0 is a valid subnet.
- deleg, _ := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) - if deleg.Netuid == 0 { + deleg, found := k.GetDelegation(ctx, event.Netuid, event.Validator.Hex(), event.Staker.Hex()) + if !found { return } deleg.Amount = types.SubBigIntString(deleg.Amount, event.Amount.String()) k.SetDelegation(ctx, deleg)
🧹 Nitpick comments (15)
x/stakework/module.go (3)
80-86: Nit: mark unused codec param in NewAppModulecdc isn’t used. Mark it intentionally unused to avoid confusion and future linter noise.
-func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { +func NewAppModule(_ codec.Codec, keeper keeper.Keeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, keeper: keeper, } }
29-33: Prefer types.ModuleName over string literal for consistencyAvoid hardcoding the module name; use the canonical constant to prevent drift.
+import ( + // ... + staketypes "github.com/hetu-project/hetu/v1/x/stakework/types" +) @@ func (AppModuleBasic) Name() string { - return "stakework" + return staketypes.ModuleName }
139-143: Harmonize ExportGenesis helper return type or remove it if unusedThe package-level ExportGenesis returns interface{} while the AppModule method returns json.RawMessage. Consider aligning the helper’s type to json.RawMessage or removing it if unused to reduce confusion.
Option A (align type):
-func ExportGenesis(ctx sdk.Context, k keeper.Keeper) interface{} { - // Simplified export, return empty state - return nil +func ExportGenesis(ctx sdk.Context, k keeper.Keeper) json.RawMessage { + // Simplified export, return empty state + return json.RawMessage(`{}`) }Option B (remove helper):
-func ExportGenesis(ctx sdk.Context, k keeper.Keeper) interface{} { - // Simplified export, return empty state - return nil -} +// removed unused helperx/event/types/genesis.go (1)
44-74: Add hex address validation for better genesis integrity.The current validation checks are good for basic structural integrity, but they don't validate address formats. Since this is dealing with blockchain addresses (likely Ethereum hex addresses based on the context), consider adding hex address validation to catch malformed addresses early.
Apply this diff to add address validation:
+import ( + "fmt" + "github.com/ethereum/go-ethereum/common" +) func (gs GenesisState) Validate() error { seenNetuid := make(map[uint16]bool) for _, s := range gs.Subnets { if seenNetuid[s.Netuid] { return fmt.Errorf("duplicate subnet netuid: %d", s.Netuid) } seenNetuid[s.Netuid] = true if s.Netuid == 0 { return fmt.Errorf("subnet netuid must be > 0") } if s.Owner == "" { return fmt.Errorf("subnet owner must not be empty for netuid %d", s.Netuid) } + if s.Owner != "" && !common.IsHexAddress(s.Owner) { + return fmt.Errorf("invalid subnet owner address format: %s", s.Owner) + } } for _, vs := range gs.ValidatorStakes { if vs.Validator == "" { return fmt.Errorf("validator stake: empty validator for netuid %d", vs.Netuid) } + if vs.Validator != "" && !common.IsHexAddress(vs.Validator) { + return fmt.Errorf("invalid validator address format in stakes: %s", vs.Validator) + } } for _, d := range gs.Delegations { if d.Validator == "" || d.Staker == "" { return fmt.Errorf("delegation: empty validator/staker for netuid %d", d.Netuid) } + if d.Validator != "" && !common.IsHexAddress(d.Validator) { + return fmt.Errorf("invalid validator address format in delegations: %s", d.Validator) + } + if d.Staker != "" && !common.IsHexAddress(d.Staker) { + return fmt.Errorf("invalid staker address format in delegations: %s", d.Staker) + } } for _, vw := range gs.ValidatorWeights { if vw.Validator == "" { return fmt.Errorf("validator weights: empty validator for netuid %d", vw.Netuid) } + if vw.Validator != "" && !common.IsHexAddress(vw.Validator) { + return fmt.Errorf("invalid validator address format in weights: %s", vw.Validator) + } + for addr := range vw.Weights { + if !common.IsHexAddress(addr) { + return fmt.Errorf("invalid weight address format: %s", addr) + } + } } return nil }x/blockinflation/keeper/alpha_token.go (3)
41-47: Prefer typed AlphaToken from SubnetInfo; avoid stringly-typed paramsLeverage the typed field added in SubnetInfo to eliminate brittle map lookups and magic strings. Fallback to params only if SubnetInfo is missing the field.
- // 2. Resolve and validate the subnet's AlphaToken address - alphaTokenAddress, ok := subnet.Params["alpha_token"] - alphaTokenAddress = strings.TrimSpace(alphaTokenAddress) - if !ok || alphaTokenAddress == "" || !common.IsHexAddress(alphaTokenAddress) { - return fmt.Errorf("invalid or missing alpha token address in subnet params: %d", netuid) - } + // 2. Resolve and validate the subnet's AlphaToken address (prefer typed field) + var alphaTokenAddress string + if info, ok := k.eventKeeper.GetSubnetInfo(ctx, netuid); ok && strings.TrimSpace(info.AlphaToken) != "" { + alphaTokenAddress = strings.TrimSpace(info.AlphaToken) + } else { + addr, ok := subnet.Params["alpha_token"] + if !ok { + return fmt.Errorf("invalid or missing alpha token address in subnet params: %d", netuid) + } + alphaTokenAddress = strings.TrimSpace(addr) + } + if !common.IsHexAddress(alphaTokenAddress) { + return fmt.Errorf("invalid or missing alpha token address in subnet params: %d", netuid) + }
70-72: Propagate big.Int through the EVM call and loggingFollow-up to the API refactor: remove SetUint64, pass the big.Int directly, and log its string form.
- // 6. Convert amount to big.Int - amountBig := new(big.Int).SetUint64(amount) + // 6. Validate amount + if amount == nil { + return fmt.Errorf("amount cannot be nil") + } + if amount.Sign() < 0 { + return fmt.Errorf("amount cannot be negative") + } @@ - recipientAddr, - amountBig, + recipientAddr, + amount, @@ - "amount", amount, + "amount", amount.String(),Also applies to: 81-84, 92-92
70-72: Short-circuit zero-amount mintsMinting zero tokens wastes gas or may revert, depending on the contract. Return early when amount == 0.
- // 6. Convert amount to big.Int - amountBig := new(big.Int).SetUint64(amount) + // 6. No-op on zero amount + if amount == 0 { + k.Logger(ctx).Debug("Skipping alpha mint: zero amount", "netuid", netuid, "recipient", recipient) + return nil + } + amountBig := new(big.Int).SetUint64(amount)x/event/types/params_keys.go (4)
48-49: Make the alias map unexported to prevent external mutation.Exporting a map invites accidental writes from other packages and potential data races. Keeping it unexported and accessing through NormalizeParamKey avoids that risk.
Apply this diff to unexport and clarify the comment:
-// KeyAliases maps camelCase keys from ABI to canonical snake_case keys -var KeyAliases = map[string]string{ +// keyAliases maps camelCase keys from ABI to canonical snake_case keys (unexported to avoid external mutation) +var keyAliases = map[string]string{
93-96: Follow-up to unexport alias map: update usage in NormalizeParamKey.If you adopt the unexport, adjust the reference accordingly.
- if canonicalKey, exists := KeyAliases[key]; exists { + if canonicalKey, exists := keyAliases[key]; exists { return canonicalKey }
51-56: Trim redundant canonical entries from KeyAliases (nit).Entries like "rho", "kappa", "tempo", "alpha", and "delta" are already in canonical snake_case. Keeping KeyAliases strictly for non-canonical forms (camelCase) reduces duplication and future maintenance risk. No functional change, just cleanup.
Also applies to: 83-86
89-97: Optional: Add a strict normalization/validation path to catch unknown keys early.Current behavior passes unknown keys through unchanged. Consider offering a strict variant to fail fast or at least report unexpected keys. I can wire tests if helpful.
Example (new code, outside the current ranges):
// NormalizeParamKeysStrict normalizes keys and returns a list of unknown keys encountered. // Unknown means: neither a canonical key nor a known alias. func NormalizeParamKeysStrict(params map[string]string, knownCanonical map[string]struct{}) (map[string]string, []string) { normalized := make(map[string]string, len(params)) var unknown []string // Build known alias set from the alias map values and keys known := make(map[string]struct{}, len(knownCanonical)) for k := range knownCanonical { known[k] = struct{}{} } for alias := range keyAliases { known[alias] = struct{}{} } for key, value := range params { if _, ok := known[key]; !ok { unknown = append(unknown, key) } normalized[NormalizeParamKey(key)] = value } return normalized, unknown }Callers can decide to reject on len(unknown) > 0 or just log it.
x/event/keeper/keeper.go (4)
28-49: Remove deprecated hard-coded topic constantsThese constants are marked deprecated and duplicate the ABI-derived topic IDs. Keeping both invites drift and confusion. Drop the constants and corresponding crypto import if unused.
-// ----------- Event topic constants ----------- -// Deprecated: Use Keeper instance fields (k.topicXXX) instead of these global constants -var ( - // Legacy events remain unchanged (backward compatibility) - SubnetRegisteredTopic = crypto.Keccak256Hash([]byte("SubnetRegistered(address,uint16,uint256,uint256,address,string)")).Hex() - StakedSelfTopic = crypto.Keccak256Hash([]byte("Staked(uint16,address,uint256)")).Hex() - UnstakedSelfTopic = crypto.Keccak256Hash([]byte("Unstaked(uint16,address,uint256)")).Hex() - StakedDelegatedTopic = crypto.Keccak256Hash([]byte("Staked(uint16,address,address,uint256)")).Hex() - UnstakedDelegatedTopic = crypto.Keccak256Hash([]byte("Unstaked(uint16,address,address,uint256)")).Hex() - WeightsSetTopic = crypto.Keccak256Hash([]byte("WeightsSet(uint16,address,(address,uint256)[])")).Hex() - - // New event topics - NetworkRegisteredTopic = crypto.Keccak256Hash([]byte("NetworkRegistered(uint16,address,address,address,uint256,uint256,uint256,string,(uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint16,uint256,uint64,uint16,uint16,uint64,bool,bool,uint64,uint64,uint256,uint256))")).Hex() - SubnetActivatedTopic = crypto.Keccak256Hash([]byte("SubnetActivated(uint16,address,uint256,uint256)")).Hex() - NetworkConfigUpdatedTopic = crypto.Keccak256Hash([]byte("NetworkConfigUpdated(string,uint256,uint256,address)")).Hex() - NeuronRegisteredTopic = crypto.Keccak256Hash([]byte("NeuronRegistered(uint16,address,uint256,bool,bool,string,uint32,string,uint32,uint256)")).Hex() - NeuronDeregisteredTopic = crypto.Keccak256Hash([]byte("NeuronDeregistered(uint16,address,uint256)")).Hex() - NeuronStakeChangedTopic = crypto.Keccak256Hash([]byte("StakeAllocationChanged(uint16,address,uint256,uint256,uint256)")).Hex() - ServiceUpdatedTopic = crypto.Keccak256Hash([]byte("ServiceUpdated(uint16,address,string,uint32,string,uint32,uint256)")).Hex() - SubnetAllocationChangedTopic = crypto.Keccak256Hash([]byte("SubnetAllocationChanged(address,uint16,uint256,uint256)")).Hex() - DeallocatedFromSubnetTopic = crypto.Keccak256Hash([]byte("DeallocatedFromSubnet(address,uint16,uint256)")).Hex() -)
969-979: Comment and code disagree on root-subnet filteringThe comment says “Filter out root subnet”, but the code includes all subnets with FirstEmissionBlock > 0. Align comment or behavior.
- if subnet.FirstEmissionBlock > 0 { // Filter out root subnet and subnets without first emission block set + if subnet.FirstEmissionBlock > 0 { // Return subnets with first emission block set (root may be included)
739-742: Handle JSON marshal errors in settersIgnoring json.Marshal errors can hide corrupted/invalid state. At least log and return early.
- bz, _ := json.Marshal(subnet) - store.Set(uint16ToBytes(subnet.Netuid), bz) + bz, err := json.Marshal(subnet) + if err != nil { + k.Logger(ctx).Error("SetSubnet: marshal failed", "netuid", subnet.Netuid, "error", err) + return + } + store.Set(uint16ToBytes(subnet.Netuid), bz)Apply similarly to other Set* methods (stakes, delegations, weights, emission data).
515-529: Log when Subnet or SubnetInfo is missing during activationCurrently silent when not found; adding logs helps trace activation issues.
- subnet, found := k.GetSubnet(ctx, event.Netuid) - if found { + subnet, found := k.GetSubnet(ctx, event.Netuid) + if found { subnet.FirstEmissionBlock = event.BlockNumber.Uint64() k.SetSubnet(ctx, subnet) - } + } else { + k.Logger(ctx).Error("Subnet not found for activation", "netuid", event.Netuid) + } @@ - subnetInfo, found := k.GetSubnetInfo(ctx, event.Netuid) - if found { + subnetInfo, found := k.GetSubnetInfo(ctx, event.Netuid) + if found { subnetInfo.IsActive = true subnetInfo.ActivatedAt = event.Timestamp.Uint64() subnetInfo.ActivatedBlock = event.BlockNumber.Uint64() k.SetSubnetInfo(ctx, subnetInfo) - } + } else { + k.Logger(ctx).Error("SubnetInfo not found for activation", "netuid", event.Netuid) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
init.sh(1 hunks)x/blockinflation/keeper/alpha_token.go(1 hunks)x/blockinflation/keeper/genesis.go(1 hunks)x/blockinflation/keeper/inflation.go(1 hunks)x/blockinflation/types/expected_keepers.go(1 hunks)x/blockinflation/types/genesis.go(1 hunks)x/blockinflation/types/subnet_reward.go(1 hunks)x/event/keeper/keeper.go(1 hunks)x/event/types/expected_keepers.go(1 hunks)x/event/types/genesis.go(1 hunks)x/event/types/params.go(1 hunks)x/event/types/params_keys.go(1 hunks)x/stakework/module.go(1 hunks)x/stakework/types/interfaces.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- x/blockinflation/keeper/inflation.go
- x/stakework/types/interfaces.go
- x/blockinflation/types/expected_keepers.go
- x/event/types/params.go
- init.sh
🧰 Additional context used
🧬 Code Graph Analysis (8)
x/event/types/genesis.go (3)
x/event/types/subnet.go (1)
Subnet(8-18)x/event/types/stake.go (2)
ValidatorStake(5-9)Delegation(13-18)x/event/types/weight.go (1)
ValidatorWeight(3-7)
x/blockinflation/keeper/genesis.go (3)
x/blockinflation/keeper/keeper.go (1)
Keeper(18-29)x/blockinflation/types/genesis.go (1)
GenesisState(11-20)x/blockinflation/types/params.go (1)
Params(29-48)
x/blockinflation/types/subnet_reward.go (1)
x/blockinflation/types/params.go (1)
Params(29-48)
x/stakework/module.go (2)
x/event/module.go (14)
AppModule(80-83)AppModule(96-96)AppModule(167-167)AppModule(170-170)AppModuleBasic(40-42)AppModuleBasic(48-50)AppModuleBasic(52-52)AppModuleBasic(53-53)AppModuleBasic(55-61)AppModuleBasic(63-69)AppModuleBasic(71-71)AppModuleBasic(72-72)AppModuleBasic(73-73)AppModuleBasic(74-74)x/stakework/keeper/keeper.go (1)
Keeper(15-19)
x/blockinflation/keeper/alpha_token.go (3)
x/event/abi/abi.go (1)
AlphaTokenABI(29-29)x/blockinflation/keeper/keeper.go (1)
Keeper(18-29)x/event/keeper/keeper.go (1)
Keeper(52-83)
x/blockinflation/types/genesis.go (3)
x/blockinflation/types/params.go (2)
Params(29-48)DefaultParams(66-81)x/event/types/genesis.go (2)
DefaultGenesisState(34-41)GenesisState(11-16)app/app.go (1)
GenesisState(284-284)
x/event/types/expected_keepers.go (4)
x/event/types/subnet.go (2)
Subnet(8-18)NeuronInfo(68-81)x/blockinflation/types/expected_keepers.go (1)
SubnetInfo(17-17)x/event/types/stake.go (1)
ValidatorStake(5-9)x/event/types/weight.go (1)
ValidatorWeight(3-7)
x/event/keeper/keeper.go (6)
x/event/types/params.go (1)
DefaultParamsMap(5-37)x/event/types/subnet.go (3)
Subnet(8-18)SubnetInfo(51-65)SubnetEmissionData(96-100)x/event/types/stake.go (2)
ValidatorStake(5-9)Delegation(13-18)x/event/types/params_keys.go (11)
KeyRho(9-9)KeyKappa(10-10)KeyImmunityPeriod(11-11)KeyTempo(12-12)KeyMaxValidators(13-13)KeyActivityCutoff(14-14)KeyMaxAllowedUids(15-15)KeyMaxAllowedValidators(16-16)KeyMinAllowedWeights(17-17)KeyMaxWeightsLimit(18-18)KeyWeightsSetRateLimit(26-26)x/event/types/weight.go (1)
ValidatorWeight(3-7)x/blockinflation/types/emission.go (1)
SubnetEmissionData(8-18)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (12)
x/stakework/module.go (2)
20-24: Verify interface conformance after signature fixes (legacy and v2 appmodule)You assert both module.AppModule and core appmodule.AppModule. After fixing the signatures above, ensure the build succeeds for both. If you don’t fully implement core appmodule yet, consider removing that assertion until it’s wired.
Would you like me to scan the repo and list other modules’ AppModule signatures to mirror the exact shape you’re targeting?
127-131: Fail-fast ProvideModule is acceptable for nowReturning a non-nil error avoids nil keeper panics at runtime until DI is properly wired. Good call.
x/event/types/genesis.go (3)
34-41: LGTM! Properly addresses the past review feedback.The implementation correctly initializes all slices to empty slices (not nil) to avoid JSON serialization issues that would produce
nullarrays instead of[].
18-31: LGTM! Constructor function is well-structured.The
NewGenesisStatefunction provides a clean way to create genesis state instances with all required fields.
10-16: LGTM! Genesis state structure is well-defined.The
GenesisStatestruct properly encompasses all the necessary components for the event module's genesis state with appropriate JSON tags for serialization.x/event/types/expected_keepers.go (1)
9-67: Cohesive, well-scoped EventKeeper interfaceThe interface cleanly centralizes subnet, price, emission, and epoch access with consistent math types. This gives other modules (e.g., BlockInflation) a stable contract to depend on. LGTM.
x/event/types/params_keys.go (1)
6-46: Good centralization of canonical parameter keys.Clear grouping and naming of constants improves cross-module consistency. This will pay off when normalizing ABI payloads.
x/blockinflation/keeper/genesis.go (2)
11-75: Solid genesis validations and clear loggingNil-check, params validation (MintDenom), denom consistency, and non-negativity checks are all in place. Good defensive posture for genesis state integrity.
77-85: ExportGenesis looks correctExported fields mirror stored state and align with the module’s GenesisState schema.
x/blockinflation/types/genesis.go (2)
22-32: Deriving default denoms from Params prevents driftDefaultGenesisState now uses Params.MintDenom for all zero coins. Nice.
34-67: Comprehensive Validate() including denom consistencyValidation covers Params, individual coins, and matching denoms with MintDenom. This will catch misconfigured genesis early.
x/event/keeper/keeper.go (1)
351-356: Potential uint64 overflow when converting weightWeights are read as big.Int and cast to uint64. If contract-side weights can exceed 64 bits, this truncates. Confirm bounds or clamp/validate before conversion.
Can you confirm the maximum weight value emitted by WeightsSet? If it may exceed 2^64-1, we should store as string/big.Int or reject values above the cap.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
init_validators.sh (2)
64-68: Guarded deletes: protect against catastrophic rm -rfUnconditional rm -rf on HOME_PREFIX can be dangerous if mis-set. Add sanity checks similar to init.sh.
Apply this diff:
-# Clean local node data -rm -rf "${HOME_PREFIX}"/* +# +# Clean local node data (guarded) +if [[ -z "${HOME_PREFIX:-}" || "$HOME_PREFIX" = "/" || "$HOME_PREFIX" = "." ]]; then + echo "Refusing to remove HOME_PREFIX=$HOME_PREFIX" + exit 1 +fi +rm -rf "${HOME_PREFIX}"/* for i in $(seq 0 $((NUM_VALIDATORS - 1))); do echo "Cleaning up local validator $i data..." - rm -rf "${HOME_PREFIX}$i" + rm -rf "${HOME_PREFIX}$i" done
132-133: Parse address robustly: use hetud’s address-only outputGrep/cut on human output is brittle. Use -a or JSON output with jq.
Apply either of these diffs:
Option A (simplest, if supported):
-node_address=$(hetud keys show "$claims_key" --keyring-backend $KEYRING --home "$HOME_PREFIX" | grep "address" | cut -c12-) +node_address=$(hetud keys show "$claims_key" -a --keyring-backend "$KEYRING" --home "$HOME_PREFIX")Option B (portable via JSON):
-node_address=$(hetud keys show "$claims_key" --keyring-backend $KEYRING --home "$HOME_PREFIX" | grep "address" | cut -c12-) +node_address=$(hetud keys show "$claims_key" --output json --keyring-backend "$KEYRING" --home "$HOME_PREFIX" | jq -r '.address')
♻️ Duplicate comments (23)
init.sh (2)
2-3: Good: strict bash modes enabledset -euo pipefail and a safe IFS are in place. Solid baseline for safer scripting.
96-101: Nit: fix macOS-only sed syntax in commented exampleAvoid leaving non-portable examples around. Use a portable -i.bak form even in comments to prevent copy/paste issues.
Apply this diff:
-# sed -i '' "s/external_address = \"\"/external_address = \"YOUR_IP:26656\"/g" "$ETHCONFIG" +# sed -i.bak "s/external_address = \"\"/external_address = \"YOUR_IP:26656\"/g" "$ETHCONFIG" && rm -f "$ETHCONFIG.bak"init_validators.sh (2)
12-15: Require exactly 5 IPs to avoid mismatch with NUM_VALIDATORS=5The script assumes 5 validators; enforce an exact arg count for consistency.
Apply this diff:
-if [ "$#" -lt 5 ]; then +if [ "$#" -ne 5 ]; then echo "Usage: $0 <validator0_ip> <validator1_ip> <validator2_ip> <validator3_ip> <validator4_ip>" echo "Example: $0 1.2.3.4 5.6.7.8 9.10.11.12 13.14.15.16 17.18.19.20" exit 1 fi
26-26: Prefer "$@" for the IP array; ensures consistency and less maintenanceUsing all positional args avoids manual indexing and scales cleanly.
Apply this diff:
-# declare -a VALIDATOR_IPS=("$1" "$2" "$3" "$4" "$5" "$6" "$7") -declare -a VALIDATOR_IPS=("$1" "$2" "$3" "$4" "$5") +# declare -a VALIDATOR_IPS=("$1" "$2" "$3" "$4" "$5" "$6" "$7") +declare -a VALIDATOR_IPS=("$@")Optionally, derive NUM_VALIDATORS from the array length to keep them in sync:
-# NUM_VALIDATORS=7 -NUM_VALIDATORS=5 +NUM_VALIDATORS=${#VALIDATOR_IPS[@]}Also applies to: 29-29
x/event/types/utils.go (2)
3-6: Imports look correct and address prior feedbackfmt is now imported for error paths; resolves the earlier “import fmt” suggestion.
36-52: Consider offering a strict subtraction variant to avoid silent underflow clampClamping a < b to "0" can mask upstream underflow. You’ve documented it (good), but consider adding a strict variant that errors on underflow and invalid inputs. Keep this one for convenience if desired.
Example strict helper to add (outside this hunk):
// SubBigIntStringStrict subtracts b from a and returns an error if parsing fails or a < b. func SubBigIntStringStrict(a, b string) (string, error) { aInt := new(big.Int) if _, ok := aInt.SetString(a, 10); !ok { return "", fmt.Errorf("invalid big integer string a: %q", a) } bInt := new(big.Int) if _, ok := bInt.SetString(b, 10); !ok { return "", fmt.Errorf("invalid big integer string b: %q", b) } if aInt.Cmp(bInt) < 0 { return "", fmt.Errorf("underflow: %s - %s", a, b) } return new(big.Int).Sub(aInt, bInt).String(), nil }x/event/types/subnet.go (2)
9-20: Amounts as strings in core type: accessors help, but consider stronger typing longer-termKeeping LockedAmount/BurnedAmount/AmmPool as strings works with event-sourced data, and you added typed accessors (nice). Longer term, consider switching these to math.Int in the core Subnet type to reduce parsing at call sites and avoid accidental misuse.
188-193: Consider aligning emission schema with BlockInflation to avoid driftTo keep parity with x/blockinflation/types/emission.go and prevent lossy conversions, consider adding the extended fields (owner_cut, root_divs, subnet_* emissions). Defaults will serialize fine.
type SubnetEmissionData struct { - TaoInEmission math.Int `json:"tao_in_emission" yaml:"tao_in_emission"` // TAO input emission - AlphaInEmission math.Int `json:"alpha_in_emission" yaml:"alpha_in_emission"` // Alpha input emission - AlphaOutEmission math.Int `json:"alpha_out_emission" yaml:"alpha_out_emission"` // Alpha output emission + TaoInEmission math.Int `json:"tao_in_emission" yaml:"tao_in_emission"` // TAO input emission + AlphaInEmission math.Int `json:"alpha_in_emission" yaml:"alpha_in_emission"` // Alpha input emission + AlphaOutEmission math.Int `json:"alpha_out_emission" yaml:"alpha_out_emission"` // Alpha output emission + OwnerCut math.Int `json:"owner_cut" yaml:"owner_cut"` + RootDivs math.Int `json:"root_divs" yaml:"root_divs"` + SubnetAlphaInEmission math.Int `json:"subnet_alpha_in_emission" yaml:"subnet_alpha_in_emission"` + SubnetAlphaOutEmission math.Int `json:"subnet_alpha_out_emission" yaml:"subnet_alpha_out_emission"` + SubnetTaoInEmission math.Int `json:"subnet_tao_in_emission" yaml:"subnet_tao_in_emission"` }x/blockinflation/keeper/keeper.go (2)
66-76: Avoid mutating state on panic in GetParams; just return defaults.Writing DefaultParams() to KVStore from a getter on panic can mask wiring errors and unexpectedly change chain state. Prefer returning defaults without SetParams, and fix wiring (ParamKeyTable registration) at init.
Apply this minimal-change diff:
- defer func() { - if r := recover(); r != nil { - k.Logger(ctx).Warn("Panic in GetParamSet, writing default params", "panic", r) - // Write default parameters to KVStore - k.SetParams(ctx, blockinflationtypes.DefaultParams()) - params = blockinflationtypes.DefaultParams() - } - }() + defer func() { + if r := recover(); r != nil { + k.Logger(ctx).Error("subspace.GetParamSet panicked; returning DefaultParams()", "panic", r) + // Do not mutate state here; just return defaults as a safe fallback. + params = blockinflationtypes.DefaultParams() + } + }()
149-170: AddToPendingSubnetRewards silently fails on denom mismatch.The function logs an error but returns silently when the denom doesn't match, which could lead to silent failures in critical reward distribution logic. Consider returning an error to make failures explicit.
-func (k Keeper) AddToPendingSubnetRewards(ctx sdk.Context, amount sdk.Coin) { +func (k Keeper) AddToPendingSubnetRewards(ctx sdk.Context, amount sdk.Coin) error { params := k.GetParams(ctx) // Validate denom if amount.Denom != params.MintDenom { k.Logger(ctx).Error("invalid denom for pending subnet rewards", "expected", params.MintDenom, "got", amount.Denom, ) - return + return fmt.Errorf("invalid denom: expected %s, got %s", params.MintDenom, amount.Denom) } currentPending := k.GetPendingSubnetRewards(ctx) newPending := currentPending.Add(amount) k.SetPendingSubnetRewards(ctx, newPending) k.Logger(ctx).Info("added to pending subnet rewards", "amount", amount.String(), "total_pending", newPending.String(), ) + return nil }x/blockinflation/keeper/inflation_test.go (2)
183-195: Tests won't run: Keeper not initialized with stores/subspace.You call Keeper methods that rely on KV stores and param subspace (SetParams, SetTotalIssuance) but createTestKeeper returns a zero-value Keeper. This will panic or fail at runtime.
Since the tests need to interact with KV stores, properly wire a test multistore and initialize params subspace in createTestKeeper:
func createTestKeeper(t *testing.T) Keeper { - // Create a fully functional test keeper - // Use mocked dependencies to avoid complex initialization - k := Keeper{ - // Here we need to set necessary fields, but since the test mainly focuses on algorithm logic - // We can create a simplified version that only tests the core calculation logic - } - - // Note: This test keeper is mainly used for testing algorithm logic - // In actual use, you may need more complete mock objects - return k + // Create test store + db := dbm.NewMemDB() + stateStore := store.NewCommitMultiStore(db) + storeKey := storetypes.NewKVStoreKey(blockinflationtypes.StoreKey) + stateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, db) + require.NoError(t, stateStore.LoadLatestVersion()) + + // Create test subspace + subspace := createTestSubspace() + + k := Keeper{ + storeKey: storeKey, + subspace: subspace, + } + return k }
392-442: ImprovedPrecision test lacks concrete assertions.The test currently logs values but doesn't assert outcomes beyond early returns, so it can't fail on regressions. Add concrete assertions to verify precision calculations.
Add assertions to verify the computed values match expected precision:
// Test boundary value precision t.Run("Boundary value precision test", func(t *testing.T) { t.Logf("=== Boundary Value Precision Test ===") // Test precision near 50% boundary boundaryTests := []struct { name string issuanceRatio string expectedResult string + expectedEmission string }{ - {"Near 50% boundary", "10499999999999999999999999", "100% reward"}, - {"Exactly 50% boundary", "10500000000000000000000000", "50% reward"}, - {"Over 50% boundary", "10500000000000000000000001", "50% reward"}, + {"Near 50% boundary", "10499999999999999999999999", "100% reward", "1000000000000000000"}, + {"Exactly 50% boundary", "10500000000000000000000000", "50% reward", "500000000000000000"}, + {"Over 50% boundary", "10500000000000000000000001", "50% reward", "500000000000000000"}, } for _, test := range boundaryTests { t.Logf("\nTest: %s", test.name) issuance, _ := math.NewIntFromString(test.issuanceRatio) totalSupply, _ := math.NewIntFromString("21000000000000000000000000") defaultEmission, _ := math.NewIntFromString("1000000000000000000") + // Calculate emission using the direct algorithm + emission := calculateBlockEmissionDirect(issuance, totalSupply, defaultEmission) + expectedEmission, _ := math.NewIntFromString(test.expectedEmission) + + // Assert the emission matches expected value + require.Equal(t, expectedEmission, emission, + "Emission mismatch for %s: expected %s, got %s", + test.name, expectedEmission.String(), emission.String()) // Use high-precision calculation ratio := issuance.ToLegacyDec().Quo(totalSupply.ToLegacyDec()) t.Logf("Issuance ratio: %s", ratio.String()) // ... rest of logging ... } })app/app.go (2)
420-423: Use typed StoreKey constants instead of string literals.Using string literals for store keys ("event", "stakework") is error-prone and can lead to runtime errors if misspelled.
Create store key constants and use them consistently:
// event keys -"event", -"stakework", +eventtypes.StoreKey, +stakeworktypes.StoreKey, blockinflationtypes.StoreKey,Also update references at Lines 707 and 720:
-keys["event"], +keys[eventtypes.StoreKey], -keys["stakework"], +keys[stakeworktypes.StoreKey],Ensure stakeworktypes defines a StoreKey constant (add to x/stakework/types/keys.go if missing):
const ( // StoreKey defines the primary module store key StoreKey = ModuleName )
426-427: Missing memory store key for BlockInflationKeeper.The memKeys only registers capabilitytypes.MemStoreKey, but Line 729 attempts to access memKeys[blockinflationtypes.MemStoreKey], which will be nil and cause panics.
Add the missing memory store key:
-memKeys := storetypes.NewMemoryStoreKeys(capabilitytypes.MemStoreKey) +memKeys := storetypes.NewMemoryStoreKeys( + capabilitytypes.MemStoreKey, + blockinflationtypes.MemStoreKey, +)x/stakework/keeper/epoch.go (6)
276-294: TODO: Implement actual validator activity checking.The calculateActive function currently marks all validators as active due to the placeholder getLastActiveBlock implementation. This defeats the purpose of the ActivityCutoff parameter.
The event module already provides the necessary APIs. Would you like me to implement the active status check using GetActiveNeuronInfosByNetuid and GetNeuronInfo? This would involve:
- Building an active set from GetActiveNeuronInfosByNetuid
- Handling immunity period for new validators
- Using RegistrationBlock instead of LastUpdate for block-based comparisons
#!/bin/bash # Verify the available EventKeeper methods for activity tracking ast-grep --pattern 'type EventKeeper interface { $$$ GetActiveNeuronInfosByNetuid($$$) $$$ GetNeuronInfo($$$) $$$ $$$ }'
570-585: Consider using binary encoding for bond storage.Storing float values as strings is inefficient and can lead to precision issues. Binary encoding would be more efficient and preserve precision.
// Read bonds value from storage bz := store.Get([]byte(key)) if bz != nil { - // Convert string back to float64 - if bondStr := string(bz); bondStr != "" { - if bondValue, err := strconv.ParseFloat(bondStr, 64); err == nil { - bonds[i][j] = bondValue - } - } + // Use binary encoding for efficiency + if len(bz) == 8 { + bonds[i][j] = math.Float64frombits(binary.BigEndian.Uint64(bz)) + } }Also update saveBonds accordingly:
-// Store float64 as string -bondStr := fmt.Sprintf("%.10f", bondValue) -store.Set([]byte(key), []byte(bondStr)) +// Store float64 as binary for efficiency +bz := make([]byte, 8) +binary.BigEndian.PutUint64(bz, math.Float64bits(bondValue)) +store.Set([]byte(key), bz)
297-301: Placeholder returns incorrect active status.The getLastActiveBlock function always returns the current block height, making all validators appear active.
This needs to be properly implemented or removed if using the event module's active neuron list directly:
-func (k Keeper) getLastActiveBlock(ctx sdk.Context, netuid uint16, validator string) uint64 { - // TODO: Get the last active time for the validator from the event module - // For now, return the current block, indicating all validators are active - return uint64(ctx.BlockHeight()) -} +func (k Keeper) getLastActiveBlock(ctx sdk.Context, netuid uint16, validator string) uint64 { + neuron, found := k.eventKeeper.GetNeuronInfo(ctx, netuid, validator) + if !found { + return 0 // No activity record + } + return neuron.RegistrationBlock +}
349-361: Fix incorrect bounds check in weights matrix construction.The condition at Line 351 checks
i < len(validators[i].Weights)which is incorrect - it's comparing the validator index against the weights array length.Remove the incorrect outer condition:
for i := 0; i < n; i++ { weights[i] = make([]float64, n) - if i < len(validators[i].Weights) { - for j := 0; j < n; j++ { - if j < len(validators[i].Weights) { - weights[i][j] = float64(validators[i].Weights[j]) - } + for j := 0; j < n; j++ { + if j < len(validators[i].Weights) { + weights[i][j] = float64(validators[i].Weights[j]) } } }
644-646: Non-deterministic math.Exp in consensus code.The use of
math.Expin the sigmoid calculation is non-deterministic across different platforms and can cause consensus failures.Replace with a deterministic approximation:
-// sigmoid = 1 / (1 + exp(-steepness * (combined_diff - 0.5))) -sigmoid := 1.0 / (1.0 + math.Exp(-params.AlphaSigmoidSteepness*(combinedDiff-0.5))) +// Use deterministic piecewise linear approximation +x := params.AlphaSigmoidSteepness * (combinedDiff - 0.5) +var sigmoid float64 +switch { +case x <= -2.0: + sigmoid = 0.1 // Saturate at low end +case x >= 2.0: + sigmoid = 0.9 // Saturate at high end +default: + // Linear interpolation in the middle range + sigmoid = 0.5 + x * 0.2 +}
689-693: Non-deterministic math.Pow in consensus code.The use of
math.Powfor the rho parameter is non-deterministic and should be replaced.Restrict rho to specific values with deterministic implementations:
// Apply rho parameter for i := range normalized { if normalized[i] > 0 { - normalized[i] = math.Pow(normalized[i], rho) + // Use deterministic power calculation for common rho values + switch rho { + case 0.5: + // Square root using Newton's method (deterministic) + normalized[i] = deterministicSqrt(normalized[i]) + case 1.0: + // No change needed + case 2.0: + normalized[i] = normalized[i] * normalized[i] + default: + k.Logger(ctx).Error("Unsupported rho value", "rho", rho) + // Fallback to no transformation + } } }Add a helper for deterministic square root:
func deterministicSqrt(x float64) float64 { if x < 0 { return 0 } // Newton's method with fixed iterations guess := x / 2.0 for i := 0; i < 10; i++ { guess = (guess + x/guess) / 2.0 } return guess }x/stakework/module.go (3)
3-18: Add abci types import (and types.ModuleName) to satisfy module interfacesWithout the CometBFT abci import, the canonical AppModule method signatures can’t be used. Also, prefer using types.ModuleName to avoid hardcoding the module name string.
Apply this diff to the import block:
import ( "encoding/json" "fmt" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/spf13/cobra" + abci "github.com/cometbft/cometbft/abci/types" + "github.com/hetu-project/hetu/v1/x/stakework/types" "github.com/hetu-project/hetu/v1/x/stakework/keeper" )
98-104: InitGenesis return type must be []abci.ValidatorUpdateReturning []interface{} won’t satisfy module.AppModule. Update to the canonical type.
-func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, bz json.RawMessage) []interface{} { +func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, bz json.RawMessage) []abci.ValidatorUpdate { // Simplified initialization, mainly setting basic state InitGenesis(ctx, am.keeper) - return []interface{}{} + return []abci.ValidatorUpdate{} }
114-117: BeginBlock signature should use abci.RequestBeginBlockUse the standard signature to satisfy module.AppModule and avoid ambiguous types.
-func (am AppModule) BeginBlock(ctx sdk.Context, _ interface{}) { +func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { }
🧹 Nitpick comments (14)
init.sh (2)
36-41: Preflight dependency checks for hetud and jqAdd explicit checks for hetud and jq before first use for faster failures and clearer diagnostics.
Apply this diff near the build section (after Line 21):
+# Verify required commands exist +command -v hetud >/dev/null 2>&1 || { echo >&2 "hetud not found in PATH"; exit 1; } +command -v jq >/dev/null 2>&1 || { echo >&2 "jq not installed. See: https://stedolan.github.io/jq/download/"; exit 1; }
43-47: Optional: batch jq mutations to reduce file churnMultiple sequential jq reads/writes increase IO and failure points. Consider composing a single jq filter to set all fields in one pass. Not blocking.
Also applies to: 52-63
init_validators.sh (3)
3-4: Enable strict bash modes for robustnessUse set -euo pipefail and a safe IFS to catch unset vars and pipeline failures.
Apply this diff:
-# Exit on error -set -e +# Exit on error with strict modes +set -euo pipefail +IFS=$'\n\t'
188-202: Nit: use port variables instead of hardcoded 26656/26657You already define P2P_PORT/RPC_PORT. Using them avoids drift.
Apply this diff:
- sed -i.bak -e '/^\[rpc\]/,/^\[/s|^laddr *= *.*|laddr = "tcp://0.0.0.0:26657"|' "$CONFIG_TOML" - sed -i.bak -e '/^\[p2p\]/,/^\[/s|^laddr *= *.*|laddr = "tcp://0.0.0.0:26656"|' "$CONFIG_TOML" + sed -i.bak -e "/^\[rpc\]/,/^\[/s|^laddr *= *.*|laddr = \"tcp://0.0.0.0:${RPC_PORT}\"|" "$CONFIG_TOML" + sed -i.bak -e "/^\[p2p\]/,/^\[/s|^laddr *= *.*|laddr = \"tcp://0.0.0.0:${P2P_PORT}\"|" "$CONFIG_TOML"
301-306: Quote remote paths in SSH/rsync and guard deletesEnsure quoting and guard remote rm similarly here.
Apply this diff:
- # ssh ubuntu@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" - ssh root@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" + # ssh ubuntu@"${TARGET_IP}" 'if [[ -z "'"$HOME_PREFIX"'" || "'"$HOME_PREFIX"'" = "/" || "'"$HOME_PREFIX"'" = "." ]]; then echo "Refusing to remove HOME_PREFIX" >&2; exit 1; fi; rm -rf "'"$HOME_PREFIX$i"'"' + ssh root@"${TARGET_IP}" 'if [[ -z "'"$HOME_PREFIX"'" || "'"$HOME_PREFIX"'" = "/" || "'"$HOME_PREFIX"'" = "." ]]; then echo "Refusing to remove HOME_PREFIX" >&2; exit 1; fi; rm -rf "'"$HOME_PREFIX$i"'"' @@ - rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/" + rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/"Optional: consider -e 'ssh -o StrictHostKeyChecking=no' for first-time hosts in CI.
x/event/types/utils.go (3)
8-20: Minor simplification: drop explicit SetInt64(0) fallbacknew(big.Int) is already zero; calling SetInt64(0) in the parse-failure branches is redundant. If you want to keep explicitness that’s fine; otherwise this small tweak trims noise.
func AddBigIntString(a, b string) string { - aInt := new(big.Int) - if _, ok := aInt.SetString(a, 10); !ok { - aInt.SetInt64(0) - } - bInt := new(big.Int) - if _, ok := bInt.SetString(b, 10); !ok { - bInt.SetInt64(0) - } + aInt := new(big.Int) + _, _ = aInt.SetString(a, 10) // on parse failure, remains 0 + bInt := new(big.Int) + _, _ = bInt.SetString(b, 10) // on parse failure, remains 0 return new(big.Int).Add(aInt, bInt).String() }
54-70: Name vs behavior: “WithError” also clamps underflow to "0"The contract is clear in the comment, but the name might imply “only parse errors.” If you keep underflow clamping here, consider providing a strict variant (see previous comment) and/or renaming for clarity in the future. No blocking change required.
8-70: Add targeted unit tests for parse failures and underflow behaviorRecommend tests covering:
- Parse errors (ensure non-error variants treat invalid as 0; error variants return errors).
- Underflow (a < b) for both Sub variants (clamp vs strict behavior if added).
- Very large values to ensure correctness of big.Int arithmetic.
I can scaffold these tests if helpful.
x/event/types/subnet.go (1)
48-76: Hyperparams: add typed accessors and basic validation for string/threshold fieldsA few fields are strings likely representing amounts/decimals (BaseNeuronCost, ValidatorThreshold, NeuronThreshold). To avoid scattered parsing logic and ensure constraints, consider:
- Adding accessors that parse these into math.Int/math.LegacyDec.
- A ValidateBasic() method enforcing sensible ranges (e.g., thresholds in [0,1]).
I can provide a focused patch once you confirm the exact Cosmos SDK version (parsing helpers differ slightly across versions).
x/stakework/keeper/epoch.go (1)
305-320: Consider adding error handling for stake parsing.The function logs errors but continues processing, which could lead to incorrect calculations if stake parsing fails.
Consider returning an error if critical validators have invalid stake values:
func (k Keeper) getStakeWeights(ctx sdk.Context, netuid uint16, validators []types.ValidatorInfo) []float64 { stake := make([]float64, len(validators)) + var parseErrors []string for i, validator := range validators { // Parse string stake to big.Int, then convert to float64 bigIntStake, err := validator.GetStakeBigInt() if err != nil { k.Logger(ctx).Error("Failed to parse stake", "validator", validator.Address, "stake", validator.Stake, "error", err) + parseErrors = append(parseErrors, fmt.Sprintf("validator %s: %v", validator.Address, err)) continue // Skip this validator or set to 0 } // ... rest of conversion } + if len(parseErrors) > 0 && len(parseErrors) > len(validators)/2 { + k.Logger(ctx).Error("Too many stake parsing errors", "errors", parseErrors) + // Consider returning an error or handling this case + } return stake }x/stakework/module.go (4)
29-33: Use types.ModuleName instead of a string literalPrevents drift and aligns with common Cosmos SDK patterns.
-func (AppModuleBasic) Name() string { - return "stakework" -} +func (AppModuleBasic) Name() string { + return types.ModuleName +}
50-54: Silence unused param warning in ValidateGenesisThe config param isn’t used; name it
_to satisfy linters without changing behavior.-func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config client.TxEncodingConfig, bz json.RawMessage) error { +func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, _ client.TxEncodingConfig, bz json.RawMessage) error { // Simplified module, no need to validate for now return nil }
80-86: Rename unused constructor arg to_cdc is not used; renaming avoids linter noise.
-func NewAppModule(cdc codec.Codec, keeper *keeper.Keeper) AppModule { +func NewAppModule(_ codec.Codec, keeper *keeper.Keeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, keeper: keeper, } }
145-148: Unify ExportGenesis helper to return json.RawMessage (or remove it)The free function returning interface{} is inconsistent with the AppModule’s ExportGenesis (json.RawMessage). Either delete the helper or return json.RawMessage here for consistency.
-func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) interface{} { - // Simplified export, return empty state - return nil -} +func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) json.RawMessage { + // Simplified export, return empty state + return json.RawMessage(`{}`) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
app/app.go(19 hunks)init.sh(1 hunks)init_validators.sh(11 hunks)x/blockinflation/client/cli/query.go(1 hunks)x/blockinflation/keeper/coinbase.go(1 hunks)x/blockinflation/keeper/inflation_test.go(1 hunks)x/blockinflation/keeper/keeper.go(1 hunks)x/blockinflation/types/keys.go(1 hunks)x/event/keeper/keeper.go(1 hunks)x/event/types/subnet.go(1 hunks)x/event/types/utils.go(1 hunks)x/evm/keeper/keeper.go(2 hunks)x/evm/keeper/state_transition.go(2 hunks)x/stakework/abci.go(1 hunks)x/stakework/keeper/epoch.go(1 hunks)x/stakework/keeper/keeper.go(1 hunks)x/stakework/module.go(1 hunks)x/stakework/types/epoch.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- x/stakework/abci.go
🚧 Files skipped from review as they are similar to previous changes (7)
- x/evm/keeper/state_transition.go
- x/evm/keeper/keeper.go
- x/blockinflation/keeper/coinbase.go
- x/blockinflation/types/keys.go
- x/stakework/types/epoch.go
- x/blockinflation/client/cli/query.go
- x/event/keeper/keeper.go
🧰 Additional context used
🧬 Code Graph Analysis (7)
x/stakework/keeper/keeper.go (2)
x/stakework/types/module.go (1)
ModuleName(4-4)x/stakework/types/interfaces.go (1)
EventKeeper(9-9)
x/stakework/keeper/epoch.go (3)
x/stakework/keeper/keeper.go (1)
Keeper(15-19)x/stakework/types/epoch.go (4)
EpochResult(11-19)ParseEpochParams(145-295)ValidatorInfo(298-303)EpochParams(48-74)x/blockinflation/types/params.go (1)
Params(29-48)
x/blockinflation/keeper/keeper.go (3)
x/blockinflation/types/keys.go (5)
StoreKey(8-8)ModuleName(5-5)TotalIssuanceKey(22-22)TotalBurnedKey(25-25)PendingSubnetRewardsKey(28-28)x/blockinflation/types/expected_keepers.go (5)
AccountKeeper(20-23)BankKeeper(26-32)EventKeeper(36-36)StakeworkKeeper(39-42)ERC20Keeper(45-47)x/blockinflation/types/params.go (2)
Params(29-48)DefaultParams(66-81)
x/blockinflation/keeper/inflation_test.go (3)
x/blockinflation/types/params.go (3)
DefaultParams(66-81)NewParams(51-63)ParamKeyTable(24-26)x/blockinflation/types/subnet_reward.go (1)
CalculateSubnetRewardRatio(12-35)x/blockinflation/keeper/keeper.go (2)
Keeper(18-29)NewKeeper(32-56)
x/event/types/subnet.go (2)
x/blockinflation/types/params.go (1)
Params(29-48)x/blockinflation/types/expected_keepers.go (1)
SubnetInfo(17-17)
x/stakework/module.go (3)
x/event/module.go (14)
AppModule(80-83)AppModule(96-96)AppModule(167-167)AppModule(170-170)AppModuleBasic(40-42)AppModuleBasic(48-50)AppModuleBasic(52-52)AppModuleBasic(53-53)AppModuleBasic(55-61)AppModuleBasic(63-69)AppModuleBasic(71-71)AppModuleBasic(72-72)AppModuleBasic(73-73)AppModuleBasic(74-74)x/blockinflation/types/codec.go (2)
RegisterLegacyAminoCodec(20-22)RegisterInterfaces(25-27)x/stakework/keeper/keeper.go (1)
Keeper(15-19)
app/app.go (12)
x/blockinflation/module.go (9)
AppModuleBasic(32-32)AppModuleBasic(35-37)AppModuleBasic(40-42)AppModuleBasic(50-56)AppModuleBasic(59-65)AppModuleBasic(68-70)AppModuleBasic(73-75)AppModuleBasic(78-80)NewAppModule(89-94)x/blockinflation/types/keys.go (3)
ModuleName(5-5)StoreKey(8-8)MemStoreKey(17-17)x/stakework/types/module.go (3)
ModuleName(4-4)StoreKey(7-7)MemStoreKey(13-13)x/event/types/keys.go (3)
ModuleName(5-5)StoreKey(8-8)MemStoreKey(14-14)x/blockinflation/types/expected_keepers.go (4)
EventKeeper(36-36)StakeworkKeeper(39-42)AccountKeeper(20-23)BankKeeper(26-32)x/stakework/types/interfaces.go (1)
EventKeeper(9-9)x/blockinflation/keeper/keeper.go (2)
Keeper(18-29)NewKeeper(32-56)x/stakework/keeper/keeper.go (2)
Keeper(15-19)NewKeeper(22-35)x/event/keeper/keeper.go (2)
Keeper(52-83)NewKeeper(86-126)x/event/abi/abi.go (7)
SubnetRegistryABI(6-6)StakingSelfABI(9-9)StakingDelegatedABI(12-12)WeightsABI(15-15)SubnetManagerABI(20-20)NeuronManagerABI(23-23)GlobalStakingABI(26-26)x/stakework/module.go (1)
NewAppModule(81-86)x/blockinflation/types/params.go (1)
ParamKeyTable(24-26)
🪛 Gitleaks (8.27.2)
init.sh
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (19)
init.sh (3)
22-30: Good: guarded reset with SKIP_RESET and sanity checksThe rm -rf is properly gated against empty or “/” paths and SKIP_RESET=1. This averts catastrophic deletions.
76-80: Good: portable sed handling (GNU/BSD)Runtime detection of GNU vs BSD sed with appropriate -i flags looks correct.
36-37: Gitleaks “Generic API Key” looks like a false positiveThis line adds a local dev key to the keyring; there is no hard-coded secret or API key in the script. Safe to ignore the finding.
If your CI still fails on this, add an allowlist rule for this path/pattern in gitleaks, or rename KEY from “dev0” to something less trigger-happy (e.g., “local_dev_key”).
init_validators.sh (1)
108-113: Genesis edits align with denom and new modulesStaking/crisis/gov/evm denom changes and blockinflation/event genesis scaffolding look consistent with the PR’s module defaults and params.
Also applies to: 114-121
x/event/types/utils.go (1)
22-34: Addition with error propagation reads cleanThe error-returning variant is correct and clear. No issues spotted.
x/event/types/subnet.go (8)
22-33: Getter for LockedAmount: LGTMClear empty-string handling and proper error propagation.
35-46: Getter for BurnedAmount: LGTMSame as above; correct implementation.
78-93: SubnetInfo schema and naming aligned; good for type aliasing in BlockInflationFields and tags are consistent with downstream usage (e.g., aliasing in x/blockinflation/types). No issues spotted.
95-132: SubnetInfo parsing helpers: LGTMConsistent empty-string and error handling across LockedAmount/BurnedAmount/PoolInitialTao.
149-163: NeuronInfo: LGTMSchema and tags look good.
165-176: Stake parsing helper: LGTMCorrect empty-string behavior and error propagation.
178-186: SubnetPriceData: LGTMTypes match expected semantics (LegacyDec for prices, Int for amounts).
134-147: Confirm: FirstEmissionBlock == activation block (ActivatedBlock)Mapping is correct — callers expect the activation block as first emission block.
- x/event/types/subnet.go: ToSubnet sets FirstEmissionBlock = s.ActivatedBlock (line ~143)
- x/event/keeper/keeper.go: handleSubnetActivated sets subnet.FirstEmissionBlock = event.BlockNumber.Uint64() and updates subnetInfo.ActivatedAt/ActivatedBlock (line ~518)
- x/event/keeper/keeper.go: GetSubnetsToEmitTo filters on subnet.FirstEmissionBlock > 0 (line ~976)
- x/event/keeper/keeper.go: callers use GetSubnetFirstEmissionBlock and compute startCallBlock := int64(firstEmissionBlock) - 1 when scanning (lines ~1289–1298) — consistent with activation-block semantics
- x/blockinflation/keeper/coinbase.go: GetSubnetFirstEmissionBlock used to include/exclude subnets from emission (line ~375)
No changes required.
x/stakework/keeper/keeper.go (1)
1-46: LGTM!The Keeper structure is well-designed with clear dependencies. The constructor properly validates the eventKeeper requirement, and methods follow pointer receiver conventions as suggested in past reviews.
app/app.go (2)
705-738: LGTM! Well-structured keeper initialization.The initialization flow properly establishes dependencies: EventKeeper → StakeworkKeeper → BlockInflationKeeper. The ABI parsing includes comprehensive error handling, and the keeper constructors are called in the correct order.
968-970: LGTM! Event handler wiring is appropriate.Setting the EventKeeper as the EVM event handler after all keepers are initialized is the correct approach for cross-module event processing.
x/stakework/keeper/epoch.go (2)
22-202: Well-structured epoch calculation implementation!The RunEpoch method follows a clear, step-by-step workflow with comprehensive debug logging at each stage. The error handling is appropriate, and the function properly manages the complete epoch lifecycle from parameter parsing to bond persistence.
553-555: Improved precision preservation in emission distribution.Good fix! Combining the weighted values before conversion minimizes precision loss compared to the previous implementation that performed separate conversions.
x/stakework/module.go (1)
132-136: ProvideModule intentionally fails fast; confirm it’s not wired at app startupReturning an error is safer than a zero-value Keeper, but this will abort DI if ProvideModule is part of the app’s provider set. Ensure app/app.go doesn’t rely on this provider yet, or wire the keeper properly before merging.
If you want, I can draft the DI wiring skeleton (store key, codec, param subspace, event keeper) to return a real keeper.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
go.mod (1)
3-3: Invalidgodirective: must not include patch version
godirective only accepts major.minor (e.g.,go 1.23), not a patch (1.23.3). This will causego mod tidyand builds to fail.Apply this diff:
-go 1.23.3 +go 1.23init_validators.sh (1)
58-63: Guard local rm -rf with HOME_PREFIX sanity checksLocal deletions lack the safety check you added for remote. Add the same guard to prevent catastrophic deletes if HOME_PREFIX is empty or unsafe.
Apply:
-# Clean local node data -rm -rf "${HOME_PREFIX}"/* +# Clean local node data (guard against unsafe HOME_PREFIX) +if [[ -z "${HOME_PREFIX:-}" || "${HOME_PREFIX}" = "/" || "${HOME_PREFIX}" = "." ]]; then + echo "Refusing to remove HOME_PREFIX: '${HOME_PREFIX}'" >&2 + exit 1 +fi +rm -rf "${HOME_PREFIX}"/*
♻️ Duplicate comments (8)
x/blockinflation/keeper/keeper.go (2)
69-83: Do not mutate chain state in GetParams panic recovery; return defaults insteadMutating KV in a getter masks wiring issues and can unexpectedly change state during a panic. Log and return defaults without SetParams.
Apply this minimal diff:
func (k Keeper) GetParams(ctx sdk.Context) blockinflationtypes.Params { var params blockinflationtypes.Params defer func() { if r := recover(); r != nil { - k.Logger(ctx).Warn("Panic in GetParamSet, writing default params", "panic", r) - // Write default parameters to KVStore - k.SetParams(ctx, blockinflationtypes.DefaultParams()) - params = blockinflationtypes.DefaultParams() + k.Logger(ctx).Error("subspace.GetParamSet panicked; returning DefaultParams()", "panic", r) + // Do not mutate state here; just return defaults as a safe fallback. + params = blockinflationtypes.DefaultParams() } }() k.subspace.GetParamSet(ctx, ¶ms) return params }
156-177: Make denom mismatch in AddToPendingSubnetRewards fatal to callersSilently returning on denom mismatch can hide reward distribution bugs. Return an error instead and let callers decide.
Apply this diff (and update callers accordingly):
-func (k Keeper) AddToPendingSubnetRewards(ctx sdk.Context, amount sdk.Coin) { +func (k Keeper) AddToPendingSubnetRewards(ctx sdk.Context, amount sdk.Coin) error { params := k.GetParams(ctx) @@ - if amount.Denom != params.MintDenom { - k.Logger(ctx).Error("invalid denom for pending subnet rewards", - "expected", params.MintDenom, - "got", amount.Denom, - ) - return - } + if amount.Denom != params.MintDenom { + err := fmt.Errorf("invalid denom for pending subnet rewards: expected %s, got %s", params.MintDenom, amount.Denom) + k.Logger(ctx).Error("invalid denom for pending subnet rewards", "err", err) + return err + } @@ k.Logger(ctx).Info("added to pending subnet rewards", "amount", amount.String(), "total_pending", newPending.String(), ) + return nil }init_validators.sh (1)
24-24: Prefer using "$@" for the array to avoid manual arg expansionThis makes the array robust and simpler, with your existing -ne 5 guard ensuring exactly five items.
Apply:
-declare -a VALIDATOR_IPS=("$1" "$2" "$3" "$4" "$5") +declare -a VALIDATOR_IPS=("$@")x/blockinflation/module.go (4)
52-58: Use proper JSON marshaling for genesis state.The DefaultGenesis method is directly marshaling the struct instead of using the codec.JSONCodec parameter. This bypasses the codec's JSON handling which may have custom serialization logic.
func (AppModuleBasic) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage { - bz, err := json.Marshal(*types.DefaultGenesisState()) - if err != nil { - panic(err) - } - return bz + return cdc.MustMarshalJSON(types.DefaultGenesisState()) }
61-67: Use codec for unmarshaling genesis state consistently.The ValidateGenesis method uses standard JSON unmarshaling while DefaultGenesis above doesn't use the codec. Use the codec consistently for both operations.
func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config client.TxConfig, bz json.RawMessage) error { var genState types.GenesisState - if err := json.Unmarshal(bz, &genState); err != nil { + if err := cdc.UnmarshalJSON(bz, &genState); err != nil { return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err) } return genState.Validate() }
123-130: Use codec for unmarshaling in InitGenesis.The InitGenesis method should use the codec parameter for consistency with the module pattern.
func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, data json.RawMessage) []abci.ValidatorUpdate { var genesisState types.GenesisState - if err := json.Unmarshal(data, &genesisState); err != nil { + if err := cdc.UnmarshalJSON(data, &genesisState); err != nil { panic(err) } am.keeper.InitGenesis(ctx, cdc, &genesisState) return []abci.ValidatorUpdate{} }
134-141: Use codec for marshaling in ExportGenesis.The ExportGenesis method should use the codec parameter consistently instead of standard JSON marshaling.
func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.RawMessage { gs := am.keeper.ExportGenesis(ctx, cdc) - bz, err := json.Marshal(gs) - if err != nil { - panic(err) - } - return bz + return cdc.MustMarshalJSON(gs) }app/app.go (1)
421-423: Use typed StoreKey constants instead of string literals.Using string literals for store keys ("event", "stakework") is error-prone. Consider defining these as constants in the respective module's types package.
🧹 Nitpick comments (4)
go.mod (1)
242-247: OTel auto SDK + OTel core bumped; consider convergence with OpenCensusYou’ve added/updated:
- go.opentelemetry.io/auto/sdk v1.1.0 (indirect)
- otel, otel/metric, otel/trace v1.35.0
The repo still depends on
go.opencensus.io(Line 56). Dual telemetry frameworks can result in duplicate exporters and overhead.Consider migrating remaining OpenCensus usage to OTel or guard one of the stacks via build tags to avoid double instrumentation.
x/blockinflation/types/params_test.go (2)
27-33: Defaults coverage looks solid; consider asserting Validate() passes for defaultsYou’re locking in the economic defaults properly. Add an assertion that DefaultParams().Validate() succeeds to ensure defaults remain internally consistent.
Apply this diff to strengthen the test:
func TestDefaultParams(t *testing.T) { params := DefaultParams() @@ require.True(t, params.EnableBlockInflation, "Block inflation should be enabled by default") // Verify subnet reward parameters require.True(t, params.SubnetRewardBase.Equal(math.LegacyNewDecWithPrec(10, 2)), "Subnet reward base should be 0.10") require.True(t, params.SubnetRewardK.Equal(math.LegacyNewDecWithPrec(10, 2)), "Subnet reward K should be 0.10") require.True(t, params.SubnetRewardMaxRatio.Equal(math.LegacyNewDecWithPrec(90, 2)), "Subnet reward max ratio should be 0.90") require.True(t, params.SubnetMovingAlpha.Equal(math.LegacyNewDecWithPrec(3, 6)), "Subnet moving alpha should be 0.000003") require.True(t, params.SubnetOwnerCut.Equal(math.LegacyNewDecWithPrec(18, 2)), "Subnet owner cut should be 0.18") + + // Defaults should always validate + require.NoError(t, params.Validate(), "Default params should validate")
42-52: Avoid brittle string-based error assertionsAsserting substrings in error messages makes tests brittle. Prefer typed/sentinel errors or minimally assert that an error occurred and optionally validate invariants via structured errors if available.
For example, if there’s no typed error, reduce to:
- require.Error(t, err, "Validation should fail when SubnetRewardBase > SubnetRewardMaxRatio") - require.Contains(t, err.Error(), "subnet reward base", "Error message should mention subnet reward base") - require.Contains(t, err.Error(), "cannot exceed max ratio", "Error message should explain the constraint") + require.Error(t, err, "Validation should fail when SubnetRewardBase > SubnetRewardMaxRatio")If you expose sentinel errors (e.g., var ErrInvalidSubnetRewardBase), prefer errors.Is.
init_validators.sh (1)
297-301: Quote remote target in ssh/rsync commandsQuote the IP to prevent accidental word splitting and keep consistency with earlier ssh usage.
Apply:
-ssh root@${TARGET_IP} "rm -rf ${HOME_PREFIX}${i}" +ssh root@"${TARGET_IP}" "rm -rf '${HOME_PREFIX}${i}'" @@ -rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:${HOME_PREFIX}${i}/" +rsync -av "${HOME_PREFIX}${i}/" "root@${TARGET_IP}:'${HOME_PREFIX}${i}/'"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (8)
go.sumis excluded by!**/*.sumproto/hetu/blockinflation/v1/genesis.pb.gois excluded by!**/*.pb.goproto/hetu/blockinflation/v1/query.pb.gois excluded by!**/*.pb.goproto/hetu/blockinflation/v1/query_grpc.pb.gois excluded by!**/*.pb.gox/blockinflation/types/generated/genesis.pb.gois excluded by!**/*.pb.go,!**/generated/**x/blockinflation/types/generated/query.pb.gois excluded by!**/*.pb.go,!**/generated/**x/blockinflation/types/generated/query.pb.gw.gois excluded by!**/*.pb.gw.go,!**/generated/**x/blockinflation/types/generated/query_grpc.pb.gois excluded by!**/*.pb.go,!**/generated/**
📒 Files selected for processing (15)
app/app.go(19 hunks)go.mod(5 hunks)init_validators.sh(11 hunks)proto/buf.gen.gogo.yaml(1 hunks)proto/buf.yaml(1 hunks)proto/hetu/blockinflation/v1/genesis.proto(1 hunks)proto/hetu/blockinflation/v1/query.proto(1 hunks)x/blockinflation/client/cli/query.go(1 hunks)x/blockinflation/keeper/grpc_query.go(1 hunks)x/blockinflation/keeper/keeper.go(1 hunks)x/blockinflation/module.go(1 hunks)x/blockinflation/types/params.go(1 hunks)x/blockinflation/types/params_test.go(1 hunks)x/blockinflation/types/query.go(1 hunks)x/blockinflation/types/subnet_reward.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- proto/buf.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- x/blockinflation/client/cli/query.go
- x/blockinflation/types/subnet_reward.go
- x/blockinflation/types/params.go
🧰 Additional context used
🧬 Code Graph Analysis (6)
x/blockinflation/keeper/grpc_query.go (5)
x/blockinflation/keeper/keeper.go (1)
Keeper(22-36)proto/hetu/blockinflation/v1/query.pb.go (15)
Params(194-207)Params(220-220)Params(235-237)QueryParamsRequest(28-32)QueryParamsRequest(45-45)QueryParamsRequest(60-62)QueryParamsResponse(65-71)QueryParamsResponse(84-84)QueryParamsResponse(99-101)QueryPendingSubnetRewardsRequest(111-115)QueryPendingSubnetRewardsRequest(128-128)QueryPendingSubnetRewardsRequest(143-145)QueryPendingSubnetRewardsResponse(148-154)QueryPendingSubnetRewardsResponse(167-167)QueryPendingSubnetRewardsResponse(182-184)x/blockinflation/types/params.go (1)
Params(30-49)x/blockinflation/types/generated/query.pb.go (15)
Params(194-207)Params(220-220)Params(235-237)QueryParamsRequest(28-32)QueryParamsRequest(45-45)QueryParamsRequest(60-62)QueryParamsResponse(65-71)QueryParamsResponse(84-84)QueryParamsResponse(99-101)QueryPendingSubnetRewardsRequest(111-115)QueryPendingSubnetRewardsRequest(128-128)QueryPendingSubnetRewardsRequest(143-145)QueryPendingSubnetRewardsResponse(148-154)QueryPendingSubnetRewardsResponse(167-167)QueryPendingSubnetRewardsResponse(182-184)x/blockinflation/types/query.go (4)
QueryParamsRequest(37-37)QueryParamsResponse(40-43)QueryPendingSubnetRewardsRequest(46-46)QueryPendingSubnetRewardsResponse(49-52)
x/blockinflation/types/query.go (3)
proto/hetu/blockinflation/v1/query.pb.go (15)
Params(194-207)Params(220-220)Params(235-237)QueryParamsRequest(28-32)QueryParamsRequest(45-45)QueryParamsRequest(60-62)QueryParamsResponse(65-71)QueryParamsResponse(84-84)QueryParamsResponse(99-101)QueryPendingSubnetRewardsRequest(111-115)QueryPendingSubnetRewardsRequest(128-128)QueryPendingSubnetRewardsRequest(143-145)QueryPendingSubnetRewardsResponse(148-154)QueryPendingSubnetRewardsResponse(167-167)QueryPendingSubnetRewardsResponse(182-184)x/blockinflation/types/params.go (2)
Params(30-49)NewParams(52-64)x/blockinflation/types/generated/query.pb.go (15)
Params(194-207)Params(220-220)Params(235-237)QueryParamsRequest(28-32)QueryParamsRequest(45-45)QueryParamsRequest(60-62)QueryParamsResponse(65-71)QueryParamsResponse(84-84)QueryParamsResponse(99-101)QueryPendingSubnetRewardsRequest(111-115)QueryPendingSubnetRewardsRequest(128-128)QueryPendingSubnetRewardsRequest(143-145)QueryPendingSubnetRewardsResponse(148-154)QueryPendingSubnetRewardsResponse(167-167)QueryPendingSubnetRewardsResponse(182-184)
x/blockinflation/keeper/keeper.go (6)
x/blockinflation/types/keys.go (5)
StoreKey(8-8)ModuleName(5-5)TotalIssuanceKey(22-22)TotalBurnedKey(25-25)PendingSubnetRewardsKey(28-28)x/stakework/types/module.go (2)
StoreKey(7-7)ModuleName(4-4)x/blockinflation/types/expected_keepers.go (5)
AccountKeeper(20-23)BankKeeper(26-32)EventKeeper(36-36)StakeworkKeeper(39-42)ERC20Keeper(45-47)x/stakework/types/interfaces.go (1)
EventKeeper(9-9)proto/hetu/blockinflation/v1/query.pb.go (3)
Params(194-207)Params(220-220)Params(235-237)x/blockinflation/types/params.go (2)
Params(30-49)DefaultParams(67-82)
x/blockinflation/types/params_test.go (1)
x/blockinflation/types/params.go (1)
DefaultParams(67-82)
x/blockinflation/module.go (5)
x/blockinflation/types/keys.go (1)
ModuleName(5-5)x/blockinflation/types/codec.go (2)
RegisterLegacyAminoCodec(20-22)RegisterInterfaces(25-27)x/blockinflation/types/genesis.go (1)
GenesisState(11-20)x/blockinflation/types/generated/query.pb.gw.go (1)
RegisterQueryHandlerFromEndpoint(132-152)x/blockinflation/keeper/keeper.go (1)
Keeper(22-36)
app/app.go (10)
x/blockinflation/types/keys.go (3)
ModuleName(5-5)StoreKey(8-8)MemStoreKey(17-17)x/stakework/types/module.go (3)
ModuleName(4-4)StoreKey(7-7)MemStoreKey(13-13)x/event/types/keys.go (3)
ModuleName(5-5)StoreKey(8-8)MemStoreKey(14-14)x/blockinflation/types/expected_keepers.go (4)
EventKeeper(36-36)StakeworkKeeper(39-42)AccountKeeper(20-23)BankKeeper(26-32)x/blockinflation/keeper/keeper.go (2)
Keeper(22-36)NewKeeper(39-63)x/evm/keeper/keeper.go (2)
Keeper(44-83)NewKeeper(90-126)x/stakework/keeper/keeper.go (2)
Keeper(15-19)NewKeeper(22-35)x/event/keeper/keeper.go (2)
Keeper(52-83)NewKeeper(86-126)x/event/abi/abi.go (7)
SubnetRegistryABI(6-6)StakingSelfABI(9-9)StakingDelegatedABI(12-12)WeightsABI(15-15)SubnetManagerABI(20-20)NeuronManagerABI(23-23)GlobalStakingABI(26-26)x/blockinflation/types/params.go (1)
ParamKeyTable(25-27)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (26)
go.mod (7)
9-9: Promotingcosmossdk.io/depinjectto a direct dependency looks appropriateGiven the new modules and app wiring, making depinject direct is sensible.
51-51: Upgrade totestify v1.10.0is fineNo known breaking changes for common assertions between v1.9 and v1.10. Tests should continue to pass.
70-70:cloud.google.com/go/compute/metadataminor bumpThis is a safe, low-risk upgrade.
151-151: Indirect upgrades (go-cmpandgo-internal)These are fine and commonly pulled by testing and toolchains.
Also applies to: 216-216
249-255: x/crypto, x/oauth2, x/sys, x/term, x/tools upgradesLooks good. These often include security and platform fixes. Ensure any syscalls or terminal behaviors used by CLI keepers still behave the same across Linux/macOS.
Please run your CLI smoke tests (init, start, query) on Linux and macOS to confirm there are no regressions in terminal handling.
258-258: Pinnedgenproto/googleapis/rpcto 2025-06-03 commitMatched with grpc v1.73.0 and protobuf v1.36.6, which should be compatible. Just ensure any vendored/generated stubs are refreshed.
If you commit generated stubs, re-run codegen to avoid method-set mismatches after the bump.
268-276: Version skew:cosmossdk.io/corerequire v0.11.1 but replace forces v0.11.0The require block asks for
cosmossdk.io/core v0.11.1, but the replace pinscosmossdk.io/core => v0.11.0. The replace wins, potentially breaking code that expects 0.11.1 APIs and causing confusing resolution.Either align the replace to 0.11.1 or downgrade the require to 0.11.0. If the comment about rosetta compatibility still applies, document why 0.11.0 is required.
If 0.11.1 is compatible, prefer this diff:
- cosmossdk.io/core => cosmossdk.io/core v0.11.0 + cosmossdk.io/core => cosmossdk.io/core v0.11.1Also run
go mod tidy -compat=1.23after fixing thegodirective to ensure a clean graph.x/blockinflation/types/params_test.go (2)
13-22: Good use of math.Int parsing checks and equality semanticsParsing with ok and comparing via Int.Equal avoids reflect pitfalls. Assertions are tight and clear.
27-31: Comment mismatch in DefaultParams() (types/params.go): code sets 0.90 but comment says 0.50In x/blockinflation/types/params.go, DefaultParams() sets SubnetRewardMaxRatio to 0.90 (LegacyNewDecWithPrec(90, 2)) but the comment says 0.50. Your test expects 0.90. Fix the misleading comment to avoid future confusion.
Apply this diff in x/blockinflation/types/params.go to update the comment:
- math.LegacyNewDecWithPrec(90, 2), // Default SubnetRewardMaxRatio (0.50) + math.LegacyNewDecWithPrec(90, 2), // Default SubnetRewardMaxRatio (0.90)proto/buf.gen.gogo.yaml (1)
3-16: Verify gogoproto usage & go_package alignment before switching codegenQuick summary (ran rg over proto/):
- Many protos still import gogoproto and use gogoproto options (e.g. proto/hetu/blockinflation/v1/query.proto, genesis.proto — customtype/nullable/goproto_*).
- go_package is set in the protos (e.g. option go_package = "github.com/hetu-project/hetu/v1/x/blockinflation/types"; similar entries for evmos/ethermint modules).
- Generated pb files exist under proto/hetu/blockinflation/v1 (query.pb.go, query_grpc.pb.go, genesis.pb.go) and include _ "github.com/cosmos/gogoproto/gogoproto", i.e. current generation relies on gogoproto behavior.
Action / risks:
- Switching buf.gen.gogo.yaml to protoc-gen-go / go-grpc (paths=source_relative) may ignore gogoproto-specific options (customtype, nullable, castrepeated, embed, etc.) and change generated types/imports — this can break compilation or change APIs.
- Please run buf generate and a full go build (or at least generate + go vet/build for the modules that import these packages) to confirm generation and imports line up.
- Verify that go_package values match how your Go code imports the generated packages (search for imports of github.com/hetu-project/hetu/v1/x/.../types and any aliasing).
Files to double-check (examples from the scan):
- proto/hetu/blockinflation/v1/{query.proto,genesis.proto} — uses gogoproto and sets go_package to github.com/hetu-project/hetu/v1/x/blockinflation/types
- proto/hetu/blockinflation/v1/{query.pb.go,query_grpc.pb.go,genesis.pb.go} — generated files showing current package = types and import of github.com/cosmos/gogoproto/gogoproto
- Multiple ethermint/ and evmos/ protos (use gogoproto customtype/nullable/castrepeated, etc.) — global impact if generator changes.
Suggested verification commands:
- buf generate
- go build ./...
- rg -n 'option\s+go_package' proto -n
- rg -nP 'gogoproto' proto -n
x/blockinflation/keeper/keeper.go (2)
90-103: LGTM: denom fallback derives from params in gettersReturning zero coins using the configured MintDenom is correct and avoids hardcoded “ahetu” pitfalls.
Also applies to: 112-125, 134-147
198-211: LGTM: underflow guard on issuance during burnSaturating to zero with a warning is a safe defensive measure and prevents negative Coin amounts or panics.
init_validators.sh (2)
73-76: Remote cleanup safety looks goodThe remote guard on HOME_PREFIX before rm -rf is a solid hardening step.
200-200: Minimum gas price update aligns with ahetu denomSetting minimum-gas-prices to 0.0001ahetu across nodes is consistent with the denom changes.
proto/hetu/blockinflation/v1/genesis.proto (2)
10-16: GenesisState structure LGTMFields and non-nullability align with the Keeper state surface (params, issuance, burned, pending subnet rewards).
12-16: Params is defined in query.proto; imports and go_package look correctVerified: message Params is declared in proto/hetu/blockinflation/v1/query.proto and proto/hetu/blockinflation/v1/genesis.proto imports that file and references Params. Both files use the same option go_package = "github.com/hetu-project/hetu/v1/x/blockinflation/types". No cyclic import detected.
Files checked:
- proto/hetu/blockinflation/v1/query.proto — defines message Params and sets go_package.
- proto/hetu/blockinflation/v1/genesis.proto — imports query.proto and references Params; same go_package.
Optional suggestion (non-blocking): consider extracting Params into a params.proto if you prefer separating shared types from the query service.
x/blockinflation/keeper/grpc_query.go (3)
14-36: LGTM! Well-structured gRPC query implementation.The
ParamsRPC implementation correctly validates input, unwraps context, and converts internal params to protobuf format. The nil check and field-by-field conversion are appropriate.
39-56: LGTM! Clean implementation of PendingSubnetRewards query.The method follows the same pattern as
Paramswith proper nil checking and type conversion. The conversion from internal sdk.Coin to protobuf is correct.
48-51: Confirm: sdk.Coin is compatible with the protobuf Coin used in the responseShort answer: This is correct — the generated protobuf response type uses the cosmos-sdk types.Coin (imported from "github.com/cosmos/cosmos-sdk/types"), and the code constructs a *sdk.Coin (same package, imported with alias sdk). The types are the same, so assigning protoCoin to pb.QueryPendingSubnetRewardsResponse.PendingSubnetRewards is valid.
Locations verified:
- proto/hetu/blockinflation/v1/query.pb.go — QueryPendingSubnetRewardsResponse.PendingSubnetRewards is *types.Coin (types = "github.com/cosmos/cosmos-sdk/types")
- x/blockinflation/keeper/grpc_query.go — creates &sdk.Coin and returns it in pb.QueryPendingSubnetRewardsResponse
Recommendation: none — no change required.
x/blockinflation/types/query.go (1)
72-83: LGTM! Query client properly forwards to gRPC implementation.The
Paramsmethod correctly delegates to the underlying gRPC client and converts the response usingProtoParamsToParams.proto/hetu/blockinflation/v1/query.proto (1)
41-75: LGTM! Well-structured protobuf definitions.The Params message correctly uses string fields with gogoproto custom types for Cosmos SDK math types, ensuring proper serialization and deserialization. The use of
nullable = falseis appropriate for required fields.x/blockinflation/module.go (1)
117-119: LGTM! Query server properly registered.The RegisterServices method correctly registers the gRPC query server using the generated protobuf server registration.
app/app.go (4)
426-429: LGTM! Memory store key properly registered.The memory store key for blockinflation module is correctly added to enable in-memory storage.
708-725: LGTM! Proper initialization order for keeper dependencies.The initialization order is correct: EventKeeper → StakeworkKeeper → BlockInflationKeeper. This ensures all dependencies are satisfied.
972-973: LGTM! EventKeeper properly set as EVM event handler.Setting the EventKeeper as the EVM event handler enables cross-module event processing, which is essential for subnet functionality.
1310-1310: LGTM! BlockInflation module parameters properly registered.The module's parameter subspace is correctly initialized with its KeyTable for parameter management.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
x/blockinflation/types/query.go (6)
47-51: Good: Response uses sdk.Coin, aligning with module state typesThis removes the previous string mismatch and increases type safety.
53-59: Good: QueryClient methods use ...grpc.CallOption, matching gRPC conventionsThis aligns with generated client signatures and improves ergonomics.
61-67: Good: Wrapper delegates to pb-generated QueryClient instead of a stubRemoves the previous Unimplemented stub footgun and enables real queries.
13-33: Stop dropping parse errors in ProtoParamsToParams; return (Params, error) and validate all fieldsSilently discarding parse failures risks zero/default params and hidden misconfigurations. Also guard against nil protoParams.
Apply this diff:
-func ProtoParamsToParams(protoParams *pb.Params) Params { - totalSupply, _ := math.NewIntFromString(protoParams.TotalSupply) - defaultBlockEmission, _ := math.NewIntFromString(protoParams.DefaultBlockEmission) - subnetRewardBase, _ := math.LegacyNewDecFromStr(protoParams.SubnetRewardBase) - subnetRewardK, _ := math.LegacyNewDecFromStr(protoParams.SubnetRewardK) - subnetRewardMaxRatio, _ := math.LegacyNewDecFromStr(protoParams.SubnetRewardMaxRatio) - subnetMovingAlpha, _ := math.LegacyNewDecFromStr(protoParams.SubnetMovingAlpha) - subnetOwnerCut, _ := math.LegacyNewDecFromStr(protoParams.SubnetOwnerCut) - - return NewParams( - protoParams.EnableBlockInflation, - protoParams.MintDenom, - totalSupply, - defaultBlockEmission, - subnetRewardBase, - subnetRewardK, - subnetRewardMaxRatio, - subnetMovingAlpha, - subnetOwnerCut, - ) -} +func ProtoParamsToParams(protoParams *pb.Params) (Params, error) { + if protoParams == nil { + return Params{}, fmt.Errorf("nil Params in QueryParamsResponse") + } + + totalSupply, ok := math.NewIntFromString(protoParams.TotalSupply) + if !ok { + return Params{}, fmt.Errorf("invalid TotalSupply: %q", protoParams.TotalSupply) + } + defaultBlockEmission, ok := math.NewIntFromString(protoParams.DefaultBlockEmission) + if !ok { + return Params{}, fmt.Errorf("invalid DefaultBlockEmission: %q", protoParams.DefaultBlockEmission) + } + subnetRewardBase, err := math.LegacyNewDecFromStr(protoParams.SubnetRewardBase) + if err != nil { + return Params{}, fmt.Errorf("invalid SubnetRewardBase: %w", err) + } + subnetRewardK, err := math.LegacyNewDecFromStr(protoParams.SubnetRewardK) + if err != nil { + return Params{}, fmt.Errorf("invalid SubnetRewardK: %w", err) + } + subnetRewardMaxRatio, err := math.LegacyNewDecFromStr(protoParams.SubnetRewardMaxRatio) + if err != nil { + return Params{}, fmt.Errorf("invalid SubnetRewardMaxRatio: %w", err) + } + subnetMovingAlpha, err := math.LegacyNewDecFromStr(protoParams.SubnetMovingAlpha) + if err != nil { + return Params{}, fmt.Errorf("invalid SubnetMovingAlpha: %w", err) + } + subnetOwnerCut, err := math.LegacyNewDecFromStr(protoParams.SubnetOwnerCut) + if err != nil { + return Params{}, fmt.Errorf("invalid SubnetOwnerCut: %w", err) + } + + return NewParams( + protoParams.EnableBlockInflation, + protoParams.MintDenom, + totalSupply, + defaultBlockEmission, + subnetRewardBase, + subnetRewardK, + subnetRewardMaxRatio, + subnetMovingAlpha, + subnetOwnerCut, + ), nil +}Additional change needed outside this range: add fmt to imports (see next comment).
69-78: Propagate parsing/response errors in Params(); guard against nil responseHandle nil res/res.Params and the new (Params, error) return to avoid panics and silent defaults.
func (c *queryClient) Params(ctx context.Context, in *QueryParamsRequest, opts ...grpc.CallOption) (*QueryParamsResponse, error) { res, err := c.grpcClient.Params(ctx, &pb.QueryParamsRequest{}, opts...) if err != nil { return nil, err } - return &QueryParamsResponse{ - Params: ProtoParamsToParams(res.Params), - }, nil + if res == nil || res.Params == nil { + return nil, fmt.Errorf("empty Params response from server") + } + + params, err := ProtoParamsToParams(res.Params) + if err != nil { + return nil, err + } + return &QueryParamsResponse{Params: params}, nil }
80-92: Nil-safe handling for PendingSubnetRewards() to avoid NPECurrently dereferences res.PendingSubnetRewards without checks. Guard against nil response/field.
func (c *queryClient) PendingSubnetRewards(ctx context.Context, in *QueryPendingSubnetRewardsRequest, opts ...grpc.CallOption) (*QueryPendingSubnetRewardsResponse, error) { res, err := c.grpcClient.PendingSubnetRewards(ctx, &pb.QueryPendingSubnetRewardsRequest{}, opts...) if err != nil { return nil, err } - return &QueryPendingSubnetRewardsResponse{ - PendingSubnetRewards: sdk.Coin{ - Denom: res.PendingSubnetRewards.Denom, - Amount: math.NewIntFromBigInt(res.PendingSubnetRewards.Amount.BigInt()), - }, - }, nil + if res == nil || res.PendingSubnetRewards == nil { + // Treat missing field as zero-value coin to avoid panics and preserve API stability. + return &QueryPendingSubnetRewardsResponse{PendingSubnetRewards: sdk.Coin{}}, nil + } + return &QueryPendingSubnetRewardsResponse{ + PendingSubnetRewards: sdk.Coin{ + Denom: res.PendingSubnetRewards.Denom, + Amount: math.NewIntFromBigInt(res.PendingSubnetRewards.Amount.BigInt()), + }, + }, nil }
🧹 Nitpick comments (2)
x/blockinflation/types/query.go (2)
3-11: Add fmt to imports for error constructionNeeded by the updated ProtoParamsToParams and query methods’ error returns.
import ( "context" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "google.golang.org/grpc" + "fmt" pb "github.com/hetu-project/hetu/v1/x/blockinflation/types/generated" )
86-90: Optional: construct coin via sdk.NewCoin for validationUsing sdk.NewCoin(denom, amount) ensures canonical validation (non-empty denom, non-negative amount). If you prefer strictness, switch creation to NewCoin and handle a potential panic using a safe constructor pattern.
Example:
coin := sdk.NewCoin(res.PendingSubnetRewards.Denom, math.NewIntFromBigInt(res.PendingSubnetRewards.Amount.BigInt()))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
x/blockinflation/client/cli/query.go(1 hunks)x/blockinflation/keeper/grpc_query.go(1 hunks)x/blockinflation/keeper/keeper.go(1 hunks)x/blockinflation/types/query.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- x/blockinflation/keeper/keeper.go
- x/blockinflation/keeper/grpc_query.go
- x/blockinflation/client/cli/query.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
x/blockinflation/types/query.go (2)
proto/hetu/blockinflation/v1/query.pb.go (15)
Params(194-207)Params(220-220)Params(235-237)QueryParamsRequest(28-32)QueryParamsRequest(45-45)QueryParamsRequest(60-62)QueryParamsResponse(65-71)QueryParamsResponse(84-84)QueryParamsResponse(99-101)QueryPendingSubnetRewardsRequest(111-115)QueryPendingSubnetRewardsRequest(128-128)QueryPendingSubnetRewardsRequest(143-145)QueryPendingSubnetRewardsResponse(148-154)QueryPendingSubnetRewardsResponse(167-167)QueryPendingSubnetRewardsResponse(182-184)x/blockinflation/types/params.go (2)
Params(30-49)NewParams(52-64)
🔇 Additional comments (2)
x/blockinflation/types/query.go (2)
69-71: Confirmed — request messages are empty; no change neededVerified that both request types are empty:
- proto/hetu/blockinflation/v1/query.proto:
message QueryParamsRequest {}andmessage QueryPendingSubnetRewardsRequest {}.- proto/hetu/blockinflation/v1/query.pb.go: the generated
QueryParamsRequestandQueryPendingSubnetRewardsRequeststructs contain only protoimpl state fields and no user fields.Using empty proto requests (as in the reviewed code) is correct.
10-11: Import path is correct — generated PB exists at the imported locationConfirmed: x/blockinflation/types/query.go imports pb "github.com/hetu-project/hetu/v1/x/blockinflation/types/generated", and x/blockinflation/types/generated/query.pb.go exists and defines the generated proto types, so this import is valid and should not break builds.
Files checked:
- x/blockinflation/types/query.go — imports pb "github.com/hetu-project/hetu/v1/x/blockinflation/types/generated" (line ~10)
- x/blockinflation/types/generated/query.pb.go — present (generated)
- proto/hetu/blockinflation/v1/query.proto — go_package = "github.com/hetu-project/hetu/v1/x/blockinflation/types" (note: canonical go_package differs from the ./generated directory; consider aligning generation output and go_package to avoid future confusion)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (45)
api/hetu/blockinflation/v1/query_grpc.pb.gois excluded by!**/*.pb.goethermint/crypto/v1/ethsecp256k1/keys.pb.gois excluded by!**/*.pb.goethermint/evm/v1/events.pb.gois excluded by!**/*.pb.goethermint/evm/v1/evm.pb.gois excluded by!**/*.pb.goethermint/evm/v1/genesis.pb.gois excluded by!**/*.pb.goethermint/evm/v1/query.pb.gois excluded by!**/*.pb.goethermint/evm/v1/query_grpc.pb.gois excluded by!**/*.pb.goethermint/evm/v1/tx.pb.gois excluded by!**/*.pb.goethermint/evm/v1/tx_grpc.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/events.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/feemarket.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/genesis.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/query.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/query_grpc.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/tx.pb.gois excluded by!**/*.pb.goethermint/feemarket/v1/tx_grpc.pb.gois excluded by!**/*.pb.goethermint/types/v1/account.pb.gois excluded by!**/*.pb.goethermint/types/v1/dynamic_fee.pb.gois excluded by!**/*.pb.goethermint/types/v1/indexer.pb.gois excluded by!**/*.pb.goethermint/types/v1/web3.pb.gois excluded by!**/*.pb.goevmos/epochs/v1/genesis.pb.gois excluded by!**/*.pb.goevmos/epochs/v1/query.pb.gois excluded by!**/*.pb.goevmos/epochs/v1/query_grpc.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/erc20.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/events.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/genesis.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/query.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/query_grpc.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/tx.pb.gois excluded by!**/*.pb.goevmos/erc20/v1/tx_grpc.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/genesis.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/inflation.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/query.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/query_grpc.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/tx.pb.gois excluded by!**/*.pb.goevmos/inflation/v1/tx_grpc.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/events.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/query.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/query_grpc.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/tx.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/tx_grpc.pb.gois excluded by!**/*.pb.goevmos/vesting/v1/vesting.pb.gois excluded by!**/*.pb.gohetu/blockinflation/v1/genesis.pb.gois excluded by!**/*.pb.gohetu/blockinflation/v1/query.pb.gois excluded by!**/*.pb.gohetu/blockinflation/v1/query_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (1)
api/hetu/blockinflation/v1/genesis.pulsar.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/hetu/blockinflation/v1/genesis.pulsar.go (1)
hetu/blockinflation/v1/genesis.pb.go (1)
GenesisState(26-35)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (5)
api/hetu/blockinflation/v1/genesis.pulsar.go (5)
340-383: Marshal size and buffer logic looks correctSize() and Marshal() are consistent: size equals the sum of present fields plus unknowns, and the reverse write fills the buffer fully (i reaches 0), avoiding zero-prefix leaks. Unknown fields are preserved at the end.
471-677: Unmarshal path robust; handles invalid lengths/EOF/overflow and preserves unknowns when not discardingPer-field parsing and allocation are correct; error handling paths look solid.
841-879: Descriptor builder wiring LGTM; guarded against multiple initTypeBuilder initialization and dependency on query_proto are wired correctly with a nil-guard, avoiding duplicate builds.
708-712: No Coin type mismatch — original comment is incorrectVerification shows all GenesisState coin fields reference cosmos.base.v1beta1.Coin (just imported under different aliases), not sdk/types.Coin — so there is no cross-type mismatch.
Evidence:
- .api/hetu/blockinflation/v1/genesis.pulsar.go:709-711 — TotalIssuance/TotalBurned/PendingSubnetRewards use *v1beta1.Coin (descriptor: // 2: cosmos.base.v1beta1.Coin).
- .hetu/blockinflation/v1/genesis.pb.go:32-34 — fields use *types.Coin; file mapping shows (*types.Coin)(nil) // 2: cosmos.base.v1beta1.Coin.
- .proto/hetu/blockinflation/v1/genesis.pb.go and .x/blockinflation/types/generated/genesis.pb.go — same *types.Coin entries mapped to cosmos.base.v1beta1.Coin.
Conclusion: ignore the original mismatch warning — no changes required to align Coin types.
Likely an incorrect or invalid review comment.
25-32: Init ordering confirmed — no recursive init cycle detectedThe generated init/proto_init calls are ordered safely: the top-level init in genesis calls file_hetu_blockinflation_v1_genesis_proto_init(), and that function invokes file_hetu_blockinflation_v1_query_proto_init() before the protoreflect descriptors are resolved — I found no evidence of a back-call from the query init to genesis.
Files/locations verified:
- api/hetu/blockinflation/v1/genesis.pulsar.go
- top-level init (≈ lines 25–32): calls file_hetu_blockinflation_v1_genesis_proto_init() then resolves md_/fd_ descriptors.
- file_hetu_blockinflation_v1_genesis_proto_init() (≈ lines 841–847): contains the nil-guard and calls file_hetu_blockinflation_v1_query_proto_init() before building descriptors.
- api/hetu/blockinflation/v1/query.pulsar.go
- file_hetu_blockinflation_v1_query_proto_init() present (top-level init calls it at ≈ line 22 and function around ≈ 2907–2908); no observed call back to genesis_proto_init().
Snippet (unchanged / verified):
func init() {
file_hetu_blockinflation_v1_genesis_proto_init()
md_GenesisState = File_hetu_blockinflation_v1_genesis_proto.Messages().ByName("GenesisState")
fd_GenesisState_params = md_GenesisState.Fields().ByName("params")
fd_GenesisState_total_issuance = md_GenesisState.Fields().ByName("total_issuance")
fd_GenesisState_total_burned = md_GenesisState.Fields().ByName("total_burned")
fd_GenesisState_pending_subnet_rewards = md_GenesisState.Fields().ByName("pending_subnet_rewards")
}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
52-65: Fix release links to point to this repository (hetu-chain) and verify asset namesRelease links currently target hetu-project/hetu, which will 404 from this repo’s README. Update to hetu-project/hetu-chain and verify the asset naming matches your actual releases.
-Alternatively, you can download pre-built binaries from the latest [release](https://github.com/hetu-project/hetu/releases). +Alternatively, you can download pre-built binaries from the latest [release](https://github.com/hetu-project/hetu-chain/releases). @@ -wget https://github.com/hetu-project/hetu/releases/download/v0.x.x/hetud-v0.x.x-linux-amd64.tar.gz +wget https://github.com/hetu-project/hetu-chain/releases/download/v0.x.x/hetud-v0.x.x-linux-amd64.tar.gzFollow-up:
- If your release artifact name differs (e.g., hetu-chain-... or a tarball containing a directory), adjust the filename and subsequent mv command accordingly.
- Consider adding a checksum verification step for downloaded artifacts.
Optional checksum/naming verification checklist (no repo access needed):
- Confirm the final asset filename on your latest release page.
- Update the tar command and mv target to match the actual extracted paths.
🧹 Nitpick comments (3)
README.md (3)
15-16: Docs repo link mismatch with rest of READMEThe docs link points to hetu-project/docs, while the rest of the README and contributing references use hetu-project/hetu-docs. Align to hetu-docs to avoid 404s and confusion.
-Our documentation is hosted in a [separate repository](https://github.com/hetu-project/docs). +Our documentation is hosted in a [separate repository](https://github.com/hetu-project/hetu-docs).
118-121: Point issue link to this repository“Open an Issue” currently points to hetu (org/repo mismatch). Update to hetu-chain issues to reduce misrouted reports.
-- [Open an Issue](https://github.com/hetu-project/hetu/issues) +- [Open an Issue](https://github.com/hetu-project/hetu-chain/issues)
126-131: Align PRs/Issues links with this repository; keep docs linkContributing links should target hetu-chain to match this repo. Docs link to hetu-docs looks correct.
-- Cloning code repo and opening a [PR](https://github.com/hetu-project/hetu/pulls). -- Submitting feature requests or [bugs](https://github.com/hetu-project/hetu/issues). +- Cloning code repo and opening a [PR](https://github.com/hetu-project/hetu-chain/pulls). +- Submitting feature requests or [bugs](https://github.com/hetu-project/hetu-chain/issues).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~20-~20: There might be a mistake here.
Context: ...k it out. ## Prerequisites - Go 1.20+ - Git - [Make](http...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...Go 1.20+](https://golang.org/dl/) - Git - [Make](https://www.gnu.org/software/make/...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: .../) - Git - Make - gcc (for cgo supp...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...g/software/make/manual/make.html) - gcc (for cgo support) ## Installation Fol...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...t_node_archive.sh ``` This script will: 1. Initialize the genesis file 2. Create a ...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...t for Ethereum smart contracts and tools - Cosmos SDK Integration: Leverage Cosmo...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...erage Cosmos ecosystem features like IBC - Bittensor-inspired Consensus: Advanced...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...nced consensus mechanism for AI networks - Subnet Architecture: Specialized subne...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...alized subnets for different AI services - Alpha Token System: Subnet-specific to...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...ub.com/hetu-project/hetu-docs) document.
(QB_NEW_EN)
⏰ 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). (1)
- GitHub Check: build
1 participant
PR Title: feat: subnet functionality – Event, Stakework & BlockInflation modules
📝 Description
This PR introduces full subnet support for the Hetu Chain by integrating three new modules—Event, Stakework, and BlockInflation—that together enable subnet-specific Alpha token minting and reward distribution.
🔑 What’s Added
SubnetInfowithAlphaTokenfield• Stores/retrieves subnet emission & price data
• Fixes subnet activation logic
RunEpoch(Bittensor-style weighted-median consensus)• Tempo-triggered reward cycles
• Detailed debug logging
• Progressive halving for sustainable issuance
• Configurable owner cut (%)
• High-precision emission math
🔄 Flow Overview
✅ Checklist
🚀 Deployment Notes
Ready for review 🙌
Summary by CodeRabbit
New Features
Documentation
Chores
Tests