likecollective-indexer: Re-read staking totals from the chain instead of applying event deltas by williamchong · Pull Request #324 · likecoin/likecoin-op

williamchong · 2026-09-17T07:11:46Z

Why

accounts, nft_classes and stakings were built by adding up event deltas, so the drift could not be recovered: a missing or wrong event didn't delay a number, it offset it, and every later event built on the wrong base. RewardDeposited drifted by design. It floored once per account, where the contract floors once per position against a reward index, so the two disagreed by dust on every deposit. That is why pending_reward_amount held 873 of the 1173 wrong columns found on mainnet.

What changes

Events trigger a chain re-read instead of supplying the amount. getStakeForUser, getPendingRewardsForUser and getTotalStake replace the stored totals. staking_events is history, so it still accumulates. Simulation keeps the delta arithmetic, since checking that arithmetic is its job.

Reads use the head, not the event's block. Nothing orders the pipeline (asynq retries, retry-failed-evm-events), so pinning to the event's block would let an older event commit older data over newer. Reading the head gives the same result in any order.
One head per pass. It's resolved once and every read is pinned to it, so a block that lands mid-pass can't mix rows from both sides of it. A head older than the event's own block fails and retries instead of storing data from before the event.
Deltas are no longer checked against re-read totals. A loaded amount may already come from a head past the event. For example, a stake of 100 then an unstake of 100, processed out of order, used to fail "staked amount is less than staked amount removed" on every retry.
One writer at a time. Every writer, including resync --apply, holds one Postgres advisory lock across load, chain read and persist, and persists on the transaction that holds the lock. The head each commit was read at is recorded in the new staking_state_heads table, and a commit from an older head is refused and retried.
resync --apply no longer needs the workers stopped. It takes the same lock before choosing its block, so workers pause for the build. The snapshot block is raised to the committed head when --confirmations would put it behind, and an explicit --block behind that head is refused. This replaces the CheckSnapshotBlock guard that main relies on today.
RewardDeposited splits by the stake at the deposit's own block. The totals come from the head, but the split must not reward whoever held the stake later. A past block's stake never changes, so pinning is safe here. It needs archive state once the event is older than the node's window (Alchemy serves this).

Also fixed:

check-received-evm-events sorted events into a variable it never used and enqueued them unsorted.
MapWithLimit could return a partly filled slice with a nil error on cancellation, and the caller treated those zeros as chain data.

Deploy

⚠️ Apply the migration (20260916000000_Add staking state head table) before deploying. Without the table, every event write fails.

Known limits

Stake read "at the deposit's block" is the state after the whole block. A stake or unstake later in the same block counts as if it came first, because the contract offers no read between two logs in one block.
claimed_reward_amount still accumulates because the contract has no matching counter, so it still can't be recovered.
resync.CheckSnapshotBlock and GetEarliestPendingStakingEvmEvent are now unused; they can be removed in a follow-up.

Copilot

🟡 Changes recommended

Missing-row reward splits, uninitialized migration state, and lock-wait cancellation can cause unrecoverable or stranded indexing state.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Reworks staking indexing to reconcile current totals from chain state while preserving event-derived history.

Changes:

Re-reads staking totals at a consistent chain head and historical deposit stakes at the deposit block.
Serializes workers and resync with an advisory lock and records committed heads.
Preserves delta-based simulation and sorts received events before enqueueing.

File summaries

