Increase delegated stakes

[effects-ai]

Based on #117

[ryle-ai]

Review: Increase Delegated Stakes Support

I've analyzed this PR's implementation of the delegated stake increase feature. Overall the logic is sound, but there are a few observations.

✅ Verified Correct

1. Stake Increase Detection (GlobalSnapshotMapper.scala lines 225-246)

   • Correctly identifies new vs. updated stakes by comparing both event hash AND currentTokenLockRef
   • Changes are properly detected when either differs

2. Token Lock Replacement Tracking (mapTokenLock lines 281-299)

   • replaceTokenLockRef is properly extracted and stored
   • Creates correct linkage from new lock to original lock

3. Transaction Isolation

   • All snapshot operations wrapped in single transaction with commit
   • Prevents race conditions within snapshot processing

4. Upsert Behavior (insertDelegatedStakingCreateCommand lines 239-275)

   • ON CONFLICT (hash) DO UPDATE correctly uses event hash as stable identifier
   • current_token_lock_hash and current_amount updated on stake increase

🟡 Observations (Low Severity)

1. Transfer History Overwrite

When a stake is increased multiple times:

ON CONFLICT (hash) DO UPDATE SET
    transfer_from_hash = EXCLUDED.transfer_from_hash,

Each subsequent increase overwrites transfer_from_hash, losing the chain of previous states.

Impact: If audit trail of all stake increases is needed, this won't capture the full history. If only "immediately previous state" is needed, this is correct.

2. Missing FK Constraint (delegated_staking.sql)

current_token_lock_hash lacks a foreign key to dag_token_locks, while lock_reference_hash has one. In practice this is unlikely to cause issues since related records are in the same snapshot.

3. Withdrawal Records Missing Current Values

insertDelegatedStakingCreateWithdrawCommand (lines 277-291) doesn't include current_token_lock_hash or current_amount, though the DelegatedStakingWithdraw schema has these fields. If withdrawal audit needs to capture the stake value at withdrawal time, this would need updating.

Dependency Notes

This PR correctly aligns with tessellation #1405's changes to DelegatedStakeRecord:

• Uses dsr.amount and dsr.tokenLockRef which return computed "current" values when stake is increased
• Tests confirm expected behavior (e.g., test expects amount = 2000L when currentAmount is set)

Verdict: The core logic is correct. The observations above are low severity and may be intentional design choices.

---

Automated review by ryle-ai

[ryle-ai]

Review: Increase Delegated Stakes

The core logic for detecting stake increases via currentTokenLockRef comparison and the upsert pattern is sound. There are two issues worth discussing.

1. DelegatedStakingWithdraw fields populated but never persisted

The DelegatedStakingWithdraw case class gains currentTokenLockHash and currentAmount fields (schema/DelegatedStaking.scala lines 25-26 in the diff), and mapDelegatedStakingWithdraw in GlobalSnapshotMapper.scala (lines 153-154 in the diff) populates them from pendingWithdrawal.currentTokenLockRef and pendingWithdrawal.currentAmount.

However, insertDelegatedStakingCreateWithdrawCommand in SnapshotDAO.scala (lines 268-282, unchanged in this PR) does not include these columns in the INSERT statement, and the delegate_stake_withdraw_events table schema has no corresponding columns either. The tests (map stakes withdrawal with currentTokenLockRef and currentAmount fields) verify the in-memory mapping works, but the data is silently dropped at persistence time.

If this is intentional (withdrawal records don't need to capture the stake value at withdrawal time), consider removing the fields from the DelegatedStakingWithdraw case class to avoid the misleading impression that they're stored. If they should be persisted, the SQL table and INSERT command both need updating.

2. Upsert does not update amount or lock_reference_hash

insertDelegatedStakingCreateCommand (SnapshotDAO.scala lines 248-282 in the diff) now uses ON CONFLICT (hash) DO UPDATE SET for current_token_lock_hash, current_amount, transfer_from_hash, and updated_at. The base amount and lock_reference_hash columns are not in the UPDATE SET.

Meanwhile, the mapper (GlobalSnapshotMapper.scala lines 200-204 in the diff) switched from dsr.event.amount / dsr.event.tokenLockRef to dsr.amount / dsr.tokenLockRef -- the computed "current" values. This means:

• First insert (new stake): amount = original, lock_reference_hash = original token lock, current_amount = NULL, current_token_lock_hash = NULL
• Upsert (stake increased): amount stays at original (not updated), lock_reference_hash stays at original (not updated), current_amount = new value, current_token_lock_hash = new ref

The in-memory DelegatedStakingCreate.amount field holds the computed current value from dsr.amount, but the DB amount column retains the original value on upsert. This divergence between in-memory representation and persisted data is not necessarily wrong -- it may be the intended design where amount = original stake and current_amount = latest -- but it is subtle and worth documenting. Downstream consumers querying amount vs current_amount need to understand this distinction.

Alternatively, if amount should always reflect the current state (as the mapper's in-memory value suggests), it should be added to the DO UPDATE SET clause.

Minor: visibility change on mapTokenLock

mapTokenLock in GlobalSnapshotMapper.scala (line 278 in the diff) changed from private to def (public). This appears to be solely for test access (the map token lock with replacementHash set tests call it directly). Consider whether a test-specific approach (e.g., testing through a higher-level public method) would be preferable to exposing an internal method.

[ryle-ai]

transfer_from_hash is self-referential on stake increase

When a stake is increased, mapDelegatedStakingCreates detects the change because oldCurrentTokenLockRef != dsr.currentTokenLockRef, and passes Some(oldStakeHash) as fromHash. But oldStakeHash was computed from dsr.event.toHashed on the previous snapshot's DelegatedStakeRecord -- and since the underlying event (Signed[UpdateDelegatedStake.Create]) never changes when a stake is increased (only currentTokenLockRef/currentAmount change), oldStakeHash == ev.hash. This means the upserted row's transfer_from_hash points to itself.

The test at line 696 confirms this implicitly: it asserts result.head.transferFrom.isDefined but doesn't check that it differs from result.head.hash. If transfer_from_hash is meant to link to a different record (like the original stake before an increase), this is a bug. If the intent is just a boolean signal that the row was updated (not a true foreign reference), consider using a simpler flag or documenting this self-referential behavior.

global_snapshot_hash frozen at first-seen snapshot

The upsert's DO UPDATE SET clause updates current_token_lock_hash, current_amount, transfer_from_hash, and updated_at -- but not global_snapshot_hash. After a stake increase is processed, the row still references the snapshot where the stake was first indexed, not the snapshot where the increase occurred. If downstream consumers use global_snapshot_hash to determine when the current state of a stake was established, they'll get the wrong answer.

[ryle-ai]

When a stake is increased, the event field on DelegatedStakeRecord is immutable -- only currentTokenLockRef/currentAmount change. So oldStakeHash (hashed from previous DSR's event) equals ev.hash (hashed from current DSR's event), making fromHash = Some(ev.hash). The resulting transfer_from_hash in the DB row will point to the row's own hash.

If this is meant as a self-referential "this row was updated" marker, it works but is unusual. If it's meant to reference a distinct prior record, this is a bug -- increased stakes don't produce new rows with different hashes, they upsert the same row.

[ryle-ai]

On upsert (stake increase), global_snapshot_hash is not in the DO UPDATE SET clause, so it retains the value from the original insert. But the DelegatedStakingCreate.snapshotHash passed in reflects the current snapshot (where the increase was detected). This means the snapshot hash in the VALUES clause is silently discarded on conflict.

If the intent is to preserve the original creation snapshot, this is fine. But if consumers need to know which snapshot triggered the latest state change, consider adding global_snapshot_hash = EXCLUDED.global_snapshot_hash to the update set, or adding a separate last_updated_snapshot_hash column.

[ryle-ai]

Using dsr.amount.value (which resolves to currentAmount.getOrElse(event.amount)) means the amount field in DelegatedStakingCreate holds the effective current amount. But in the DB, amount is only written on first INSERT and not updated by the upsert -- only current_amount is updated.

This creates a data inconsistency depending on indexer start time: if the indexer first encounters a stake that already has an increase, amount gets the current value (e.g. 2000). If it indexed the stake from creation and later processes the increase, amount stays at the original (e.g. 1000). Consider using dsr.event.amount.value here to always store the original event amount, making the amount column semantics consistent regardless of when indexing started.