feat: forked transaction-view for increased limits - #33
Conversation
|
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. |
de564a1 to
acf36a7
Compare
e7813be to
ef28038
Compare
ef28038 to
5136be2
Compare
67bda80 to
7ef5704
Compare
7ef5704 to
4d84bd9
Compare
54a8790 to
9aecd4e
Compare
9aecd4e to
38631fc
Compare
38631fc to
89c5db4
Compare
89c5db4 to
5daf3d5
Compare
5daf3d5 to
cb6d033
Compare
cb6d033 to
86c2748
Compare
|
@CodeRabbit review |
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning
|
| Layer / File(s) | Summary |
|---|---|
Workspace packaging and documentation Cargo.toml, solana/transaction-view/Cargo.toml, solana/transaction-view/README.md, solana/transaction-view/benches/* |
The workspace uses the local fork, package metadata declares version 4.1.1, benchmarks retain their configuration, and the README documents supported formats and validation rules. |
Checked compact-length parsing solana/transaction-view/src/bytes.rs, solana/transaction-view/src/instructions_frame.rs |
Byte readers return ParseError when offset arithmetic overflows. Instruction-frame construction and tests are reformatted, with the V1 test using Message::serialize() and offset 42. |
Bounded zero-copy accessors and validation solana/transaction-view/src/transaction_frame.rs, solana/transaction-view/src/transaction_view.rs, solana/transaction-view/src/sanitize.rs, solana/transaction-view/src/resolved_transaction_view.rs |
Pubkey and Hash references are created from bounded slices before casting. Related validation, view, resolved-view, and test expressions are reformatted without behavior changes. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Sequence Diagram(s)
sequenceDiagram
participant TransactionBytes
participant ByteReader
participant InstructionFrame
participant Sanitizer
TransactionBytes->>ByteReader: Read bytes and compact-u16 lengths
ByteReader-->>InstructionFrame: Return values or ParseError
InstructionFrame->>Sanitizer: Provide parsed frame metadata
Sanitizer-->>InstructionFrame: Validate transaction limits
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the fork and its purpose of supporting increased transaction limits. |
| Description check | ✅ Passed | The description directly explains the fork, parser changes, validation safeguards, limits, and linked issue. |
| Linked Issues check | ✅ Passed | The changes address the coding objectives in [#34], including dependency patching, canonical lengths, checked offsets, limits, and validation boundaries. |
| Out of Scope Changes check | ✅ Passed | The changes remain within scope; documentation, metadata, formatting, and benchmark edits support the transaction-view fork. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
transaction-view
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 @coderabbitai help to get the list of available commands.
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Cargo.toml`:
- Around line 50-55: Align the version requirements for agave-transaction-view
in the root dependency configuration and solana/transaction-view/Cargo.toml so
the local [patch.crates-io] entry applies. Preserve the intended local fork
resolution by making the declared dependency version compatible with the patched
package version.
In `@solana/transaction-view/README.md`:
- Around line 27-30: Update the transaction size and signature-limit
documentation to distinguish parser-enforced boundaries from
sanitization-enforced limits. In the README section describing Legacy, v0, V1,
and Magicblock transactions, state that Legacy parsing rejects the
over-signature case through TransactionView::try_new_unsanitized with
ParseError, while V1 signature validation occurs in sanitize_signatures; keep
other structural sanitization limits described separately.
🪄 Autofix
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 Plus
Run ID: b02b19fe-fd5a-4abe-bbf8-733802fe596c
📒 Files selected for processing (11)
Cargo.tomlsolana/transaction-view/Cargo.tomlsolana/transaction-view/README.mdsolana/transaction-view/benches/bytes.rssolana/transaction-view/benches/transaction_view.rssolana/transaction-view/src/bytes.rssolana/transaction-view/src/instructions_frame.rssolana/transaction-view/src/resolved_transaction_view.rssolana/transaction-view/src/sanitize.rssolana/transaction-view/src/transaction_frame.rssolana/transaction-view/src/transaction_view.rs
86c2748 to
e09f437
Compare
e09f437 to
d2510cc
Compare
d2510cc to
998ebbe
Compare
998ebbe to
50b9a07
Compare

What changed
Forked
agave-transaction-viewinto the workspace and adapted its framing parser for engine transaction sizes.Why
Engine-private transactions can exceed packet-sized assumptions, so their zero-copy view must parse complete canonical lengths and validate larger offsets before unchecked access.
Closes #34.
Impact
Reviewer notes
Unchecked iterators and typed views rely on the initial frame validation. Review the offset and length checks together with the standard-versus-Magicblock limit boundary.
Follow-up
The transaction-context and runtime forks consume this parser upstack.