feat: add customized solana-account crate - #13
Open
bmuddha wants to merge 1 commit into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changessolana-account crate
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant AccountSharedData
participant CoWAccount
participant BorrowedAccount
participant AccountHeader
Caller->>AccountSharedData: update account data
AccountSharedData->>CoWAccount: request mutable backing
CoWAccount->>BorrowedAccount: translate or promote
BorrowedAccount->>AccountHeader: stage shadow image
Caller->>BorrowedAccount: commit
BorrowedAccount->>AccountHeader: publish sequence
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Collaborator
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 23, 2026
bmuddha
force-pushed
the
solana-account
branch
3 times, most recently
from
April 28, 2026 13:23
726ddf7 to
b00291b
Compare
bmuddha
force-pushed
the
solana-account
branch
2 times, most recently
from
May 7, 2026 17:35
a0e2c91 to
7501614
Compare
bmuddha
force-pushed
the
solana-account
branch
4 times, most recently
from
May 15, 2026 14:52
9acbc90 to
2e43191
Compare
This was referenced May 28, 2026
bmuddha
force-pushed
the
solana-account
branch
7 times, most recently
from
June 2, 2026 18:26
9f791a8 to
aab5a4e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@solana/account/src/lib.rs`:
- Around line 13-15: Update the cfg attribute on the test_utils module to expose
it during ordinary unit-test builds as well as when the dev-context-only-utils
feature is enabled. Preserve the existing feature-gated behavior and ensure
tests/account.rs can resolve crate::test_utils under cfg(test).
In `@solana/account/src/test_utils.rs`:
- Around line 43-57: Make init_borrowed_account, borrowed_shared_data, and
active_borrowed_data unsafe or otherwise bind returned views to owned storage,
preventing arbitrary buffers from creating lifetime-free borrowed-layout views.
Document explicit caller obligations to provide a valid BorrowedAccount layout
and keep the backing buffer alive for every returned view; preserve
active_borrowed_data’s validation requirement as well.
In `@solana/account/src/tests/account.rs`:
- Around line 277-285: Update solana/account/src/tests/account.rs lines 277-285
in test_account_cow_borrowed_extend_promotes to append capacity - len + 1 bytes
and assert shared.cow() matches CoWAccount::Owned(_); update lines 342-353 in
the related set_data_at promotion test to either force and assert promotion or
rename it to describe in-capacity behavior, keeping the test names and
assertions factually consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 094db5d7-ce72-45fb-be2a-717761c2f6eb
📒 Files selected for processing (22)
.gitattributes.gitignoreCargo.tomlsolana/account/Cargo.tomlsolana/account/README.mdsolana/account/src/account.rssolana/account/src/codec.rssolana/account/src/cow/borrowed.rssolana/account/src/cow/mod.rssolana/account/src/cow/owned.rssolana/account/src/cow/tests.rssolana/account/src/lib.rssolana/account/src/patch.rssolana/account/src/state_traits.rssolana/account/src/sysvar.rssolana/account/src/test_utils.rssolana/account/src/tests/account.rssolana/account/src/tests/mod.rssolana/account/src/tests/state_traits.rssolana/account/src/tests/sysvar.rssolana/account/src/traits.rssrc/lib.rs
💤 Files with no reviewable changes (1)
- src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (17)
- solana/account/src/tests/sysvar.rs
- .gitignore
- solana/account/src/tests/state_traits.rs
- solana/account/src/tests/mod.rs
- .gitattributes
- solana/account/src/codec.rs
- solana/account/src/cow/tests.rs
- solana/account/Cargo.toml
- Cargo.toml
- solana/account/README.md
- solana/account/src/state_traits.rs
- solana/account/src/patch.rs
- solana/account/src/cow/owned.rs
- solana/account/src/cow/mod.rs
- solana/account/src/traits.rs
- solana/account/src/sysvar.rs
- solana/account/src/account.rs
Comment on lines
+13
to
+15
| /// Test-only helpers for borrowed account buffers. | ||
| #[cfg(feature = "dev-context-only-utils")] | ||
| pub mod test_utils; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Expose these helpers when compiling unit tests.
tests/account.rs imports crate::test_utils, but cfg(test) alone does not enable this feature, causing unresolved imports during ordinary unit-test builds.
Proposed fix
-#[cfg(feature = "dev-context-only-utils")]
+#[cfg(any(test, feature = "dev-context-only-utils"))]
pub mod test_utils;As per path instructions, Rust workspace reviews must prioritize API contract correctness.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Test-only helpers for borrowed account buffers. | |
| #[cfg(feature = "dev-context-only-utils")] | |
| pub mod test_utils; | |
| /// Test-only helpers for borrowed account buffers. | |
| #[cfg(any(test, feature = "dev-context-only-utils"))] | |
| pub mod test_utils; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@solana/account/src/lib.rs` around lines 13 - 15, Update the cfg attribute on
the test_utils module to expose it during ordinary unit-test builds as well as
when the dev-context-only-utils feature is enabled. Preserve the existing
feature-gated behavior and ensure tests/account.rs can resolve crate::test_utils
under cfg(test).
Source: Path instructions
Comment on lines
+277
to
+285
| #[test] | ||
| // Writing past borrowed capacity should promote to owned storage. | ||
| fn test_account_cow_borrowed_extend_promotes() { | ||
| let (_buf, mut shared) = make_borrowed(vec![7, 8]); | ||
|
|
||
| shared.extend_from_slice(&[9]); | ||
|
|
||
| assert_eq!(shared.data(), &[7, 8, 9]); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the promotion tests actually cross capacity and assert owned storage.
Both tests currently verify only resulting bytes, so they can pass while storage remains borrowed.
solana/account/src/tests/account.rs#L277-L285: appendcapacity - len + 1bytes and assertmatches!(shared.cow(), CoWAccount::Owned(_)).solana/account/src/tests/account.rs#L342-L353: either force and assert one promotion or rename the test to describe in-capacityset_data_atbehavior.
As per path instructions, Rust tests and documentation must remain factually consistent with behavior.
📍 Affects 1 file
solana/account/src/tests/account.rs#L277-L285(this comment)solana/account/src/tests/account.rs#L342-L353
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@solana/account/src/tests/account.rs` around lines 277 - 285, Update
solana/account/src/tests/account.rs lines 277-285 in
test_account_cow_borrowed_extend_promotes to append capacity - len + 1 bytes and
assert shared.cow() matches CoWAccount::Owned(_); update lines 342-353 in the
related set_data_at promotion test to either force and assert promotion or
rename it to describe in-capacity behavior, keeping the test names and
assertions factually consistent.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
solana/account/Cargo.toml (1)
15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGate the
serdefeature forbitflags.The
bitflagsdependency currently enables itsserdefeature unconditionally, which pullsserdeinto the dependency tree even when the crate'sserdefeature is disabled. Consider gating it behind this crate'sserdefeature to ensure true optionality.♻️ Proposed refactor
[features] bincode = ["dep:bincode", "dep:solana-sysvar", "serde"] -serde = ["dep:serde", "dep:serde_bytes", "serde/rc", "solana-pubkey/serde"] +serde = ["dep:serde", "dep:serde_bytes", "serde/rc", "solana-pubkey/serde", "bitflags/serde"] testkit = ["bincode"] wincode = ["bincode", "dep:wincode", "solana-pubkey/wincode"] [dependencies] bincode = { workspace = true, optional = true } -bitflags = { workspace = true, features = ["serde"] } +bitflags = { workspace = true } serde = { workspace = true, optional = true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@solana/account/Cargo.toml` around lines 15 - 21, Update the bitflags dependency declaration in the crate’s [dependencies] section to stop enabling its serde feature unconditionally; gate bitflags’ serde support through this crate’s serde feature while preserving the existing dependency and feature behavior otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@solana/account/src/cow/mod.rs`:
- Around line 39-44: Update AccountSharedData and its CoWAccount implementations
so cloning a Borrowed value materializes an owned snapshot rather than sharing
external backing storage with independent DirtyMarkers. Restrict DerefMut for
borrowed representations to prevent aliased mutable AccountCore access, and
remove any Sync implementation or bound until backing-memory synchronization
guarantees data-race-free access.
---
Nitpick comments:
In `@solana/account/Cargo.toml`:
- Around line 15-21: Update the bitflags dependency declaration in the crate’s
[dependencies] section to stop enabling its serde feature unconditionally; gate
bitflags’ serde support through this crate’s serde feature while preserving the
existing dependency and feature behavior otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b51a94e-77a8-4a0d-9eb9-e29e3dfc7a6f
📒 Files selected for processing (22)
.gitattributes.gitignoreCargo.tomlsolana/account/Cargo.tomlsolana/account/README.mdsolana/account/src/account.rssolana/account/src/codec.rssolana/account/src/cow/borrowed.rssolana/account/src/cow/mod.rssolana/account/src/cow/owned.rssolana/account/src/cow/tests.rssolana/account/src/lib.rssolana/account/src/patch.rssolana/account/src/state_traits.rssolana/account/src/sysvar.rssolana/account/src/testkit.rssolana/account/src/tests/account.rssolana/account/src/tests/mod.rssolana/account/src/tests/state_traits.rssolana/account/src/tests/sysvar.rssolana/account/src/traits.rssrc/lib.rs
💤 Files with no reviewable changes (1)
- src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (18)
- .gitattributes
- solana/account/src/codec.rs
- solana/account/README.md
- solana/account/src/tests/sysvar.rs
- solana/account/src/tests/state_traits.rs
- .gitignore
- solana/account/src/lib.rs
- Cargo.toml
- solana/account/src/cow/tests.rs
- solana/account/src/patch.rs
- solana/account/src/tests/mod.rs
- solana/account/src/traits.rs
- solana/account/src/sysvar.rs
- solana/account/src/cow/borrowed.rs
- solana/account/src/state_traits.rs
- solana/account/src/cow/owned.rs
- solana/account/src/tests/account.rs
- solana/account/src/account.rs
Comment on lines
+39
to
+44
| #[derive(PartialEq, Eq, Clone, Default)] | ||
| pub struct AccountSharedData { | ||
| /// Backing storage, borrowed until promotion or direct construction. | ||
| pub(crate) cow: CoWAccount, | ||
| /// Fields changed through the writable APIs. | ||
| pub(crate) dirty: DirtyMarkers, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
Do not make borrowed storage safely cloneable, mutable, and Sync.
A cloned Borrowed variant still targets the same external image but has independent dirty state. Two clean clones can therefore translate the same active image into the same shadow buffer, overwriting edits; DerefMut can also produce aliased &mut AccountCore references. Declaring this representation Sync further permits data races that sequence retries cannot make memory-safe.
Make borrowed clones produce owned snapshots, restrict direct DerefMut, and only implement Sync once the backing-memory synchronization contract guarantees data-race-free access.
As per path instructions, Rust workspace code must prioritize ownership, unsafe usage, concurrency, and API contract correctness.
Also applies to: 81-87, 408-414, 637-639
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@solana/account/src/cow/mod.rs` around lines 39 - 44, Update AccountSharedData
and its CoWAccount implementations so cloning a Borrowed value materializes an
owned snapshot rather than sharing external backing storage with independent
DirtyMarkers. Restrict DerefMut for borrowed representations to prevent aliased
mutable AccountCore access, and remove any Sync implementation or bound until
backing-memory synchronization guarantees data-race-free access.
Source: Path instructions
bmuddha
force-pushed
the
solana-account
branch
4 times, most recently
from
July 21, 2026 19:46
9d337aa to
0905923
Compare
bmuddha
force-pushed
the
solana-account
branch
2 times, most recently
from
July 27, 2026 13:19
28eacdd to
3b5ca59
Compare
bmuddha
force-pushed
the
solana-account
branch
2 times, most recently
from
July 28, 2026 14:25
54be71a to
d1e8c56
Compare
bmuddha
force-pushed
the
solana-account
branch
3 times, most recently
from
July 31, 2026 13:48
f30b82c to
8af975f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Customized the imported
solana-accountbaseline with owned and copy-on-writeaccount representations, borrowed account layout helpers, codec helpers, sysvar
support, and focused tests.
Why
accountsdbneeds an account representation that can borrow from mapped storageand promote to owned storage only when writes require it.
Closes #5.
Impact
Account,AccountSharedData, and a borrowed/owned copy-on-write model.routing decisions above this crate.
Reviewer notes
The borrowed layout is layout-bound and must stay 8-byte aligned. This crate
should remain storage-source agnostic.
Follow-up
transaction-context,program-runtime, andsvmadapt to this representationin the next stack PRs; storage consumers arrive later.