Skip to content

Conversation

@thlorenz
Copy link
Collaborator

@thlorenz thlorenz commented Nov 21, 2025

Summary

Refine account filtering on startup to preserve undelegating and delegated accounts by distinguishing between delegated-only and undelegating states.

Details

magicblock-chainlink Account Filtering

The startup account filtering logic was updated to provide better tracking and preservation of account states:

  • Removed DLP-owned account filtering: Previously, DLP-owned non-delegated accounts were explicitly filtered out during startup. This behavior has been removed, allowing these accounts to be preserved.

  • Refined delegated account tracking: The delegated account tracking now distinguishes between two states:

    • delegated_only: Accounts that are delegated but not undelegating
    • undelegating: Accounts that are both delegated and in an undelegating state
  • Updated filtering logic: Accounts are now kept on startup if they are delegated (whether delegated-only or undelegating), ensuring that accounts in transition states are preserved for proper state management.

  • Improved logging: The startup logs now provide a clearer breakdown of account states, making it easier to track what's being filtered and what's being kept.

Summary by CodeRabbit

  • Refactor
    • Improved internal delegation-account tracking to separate currently undelegating from delegated-only accounts for clearer logic.
  • Bug Fixes
    • Fixed delegation counting so reported counts and percentages are accurate after the new partitioning.
  • Logs
    • Updated log output to show a clearer breakdown (delegated-only vs undelegating) and combined totals for easier reporting.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link

github-actions bot commented Nov 21, 2025

Manual Deploy Available

You can trigger a manual deploy of this PR branch to testnet:

Deploy to Testnet 🚀

Alternative: Comment /deploy on this PR to trigger deployment directly.

⚠️ Note: Manual deploy requires authorization. Only authorized users can trigger deployments.

Comment updated automatically when the PR is synchronized.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

📝 Walkthrough

Walkthrough

This change refactors classification and logging inside reset_accounts_bank. Two counters were renamed and repurposed: delegateddelegated_only and dlp_owned_not_delegatedundelegating. Accounts where account.delegated() || account.undelegating() are now treated together: increment undelegating when undelegating() is true, otherwise delegated_only. The previous DLP-owned non-delegated special-case was removed. Log output and final aggregation were updated to read concrete values of delegated_only and undelegating (converted from atomics) and to report their sum as the delegated total.

Possibly related PRs

Suggested reviewers

  • bmuddha
  • GabrielePicco
  • taco-paco
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch thlorenz/keep-undelegating

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9334f7 and e5d31e9.

📒 Files selected for processing (1)
  • magicblock-chainlink/src/chainlink/mod.rs (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579

Applied to files:

  • magicblock-chainlink/src/chainlink/mod.rs
🔇 Additional comments (4)
magicblock-chainlink/src/chainlink/mod.rs (4)

152-153: LGTM: Clear counter naming.

The renamed counters delegated_only and undelegating clearly express their purpose and align with the PR objectives to distinguish between delegated-only and undelegating states.


163-173: LGTM: Filtering logic correctly preserves undelegating accounts.

The logic correctly implements the PR objectives:

  • Uses OR condition to keep accounts that are either delegated or undelegating
  • Properly classifies kept accounts into undelegating (if undelegating) or delegated_only (otherwise)
  • The comment accurately explains the future-proofing rationale

191-192: LGTM: Proper atomic conversion.

Converting the atomic counters to regular values before logging is correct and ensures consistent values throughout the log statement.


194-207: LGTM: Log parameters now correctly match format strings.

The previous critical issue where log parameters were swapped has been resolved. The parameters now correctly map to their format string labels:

  • delegated_only → "delegated not undelegating" ✓
  • undelegating → "delegated and undelegating" ✓

The logging provides clear visibility into account filtering during startup.


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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9613d0 and 861b711.

📒 Files selected for processing (1)
  • magicblock-chainlink/src/chainlink/mod.rs (3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.

Applied to files:

  • magicblock-chainlink/src/chainlink/mod.rs
📚 Learning: 2025-11-18T08:47:39.702Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: magicblock-chainlink/tests/04_redeleg_other_separate_slots.rs:158-165
Timestamp: 2025-11-18T08:47:39.702Z
Learning: In magicblock-chainlink tests involving compressed accounts, `set_remote_slot()` sets the slot of the `AccountSharedData`, while `compressed_account_shared_with_owner_and_slot()` sets the slot of the delegation record. These are two different fields and both calls are necessary.

Applied to files:

  • magicblock-chainlink/src/chainlink/mod.rs
🔇 Additional comments (1)
magicblock-chainlink/src/chainlink/mod.rs (1)

152-168: LGTM! Clear differentiation between delegation states.

The refactored logic correctly distinguishes between accounts that are delegated-only versus those in an undelegating state, and appropriately preserves both on startup as intended by the PR objectives.

@GabrielePicco GabrielePicco self-requested a review November 21, 2025 18:14
@thlorenz thlorenz merged commit 710792e into master Nov 21, 2025
7 of 8 checks passed
@thlorenz thlorenz deleted the thlorenz/keep-undelegating branch November 21, 2025 18:15
thlorenz added a commit that referenced this pull request Nov 21, 2025
* master:
  chore: preserve undelegating and delegated accounts on startup (#670)
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