File	Description
`likecollective-indexer/README.md`	Documents reconciliation and locking behavior.
`internal/logic/stakingstate/staking_state.go`	Adds chain-backed state tracking.
`internal/logic/stakingstate/staking_state_test.go`	Updates persistence calls.
`internal/logic/stakingstate/staking_state_mock_test.go`	Updates persistor mock.
`internal/logic/stakingstate/staking_event_application.go`	Separates history deltas from chain totals.
`internal/logic/stakingstate/persistor/persistor.go`	Adds locking and committed-head persistence.
`internal/logic/stakingstate/evm_event_processor.go`	Integrates reconciliation into event processing.
`internal/logic/stakingstate/deposit_stakes.go`	Reads deposit-time stakes.
`internal/logic/stakingstate/deposit_stakes_test.go`	Tests historical reward splitting.
`internal/logic/stakingstate/chain_reconciler.go`	Implements chain reconciliation.
`internal/logic/stakingstate/chain_reconciler_test.go`	Tests reconciliation behavior.
`internal/logic/simulate/persistor.go`	Adapts simulation persistence.
`internal/database/staking_state_lock.go`	Implements advisory locking.
`internal/database/staking_state_head.go`	Manages committed chain heads.
`internal/database/database.go`	Exposes the SQL database handle.
`ent/tx.go`	Registers the new transactional client.
`ent/stakingstatehead/where.go`	Adds generated predicates.
`ent/stakingstatehead/stakingstatehead.go`	Defines generated entity metadata.
`ent/stakingstatehead.go`	Adds the generated entity model.
`ent/stakingstatehead_update.go`	Adds generated update builders.
`ent/stakingstatehead_query.go`	Adds generated query builders.
`ent/stakingstatehead_delete.go`	Adds generated delete builders.
`ent/stakingstatehead_create.go`	Adds generated create builders.
`ent/schema/stakingstatehead.go`	Defines the committed-head schema.
`ent/runtime.go`	Initializes schema runtime defaults.
`ent/predicate/predicate.go`	Registers generated predicates.
`ent/mutation.go`	Adds generated mutation support.
`ent/migrate/schema.go`	Registers the new table.
`ent/migrate/migrations/atlas.sum`	Updates migration checksums.
`ent/migrate/migrations/20260916000000_Add staking state head table.sql`	Creates the committed-head table.
`ent/hook/hook.go`	Adds generated mutation-hook support.
`ent/ent.go`	Registers column validation.
`ent/client.go`	Adds generated client operations.
`cmd/worker/task/check_received_evm_events.go`	Enqueues events in processing order.
`cmd/cli/cmd/simulate.go`	Selects delta-based simulation processing.
`cmd/cli/cmd/resync.go`	Locks and head-guards snapshot application.

Review details

Files not reviewed (15)

likecollective-indexer/ent/client.go: Generated file
likecollective-indexer/ent/ent.go: Generated file
likecollective-indexer/ent/hook/hook.go: Generated file
likecollective-indexer/ent/migrate/schema.go: Generated file
likecollective-indexer/ent/mutation.go: Generated file
likecollective-indexer/ent/predicate/predicate.go: Generated file
likecollective-indexer/ent/runtime.go: Generated file
likecollective-indexer/ent/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/where.go: Generated file
likecollective-indexer/ent/stakingstatehead_create.go: Generated file
likecollective-indexer/ent/stakingstatehead_delete.go: Generated file
likecollective-indexer/ent/stakingstatehead_query.go: Generated file
likecollective-indexer/ent/stakingstatehead_update.go: Generated file
likecollective-indexer/ent/tx.go: Generated file

