Skip to content

feat(indexer): cosmos staking, distribution and governance handlers - #3591

Open
baktun14 wants to merge 2 commits into
feat/indexer-balance-ledger-activity-logfrom
feat/indexer-cosmos-staking-gov-handlers
Open

feat(indexer): cosmos staking, distribution and governance handlers#3591
baktun14 wants to merge 2 commits into
feat/indexer-balance-ledger-activity-logfrom
feat/indexer-cosmos-staking-gov-handlers

Conversation

@baktun14

Copy link
Copy Markdown
Contributor

Why

The indexer stopped at the bank module: cosmos staking, distribution, slashing and governance went unindexed, so the validator set, delegations, and every proposal and vote were invisible, and reward/commission/slash balance changes weren't told apart. This adds them, finishing the cosmos side before the akash module handlers.

Closes CON-810.

Stacks on L-4 (CON-809) — based on feat/indexer-balance-ledger-activity-log, so this diff is L-5 only; review/merge L-4 first.

What

Staking — validators, delegations and unbonding match chain queries. Delegation shares can't be derived exactly from messages (the token↔share rate moves with every reward and slash), so a periodic snapshot reconciles them against the chain's own staking queries:

  • validators gains bond state (status, jailed, tokens, delegator_shares, unbonding height/time); new unbonding_delegations table and validator_status enum (migration 0003).
  • StakingSnapshotService fetches the full validator/delegation/unbonding set over abci_query, then upserts each validator's row and fully replaces delegations and unbonding in one transaction, interning delegators first.
  • Sync triggers a snapshot only once caught up to the tip, throttled by STAKING_SNAPSHOT_INTERVAL_BLOCKS; a snapshot failure is logged and retried, never halting sync.

Governance — proposals and votes indexed. From messages plus EndBlock events (votes are pruned from chain state once voting ends, so they have to be captured per block):

  • proposals / proposal_votes / proposal_deposits tables and proposal_status / vote_option enums (migration 0004).
  • Handles MsgSubmitProposal (v1 and v1beta1), MsgVote/MsgVoteWeighted and MsgDeposit; takes each proposal's id from its submit_proposal event and its lifecycle from the EndBlock active_proposal/inactive_proposal events. A vote promotes its proposal into voting_period without regressing a terminal result.
  • GovWriter persists them inside the block transaction, resolving proposer/voter/depositor from the account ids the committer already interned (governance actors are always the message signer).

Distribution reasons sharpened. classifyReason is now direction-aware: a credit from the distribution module is a reward or commission, but a fund-community-pool debit is a plain transfer, not a reward.

Verification

  • 302 unit tests, tsc and lint all green.
  • Live abci_query-vs-LCD on sandbox: validator tokens, delegator shares, commission rate, status, jailed and moniker plus delegation shares all match exactly, confirming the LegacyDec scaling and proto decoding.
  • Migrations 0003 and 0004 apply cleanly on Postgres 14. Both are additive (new tables, columns with defaults, and enums), so they don't rewrite or block existing tables.

Notes

  • Final power-weighted proposal tally is left null — it isn't in the events; a later jobs-role reconcile can fill it. A v1beta1 proposal's title/summary are null with its legacy content kept in messages. Authz-wrapped governance isn't unwrapped yet (the decoder is top-level only — that's the akash adapter layer).
  • The CI test/lint/tsc matrix only runs on PRs based on main, so this was verified locally.

Reconcile the validator set against the chain's own staking queries so it
matches at the tip. Delegation shares can't be derived exactly from
messages, since the token/share exchange rate moves with every reward and
slash, so an authoritative snapshot is the only way to keep them exact.

- validators gains bond state (status, jailed, tokens, delegator_shares,
  unbonding height/time); new unbonding_delegations table; validator_status enum
- StakingSnapshotService fetches the full set over abci_query, then upserts
  each validator's row and fully replaces delegations and unbonding in one
  transaction, interning delegators first
- sync triggers a snapshot only once caught up to the tip, throttled by
  STAKING_SNAPSHOT_INTERVAL_BLOCKS, and never lets a snapshot failure halt sync

Part of CON-810.
Index governance proposals, votes and deposits per block, and tighten the
balance-ledger reason for distribution flows.

- proposals/proposal_votes/proposal_deposits tables plus proposal_status and
  vote_option enums
- gov deriver reads MsgSubmitProposal (v1 and v1beta1), MsgVote/MsgVoteWeighted
  and MsgDeposit, taking each proposal's id from its submit_proposal event and
  its lifecycle from the EndBlock active_proposal/inactive_proposal events; a
  vote promotes its proposal into voting_period without regressing a result
- GovWriter persists them inside the block transaction, resolving proposer,
  voter and depositor from the account ids the committer already interned
  (governance actors are always the message signer)
- decoder now keeps the gov events; committer calls the gov writer
- classifyReason is direction-aware: a credit from the distribution module is a
  reward/commission, but a fund-community-pool debit is a plain transfer, not a
  reward

Part of CON-810.
@baktun14
baktun14 requested a review from a team as a code owner August 13, 2026 07:35
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c6156591-af98-4bae-a9c1-fe57217869cc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment on lines +35 to +47
const rows = proposals.map(proposal => ({
id: proposal.id,
proposerAccountId: proposal.proposerAddress ? accountIds.get(proposal.proposerAddress) ?? null : null,
title: proposal.title,
summary: proposal.summary,
messages: proposal.messages,
metadata: proposal.metadata,
status: "deposit_period" as const,
submitTime: proposal.submitTime,
totalDeposit: proposal.initialDeposit.length > 0 ? proposal.initialDeposit : null,
submitHeight: proposal.submitHeight
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 In GovWriter#writeProposals (gov-writer.service.ts:35-47), Proposals.totalDeposit is set once from the MsgSubmitProposal initial deposit and the row is inserted with onConflictDoNothing, so it's never updated again. Every later MsgDeposit is recorded in proposal_deposits but never folded back into Proposals.totalDeposit, so the column silently freezes at the submission-time value instead of tracking the running total cosmos uses for the deposit-period threshold. Since proposal_deposits retains the full history, consider accumulating totalDeposit on each deposit (e.g. via onConflictDoUpdate) or computing it on read instead of persisting a stale partial value.

Extended reasoning...

What the bug is: GovWriter#writeProposals (gov-writer.service.ts:35-47) sets Proposals.totalDeposit exactly once, from proposal.initialDeposit captured at MsgSubmitProposal time. That row is then inserted via insertChunked, which defaults to onConflictDoNothing: true (see insert-chunked.ts), so once the proposal row exists nothing in this codebase ever writes to totalDeposit again. Meanwhile #writeDeposits faithfully records every subsequent MsgDeposit as its own row in proposal_deposits, and #applyStatusUpdates only ever touches the status column. There is no reconciliation step anywhere in src that folds the deposit history back into the proposal's running total.

Why this diverges from chain semantics: on a real cosmos chain, Proposal.total_deposit is the running sum of every deposit the proposal has received — it's the value gov module logic checks against the minimum deposit to decide when a proposal exits deposit_period. A column named total_deposit that only ever reflects the first deposit is misleading to any downstream consumer (API endpoint, dashboard, analytics query) that reads it expecting on-chain semantics.

Concrete walkthrough: Suppose proposal 42 is submitted with an initial deposit of 500 uakt. #writeProposals inserts { id: 42, totalDeposit: [{denom: 'uakt', amount: '500'}], status: 'deposit_period', ... }. Two blocks later, a second account deposits 600 uakt via MsgDeposit. #writeDeposits inserts a new row into proposal_deposits for that deposit — this succeeds and is durably recorded. But the Proposals table's own insert for id 42 already happened, so onConflictDoNothing means the second write attempt (if it were ever made, which it isn't) would be a no-op anyway; in practice, no code path even attempts it. The result: Proposals.totalDeposit for proposal 42 permanently reads 500 uakt, even though the real running total (and the true chain state) is 1100 uakt. A consumer querying Proposals directly for "how close is this proposal to its minimum deposit" gets a wrong, understated answer forever.

Why nothing currently prevents this: the write path is intentionally split — proposals are inserted once (ids are assigned externally, by the `submit_proposal" event, so there's no natural place that revisits an existing row) and deposits accumulate independently in their own table. That's a reasonable design for preserving full deposit history, but it silently drops the derived aggregate unless something explicitly recomputes it.

Fix: either (a) change the deposit-insert path to also perform an onConflictDoUpdate on Proposals that adds the new deposit's coins to the existing totalDeposit JSON array (matching by denom), or (b) leave totalDeposit null/undocumented as a submission-only snapshot and compute the true running total on read by summing proposal_deposits for the given proposal id — the latter is actually simpler since proposal_deposits already retains complete history.

Severity: all verifiers agree this is real but low-impact: the complete deposit history survives in proposal_deposits (no data loss, and the true total is recoverable), the indexer drives proposal status transitions from EndBlock events rather than by computing the deposit threshold itself (so this doesn't corrupt status), and nothing crashes. It's a stored-value accuracy gap on one denormalized column, not a functional regression — hence nit rather than blocking.

Comment on lines +58 to +68

/**
* A credit from the distribution module is a delegator reward, or commission when the withdraw came from a
* validator's own commission message. A debit *to* the module (e.g. MsgFundCommunityPool) is not a reward, so
* it falls back to a plain transfer.
*/
function classifyDistributionFlow(ctx: ReasonContext): BalanceReason {
if (!ctx.isCredit) {
return "transfer";
}
return ctx.msgTypeUrl === WITHDRAW_VALIDATOR_COMMISSION ? "commission" : "reward";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 classifyDistributionFlow (reason-classifier.ts:58-68) keys its reward/commission-vs-transfer decision off ctx.isCredit, which balance-deriver.ts sets relative to the row's own holder, not relative to the distribution module. When the distribution module itself is the holder of a ledger row (its own coin_spent/coin_received leg, matched via the roleOf(address) fallback), the direction is inverted: a reward/commission payout's module-side outflow is now classified 'transfer' instead of 'reward'/'commission', producing two ledger rows for the same event with inconsistent reasons. A one-line fix — classifying by direction relative to the distribution module rather than the row's holder — would resolve it.

Extended reasoning...

The bug. classifyDistributionFlow decides reward/commission vs. transfer purely from ctx.isCredit. But isCredit is set in balance-deriver.ts as direction === "received" — i.e. relative to whichever address is the holder of the current ledger row, not relative to the distribution module. Role resolution (reason-classifier.ts:35) is roleOf(counterparty) ?? roleOf(address), so when the distribution module's own account is the row's holder and the counterparty (a validator/delegator) has no registered role, the module's own leg still resolves role === "distribution" and calls classifyDistributionFlow — but now keyed off the module's own direction, which is inverted relative to 'money flowing out of the module'.

Concrete effect. For a normal reward/commission withdrawal, the distribution module emits a coin_spent leg for itself (direction=spent → isCredit=false), so its own row is now classified transfer, while the delegator/validator's coin_received leg is correctly classified reward/commission. Before this PR, both legs resolved via the counterparty-only msgTypeUrl check and were consistently reward/commission. So this is a real, PR-introduced regression: the double-entry ledger now carries two different reasons for the two legs of the same economic event, on every reward/commission payout. The existing test balance-deriver.spec.ts 'classifies a distribution reward withdrawal...' only asserts the recipient's row, so the module's own mislabeled leg went unasserted and unnoticed.

Why existing tests don't catch it. The new reason-classifier.spec.ts cases only exercise classifyReason with a single ctx object per scenario (one holder at a time) — they never construct the pair of contexts that balance-deriver.ts actually derives for both sides of one event, so the cross-row inconsistency is invisible at the unit level.

Step-by-step proof: (1) A validator withdraws a reward via MsgWithdrawDelegatorReward. (2) The chain emits coin_spent{spender=distribution_module, amount=100uakt} and coin_received{receiver=akash1val, amount=100uakt} (plus a transfer event correlating them). (3) deriveBalanceChanges produces two DerivedBalanceChange rows: holder=distribution_module, direction=spent, isCredit=false; and holder=akash1val, direction=received, isCredit=true. (4) For the distribution_module row, counterparty=akash1val has no role, so role falls back to roleOf(distribution_module) = "distribution" → classifyDistributionFlow({isCredit:false,...}) → returns "transfer". (5) For the akash1val row, counterparty=distribution_module resolves role="distribution" → classifyDistributionFlow({isCredit:true,...}) → returns "reward". Result: one reward payout, two ledger rows, reasons transfer and reward.

Addressing the refutation. One reviewer argued this is intentional/an improvement, because SUM(delta) WHERE reason='reward' now nets to the true payout total instead of zero, and a module account 'isn't earning a reward' when it pays one out. That's a defensible take on what the module's row should mean, and it's plausible the author intended isCredit to be holder-relative by design. But two things stand: first, it's inconsistent with how the PR treats fee flows — the fee test explicitly asserts both legs of a fee movement share the same reason: 'fee' (balance-deriver.spec.ts:72), so the ledger's own convention elsewhere is same-reason-per-event, making distribution's split an outlier rather than a considered design choice. Second, even granting the 'aggregate is more correct' argument, the PR's own doc comment on classifyDistributionFlow describes flows 'from/to the distribution module' in module-relative terms, but the code executes holder-relative logic — the code doesn't implement what its own comment says it does. The refuter's separate point about MsgFundCommunityPool's module-side inflow being mislabeled reward is correct that this specific instance is pre-existing and unchanged by this PR (before, classifyReason returned reward/commission regardless of direction, since isCredit didn't exist yet) — so that particular case is not a new regression, only the reward/commission module-side outflow is.

Fix. Compute isCredit (or an equivalent direction flag) relative to the distribution module specifically inside classifyDistributionFlow, e.g. by comparing ctx.address against the module's own address rather than trusting the ambient holder-relative isCredit, or by deriving reason once per event rather than once per row.

Comment on lines +70 to +79
async #writeDeposits(tx: ChainTransaction, deposits: DerivedDeposit[], accountIds: Map<string, number>): Promise<void> {
const rows = deposits
.map(deposit => {
const depositorAccountId = accountIds.get(deposit.depositorAddress);
return depositorAccountId === undefined ? null : { proposalId: deposit.proposalId, depositorAccountId, amount: deposit.amount, height: deposit.height };
})
.filter((row): row is NonNullable<typeof row> => row !== null);

await insertChunked(tx, ProposalDeposits, rows);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 ProposalDeposits' primary key (proposalId, depositorAccountId, height) has no tx/message discriminator, and GovWriter#writeDeposits inserts via insertChunked's default onConflictDoNothing=true. If the same depositor deposits to the same proposal twice in one block (e.g. an explicit MsgDeposit in the same block as their initial proposal deposit, or two separate deposit txs), the second row silently collides on the PK and is dropped — contradicting the table's own doc comment that 'a depositor's repeated deposits are each kept'. Fix by adding a message/tx-index discriminator to the primary key.

Extended reasoning...

The bug: ProposalDeposits (schema.ts) is keyed by (proposalId, depositorAccountId, height) with no per-message or per-tx discriminator:

export const ProposalDeposits = cosmosSchema.table(
  "proposal_deposits",
  {
    proposalId: bigint("proposal_id", { mode: "number" }).notNull(),
    depositorAccountId: integer("depositor_account_id").notNull().references(() => Accounts.id),
    amount: jsonb("amount").$type<FeeCoin[]>().notNull(),
    height: bigint("height", { mode: "number" }).notNull()
  },
  t => [primaryKey({ columns: [t.proposalId, t.depositorAccountId, t.height] })]
);

GovWriter#writeDeposits (gov-writer.service.ts:70-79) writes rows for this table via insertChunked(tx, ProposalDeposits, rows) with no options override. insertChunked's default is onConflictDoNothing: true (db/insert-chunked.ts), so any row whose PK collides with an already-inserted row in the same statement/transaction is silently dropped rather than erroring or merging.

How the collision happens: gov-deriver.ts's addProposal pushes a deposit for the proposer's initialDeposit derived from MsgSubmitProposal, and addDeposit pushes one for every standalone MsgDeposit. Neither path aggregates or dedupes by (proposalId, depositor, height) — each call just appends. So if the same depositor address deposits to the same proposal more than once within a single block, both derived rows carry an identical (proposalId, depositorAccountId, height) key.

Concrete trigger — step by step:

  1. At height H, account A submits MsgSubmitProposal for proposal 7 with an initialDeposit of 1000uakt. addProposal derives a deposit row: {proposalId: 7, depositor: A, amount: [1000uakt], height: H}.
  2. In the same block H (a separate tx, or even the same multi-message tx), account A also sends an explicit MsgDeposit of 500uakt to proposal 7. addDeposit derives: {proposalId: 7, depositor: A, amount: [500uakt], height: H}.
  3. Both rows resolve to the same depositorAccountId (interned once for address A) and the same height, so both share PK (7, A's id, H).
  4. writeDeposits inserts both rows in the same insertChunked call. The second insert conflicts on the PK and — because onConflictDoNothing is the default — is silently discarded.
  5. Result: the 500uakt deposit is permanently missing from proposal_deposits, even though it happened on-chain and is a legitimate, independent deposit.

Why nothing else catches this: there's no aggregation step that would merge same-key deposits into one row (which would at least preserve the total, if not the individual records), and no uniqueness violation is raised to surface the problem — onConflictDoNothing swallows it entirely. Existing tests (gov-writer.service.spec.ts) only cover the single-deposit case per block/proposal/depositor, so this path isn't exercised.

Impact: This directly contradicts the table's own doc comment: "One row per deposit ... a depositor's repeated deposits are each kept." It's an auxiliary indexing table — the actual coin movement is still captured correctly in the balance ledger — so nothing crashes or halts sync, and the scenario (same depositor, same proposal, same block, multiple deposits) is relatively rare. But it is a genuine, silent data-completeness gap relative to the documented design intent.

Fix: Add a tx-index/message-index discriminator (e.g. txIndex and/or message index) to the ProposalDeposits primary key so that two deposits from the same depositor to the same proposal in the same block get distinct keys and both survive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant