Upgrade to Solana 3.0 and handle V0_0_3 Geyser transactions - #101
Merged
Conversation
- Bump solana-sdk, solana-transaction-status, agave-geyser-plugin-interface and related crates to 3.0.0 - Add ReplicaTransactionInfoV3 shim and serialize_transaction_v3 for VersionedTransaction support in V0_0_3 - Handle V0_0_3 in plerkle's notify_transaction with proper vote/error filtering, transaction selection, and caching - Add roundtrip unit tests for serialize_transaction_v3 - Remove plerkle_snapshot crate (unmaintained, not needed for DAS) - Update Rust toolchain to 1.89.0, Solana CLI to v3.1.14 - Update Solana.Dockerfile to v3.1.14 - Bump plerkle to 3.0.0, plerkle_serialization to 3.0.0 - Fix typo in release notes
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughToolchains bumped (Rust 1.83→1.89, Solana v2.1.11→v3.1.14); workspace drops Changes
Sequence DiagramsequenceDiagram
participant Geyser as Geyser Plugin
participant Handler as notify_transaction Handler
participant Selector as Transaction Selector
participant Serializer as serialize_transaction_v3
participant FlatBuf as FlatBufferBuilder
Geyser->>Handler: ReplicaTransactionInfoVersions::V0_0_3
Handler->>Handler: Check is_vote
alt vote
Handler-->>Geyser: Return Ok()
end
Handler->>Handler: Check transaction_status_meta.status
alt error
Handler-->>Geyser: Return Ok()
end
Handler->>Selector: is_transaction_selected(message.static_account_keys())
alt not selected / no selector
Handler-->>Geyser: Return Ok()
end
Handler->>Serializer: serialize_transaction_v3(ReplicaTransactionInfoV3)
Serializer->>FlatBuf: Build TransactionInfo FlatBuffer (keys, logs, inner/outer instructions, metadata)
FlatBuf-->>Serializer: SerializedData
Serializer-->>Handler: SerializedData
Handler->>Handler: Store (index, SerializedData) and emit metric
Handler-->>Geyser: Return Ok()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Solana.Dockerfile (2)
28-33: 🧹 Nitpick | 🔵 TrivialAdd a container HEALTHCHECK for faster failure detection.
No
HEALTHCHECKis defined; orchestrators can’t detect unhealthy containers proactively.🩺 Example HEALTHCHECK
FROM anzaxyz/agave:$SOLANA_VERSION COPY --from=builder /rust/target/release/libplerkle.so /plugin/plugin.so COPY ./docker . RUN chmod +x ./*.sh +HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \ + CMD test -f /plugin/plugin.so || exit 1 ENTRYPOINT [ "./runs.sh" ] CMD [""]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Solana.Dockerfile` around lines 28 - 33, Add a Docker HEALTHCHECK to the image so orchestrators can detect service failures: modify the Dockerfile near the existing ENTRYPOINT/CMD/./runs.sh sequence to add a HEALTHCHECK instruction that periodically runs a lightweight probe (e.g., calling ./runs.sh --health or curl/pgrep against the process), and include sensible options (interval, timeout, start-period, retries) so failures are detected quickly but transient startup delays are tolerated; ensure the probe returns proper exit codes (0 healthy, non‑zero unhealthy) and update ENTRYPOINT/CMD references (./runs.sh) if needed so the health probe can access the running service.
28-33:⚠️ Potential issue | 🟠 MajorRun the final image as a non-root user.
The runtime stage executes as root, which increases blast radius on container escape or script compromise. Add a dedicated unprivileged user before
ENTRYPOINT.🔒 Proposed hardening change
FROM anzaxyz/agave:$SOLANA_VERSION COPY --from=builder /rust/target/release/libplerkle.so /plugin/plugin.so COPY ./docker . RUN chmod +x ./*.sh +RUN useradd --system --uid 10001 --create-home plerkle \ + && chown -R plerkle:plerkle /plugin /root +USER plerkle ENTRYPOINT [ "./runs.sh" ] CMD [""]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Solana.Dockerfile` around lines 28 - 33, The final Dockerfile stage runs as root; add a dedicated unprivileged user and switch to it before ENTRYPOINT to reduce blast radius—create a user/group (e.g., plerkle), chown the plugin and any scripts (references: COPY ./docker ., COPY --from=builder /rust/target/release/libplerkle.so /plugin/plugin.so, runs.sh) so that the new user can access them, set appropriate executable bits (chmod +x ./*.sh) as root, then add a USER plerkle (or UID/GID) line immediately before ENTRYPOINT to run the container non-root.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 15-16: The cache key in the workflow uses an undefined environment
variable RUST_STABLE; update the cache key (and any occurrences referencing
env.RUST_STABLE) to use the defined variable RUST_VERSION instead so the key
partitions by the actual Rust toolchain; search for and replace env.RUST_STABLE
with env.RUST_VERSION (and confirm SOLANA_VERSION_STABLE usage remains
unchanged) to avoid incorrect cache misses.
In `@plerkle_serialization/src/serializer/serializer_stable.rs`:
- Around line 583-635: make_test_transaction currently constructs a
VersionedMessage::Legacy, so update it to include an additional fixture that
builds a true VersionedMessage::V0: create a MessageV0 (or VersionedMessage::V0)
with account_keys and a separate vector of address_table_lookups that references
at least one AddressLookupTableAccount (use unique Pubkeys for lookup table keys
and entries), ensure the TransactionStatusMeta.loaded_addresses
writable/readonly entries correspond to the lookup table entries (to exercise
the ALT lookup path), and return or expose this V0 tx alongside the existing
legacy tx so tests around make_test_transaction and the assertions in the later
tests (the V0 branch at the 638-714 area) can assert ALT-backed key ordering and
indexing behavior (reference make_test_transaction, VersionedMessage::V0,
AddressLookupTableAccount, and TransactionStatusMeta.loaded_addresses).
- Around line 294-299: The serializer currently hardcodes stack_height: 0 when
constructing CompiledInnerInstruction via CompiledInnerInstruction::create with
CompiledInnerInstructionArgs, which discards the original CPI nesting; update
the construction to preserve the incoming instruction's stack height (use the
existing compiled.compiled_instruction.stack_height or compiled.stack_height
value from the source metadata instead of 0) so the serialized V3 inner
instruction retains the original stack_height.
In `@plerkle/src/geyser_plugin_nft.rs`:
- Around line 723-729: The transaction selection currently passes only
static_account_keys() to transaction_selector.is_transaction_selected, which
drops v0 transactions that rely on address lookup tables; update the call in the
transaction filtering block (where transaction_selector and
is_transaction_selected are used) to pass the full set of account keys for the
message by combining ti.transaction.message.static_account_keys() with the v0
loaded lookup-table addresses (both writable and readonly loaded addresses on
the message / ti.transaction.message.loaded_addresses) so the selector sees
static + loaded writable + loaded readonly accounts; ensure the combined
collection is provided in the same boxed-iterator form the selector expects so
behavior matches the serializer and legacy path.
---
Outside diff comments:
In `@Solana.Dockerfile`:
- Around line 28-33: Add a Docker HEALTHCHECK to the image so orchestrators can
detect service failures: modify the Dockerfile near the existing
ENTRYPOINT/CMD/./runs.sh sequence to add a HEALTHCHECK instruction that
periodically runs a lightweight probe (e.g., calling ./runs.sh --health or
curl/pgrep against the process), and include sensible options (interval,
timeout, start-period, retries) so failures are detected quickly but transient
startup delays are tolerated; ensure the probe returns proper exit codes (0
healthy, non‑zero unhealthy) and update ENTRYPOINT/CMD references (./runs.sh) if
needed so the health probe can access the running service.
- Around line 28-33: The final Dockerfile stage runs as root; add a dedicated
unprivileged user and switch to it before ENTRYPOINT to reduce blast
radius—create a user/group (e.g., plerkle), chown the plugin and any scripts
(references: COPY ./docker ., COPY --from=builder
/rust/target/release/libplerkle.so /plugin/plugin.so, runs.sh) so that the new
user can access them, set appropriate executable bits (chmod +x ./*.sh) as root,
then add a USER plerkle (or UID/GID) line immediately before ENTRYPOINT to run
the container non-root.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e1221fe-1251-45d2-94e5-47814d4699c6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.github/workflows/release.yml.github/workflows/test.ymlCargo.tomlSolana.Dockerfileplerkle/Cargo.tomlplerkle/src/geyser_plugin_nft.rsplerkle_serialization/Cargo.tomlplerkle_serialization/src/deserializer/solana.rsplerkle_serialization/src/serializer/serializer_stable.rsplerkle_serialization/src/solana_geyser_plugin_interface_shims.rsplerkle_snapshot/Cargo.tomlplerkle_snapshot/LICENSE.mdplerkle_snapshot/README.mdplerkle_snapshot/src/append_vec.rsplerkle_snapshot/src/archived.rsplerkle_snapshot/src/bin/solana-snapshot-etl/accounts_selector.rsplerkle_snapshot/src/bin/solana-snapshot-etl/geyser.rsplerkle_snapshot/src/bin/solana-snapshot-etl/main.rsplerkle_snapshot/src/bin/solana-snapshot-etl/mpl_metadata.rsplerkle_snapshot/src/lib.rsplerkle_snapshot/src/parallel.rsplerkle_snapshot/src/solana.rsplerkle_snapshot/src/unpacked.rsrust-toolchain.toml
💤 Files with no reviewable changes (14)
- Cargo.toml
- plerkle_snapshot/LICENSE.md
- plerkle_snapshot/README.md
- plerkle_snapshot/Cargo.toml
- plerkle_snapshot/src/bin/solana-snapshot-etl/accounts_selector.rs
- plerkle_snapshot/src/archived.rs
- plerkle_snapshot/src/bin/solana-snapshot-etl/mpl_metadata.rs
- plerkle_snapshot/src/bin/solana-snapshot-etl/main.rs
- plerkle_snapshot/src/parallel.rs
- plerkle_snapshot/src/solana.rs
- plerkle_snapshot/src/append_vec.rs
- plerkle_snapshot/src/bin/solana-snapshot-etl/geyser.rs
- plerkle_snapshot/src/unpacked.rs
- plerkle_snapshot/src/lib.rs
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
plerkle_serialization/src/serializer/serializer_stable.rs (2)
294-299:⚠️ Potential issue | 🟠 MajorPreserve the incoming
stack_heightin the V3 path.This still hardcodes
0even though the source metadata already carriescompiled_instruction.stack_height(and the new fixture sets it toSome(2)). That drops CPI nesting information and breaks round-trip fidelity.Proposed fix
instructions_fb_vec.push(CompiledInnerInstruction::create( &mut builder, &CompiledInnerInstructionArgs { compiled_instruction: Some(compiled), - stack_height: 0, // Available since Solana 1.15 but unused by DAS consumers + stack_height: compiled_instruction.stack_height.unwrap_or(0), }, ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plerkle_serialization/src/serializer/serializer_stable.rs` around lines 294 - 299, The V3 serialization path is incorrectly hardcoding stack_height to 0 when building CompiledInnerInstructionArgs; instead, preserve the incoming compiled_instruction.stack_height (or its Option) so CPI nesting is retained: locate the call to CompiledInnerInstruction::create that constructs CompiledInnerInstructionArgs in the V3 branch and replace the literal 0 with the source metadata value (e.g., compiled_instruction.stack_height or compiled.stack_height) ensuring types match (Option<u8> vs u8) or map/unwrap appropriately to keep the original value.
724-775: 🧹 Nitpick | 🔵 TrivialUse a real ALT-backed V0 fixture in this test.
This constructs
VersionedMessage::V0, butaddress_table_lookupsis empty and the instruction only references static account indexes. It never exercises the lookup-table expansion/indexing this PR is meant to support, so an ALT ordering bug would still pass here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plerkle_serialization/src/serializer/serializer_stable.rs` around lines 724 - 775, The test builds a VersionedMessage::V0 (v0_message) but leaves address_table_lookups empty and only uses static account indices, so it never exercises ALT expansion/indexing; update the fixture to include a real AddressTableLookup entry on v0_message.address_table_lookups that contains atl_writable and atl_readonly, then change the SolanaCompiledInstruction.accounts to reference the lookup-table indices (so some account indices point into the lookup entries rather than only the static account_keys) so serialize_transaction_v3 actually expands lookup addresses; finally update the expected account_keys/assertions (keys.len() and positions for atl_writable/atl_readonly) to reflect the ALT-expanded ordering produced by serialize_transaction_v3 when handling ReplicaTransactionInfoV3 with lookup entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Line 13: The workflow currently uses runs-on: ubuntu-latest while the apt
provisioning still targets Bionic (older LLVM APT repo and libssl1.1) and uses
--allow-unauthenticated; update the release job to either pin the runner to a
known compatible image (e.g., ubuntu-20.04) or modernize the apt steps for
Ubuntu 24.04 by removing the bionic LLVM repo, switching to the current LLVM APT
repository for the runner release, replacing libssl1.1 with the distro-native
libssl (e.g., libssl3 or openssl packages appropriate for 24.04), and remove any
--allow-unauthenticated flags; locate the runs-on: ubuntu-latest declaration in
.github/workflows/release.yml and update the apt-add-repository / apt-get
install lines and flags in the same job (the release workflow job) accordingly.
In @.github/workflows/test.yml:
- Line 20: The workflow's runner setting uses the runs-on key value
"ubuntu-latest" which currently maps to an OS without libssl1.1; change the
runner to a pinned, compatible image (set runs-on to "ubuntu-20.04") OR update
the build steps that reference libssl1.1 to use modern OpenSSL 3.x packages and
corresponding package names (remove/replace any --allow-unauthenticated
workarounds) so the job (the workflow using the runs-on key) runs on a
compatible distribution or uses updated crypto packages.
In `@plerkle_serialization/src/serializer/serializer_stable.rs`:
- Around line 399-400: Update the doc comment that currently says "The
Transaction must be base54 encoded." to the correct encoding name "base58" in
the serializer_stable.rs documentation for the function handling
[`EncodedConfirmedTransactionWithStatusMeta`]; locate the doc block above the
serializer function that serializes EncodedConfirmedTransactionWithStatusMeta
into a FlatBuffer and replace "base54" with "base58" so the API docs are
accurate.
---
Duplicate comments:
In `@plerkle_serialization/src/serializer/serializer_stable.rs`:
- Around line 294-299: The V3 serialization path is incorrectly hardcoding
stack_height to 0 when building CompiledInnerInstructionArgs; instead, preserve
the incoming compiled_instruction.stack_height (or its Option) so CPI nesting is
retained: locate the call to CompiledInnerInstruction::create that constructs
CompiledInnerInstructionArgs in the V3 branch and replace the literal 0 with the
source metadata value (e.g., compiled_instruction.stack_height or
compiled.stack_height) ensuring types match (Option<u8> vs u8) or map/unwrap
appropriately to keep the original value.
- Around line 724-775: The test builds a VersionedMessage::V0 (v0_message) but
leaves address_table_lookups empty and only uses static account indices, so it
never exercises ALT expansion/indexing; update the fixture to include a real
AddressTableLookup entry on v0_message.address_table_lookups that contains
atl_writable and atl_readonly, then change the
SolanaCompiledInstruction.accounts to reference the lookup-table indices (so
some account indices point into the lookup entries rather than only the static
account_keys) so serialize_transaction_v3 actually expands lookup addresses;
finally update the expected account_keys/assertions (keys.len() and positions
for atl_writable/atl_readonly) to reflect the ALT-expanded ordering produced by
serialize_transaction_v3 when handling ReplicaTransactionInfoV3 with lookup
entries.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 793bf804-d82d-4c72-85b9-e92aaa8d4c95
📒 Files selected for processing (3)
.github/workflows/release.yml.github/workflows/test.ymlplerkle_serialization/src/serializer/serializer_stable.rs
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
blockiosaurus
approved these changes
Apr 22, 2026
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
2 participants
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.
Notes