Skip to content

feat: add undelegating flag and fix COW sync issues#21

Merged
thlorenz merged 1 commit into
mainfrom
thlorenz/undelegating-flag
Nov 18, 2025
Merged

feat: add undelegating flag and fix COW sync issues#21
thlorenz merged 1 commit into
mainfrom
thlorenz/undelegating-flag

Conversation

@thlorenz
Copy link
Copy Markdown
Contributor

@thlorenz thlorenz commented Nov 18, 2025

Summary by CodeRabbit

  • New Features

    • Added undelegating flag support to account data with getter and setter methods.
    • Extended debug output to display undelegating state.
  • Tests

    • Added test coverage for undelegating flag functionality, including state persistence and flag isolation.

Adds support for an undelegating flag and fixes copy-on-write (COW) synchronization issues in the Account implementation:

  • New undelegating flag (index 4) with set_undelegating() and undelegating() methods
  • Fixed COW sync bugs where borrowed account modifications weren't triggering COW before data changes
  • Updated Debug impl to include undelegating flag
  • Comprehensive test coverage for undelegating flag behavior

Details

Undelegating Flag

Added a new account state flag for tracking delegation transitions. The flag is persisted with the same mechanism as other flags (executable, delegated, privileged, compressed) and includes both accessor and mutator methods.

COW Synchronization Fixes

Fixed several places in the code where borrowed accounts were being modified without triggering copy-on-write operations:

  • set_executable(): Now calls acc.cow() before modifying flags on borrowed accounts
  • truncate_data(): Now calls acc.cow() before modifying data length on borrowed accounts
  • spare_data_capacity_mut(): Now calls acc.cow() at the beginning for borrowed accounts

These fixes ensure that any modifications to borrowed accounts properly create a local copy before changes are applied, maintaining data consistency.

Testing

Added test coverage for the undelegating flag including:

  • Flag persistence across borrowing operations
  • Setting and clearing the flag
  • Interaction with other flags to ensure independence

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 18, 2025

Walkthrough

Introduces support for an undelegating flag on account data structures. Adds the UNDELEGATING_FLAG_INDEX constant, implements getter and setter methods for the flag, extends debug output to include the undelegating state, and provides test coverage for the new functionality.

Changes

Cohort / File(s) Summary
Flag constant definition
src/cow.rs
Adds UNDELEGATING_FLAG_INDEX constant (value 4) with documentation for the new undelegating flag in AccountBorrowed and AccountOwned flag docs.
Flag accessor methods and debug output
src/lib.rs
Adds set_undelegating() and undelegating() methods to AccountSharedData enum for mutating and querying the undelegating flag across Owned and Borrowed variants. Extends Debug implementation to include undelegating state. Imports UNDELEGATING_FLAG_INDEX for use in flag logic.
Test coverage
src/cow.rs, src/lib.rs
Adds test_undelegating_flag_persistence() in cow.rs to verify flag persistence via existing helpers, and test_undelegating_flag() in lib.rs to validate default state, mutability, and non-interference with other flags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Follows established flag-handling patterns already present in the codebase
  • Straightforward addition of getter/setter methods with appropriate cow() invocation for Borrowed variants
  • Comprehensive test coverage included
  • Debug output extension is minimal and consistent with existing patterns

Possibly related PRs

Suggested reviewers

  • bmuddha

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add undelegating flag and fix COW sync issues' directly addresses both major changes in the PR: introducing the undelegating flag feature and fixing copy-on-write synchronization bugs.
✨ 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/undelegating-flag

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f454d4a and ffbadd2.

📒 Files selected for processing (2)
  • src/cow.rs (4 hunks)
  • src/lib.rs (7 hunks)
🔇 Additional comments (11)
src/cow.rs (4)

81-81: LGTM: Flag index follows established pattern.

The new UNDELEGATING_FLAG_INDEX constant correctly follows the sequential numbering scheme used by existing flags (executable=0, delegated=1, privileged=2, compressed=3, undelegating=4).


126-126: LGTM: Documentation updated consistently.

The documentation correctly includes the undelegating flag in the AccountBorrowed flags list, maintaining consistency with the existing flag documentation pattern.


287-287: LGTM: Documentation updated consistently.

The documentation correctly includes the undelegating flag in the AccountOwned flags list, maintaining consistency with the existing flag documentation pattern.


1054-1061: LGTM: Test coverage is comprehensive.

The test follows the established pattern using the generic test_flag_persistence helper, ensuring the undelegating flag is properly persisted across COW operations, set/clear cycles, and serialization/deserialization. This is consistent with how other flags are tested.

src/lib.rs (7)

6-6: LGTM: Import addition is correct.

The UNDELEGATING_FLAG_INDEX import follows the established pattern and is necessary for the new undelegating flag functionality.


335-338: Critical fix: COW synchronization bug resolved.

This change correctly fixes a critical bug where modifying the executable flag on a borrowed account would write directly to the memory-mapped region without triggering COW first. Without this fix, modifications could corrupt the primary buffer when the shadow buffer should be used, leading to inconsistent account state.

The fix brings set_executable() into consistency with other mutating methods like set_delegated() and set_compressed(), which already call cow() before modification.


460-460: LGTM: Debug output includes new flag.

The undelegating field is correctly added to the Debug implementation, maintaining consistency with other flags (delegated, compressed, privileged).


699-716: LGTM: Methods follow established patterns.

The set_undelegating() and undelegating() methods correctly follow the same implementation pattern as the existing set_compressed() and compressed() methods:

  • set_undelegating() properly calls cow() for borrowed accounts before modifying flags
  • Both methods handle both Owned and Borrowed variants consistently
  • The implementation correctly uses UNDELEGATING_FLAG_INDEX for bit manipulation

799-802: Critical fix: COW synchronization bug in truncation path.

This change correctly fixes a critical bug where shrinking account data (truncation path in resize()) would modify the data length field directly in memory without triggering COW first. Without this fix, the length modification would write to the primary buffer even when other modifications had already been written to the shadow buffer, causing inconsistent state.

The fix ensures that cow() is called before set_len(), bringing the shrinking path into consistency with other data-modifying operations.


896-896: Critical fix: COW synchronization bug in capacity access.

This change correctly fixes a critical bug where obtaining mutable access to spare capacity would allow direct writes to the memory-mapped region, completely bypassing the COW mechanism. Without this fix, any writes to the returned mutable slice would corrupt the primary buffer instead of writing to the shadow buffer.

The fix ensures cow() is called at the function entry for borrowed accounts, guaranteeing that all subsequent writes target the shadow buffer.


1259-1288: LGTM: Comprehensive test coverage for undelegating flag.

The test thoroughly validates the undelegating flag functionality:

  • Verifies default state is false
  • Tests setting the flag to true and false
  • Confirms independence from other flags (delegated, executable, compressed)

This test coverage is consistent with and comparable to tests for other flags like test_compressed_flag.


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.

@thlorenz thlorenz merged commit 3938e32 into main Nov 18, 2025
2 checks passed
@thlorenz thlorenz deleted the thlorenz/undelegating-flag branch November 18, 2025 13:25
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.

1 participant