Files reviewed: 21/36 changed files
Comments generated: 3
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+	return e.stakingStatePersistor.WithLock(ctx, func(ctx context.Context) error {
+		stakingState, err := loadStakingState(
+			ctx, e.stakingStateLoader, stakingEvents, e.reconcileFromChain,
+		)


Copilot

🟡 Changes recommended

Event ordering remains incorrect, historical holder validation has a correctness gap, and rolling deployment can bypass the new writer lock.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (15)

likecollective-indexer/ent/client.go: Generated file
likecollective-indexer/ent/ent.go: Generated file
likecollective-indexer/ent/hook/hook.go: Generated file
likecollective-indexer/ent/migrate/schema.go: Generated file
likecollective-indexer/ent/mutation.go: Generated file
likecollective-indexer/ent/predicate/predicate.go: Generated file
likecollective-indexer/ent/runtime.go: Generated file
likecollective-indexer/ent/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/where.go: Generated file
likecollective-indexer/ent/stakingstatehead_create.go: Generated file
likecollective-indexer/ent/stakingstatehead_delete.go: Generated file
likecollective-indexer/ent/stakingstatehead_query.go: Generated file
likecollective-indexer/ent/stakingstatehead_update.go: Generated file
likecollective-indexer/ent/tx.go: Generated file

Suppressed comments (1)

likecollective-indexer/internal/logic/stakingstate/deposit_stakes.go:105

Allowing every sum >= total case defeats the missing-holder guard when the two documented conditions overlap. For example, if the contract total is 90 because of the historical drift, indexed positions sum to 100, and an unindexed holder has 5, this check passes and permanently omits that holder's distribution row. Detecting a shortfall only works when the missing stake is larger than the existing surplus; replay is not safe without another way to establish the holder set (or an explicit policy for these ambiguous blocks).

		// Only a shortfall is refused. The contract's total has drifted below
		// the sum of its positions before (restake and increaseStakeToPosition
		// left pending rewards out of it), and a past block's reads never
		// change, so refusing a surplus would fail such a deposit forever.
		if sum.Lt(total) {

Files reviewed: 22/37 changed files
Comments generated: 2
Review effort level: Balanced

+	// Enqueue oldest first. GetEVMEventsByStatus does not order, so without
+	// this the deltas that build staking_events are recorded in whatever order
+	// the query happened to return.
+	slices.SortFunc(receivedEvents, model.EvmEventsProcessingComparator)


+// WithStakingStateLock runs fn holding the advisory lock that every writer of
+// the staking state takes around its load, chain read and persist.


…y event accounts, nft_classes and stakings were built by accumulating event deltas. That is what made the drift unrecoverable: a missing or wrong event did not delay a number, it offset it, and every later event built on the wrong base. RewardDeposited was drifting by construction rather than by accident. It re-derived the per-staker split as one integer floor per account against the indexer's own mirrored totals, where the contract takes one floor per position against a reward index. The two disagree by dust on every single deposit, which is why pending_reward_amount held 873 of the 1173 wrong columns found on mainnet. So the event now triggers a re-read instead of supplying the amount: getStakeForUser, getPendingRewardsForUser and getTotalStake replace the totals. The deltas still write staking_events, which is history and has to stay accumulated. The reads are not pinned to the event's block. Nothing orders the pipeline -- asynq retries land late and retry-failed-evm-events re-drives old events on purpose -- so a pinned read would let an older event commit an older truth over a newer one and leave it there, which is worse than the accumulation it replaces. Reading latest converges whatever order writers run in, and needs no archive state. One head does serve the whole pass, though. A RewardDeposited fans out into a read per staker, and with each read taking whatever block the node happened to be at, a block landing mid-pass left some rows from before it and some from after -- a set of totals no block ever held, and nothing corrects the rows read early unless some later event happens to touch them. The head is resolved once and every read in the pass is pinned to it, so the pass commits one block's answer or none. The head is also required to be at least as new as the event being applied: a node that has not caught up to the event's own block would otherwise return a truth predating the event and persist it, and since the event is then marked processed, nothing is left to re-drive it. Failing instead hands the event back to asynq, which is what retries are for. On chain-backed state the applications write only the history. Moving the totals by the event's delta first, and refusing a removal larger than the loaded amount, does not mix with re-reading them. A loaded amount can already be from a head past the event being applied: stake 100 and unstake 100, and if the Staked event is processed after the unstake landed, its re-read stores 0. The Unstaked event then fails "staked amount is less than staked amount removed" on every retry, and resync writes the same 0 back, so nothing can ever get it through. The staked and pending totals are now left as loaded, neither moved nor checked, and the reconciler replaces them. Leaving them untouched rather than clamping the subtraction matters for the account row: it is corrected by how far each staking moved from what was loaded, so a clamp on both would have zeroed what the account holds in pools the event never loaded. An account totals stakings that are not all loaded, so it moves by the correction rather than being recomputed; the clamp that keeps that from wrapping now reports itself, since it only fires on an already corrupt row. Rows holding nothing on either side are skipped. A pool keeps its fully unstaked rows forever, and a deposit loads all of them, so re-reading those would tie its cost to the pool's whole history rather than its current stakers. The rest are read through a bounded worker pool. A staking an event names is re-read even when it loads empty: with the delta no longer applied, a stake into an empty row would otherwise look like a row holding nothing and be skipped. Such a row differing from the chain is expected, so it is logged at debug, keeping the warning for drift on rows no event explained. Simulation keeps the old path: it exists to check the delta arithmetic against a known expected state, and reconciling would have it comparing the chain with itself. Three adjacent bugs the same review turned up: - check-received-evm-events sorted the events into a variable it then never read, and enqueued the unsorted query result. - pool_share reached its operands through Int64(), which wraps above MaxInt64 on the uint64 amount columns. - MapWithLimit could return a partially filled slice with a nil error: the producer stops handing out items once the context is done, leaving the untouched entries at their zero value with nothing to say so. A caller cancelled mid-map read those zeros as chain truth and would have written them.

…itted head The totals are absolute values read at a chain head, and nothing stopped an older one landing over a newer one. With worker concurrency above one or several replicas, a pass that read head H1 could stall while another read and committed H2, then commit H1 over it. Account rows were worse off: they are written as the loaded value moved by a correction, so two passes that loaded the same account before either committed each overwrote the other's correction. Even at concurrency one a load-balanced RPC node lagging the one the previous pass asked hands back an older head. resync had both problems at once. Its default snapshot is five blocks behind the head the workers read at, so applying it rolled back rows whose events were already marked processed and would never re-read them. Its build takes minutes, and a claim processed meanwhile was rolled back by the claimed_reward_amount the snapshot carried over from the database. Every writer now holds one Postgres advisory lock across load, chain read and persist. It is transaction scoped, so a writer that dies cannot leave it held, and the persist runs its statements on that same transaction: a lock held by an idle transaction of its own could be terminated or lose its connection during a long chain read, unnoticed, leaving the writer free to commit after another writer had loaded and committed. The transaction commits only when the locked function returns nil, so a lost lock fails the write instead of landing it out of order. Reads still go through other connections. The head each commit was read at is recorded in the new staking_state_heads table, advanced by a conditional update in the same transaction as the totals, and a commit from an older head is refused and retried. The migration seeds that head with the newest applied staking event's block rather than leaving the table empty, which reads as head 0: resync checks its snapshot block against the committed head alone, so until the first worker commit an empty table would let resync --apply --block write a snapshot older than events the delta-built state already holds, and those events are marked processed and would never run again. The delta-built state is right up to that block, and a worker always reads at or past its event's block, so the first commit still advances it. resync --apply takes the same lock before choosing its block and holds it until the write commits, so workers pause for the build. The snapshot block is raised to the committed head when --confirmations would put it behind, and an explicit --block behind it is refused. The migration has to be applied before this is deployed, or every event write fails on the missing table. The event workers also have to be drained once: the lock only holds back writers that take it, and a rolling deploy onto workers from before it would leave an old pod applying deltas without it alongside a new pod writing chain-read totals, each overwriting the other. After that upgrade every pod takes the lock and rolling deploys are safe.

…eposit On chain-backed state a RewardDeposited was split by the loaded staked amounts, but those are what the reconciler last wrote, read at a head. When the queue is behind, an earlier event can have re-read the pool at a head past the deposit, so the reward_deposit_distributed rows handed the reward to whoever held the stake later. The applications no longer move those amounts either, so a stake just before the deposit went uncounted. The rows are history served by the API, and nothing corrects them. The pool's stake is now read at the deposit's own block -- the total and every loaded staker, empty rows included, since holding nothing at the head says nothing about the deposit -- and the split uses that. The materialized totals still come from the head. A past block's stake never changes, so unlike the totals these reads are safe to pin; they do need archive state once the event leaves the node's window, which Alchemy serves. The read is of the state after the whole block, so a stake or unstake later in the same block counts as if it came first. The contract offers no read between two logs of one block. That stake is read only for stakings already loaded, while the total is the pool's own. A holder with no row -- a Staked not yet processed, or never delivered -- would be silently left out of the reward_deposit_distributed rows, and resync does not rewrite history. When the stakings read add up to less than the total, the event fails with ErrHolderNotIndexed and is retried once the row exists. A surplus is still split: the contract's total has drifted below the sum of its positions before, when restake and increaseStakeToPosition left pending rewards out of it, and a past block's reads never change, so refusing it would fail those deposits forever. Simulation keeps splitting by its delta-tracked amounts.

… the task The deferred failed-status update reused the task's context and dropped its error. Workers now wait on the staking state lock, possibly behind a long resync, so a task timing out or a worker shutting down leaves the context cancelled, the update fails, and the event stays processing. Neither the received scanner nor the failed retry picks it up again. The update now runs detached from cancellation, under its own timeout so a stuck database cannot hold up shutdown, and its error is logged.

topic0OrderMap is empty, so cmpTopic0 found neither topic ranked and returned 1 for every pair, an event against itself included. The comparator never reached the log index, and broke the contract slices.SortFunc relies on. That went unnoticed while check-received-evm-events sorted a copy it never used, but it now enqueues in the sorted order, so logs of one transaction could still go out of order. Two unranked topics now compare equal and fall through to the log index; a ranked topic still sorts before an unranked one.

Copilot

🔵 Needs a closer look

It changes financial state reconciliation, transaction locking, historical reward attribution, and deployment-critical schema behavior.

Review details

Files not reviewed (15)

likecollective-indexer/ent/client.go: Generated file
likecollective-indexer/ent/ent.go: Generated file
likecollective-indexer/ent/hook/hook.go: Generated file
likecollective-indexer/ent/migrate/schema.go: Generated file
likecollective-indexer/ent/mutation.go: Generated file
likecollective-indexer/ent/predicate/predicate.go: Generated file
likecollective-indexer/ent/runtime.go: Generated file
likecollective-indexer/ent/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/stakingstatehead.go: Generated file
likecollective-indexer/ent/stakingstatehead/where.go: Generated file
likecollective-indexer/ent/stakingstatehead_create.go: Generated file
likecollective-indexer/ent/stakingstatehead_delete.go: Generated file
likecollective-indexer/ent/stakingstatehead_query.go: Generated file
likecollective-indexer/ent/stakingstatehead_update.go: Generated file
likecollective-indexer/ent/tx.go: Generated file

Files reviewed: 24/39 changed files
Comments generated: 1
Review effort level: Balanced

+	// Enqueue oldest first. GetEVMEventsByStatus does not order, so without
+	// this the deltas that build staking_events are recorded in whatever order
+	// the query happened to return.


The comment said GetEVMEventsByStatus does not order, and that without the sort the staking_events deltas are recorded in whatever order the query returned. It does order: it goes through BaseQuery, which sorts by block number, transaction index and log index, and block_number is a numeric column, so that ordering is not lexicographic either. The claim described the state before the sort was applied to the slice the loop actually iterates. The sort stays. The comparator is the one definition of processing order and carries a topic0 rank the query has no equivalent for, inert only while topic0OrderMap is empty, and retry-failed-evm-events re-drives through the same comparator.

williamchong requested a balanced review from Copilot September 17, 2026 07:11

Copilot started reviewing on behalf of williamchong September 17, 2026 07:12

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

williamchong requested a balanced review from Copilot September 17, 2026 15:53

Copilot started reviewing on behalf of williamchong September 17, 2026 15:54

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

williamchong added 5 commits September 18, 2026 02:10

williamchong force-pushed the split/chain-reconcile branch from 93e6dec to 1d7db98 Compare September 17, 2026 18:13

williamchong requested a balanced review from Copilot September 17, 2026 18:13

Copilot started reviewing on behalf of williamchong September 17, 2026 18:14

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

williamchong merged commit 42a94b4 into likecoin:main Sep 17, 2026
9 checks passed

Navigation Menu

Repository navigation

likecollective-indexer: Re-read staking totals from the chain instead of applying event deltas - #324

likecollective-indexer: Re-read staking totals from the chain instead of applying event deltas#324
williamchong merged 6 commits into
likecoin:mainfrom
williamchong:split/chain-reconcile

williamchong commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// WithStakingStateLock runs fn holding the advisory lock that every writer of
		// the staking state takes around its load, chain read and persist.

Conversation

williamchong commented Sep 17, 2026

Why

What changes

Deploy

Known limits

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (15)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (15)

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Files not reviewed (15)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants