feat(sealevel): migrate to Solana SDK v3.x and Agave CLI v3.0.14#7999
feat(sealevel): migrate to Solana SDK v3.x and Agave CLI v3.0.14#7999
Conversation
This is a major migration from the forked Solana v1.14.13 dependencies to
official Solana SDK v3.x releases. The migration enables building programs
with modern tooling and removes the need for forked Solana dependencies.
## Workspace Configuration
- Added `resolver = "2"` to rust/sealevel/Cargo.toml
- Critical fix for proper Cargo feature unification
- Prevents getrandom 0.3.x from being pulled in incorrectly
- Ensures dev-dependency features don't leak into SBF builds
- Updated dependency versions:
- solana-program: 1.14.13 → 3.0.0
- solana-sdk: 1.14.13 → 3.0.0
- solana-client: 1.14.13 → 3.0.7
- solana-program-test: 1.14.13 → 3.0.7
- spl-token: 3.x → 9.0
- spl-token-2022: 0.x → 10.0
- spl-associated-token-account: 1.x → 8.0
- prometheus: 0.13 → 0.14 (required for protobuf 3 compat)
- protobuf: "*" → 3 (pinned for stability)
- Replaced forked solana-* dependencies with official crates
- Added new interface crates (SDK v3 module split):
- solana-system-interface
- solana-compute-budget-interface
- solana-loader-v3-interface
- solana-commitment-config
- solana-stake-interface
## API Migration
### BorshIoError Signature Change
Changed from `BorshIoError(String)` to unit variant `BorshIoError`.
Affected ~40 locations across:
- programs/mailbox/src/instruction.rs
- programs/validator-announce/src/instruction.rs
- programs/hyperlane-sealevel-igp/src/instruction.rs
- libraries/account-utils/src/*.rs
- libraries/message-recipient-interface/src/lib.rs
- libraries/interchain-security-module-interface/src/lib.rs
- libraries/hyperlane-sealevel-connection-client/src/lib.rs
### Keccak API Changes
The keccak module API changed from streaming hasher to direct functions:
- `keccak::Hasher::default() + hash() + result()` → `keccak::hash()`
- For multiple inputs: → `keccak::hashv(&[...])`
Files affected:
- libraries/ecdsa-signature/src/lib.rs
- programs/validator-announce/src/instruction.rs
### AccountInfo::new() Parameter Removal
Removed the `rent_epoch` (Epoch) parameter from AccountInfo::new() calls.
Previously 8 params, now 7 params. Affected test code in:
- libraries/access-control/src/lib.rs (6 occurrences)
### System Program Import
Changed from `solana_program::system_program` to `solana_system_interface::program`.
Affected all programs and libraries that interact with system program.
### Instruction Imports
Changed `solana_program::loader_v3_instruction` to
`solana_loader_v3_interface::instruction` for BPF loader interactions.
### Clock/Sysvar Changes
Updated sysvar imports from `solana_program::sysvar` to use the new
module organization where applicable.
## Program Build Configuration
- Updated build-programs.sh: SOLANA_CLI_VERSION 2.2.1 → 3.0.14
- Added `#![allow(unexpected_cfgs)]` to all program lib.rs files
- Required for Solana's custom cfg values: target_os="solana",
feature="custom-heap", feature="custom-panic"
- Added `getrandom = { workspace = true, features = ["custom"] }` to
all program Cargo.toml files
- Required for SBF determinism (programs must use custom RNG)
- Updated rust-toolchain to nightly-2025-01-15 (required for SDK v3)
## Programs Successfully Building
All 7 production programs now build with `cargo build-sbf`:
- hyperlane_sealevel_mailbox.so (176KB)
- hyperlane_sealevel_igp.so (235KB)
- hyperlane_sealevel_multisig_ism_message_id.so (161KB)
- hyperlane_sealevel_token.so (319KB)
- hyperlane_sealevel_token_collateral.so (324KB)
- hyperlane_sealevel_token_native.so (298KB)
- hyperlane_sealevel_validator_announce.so (129KB)
## rust/main Compatibility Updates
Updated rust/main/Cargo.toml for shared dependency compatibility:
- prometheus: 0.13 → 0.14
- protobuf: "*" → 3
Note: Full rust/main migration (Phase 3) is pending. The changes here
maintain compatibility between both workspaces.
## Next Steps
- Phase 3: Complete rust/main migration (ed25519-dalek API changes)
- Phase 4: Run E2E tests to validate program functionality
- Phase 5: Evaluate non-Solana patches in rust/main
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updates rust/main crates to work with Solana SDK v3.x: - Add new Solana interface crate dependencies: - solana-commitment-config - solana-compute-budget-interface - solana-rpc-client - solana-sdk-ids - solana-system-interface - Update API calls for v3.x: - Pubkey::new() -> Pubkey::try_from() - Signature::new() -> Signature::try_from() - Keypair::from_bytes() -> Keypair::try_from() - Memcmp struct init -> Memcmp::new() - borsh try_to_vec() -> borsh::to_vec() - Add missing struct fields for v3.x: - RpcProgramAccountsConfig.sort_results - UiConfirmedBlock.num_reward_partitions - UiTransactionStatusMeta.cost_units - UiPartiallyDecodedInstruction.stack_height - RpcSimulateTransactionResult new optional fields - Fix protobuf v2/v3 compatibility for hyperlane-cosmos (injective-protobuf requires v2) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
tkporter
force-pushed
the
trevor/solana-update-deps
branch
from
01718de to
a116105
Compare
SPL Token 2022 v10.0 and SPL ATA v8.0 use solana_cpi::invoke which does not support syscall stubs for offchain testing. When using processor!() to add SPL programs, invoke calls return Ok(()) without executing. This caused ATA's GetAccountDataSize call to Token 2022 to silently fail, resulting in get_return_data() returning None and InvalidInstructionData. Fix: Remove processor!()-based SPL programs and use ProgramTest's bundled BPF programs (spl_token_2022-8.0.0.so, spl_associated_token_account-1.1.1.so) which use proper syscalls that ProgramTest can intercept. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The e2e test runner was downloading solana-labs/solana v1.14.20 which doesn't support Rust 2024 edition. Update to use Agave v3.0.14 which is compatible with the SDK v3.x migration. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…na v3.x Solana v3.x changed UpgradeableLoaderState serialization from Borsh to bincode. - Use solana_loader_v3_interface::state::UpgradeableLoaderState with bincode::deserialize - Handle Pubkey type conversion from solana_pubkey to solana_sdk - Define BPF_LOADER_UPGRADEABLE_ID constant directly using pubkey! macro Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Return Box<Outbox> instead of Outbox by value in verify_account_and_fetch_inner (Outbox is 1118 bytes, exceeding Solana's 4KB stack limit during CPI) - Add #[inline(never)] to AccountData::deserialize_boxed to isolate stack frames - Add custom BorshDeserialize for IncrementalMerkle to avoid 1024-byte branch array stack allocation (deserialize one H256 at a time) These changes fix "Access violation in stack frame" errors that occurred when programs called the mailbox via CPI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Solana SDK v3.x brings in solana-remote-wallet which depends on hidapi, requiring libudev-dev on Linux for USB hardware wallet support. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused import borsh::BorshSerialize - Fix doc comment list formatting (use - prefix) - Allow dead_code on UpgradeableLoaderInstruction enum - Allow needless_lifetimes on TxnBuilder impl - Use is_some_and instead of map_or Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@codex review |
|
@claude review |
|
Claude finished @paulbalaji's task in 1m 51s —— View job Reviewing PR: Solana SDK v3.x Migration
|
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
paulbalaji
left a comment
paulbalaji
left a comment
There was a problem hiding this comment.
Review (Please disregard previous comments)
Good migration work overall. Issues to address:
HIGH: Use SDK instruction types instead of local definitions
File: rust/sealevel/client/src/router.rs:17-56
solana_loader_v3_interface::instruction already exports:
UpgradeableLoaderInstructionenumset_upgrade_authority()function
The local definitions duplicate these and risk encoding drift. Replace with:
use solana_loader_v3_interface::instruction::set_upgrade_authority;Then delete lines 13-56 (local BPF_LOADER_UPGRADEABLE_ID, enum, and function).
HIGH: Avoid stack copy with Box<Outbox>
File: rust/sealevel/programs/mailbox/src/processor.rs (lines 721, 862, 925)
// Current: dereferences Box, copies 1.1KB onto stack, then re-boxes
OutboxAccount::from(*outbox).store(outbox_info, true)?;
// Fix: use From<Box<T>> impl directly
OutboxAccount::from(outbox).store(outbox_info, true)?;MEDIUM: Missing roundtrip test for custom BorshDeserialize
File: rust/main/hyperlane-core/src/accumulator/incremental.rs:22
The custom IncrementalMerkle::BorshDeserialize lacks an explicit roundtrip test:
#[test]
fn borsh_roundtrip() {
let mut tree = IncrementalMerkle::default();
tree.ingest(H256::from([1u8; 32]));
let serialized = borsh::to_vec(&tree).unwrap();
let deserialized: IncrementalMerkle = borsh::from_slice(&serialized).unwrap();
assert_eq!(tree, deserialized);
}MINOR: Extract spl_noop Pubkey helper
10 instances of Pubkey::new_from_array(spl_noop::id().to_bytes()). Consider a shared constant.
LGTM once HIGH issues addressed.
Resolved conflicts: - rust/main/Cargo.toml: Keep v3.x Solana deps, drop old forked patches, add solana-address-lookup-table-interface - rust/sealevel/Cargo.toml: Same as above - Source files: Use v3.x import paths, incorporate new ALT/versioned transaction types from main - Additional fixes: Update ALT imports to interface crate, fix system_program path, add missing RpcSimulateTransactionResult fields Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ip test - Changed `OutboxAccount::from(*outbox)` to `OutboxAccount::from(outbox)` at 3 call sites. The deref copied 1.1KB Outbox onto the stack before re-boxing; `From<Box<T>>` impl already exists and avoids the copy. - Added Borsh serialize/deserialize roundtrip tests for IncrementalMerkle's custom BorshDeserialize impl (empty and non-empty trees). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…finition Bumped solana-loader-v3-interface to 6.1.0 (compatible solana-pubkey version) and replaced the local UpgradeableLoaderInstruction enum and set_upgrade_authority_instruction function with the SDK's set_upgrade_authority(). Removes ~40 lines of duplicated logic that risked encoding drift. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaced 18 inline `Pubkey::new_from_array(spl_noop::id().to_bytes())` conversions with a shared `account_utils::SPL_NOOP_PROGRAM_ID` constant. The conversion is needed because spl-noop 1.0 depends on solana-program ^2, yielding a different Pubkey type than solana-sdk 3.0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Validated gist findings against current head. Overall migration looks solid and prior blocking feedback is resolved.
Confirmed issues/suggestions:
- Low: client keypair loading now reconstructs from first 32 bytes only; this drops full keypair consistency validation that existed before.
- Low/non-blocking: custom IncrementalMerkle deserialize tests exist, but coverage can be strengthened with a deeper-fill roundtrip case.
Non-inlineable follow-up finding:
rust/main/utils/run-locally/src/sealevel/mod.rs:234has a stale comment: it says "use the agave 2.x validator version" while this PR setsSOLANA_NETWORK_CLI_VERSIONto3.0.14inrust/main/utils/run-locally/src/sealevel/solana.rs. Please update the comment to match runtime behavior and avoid operator confusion.
Everything else I checked from the gist (SPL noop constant correctness, stack-overflow mitigations, bincode loader state migration, CI libudev updates, and Solana v3 API migrations) is consistent with the code in this PR.
Review gist: https://gist.github.com/paulbalaji/b670184e4bc81957dfc6bb9b1d3771a6
|
confirmed backward compatible with these msgs delivered by the rc relayer: |
Use Keypair::try_from on full 64-byte slice instead of extracting only the 32-byte secret key. This preserves the secret/public key consistency check and detects malformed or tampered keypair files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tkporter
force-pushed
the
trevor/solana-update-deps
branch
from
1709a3d to
71a7107
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
paulbalaji
left a comment
paulbalaji
left a comment
There was a problem hiding this comment.
Inline note on CLI rollback behavior.
paulbalaji
left a comment
paulbalaji
left a comment
There was a problem hiding this comment.
LGTM overall. Approving; left one non-blocking inline note on CLI rollback behavior.
release.anza.xyz only serves Agave (v2+) binaries, so rollback to a 1.x version would fail. Added major-version check to route v1.x installs through release.solana.com. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR upgrades Solana-related crates to 3.0+ and other dependencies (borsh, ed25519-dalek, prometheus, getrandom), updates many Cargo manifests, migrates import paths to new interface crates, replaces Borsh try_to_vec()/deserialize usages with borsh::to_vec()/reader-based deserializers, tightens pubkey/signature parsing, and adds a CI step installing libudev-dev. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![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 (4)
rust/sealevel/libraries/test-utils/src/lib.rs (1)
79-83:⚠️ Potential issue | 🟡 Minor
AccountMeta::new(writable) for system program ininitialize_mailboxshould benew_readonly.Line 405 in the same file correctly uses
new_readonlyfor the system program. This inconsistency mirrors the pattern flagged infunctional.rsandinstruction.rs.🔧 Proposed fix
- AccountMeta::new(system_program::ID, false), + AccountMeta::new_readonly(system_program::ID, false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/libraries/test-utils/src/lib.rs` around lines 79 - 83, The system program AccountMeta in the initialize_mailbox invocation is incorrectly created as writable using AccountMeta::new(system_program::ID, false); update the call to use AccountMeta::new_readonly(system_program::ID, false) so the system program is treated as read-only (match the usage in line 405 and the patterns in functional.rs/instruction.rs); search for initialize_mailbox and replace the AccountMeta::new(...) for system_program::ID with AccountMeta::new_readonly(...) to fix the inconsistency.rust/sealevel/programs/mailbox/src/instruction.rs (1)
123-127:⚠️ Potential issue | 🟡 Minor
AccountMeta::new(writable) for system program ininit_instructionshould benew_readonly.The comment above this block says
0. [executable] The system program.— no writable flag. TheRpcProgramAccountsConfigdocs confirm that executable accounts do not need to be marked writable; in Solana, marking an executable account writable in account metas can trigger write-privilege escalation errors on stricter runtime configurations.🔧 Proposed fix
- AccountMeta::new(system_program::ID, false), + AccountMeta::new_readonly(system_program::ID, false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/mailbox/src/instruction.rs` around lines 123 - 127, The system program account in the init_instruction account metas is incorrectly created as writable; change the AccountMeta entry for system_program::ID from AccountMeta::new(system_program::ID, false) to a readonly meta using AccountMeta::new_readonly(system_program::ID, false) in the init_instruction (and any similar builders) so the system program is not marked writable and avoids write-privilege escalation errors.rust/sealevel/programs/mailbox/src/processor.rs (1)
778-790:⚠️ Potential issue | 🔴 CriticalOff-by-one panic:
ret_buf[0..31]expects 31 bytes, butH256is 32 bytes.The buffer setup is layered correctly—it's got room for 32 bytes of root plus 4 bytes of count, making 36 total. But the slice
ret_buf[0..31]only takes 31 bytes whileoutbox.tree.root()returns anH256, which is defined as a 32-byte fixed hash (seeH256(32)in primitive-types.rs). Whencopy_from_sliceruns, the source (32 bytes from root) and destination (31 bytes from the slice) won't match, and it'll panic. Validators callingoutbox_get_latest_checkpointwill hit this every time.This isn't new code—it's pre-existing—but it's a critical one. The fix is straightforward:
Fix
- ret_buf[0..31].copy_from_slice(root.as_ref()); + ret_buf[0..32].copy_from_slice(root.as_ref());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/mailbox/src/processor.rs` around lines 778 - 790, The code copies a 32-byte H256 root into a 36-byte ret_buf using ret_buf[0..31], which is an off-by-one destination slice and will panic; change the slice to ret_buf[0..32].copy_from_slice(root.as_ref()) so the full 32-byte root is copied, leaving ret_buf[32..36].copy_from_slice(&count.to_le_bytes()) (or ret_buf[32..] as used) to store the 4-byte count before serializing via SimulationReturnData::new and calling set_return_data in outbox_get_latest_checkpoint.rust/sealevel/programs/hyperlane-sealevel-token-collateral/tests/functional.rs (1)
788-798:⚠️ Potential issue | 🟡 MinorBoth
transfer_remotetests are usingspl_token_2022::id()— the standard SPL Token path isn't being tested.
test_transfer_remote_spl_tokenandtest_transfer_remote_spl_token_2022both calltest_transfer_remote(spl_token_2022::id()). The first one should be passingspl_token::id()to exercise the non-2022 code path, which is how thetransfer_from_remotetests handle it correctly (lines 935 and 975).Proposed fix
// Test transfer_remote with spl_token #[tokio::test] async fn test_transfer_remote_spl_token() { - test_transfer_remote(spl_token_2022::id()).await; + test_transfer_remote(spl_token::id()).await; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token-collateral/tests/functional.rs` around lines 788 - 798, Two tests call test_transfer_remote with the same token id; change test_transfer_remote_spl_token to pass spl_token::id() instead of spl_token_2022::id() so the non-2022 code path is exercised. Edit the test function named test_transfer_remote_spl_token to call test_transfer_remote(spl_token::id()). Leave test_transfer_remote_spl_token_2022 as test_transfer_remote(spl_token_2022::id()).
🧹 Nitpick comments (26)
rust/sealevel/libraries/ecdsa-signature/Cargo.toml (1)
14-14: Consider linking to official docs instead of a news article.The new URL points to
solana.com/news/rust-to-solana, a marketing/blog article, rather than the official developer docs. News articles tend to move or go stale faster, and the page itself shows the v1.xfeatures = ["dummy"]snippet while the code correctly uses the v2.x"custom"feature — a minor conceptual mismatch for future readers.https://solana.com/news/rust-to-solanadoes cover the determinism constraint and whyrand/getrandomneed special handling in Solana programs, so it's not wrong, just not the most durable reference.A more canonical target would be something under
https://solana.com/docsorhttps://docs.solanalabs.com, which are kept in sync with SDK changes.💡 Suggested comment tweak
-# https://solana.com/news/rust-to-solana +# https://solana.com/docs/programs/rust🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/libraries/ecdsa-signature/Cargo.toml` at line 14, The Cargo.toml README link references a marketing/news URL (https://solana.com/news/rust-to-solana) which can go stale and shows a v1.x `features = ["dummy"]` snippet that mismatches the repo's v2.x `"custom"` usage; replace that URL with the canonical developer docs (e.g., https://docs.solana.com or https://docs.solanalabs.com) and update the accompanying comment so it references the official docs and the correct v2 guidance about determinism and `rand`/`getrandom` handling to keep the reference durable and conceptually consistent with the `Cargo.toml` usage.rust/sealevel/programs/ism/test-ism/src/lib.rs (1)
4-5: Looks good — the fix is right, and there's a slightly tidier path if you ever want it.The
#![allow(unexpected_cfgs)]correctly prevents#![deny(warnings)]from turning thesolana_program::entrypoint!macro's internalcfg(target_os = "solana")into a hard compile error. Specific lint attributes always override lint-group attributes in Rust, so the ordering relative to#![deny(warnings)]on line 6 is fine.The only thing worth knowing: this silences all unknown cfg keys in the crate, not just the Solana-specific one. If you'd prefer a more surgical approach,
Cargo.tomllets you declare the expected value:[lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(target_os, values("solana"))'] }That way the compiler still yells if some other unexpected cfg sneaks in. For a test-only ISM crate this is pretty low stakes either way — just something to keep in your back pocket.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/ism/test-ism/src/lib.rs` around lines 4 - 5, The crate currently silences all unknown cfg keys via the crate attribute #![allow(unexpected_cfgs)] in lib.rs to avoid the solana_program::entrypoint! macro triggering errors (and #![deny(warnings)] remains okay); to be more surgical, remove that crate-level attribute and instead add a Cargo.toml lints.rust entry that sets unexpected_cfgs to a non-fatal level and constrains check-cfg to only expect cfg(target_os = "solana") (so the compiler still warns on other unknown cfgs) — if you prefer to keep the current approach, no change is needed since the attribute correctly overrides the deny(warnings) lint.rust/sealevel/libraries/hyperlane-sealevel-connection-client/src/lib.rs (1)
22-24:map_errbeforeunwrap()is dead-weight — the mappedProgramErroris never surfaced.
ProgramError::BorshIoErroris correctly used as a unit variant for SDK v3.x (the(String)payload was dropped). That part's solid.The wrinkle is structural:
.map_err(|_| ProgramError::BorshIoError)converts the error into aProgramError, but the very next.unwrap()throws it straight into a panic anyway. The mapped value is constructed and immediately discarded — it never propagates anywhere because the method returns(). In a Solana BPF program, a panic kills the transaction without a clean error code, same as before.Since
borsh::to_veconOption<Pubkey>is effectively infallible in practice (fixed-size type, no I/O), themap_errnoise can just be cut:🧹 Suggested simplification
set_return_data( - &borsh::to_vec(&ism) - .map_err(|_| ProgramError::BorshIoError) - .unwrap()[..], + &borsh::to_vec(&ism) + .expect("borsh::to_vec of Option<Pubkey> is infallible")[..], );Alternatively, if you'd rather propagate errors cleanly, change the trait method to return
Result<(), ProgramError>and use?— though that's a bigger, breaking change to the trait surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/libraries/hyperlane-sealevel-connection-client/src/lib.rs` around lines 22 - 24, The map_err call before unwrap on borsh::to_vec(&ism) is dead-weight because the mapped ProgramError is never propagated; remove the `.map_err(|_| ProgramError::BorshIoError)` invocation so you either call borsh::to_vec(&ism) directly (keeping the existing unwrap/expect semantics) or, if you want proper error propagation, change the enclosing trait/method to return Result<(), ProgramError> and replace the unwrap with `?` so ProgramError::BorshIoError can actually be returned (refer to borsh::to_vec(&ism) and ProgramError::BorshIoError in lib.rs).rust/sealevel/rust-toolchain (1)
1-3: Optional: rename torust-toolchain.tomlfor modern convention.The legacy
rust-toolchainfilename (no.tomlsuffix) with a TOML body is still supported, but the officially recommended name isrust-toolchain.toml. A rename keeps things consistent with both rustup's guidance and the Agave repo itself.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/rust-toolchain` around lines 1 - 3, Rename the file currently named "rust-toolchain" to "rust-toolchain.toml" and keep its TOML content ([toolchain] channel = "1.85.0" profile = "default") unchanged; this aligns with rustup's recommended modern convention (use the .toml suffix) and preserves the toolchain configuration used by functions/processes that read rust-toolchain.toml.rust/main/chains/hyperlane-sealevel/src/mailbox_indexer.rs (1)
234-235: Optional: bringdelivered_message_accountin line with the newly hardeneddispatched_message_account.
H256::from_slicepanics if the slice isn't exactly 32 bytes, while the sibling method now uses a fallibletry_from. Both methods are safe in practice becausesearch_accounts_by_discriminatorfilters to exactly 32 bytes upstream, but they're now asymmetric in defensive posture.♻️ Optional consistency fix
fn delivered_message_account(&self, account: &Account) -> ChainResult<Pubkey> { - let message_id = H256::from_slice(&account.data); + let message_id: H256 = account + .data + .as_slice() + .try_into() + .map(H256::from) + .map_err(|e| ChainCommunicationError::from_other_str(&format!("Invalid message id: {e}")))?;Or more simply, since
H256::try_from/TryFrom<&[u8]>is available viaprimitive-types:- let message_id = H256::from_slice(&account.data); + let message_id = H256::try_from(account.data.as_slice()) + .map_err(|e| ChainCommunicationError::from_other_str(&format!("Invalid message id: {e}")))?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/main/chains/hyperlane-sealevel/src/mailbox_indexer.rs` around lines 234 - 235, The delivered_message_account function currently uses H256::from_slice(&account.data) which can panic on non-32-byte slices; make it defensive like dispatched_message_account by using H256::try_from (or TryFrom<&[u8]>) to convert account.data into an H256 and propagate a ChainResult error on failure, keeping the same return type and error handling style as dispatched_message_account and preserving the rest of delivered_message_account logic.rust/sealevel/programs/hyperlane-sealevel-token-collateral/src/lib.rs (1)
3-3: Consider scopingunexpected_cfgsviaCargo.tomlinstead of a blanket crate-wideallow.
#![allow(unexpected_cfgs)]is the correct workaround to keep#![deny(warnings)]from failing the build due to Solana SBF custom cfg flags — but it's a blunt instrument. A cfg typo anywhere in this file would now be silently swallowed.Cargo provides a
[lints]table to statically declare expected cfgs, allowing you to keep the lint active while only whitelisting what's actually expected:# In Cargo.toml [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(target_os, values("solana"))', 'cfg(solana_program_runtime)'] }This keeps the lint's typo-detection value intact rather than silencing it entirely. If the triggering cfgs come purely from transitive SPL/SDK dependencies (not from this crate's own
#[cfg(...)]usage), then the blanketallowis the pragmatic choice and is fine to keep as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token-collateral/src/lib.rs` at line 3, The crate-wide attribute `#![allow(unexpected_cfgs)]` at the top of lib.rs is too broad; instead, remove it and declare the expected cfgs in Cargo.toml's lints table so the unexpected_cfgs lint stays enabled but only whitelists the Solana-specific cfgs (e.g., add a [lints.rust] entry that sets unexpected_cfgs level and supplies check-cfg with the Solana cfg expressions like cfg(target_os, values("solana")) and cfg(solana_program_runtime)); if you confirm the warning originates only from transitive dependencies rather than this crate's own #[cfg(...)] uses, you may keep the crate-level allow but prefer the Cargo.toml-scoped approach for better typo detection.rust/sealevel/programs/helloworld/src/processor.rs (1)
462-462: Serialization migration andBorshIoErrorunit-variant usage are correct.
borsh::to_vecis the proper borsh 1.x API replacing the oldtry_to_vec()method, and theBorshIoErrorunit-variant (noStringfield) is the confirmed SDK v3 shape. "Changed BorshIoError(String) -> BorshIoError, so no more string parameter exists in either InstructionError or ProgramError."The
map_err(|_| ProgramError::BorshIoError)silently swallows the underlyingio::Errordetail. In an SBF context this is generally acceptable — just worth a quick call-out so future debuggers know the error has been intentionally elided.Also applies to: 478-479
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/helloworld/src/processor.rs` at line 462, The current call set_return_data(&borsh::to_vec(&storage.ism).map_err(|_| ProgramError::BorshIoError)?[..]); intentionally drops the underlying io::Error when mapping into ProgramError::BorshIoError; add a short inline comment next to this expression (and the similar occurrence around lines handling serialization at the other site you noted) stating that the io::Error detail is intentionally elided for SBF/runtime constraints so future readers know this is deliberate, and ensure the same comment is added where borsh::to_vec is mapped to ProgramError::BorshIoError in the other block (the serialization near the referenced 478-479).rust/main/utils/run-locally/src/sealevel/solana.rs (1)
20-26: Version alignment looks good, but the comment on line 24 might need a wee update.Both CLI versions are now unified at
3.0.14pointing toanza-xyz/agave, which is right proper for this migration. However, line 24's comment says "Solana version used by mainnet validators" — worth confirming that's still the intent here, or if it should just say something like "Agave CLI version for running the local test validator," since this is e2e infrastructure and mainnet validators may run a different version.🧅 Optional comment tweak
-/// Solana version used by mainnet validators +/// Agave CLI version for running the local test validator🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/main/utils/run-locally/src/sealevel/solana.rs` around lines 20 - 26, The comment above SOLANA_NETWORK_CLI_VERSION is misleadingly describing "Solana version used by mainnet validators"; update the comment to reflect the actual intent (e.g., that this constant represents the Agave CLI version used for running the local test validator/E2E infra) or restore it to reference Agave if you intend it for validators—change the comment on the line near SOLANA_NETWORK_CLI_VERSION to a concise, accurate description clarifying it's the Agave CLI/runtime used locally rather than implying mainnet validator versions; confirm consistency with SOLANA_CONTRACTS_CLI_VERSION and SOLANA_NETWORK_CLI_RELEASE_URL when editing.rust/sealevel/client/src/squads.rs (1)
328-336:bincodeis properly declared; thePubkeyconversion is safe but potentially unnecessary.
bincodedependency: Confirmed—it's in the client crate'sCargo.tomlasbincode.workspace = true(line 11), so callingbincode::deserializewithout a use import is perfectly fine and not fragile.
Pubkeyconversion (line 333): The workaround is correct.UpgradeableLoaderState::Programcomes fromsolana_loader_v3_interface, and when Cargo resolves workspace dependencies with a single version, the underlyingsolana_pubkeytypes line up. The round-trip throughto_bytes()→Pubkey::new_from_array()is defensive but likely redundant—if the workspace has a clean version resolution, you could probably just usePubkey::new_from_array(programdata_address.to_bytes())or even cast directly. That said, the current approach is safe and the comment clearly explains the intent, so no rush to refactor unless you clean up the Solana dependencies elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/client/src/squads.rs` around lines 328 - 336, bincode::deserialize usage is fine; verify the client crate's Cargo.toml includes bincode.workspace = true and leave the deserialize call in squads::handler that reads program_account.data; for the Pubkey conversion around programdata_address (from UpgradeableLoaderState::Program) either (A) if your workspace guarantees a single solana_pubkey/solana_sdk resolution, simplify by using the programdata_address value directly as the solana_sdk::pubkey::Pubkey where used (removing the defensive to_bytes()/Pubkey::new_from_array round‑trip), or (B) if versions may differ, keep the existing defensive conversion (programdata_address.to_bytes() → Pubkey::new_from_array()) and retain the explanatory comment; update code/comments in the block handling program_account/program accordingly.rust/sealevel/programs/validator-announce/Cargo.toml (1)
32-32: Redundant[dev-dependencies]entry forsolana-system-interface.It's already listed in
[dependencies](Line 17), and Cargo makes every[dependencies]crate available to test/benchmark code by default. The duplicate entry doesn't cause harm, but it's dead weight you could trim from the swamp.🧹 Suggested cleanup
[dev-dependencies] hex.workspace = true solana-program-test.workspace = true solana-sdk.workspace = true -solana-system-interface.workspace = true hyperlane-test-utils = { path = "../../libraries/test-utils" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/validator-announce/Cargo.toml` at line 32, Remove the redundant dev-dependency entry for solana-system-interface from Cargo.toml: since solana-system-interface is already declared under [dependencies] it is available to tests and benchmarks, so delete the solana-system-interface line from the [dev-dependencies] section to avoid duplication.rust/sealevel/libraries/ecdsa-signature/src/lib.rs (1)
66-73: Parameterhashshadowed by innerlet hash— consider a distinct nameThe function parameter
hash(line 68, the message digest) is correctly consumed bysecp256k1_recoveron line 70, and then a new locallet hash(line 73) shadows it in the block. While harmless today, the reuse of the same identifier for two semantically differenthashvalues (message digest vs. public-key keccak digest) can trip up a future reader. Naming the inner variablepubkey_hashorkeccak_resultwould remove the ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/libraries/ecdsa-signature/src/lib.rs` around lines 66 - 73, In secp256k1_recover_ethereum_address rename the inner let hash that holds keccak(&public_key.to_bytes()) to a distinct name (e.g., pubkey_hash or keccak_hash) to avoid shadowing the function parameter `hash`; update any subsequent references inside that block to use the new identifier so the parameter `hash` (message digest) remains unshadowed and code intent is clear.rust/sealevel/programs/ism/multisig-ism-message-id/src/instruction.rs (1)
109-175:init_instructionmigrated tosystem_program::ID, butset_validators_and_threshold_instructionstill callssystem_program::id().Both resolve to the same address, so this is purely a style/consistency gap — not a bug. But since the rest of the PR standardises on the
IDconstant, it's worth tidying up line 174 as well.♻️ Suggested consistency fix
- AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(system_program::ID, false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/ism/multisig-ism-message-id/src/instruction.rs` around lines 109 - 175, In set_validators_and_threshold_instruction you should use the same system program constant as elsewhere for consistency: replace the AccountMeta that currently references system_program::id() with system_program::ID (i.e., change AccountMeta::new_readonly(system_program::id(), false) to AccountMeta::new_readonly(system_program::ID, false)) inside the set_validators_and_threshold_instruction function so it matches the rest of the PR.rust/sealevel/programs/build-programs.sh (1)
62-77: Version-branching logic is sound, but consider quoting variables for robustness.The Agave v2+ vs Solana Labs v1.x branching is correct. A wee thing though — the unquoted
$NEW_VERSIONon lines 69, 71, 75 and$MAJOR_VERSIONon line 66 could cause issues if they're ever empty (e.g.,solananot installed at all). Not a swamp-level crisis, but quoting shell variables is good hygiene.🧅 Suggested quoting improvements
- MAJOR_VERSION=$(echo $NEW_VERSION | cut -d. -f1) + MAJOR_VERSION=$(echo "$NEW_VERSION" | cut -d. -f1) - if [ "$MAJOR_VERSION" -ge 2 ] 2>/dev/null; then + if [ "${MAJOR_VERSION:-0}" -ge 2 ] 2>/dev/null; then # Agave CLI v2+: use agave-install if available, otherwise curl install if command -v agave-install &> /dev/null; then - agave-install init $NEW_VERSION + agave-install init "$NEW_VERSION" else - sh -c "$(curl -sSfL https://release.anza.xyz/v$NEW_VERSION/install)" + sh -c "$(curl -sSfL "https://release.anza.xyz/v${NEW_VERSION}/install")" fi else # Pre-Agave (v1.x): use Solana Labs release - sh -c "$(curl -sSfL https://release.solana.com/v$NEW_VERSION/install)" + sh -c "$(curl -sSfL "https://release.solana.com/v${NEW_VERSION}/install")" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/build-programs.sh` around lines 62 - 77, In set_solana_cli_version, quote shell variables and guard the numeric test: change the comparison to use a default for MAJOR_VERSION (e.g., ${MAJOR_VERSION:-0}) and quote it when used in the test, and quote NEW_VERSION when passed to agave-install and when interpolated into the curl URLs (use "$NEW_VERSION" or "v${NEW_VERSION}" style) so empty or whitespace-containing values won't break the condition or the commands; references: function set_solana_cli_version, variables NEW_VERSION and MAJOR_VERSION, commands agave-install init and the curl install sh -c invocations.rust/sealevel/programs/hyperlane-sealevel-igp/src/processor.rs (2)
458-460: Inconsistentborsh::to_vecerror-handling style across the PR.Every other changed processor/program file (
test-ism/program.rs,multisig-ism/processor.rs) spells out:borsh::to_vec(...).map_err(|_| ProgramError::BorshIoError)?This file alone uses bare
?. Both work becauseFrom<borsh::io::Error>exists forProgramError, but sticking with the explicitmap_errkeeps the codebase consistent and makes the error-mapping intent clear.♻️ Proposed alignment
- set_return_data(&borsh::to_vec(&SimulationReturnData::new( - required_payment, - ))?); + set_return_data( + &borsh::to_vec(&SimulationReturnData::new(required_payment)) + .map_err(|_| ProgramError::BorshIoError)?[..], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-igp/src/processor.rs` around lines 458 - 460, Replace the bare `?` error propagation on the borsh serialization with the project's explicit mapping to ProgramError: change the call that sets return data for SimulationReturnData (the invocation using set_return_data(&borsh::to_vec(&SimulationReturnData::new(required_payment,))?);) to use borsh::to_vec(...).map_err(|_| ProgramError::BorshIoError)? so the serialization error is explicitly mapped to ProgramError::BorshIoError (refer to set_return_data, SimulationReturnData::new, and required_payment to locate the exact invocation).
547-602: Inconsistent system-program key comparison style within the same file.Lines 100, 218, 273, 422, and 549 use
*system_program_info.key != system_program::ID(dereference), while lines 600 and 647 usesystem_program_info.key != &system_program::ID(reference). Both are equivalent in Rust but a consistent style across all six call sites would keep things tidy.♻️ Proposed normalisation (lines 600 and 647)
- if system_program_info.key != &system_program::ID { + if *system_program_info.key != system_program::ID {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-igp/src/processor.rs` around lines 547 - 602, Normalize the system program key comparisons to the same style used earlier in the file: replace the instances that compare with a reference (system_program_info.key != &system_program::ID) in the set_destination_gas_overheads function with the dereference style (*system_program_info.key != system_program::ID) so all checks (e.g., the checks involving system_program_info.key in processor.rs and the one in set_destination_gas_overheads) use the same dereference form for consistency.rust/sealevel/programs/hyperlane-sealevel-igp/src/instruction.rs (1)
52-54: Internal inconsistency:into_instruction_datausesmap_errwhile all builder functions use bare?.
into_instruction_dataat line 53 spells out.map_err(|_| ProgramError::BorshIoError), but every builder function in the same file (init_instruction,init_igp_instruction, etc.) just usesborsh::to_vec(&ixn)?. Pick one style and stay with it across the file — the explicitmap_erris the safer choice because it's unambiguous about the target error variant.♻️ Proposed fix — normalise builders to use map_err (example for init_instruction)
- data: borsh::to_vec(&ixn)?, + data: borsh::to_vec(&ixn).map_err(|_| ProgramError::BorshIoError)?,Apply the same change to all remaining
borsh::to_vec(&ixn)?sites in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-igp/src/instruction.rs` around lines 52 - 54, The file mixes two error-conversion styles: into_instruction_data uses borsh::to_vec(...).map_err(|_| ProgramError::BorshIoError) while builder functions (init_instruction, init_igp_instruction, etc.) use borsh::to_vec(...)?; make them consistent by replacing the fallible ? uses with explicit map_err calls that map the borsh serialization error to ProgramError::BorshIoError (e.g., change borsh::to_vec(&ixn)? to borsh::to_vec(&ixn).map_err(|_| ProgramError::BorshIoError)), applying this change across all builder functions in the file so the same unambiguous error variant is used everywhere.rust/sealevel/programs/hyperlane-sealevel-token/src/plugin.rs (1)
79-81: Add a test or compile-time comment anchoringMINT_ACCOUNT_SIZEto the specificspl-token-2022version.The constant 234 is correct for
spl-token-2022 = "10.0"withMetadataPointer, but the comment doesn't mention the pinned version. Future version bumps on the token crate could silently produce a wrong-sized allocation — the account would either fail to initialize or waste lamports. Consider a#[cfg(test)]test that dynamically computes the expected size and asserts equality, or at least augment the comment with the version it was validated against.🛡️ Suggested comment augmentation
- // Hardcoded because Pack trait version conflicts between solana-program-pack 3.0 and spl-token-2022's internal 2.2. - // It was calculated by calling `ExtensionType::try_calculate_account_len::<Mint>(vec![ExtensionType::MetadataPointer]).unwrap()` + // Hardcoded because Pack trait version conflicts between solana-program-pack 3.0 and spl-token-2022's internal 2.2. + // It was calculated by calling `ExtensionType::try_calculate_account_len::<Mint>(vec![ExtensionType::MetadataPointer]).unwrap()` + // Validated against spl-token-2022 = "10.0". Re-verify when bumping that dependency. const MINT_ACCOUNT_SIZE: usize = 234;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token/src/plugin.rs` around lines 79 - 81, The hardcoded MINT_ACCOUNT_SIZE (constant MINT_ACCOUNT_SIZE = 234) is only valid for a specific spl-token-2022 release and should be anchored; either add a compile-time/test assertion or augment the comment with the exact crate version used to derive the value. Fix by adding a #[cfg(test)] unit test that calls ExtensionType::try_calculate_account_len::<Mint>(vec![ExtensionType::MetadataPointer]).unwrap() and asserts it equals MINT_ACCOUNT_SIZE (use symbols: MINT_ACCOUNT_SIZE, ExtensionType::try_calculate_account_len, Mint, ExtensionType::MetadataPointer), or at minimum update the inline comment to state the validated spl-token-2022 = "10.0" (or exact Cargo.lock version) and note how the value was computed. Ensure the test runs in CI or the comment clearly documents the derivation and version so future bumps are caught.rust/main/chains/hyperlane-sealevel/src/validator_announce.rs (1)
19-19:system_program::id()call at line 185 wasn't updated tosystem_program::ID.The import at line 19 was updated to bring in the alias, but the usage at line 185 still calls
system_program::id()(a function) rather than thesystem_program::IDconstant that every other file in this PR consistently uses. Both resolve to the samePubkey, so this is a style inconsistency only — but it sticks out right after the import migration.✨ Proposed change for consistency
- AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(system_program::ID, false),Also applies to: 183-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/main/chains/hyperlane-sealevel/src/validator_announce.rs` at line 19, The code imports solana_system_interface::program as system_program but still calls the function system_program::id() at the usage site; replace that call with the constant system_program::ID to match the rest of the PR and the updated import. Locate the usage(s) of system_program::id() in validator_announce.rs (around the existing block that references system_program) and change them to system_program::ID so the Pubkey is obtained from the constant rather than the function.rust/sealevel/libraries/account-utils/src/lib.rs (1)
138-141:ProgramError::BorshIoErrordrops original error context.
map_err(|_| ProgramError::BorshIoError)silently discards the I/O error details on both theWriteZeropath and the generic path. On-chain programs can't propagate rich errors, so this is acceptable, but amsg!("{}", err)log before discarding would keep the info visible in program logs without changing the return type.🪵 Proposed improvement to preserve error visibility
- _ => return Err(ProgramError::BorshIoError), + _ => { + msg!("store serialization error: {:?}", err); + return Err(ProgramError::BorshIoError); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/libraries/account-utils/src/lib.rs` around lines 138 - 141, The code currently discards underlying I/O errors by returning ProgramError::BorshIoError via map_err(|_| ...) and in the generic match arm; update those spots to log the original error before returning so the details appear in on-chain logs. Specifically, replace map_err(|_| ProgramError::BorshIoError) with map_err(|err| { msg!("{}", err); ProgramError::BorshIoError }) and in the generic `_ => return Err(ProgramError::BorshIoError)` path call msg!("{}", err) (or log the captured error) prior to returning; apply this to both the WriteZero handling and the other error branch so ProgramError::BorshIoError is still returned but the original error is emitted to logs.rust/sealevel/programs/hyperlane-sealevel-token-native/src/lib.rs (1)
12-12: Remove the unusedpub use spl_noopre-export to drop the transitive dependency.This crate pulled in
spl-noopvia direct import (line 12), but nobody's actually using it. The whole point ofaccount_utils::SPL_NOOP_PROGRAM_IDwas to dodge thatsolana-program ^2dependency chain in the first place, yeah? Havingpub use spl_noopsitting there means you're keeping it alive for anyone who imports from this crate. Since there's no callers relying on it, might as well trim the fat.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token-native/src/lib.rs` at line 12, Remove the unused public re-export "pub use spl_noop" from lib.rs to eliminate the transitive spl-noop dependency; locate the line that declares the re-export (the "pub use spl_noop" statement) and delete it so the crate no longer exposes or pulls in spl-noop transitively (keep account_utils::SPL_NOOP_PROGRAM_ID as-is).rust/sealevel/programs/mailbox-test/src/utils.rs (1)
55-56: Minor inconsistency:system_program::id()on line 55 vssystem_program::IDused elsewhere.Line 55 (unchanged) uses
system_program::id()while the rest of the migrated codebase consistently usessystem_program::ID. Both resolve to the same pubkey, so this is purely cosmetic — but if you're already in the neighborhood tidying things up, might as well make it match. Not worth losing sleep over though.Optional consistency fix
- AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(system_program::ID, false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/mailbox-test/src/utils.rs` around lines 55 - 56, The AccountMeta entry using system_program::id() is inconsistent with the codebase convention; change the call in the AccountMeta::new_readonly(...) where system_program::id() is used to reference system_program::ID instead so it matches other usages (locate the AccountMeta::new_readonly call in utils.rs near the mailbox test setup).rust/sealevel/programs/mailbox-test/src/functional.rs (1)
55-58: Duplicate mailbox program registration.
ProgramTest::new(...)on line 55 already registers the mailbox program. Theadd_program(...)call on lines 76–80 re-registers the exact same program ID and processor — it's a no-op at best and slightly confusing at worst.♻️ Remove redundant add_program call
- let mailbox_program_id = mailbox_id(); - program_test.add_program( - "hyperlane_sealevel_mailbox", - mailbox_program_id, - processor!(hyperlane_sealevel_mailbox::processor::process_instruction), - ); - program_test.add_program(Also applies to: 76-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/mailbox-test/src/functional.rs` around lines 55 - 58, The test registers the mailbox program via ProgramTest::new(...) using program_id and processor!(hyperlane_sealevel_mailbox::processor::process_instruction) and then re-registers the identical program via add_program(...), which is redundant; remove the add_program(...) call that references the same program_id and processor to avoid duplicate registration and confusion (keep the initial ProgramTest::new(...) registration and delete the add_program(...) block).rust/sealevel/programs/hyperlane-sealevel-token-collateral/src/plugin.rs (1)
19-19: Consider renaming thesystem_programimport alias to avoid the shadowing with the function parameter.The module alias
system_program(line 19) and theinitializefunction parametersystem_program: &'a AccountInfo<'b>(line 112) share the same name. Rust resolves them correctly through separate namespaces (value vs. type/path), sosystem_program::IDstill resolves to the module constant and baresystem_programresolves to the parameter — but it's genuinely confusing to read, and easy to misread or mis-edit in future.A small rename of the alias keeps things unambiguous:
♻️ Proposed rename
-use solana_system_interface::program as system_program; +use solana_system_interface::program as system_program_interface;Then update the single usage:
- &system_program::ID, + &system_program_interface::ID,Also applies to: 218-219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token-collateral/src/plugin.rs` at line 19, The import alias system_program in plugin.rs shadows the initialize function parameter named system_program and can be renamed to avoid confusion; change the import "use solana_system_interface::program as system_program;" to a distinct alias (e.g., system_program_interface) and update all uses of the module constant (system_program::ID -> system_program_interface::ID) in the file (including the initialize function and the other occurrence around lines referenced in the review), leaving the initialize parameter name unchanged so references to the parameter remain correct.rust/sealevel/client/src/main.rs (1)
1202-1202: Minor style inconsistency: unchanged lines still callsystem_program::id()rather thansystem_program::ID.Lines 1202, 1260, and 1482 still use
system_program::id()(the old function form), while the rest of the migrated codebase moves tosystem_program::ID. Both are functionally identical sincedeclare_id!generates both, but mixing the two forms in the same file makes it look like some spots were missed. Worth tidying up in a follow-up.♻️ Suggested cleanup (representative example)
-AccountMeta::new_readonly(system_program::id(), false), +AccountMeta::new_readonly(system_program::ID, false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/client/src/main.rs` at line 1202, Replace occurrences of the old function form system_program::id() with the constant system_program::ID to keep style consistent; specifically update the AccountMeta::new_readonly(system_program::id(), false) (and the other instances at the same spots) to use system_program::ID so all references to the program ID in this module use the constant form.rust/sealevel/programs/hyperlane-sealevel-token-native/src/plugin.rs (1)
21-21: Same naming collision as the collateral plugin —system_programlocal variables shadow the module alias.In
transfer_in(line 129) andtransfer_out(line 160), local variables namedsystem_program(fromnext_account_info) coexist with the module alias imported at line 21. Rust resolvessystem_program.keyagainst the local variable (value namespace) andsystem_program::IDagainst the module (type namespace), so it compiles fine — but the dual meaning in the same block is a readability trap. The same alias rename suggested for the collateral plugin applies here.♻️ Suggested rename
-use solana_system_interface::{instruction as system_instruction, program as system_program}; +use solana_system_interface::{instruction as system_instruction, program as system_program_interface};Then update line 104, 130, 161, 200:
- &system_program::ID, + &system_program_interface::ID,Also applies to: 129-131, 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/hyperlane-sealevel-token-native/src/plugin.rs` at line 21, The imported module alias system_program conflicts with local variables named system_program returned from next_account_info in transfer_in and transfer_out; rename the module alias (e.g., to system_program_mod or system_interface) in the use statement and update all references to the module (e.g., change system_program::ID to system_program_mod::ID), or alternatively rename the local account variables (e.g., to system_program_acc) so the module and local names no longer collide; apply the same rename consistently for every occurrence in transfer_in, transfer_out and any other functions that reference system_program for either the module or an account.rust/sealevel/programs/validator-announce/tests/functional.rs (1)
103-104: Align test file to usesystem_program::IDconstant instead ofsystem_program::id()function.The test file uses
system_program::id()at lines 103 and 208, while the correspondinginstruction.rsandprocessor.rsfiles use thesystem_program::IDconstant. This misalignment breaks the pattern established throughout the codebase. Both work fine sincedeclare_id!generates both forms, but the constant is the preferred idiom here.♻️ Suggested alignment
- AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(system_program::ID, false),(Same change applies on line 208.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/sealevel/programs/validator-announce/tests/functional.rs` around lines 103 - 104, Replace uses of the function call system_program::id() in the test file with the constant system_program::ID to match the codebase idiom; update the AccountMeta entries that currently call system_program::id() (e.g., the AccountMeta::new_readonly(...) and the other AccountMeta::new(...) instance in tests/functional.rs) to use system_program::ID so the tests align with instruction.rs and processor.rs which use the constant.
rust/sealevel/programs/hyperlane-sealevel-token-native/tests/functional.rs
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7999 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
…ests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@rust/sealevel/rust-toolchain`:
- Line 2: No code changes required: the rust toolchain channel is correctly set
to "1.86.0" and matches Agave v3.0.14, so leave the rust/sealevel/rust-toolchain
file's channel value unchanged (channel = "1.86.0") and proceed with the
approved update.
🦀 Rust Agent Docker Image Built Successfully
Full image paths |
The old two-version dance (1.14.20 for building, 1.18.18 for deploying) was removed in #7999 which migrated to Agave CLI v3.0.14. This script is no longer referenced anywhere. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Summary
rust/sealevelandrust/mainfrom forked Solana SDK v1.14.13 to official Solana SDK v3.xWhy v3.0.0?
solana-sdkis now independently versioned from the Agave validator. v3.0.0 is the version Anza's migration docs target. v4.0.0 just dropped with no docs yet — we can bump in a follow-up once it stabilizes.Stack Overflow Prevention
Solana BPF has a 4KB stack limit per function frame. Several structures exceeded this during CPI calls:
Outbox::verify_account_and_fetch_inner- Now returnsBox<Outbox>instead ofOutboxby value (1118 bytes → heap)AccountData::deserialize_boxed- New#[inline(never)]helper to isolate large stack frames during deserializationIncrementalMerkle(hyperlane-core) - CustomBorshDeserializeimplementation that deserializes the 1024-byte branch array one element at a time instead of allocating on stackThese fixes resolve "Access violation in stack frame" errors during CPI.
Test Infrastructure
processor!()macroprocessor!()approach doesn't work withsolana_cpi::invoke(returnsOk(())offchain without executing)Client Changes
Serialization Format Change
UpgradeableLoaderStatechanged from Borsh to bincode in Solana v3.xrouter.rsandsquads.rsto usebincode::deserializeOther Changes
rust/sealevel
solana-*dependencies to v3.0resolver = "2"to workspace Cargo.tomlBorshIoErrorAPI (now unit variant)AccountInfo::newcalls (removedrent_epochparam)keccak::hashv→keccak::hashrust/main
solana-commitment-configsolana-compute-budget-interfacesolana-rpc-clientsolana-sdk-idssolana-system-interfacePubkey::new()→Pubkey::try_from()Signature::new()→Signature::try_from()Keypair::from_bytes()→Keypair::try_from()Memcmpstruct init →Memcmp::new()borsh::try_to_vec()→borsh::to_vec()SigningKey::from_bytesPatches retained
primitive-types/rlppatches (Borsh support for U256/H256)curve25519-dalekpatch (required by cosmwasm ecosystem)Test plan
cargo check --all-targetspasses for rust/maincargo clippypasses for modified packagescargo build-sbf🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests