Skip to content

Conversation

@serichoi65
Copy link
Contributor

@serichoi65 serichoi65 commented Nov 14, 2025

Description

1. Database Migration (202511141700_withdrawalQueueAndAllocationRounding)

  • Enhanced queued_slashing_withdrawals table with timing columns for 14-day withdrawal tracking:
    • completed, completion_* - Completion tracking
  • Enhanced operator_allocations table with rounding support:
    • effective_date - Rounded date when allocation/deallocation takes effect

2. Allocation/Deallocation Rounding Logic (Sabine Fork)

Implements day-boundary rounding to prevent gaming and mimic existing registration behavior:

  • Allocations (increases): Round UP to next day → starts earning tomorrow
  • Deallocations (decreases): Round DOWN to current day → stops earning immediately
  • No change: Round DOWN to current day (conservative default)

3. Dual Behavior Implementation

Maintains backward compatibility with fork-based behavior switching:

  • Before Sabine fork: Uses old behavior (no rounding, date fields are NULL)
  • After Sabine fork: Applies rounding logic and populates date fields
  • Model continues processing allocations from Brazos fork onwards

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Unit tests

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@serichoi65 serichoi65 requested a review from a team as a code owner November 14, 2025 21:09
@serichoi65 serichoi65 force-pushed the seri.choi/withdrawal-deallocation-queue branch 3 times, most recently from 3e7ab11 to 87d7809 Compare November 17, 2025 20:15
@serichoi65 serichoi65 force-pushed the seri.choi/withdrawal-deallocation-queue branch from 267d580 to f51270b Compare November 30, 2025 19:16
@serichoi65 serichoi65 force-pushed the seri.choi/withdrawal-deallocation-queue branch 3 times, most recently from bfc21ac to 4d8299b Compare December 1, 2025 17:08
strategy,
operator_set_id,
magnitude,
block_time,
Copy link
Contributor

Choose a reason for hiding this comment

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

So the AllocationUpdated event emits an effectBlock. I'm not sure if we're using that anywhere here. The effectBlock is the block at which the allocation/dealloction becomes active

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed join from oa.block_number to oa.effective_block

Copy link
Contributor

Choose a reason for hiding this comment

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

We're joining on the effective_block, but now we also need to make sure that we're making sure that the allocation is counted at the effective block and its associated day, not the block at which the event was emitted.

Example is when the event is emitted at block 1 but the allocation is activated at block 10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct,

INNER JOIN blocks b ON oa.effective_block = b.number
WHERE b.block_time < TIMESTAMP '{{.cutoffDate}}'

The join is on effective_block, so b.block_time comes from the effective block, not the emission block. The new test case added for the future allocation scenario verifies this

Copy link
Contributor

@ypatil12 ypatil12 left a comment

Choose a reason for hiding this comment

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

One minor comment on the effective_block and a test to add. Other than looking solid!

strategy,
operator_set_id,
magnitude,
block_time,
Copy link
Contributor

Choose a reason for hiding this comment

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

We're joining on the effective_block, but now we also need to make sure that we're making sure that the allocation is counted at the effective block and its associated day, not the block at which the event was emitted.

Example is when the event is emitted at block 1 but the allocation is activated at block 10

"go.uber.org/zap"
)

const operatorAllocationSnapshotsQuery = `
Copy link
Contributor

Choose a reason for hiding this comment

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

Taking a look at operatorAVSRegistrationSnapshots.go, I think we might be missing a few cases in general:

From that file:

// Operator AVS Registration Windows: Ranges at which an operator has registered for an AVS
// 0. Ranked: Rank the operator state changes by block_time and log_index since sqlite lacks LEAD/LAG functions
// 1. Marked_statuses: Denote which registration status comes after one another
// 2. Removed_same_day_deregistrations: Remove a pairs of (registration, deregistration) that happen on the same day
// 3. Registration_periods: Combine registration together, only select registrations with:
// a. (Registered, Unregistered)
// b. (Registered, Null). If null, the end time is the current timestamp
// 4. Registration_snapshots: Round up each start_time to  0 UTC on NEXT DAY and round down each end_time to 0 UTC on CURRENT DAY
// 5. Operator_avs_registration_windows: Ranges that start and end on same day are invalid
// Note: We cannot assume that the operator is registered for avs at end_time because it is
// Payments calculations should only be done on snapshots from the PREVIOUS DAY. For example say we have the following:
// <-----0-------1-------2------3------>
// ^           ^
// Entry        Exit
// Since exits (deregistrations) are rounded down, we must only look at the day 2 snapshot on a pipeline run on day 3.

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. If an allocation and a non-zero deallocation happens on the same day (this is just on testnet), we should just use the deallocation and not round down. Rounding down would just give the user an extra day of stake.
  2. If an allocation and zero deallocation happens on the same day, then discard the entire allocation/deallocation pair

Again, these cases only exist on testnet since allocation/deallocation can happen in the same day. In practice mainnet would never have this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it looks like we properly handle these cases based on the tests. I just want to leave this comment up to make sure you agree that we're not missing anything based on the what we did in the above logic too :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, tests properly handle allocation/deallocation on same day by taking latest record!

fix

fix test
@serichoi65 serichoi65 force-pushed the seri.choi/withdrawal-deallocation-queue branch from 41d0383 to 691afe7 Compare December 3, 2025 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants