Enhance Solana transaction support and add new dependencies #78
Conversation
- Introduced `SolanaSigner` to the engine core for improved Solana transaction handling. - Added `base64`, `bincode`, `solana-client`, and `solana-sdk` as dependencies in `Cargo.toml`. - Updated server state to include `solana_signer` and added new route for signing Solana transactions.
WalkthroughAdds Solana transaction signing: workspace dependency additions and an integration-tests crate, a POST /v1/solana/sign/transaction Axum route that builds, signs and returns a base64 bincode-serialized VersionedTransaction, and wires SolanaSigner + SolanaRpcCache into server state and startup. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Handler as sign_solana_transaction
participant RpcCache as SolanaRpcCache
participant RPC as Solana RPC
participant Signer as SolanaSigner
Client->>Handler: POST /v1/solana/sign/transaction\n(request + credentials)
Handler->>RpcCache: get_rpc_client(chain_id)
RpcCache->>RPC: connect / forward requests
RPC-->>RpcCache: rpc client / responses
RpcCache-->>Handler: RPC client
Handler->>Handler: fetch_latest_blockhash(via RPC client)
Handler->>Handler: build VersionedTransaction\n(apply compute unit options)
Handler->>Signer: sign_transaction(tx, credentials)
Signer-->>Handler: signed transaction (signatures)
Handler->>Handler: bincode_serialize -> base64_encode
Handler-->>Client: 200 OK\n{ signature, signed_transaction }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
server/Cargo.toml(2 hunks)server/src/http/routes/mod.rs(1 hunks)server/src/http/routes/sign_solana_transaction.rs(1 hunks)server/src/http/server.rs(3 hunks)server/src/main.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
server/src/http/routes/mod.rs (1)
server/src/http/routes/sign_solana_transaction.rs (1)
sign_solana_transaction(66-144)
server/src/main.rs (3)
server/src/http/server.rs (1)
new(48-116)core/src/signer.rs (2)
new(211-216)new(561-566)server/src/queue/manager.rs (1)
new(52-318)
server/src/http/routes/sign_solana_transaction.rs (1)
server/src/http/server.rs (1)
new(48-116)
server/src/http/server.rs (1)
server/src/http/routes/sign_solana_transaction.rs (1)
sign_solana_transaction(66-144)
- Introduced `SolanaRpcCache` to manage RPC URLs for Solana transactions. - Updated server state to include `solana_rpc_cache` for efficient RPC client retrieval. - Refactored transaction signing logic to utilize the new RPC cache, improving performance and reliability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
server/src/main.rs (2)
63-64: Consider cloningiaw_clienton line 64 for consistency.Line 63 uses
iaw_client.clone()forEoaSigner, but line 64 movesiaw_clientforSolanaSigner. While moving is more efficient sinceiaw_clientisn't used afterward, using.clone()on both lines would improve consistency with the established pattern (including inqueue/manager.rs).Apply this diff for consistency:
let eoa_signer = Arc::new(EoaSigner::new(vault_client.clone(), iaw_client.clone())); -let solana_signer = Arc::new(SolanaSigner::new(vault_client.clone(), iaw_client)); +let solana_signer = Arc::new(SolanaSigner::new(vault_client.clone(), iaw_client.clone()));
75-82: Consider consolidating duplicated Solana RPC cache creation.A separate
SolanaRpcCachewith identical configuration is also created inqueue/manager.rs(lines 66-69 of the relevant snippet). This results in two independent cache instances serving different parts of the system (HTTP routes vs queue workers), which can lead to:
- Wasted memory maintaining duplicate caches
- Configuration drift if URLs are updated in one place but not the other
- Inconsistent cache state
Consider creating the cache once here and passing it as a parameter to
QueueManager::new(), similar to howauthorization_cacheis handled (line 91).To consolidate, modify
QueueManager::new()to acceptsolana_rpc_cacheas a parameter instead of creating it internally:let queue_manager = QueueManager::new( redis_client.clone(), &config.queue, &config.solana, chains.clone(), signer.clone(), eoa_signer.clone(), authorization_cache.clone(), kms_client_cache.clone(), + solana_rpc_cache.clone(), ) .await?;Then update the
QueueManager::newsignature inqueue/manager.rsto accept the cache instead of creating it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
server/src/http/routes/sign_solana_transaction.rs(1 hunks)server/src/http/server.rs(3 hunks)server/src/main.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/src/http/server.rs
- server/src/http/routes/sign_solana_transaction.rs
🧰 Additional context used
🧬 Code graph analysis (1)
server/src/main.rs (3)
server/src/http/server.rs (1)
new(50-118)core/src/signer.rs (2)
new(211-216)new(561-566)server/src/queue/manager.rs (1)
new(52-318)
🔇 Additional comments (2)
server/src/main.rs (2)
3-4: LGTM!The new imports for Solana support are appropriate and all are used in the initialization flow.
137-138: LGTM!The
solana_signerandsolana_rpc_cacheare correctly added toEngineServerStatefollowing the established pattern for shared resources, with appropriateArccloning for thread-safe access.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
integration-tests/README.md (2)
111-112: Wrap bare URLs in markdown link syntax.Lines 111-112 and 276 contain bare URLs that should be wrapped in markdown link syntax for proper rendering and accessibility. This also resolves
MD034linting warnings.Apply this diff:
- ✅ **`thirdweb.secret_key`** - Get from https://thirdweb.com/dashboard - ✅ **`thirdweb.client_id`** - Get from https://thirdweb.com/dashboard + ✅ **`thirdweb.secret_key`** - Get from [https://thirdweb.com/dashboard](https://thirdweb.com/dashboard) + ✅ **`thirdweb.client_id`** - Get from [https://thirdweb.com/dashboard](https://thirdweb.com/dashboard)And similarly for line 276:
- Get credentials from https://thirdweb.com/dashboard + Get credentials from [https://thirdweb.com/dashboard](https://thirdweb.com/dashboard)Also applies to: 276-276
186-197: Specify language identifier for code fence block.The YAML configuration file structure block (lines 186-197) is a code fence without a language specifier, which resolves
MD040linting violations and improves syntax highlighting.Apply this diff:
-``` +```yaml integration-tests/ ├── configuration/Cargo.toml (1)
32-48: Avoid duplicate Solana dependency declarations
solana-system-interfaceandspl-token-interface-2022are already declared inserver/Cargo.tomlwith the same versions/features. Duplicating them in[workspace.dependencies]forces every crate to inherit these heavier deps (and feature sets) even if unused, bloating compilation and risking version skew with crates that rely on workspace defaults. Please keep them scoped in the crate(s) that actually need them.integration-tests/Cargo.toml (2)
24-28: Reuse workspace alias forsolana-system-interface
Here we pinsolana-system-interface = { version = "2.0", features = ["bincode"] }directly instead of consuming the workspace alias you just introduced. This defeats the point of centralizing versions and can drift. Please switch to{ workspace = true }so the integration crate stays in lockstep with the workspace declaration (once the workspace entry remains necessary).
24-28: Drop redundantsolana-system-interfacedependency
Once you scope the workspace dependency appropriately (previous comment), this crate no longer needs its own explicit declaration; it can inherit viasolana-coreor add aworkspace = trueentry. Keeping both declarations will cause cargo to treat them as distinct specs.integration-tests/tests/setup.rs (4)
48-74: Test config loader never validatesenvironmentmatches an actual override file
We accept anyTEST_ENVIRONMENTstring and silently ignore typos because.required(false)suppresses errors. That makes debugging painful (e.g., settingTEST_ENVIRONMENT=devwhen onlytest_local.yamlexists). Please add a check that at least one override file exists when a non-default env is specified, or log a warning so misconfigured runs don’t continue with the base config unexpectedly.
187-209: QueueConfig clone is unnecessary and leaks test namespace logic
You recreateQueueConfigmanually instead of using the deserializedconfig.queue, duplicating fields and risking divergence if new fields appear. Better to cloneconfig.queue, then overrideexecution_namespacevia struct update syntax to keep future additions safe.
211-224: RebuildingSolanaConfigduplicates nested structs
Same issue as above: manual reconstruction will drift if config schema changes. Clone the existingconfig.solanaand tweak only what’s test-specific, or simply passconfig.solana.clone().
274-313: Double-wrappingArcaround already shared components
You wrapvault_clientinArctwice (Arc::new(vault_client.clone())), creating redundant reference counting and copies. Reuse the existingArcclone: passvault_client.clone()directly. Similar forsigner,solana_signer,queue_manager.integration-tests/tests/sign_solana_transaction.rs (1)
265-284: Surface error body in invalid-transaction test
The test asserts failure but never inspects the error payload, missing regression coverage on the error schema (status code vs. JSON body). Please parse the body and assert it matches the documented error structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.toml(2 hunks)integration-tests/.gitignore(1 hunks)integration-tests/Cargo.toml(1 hunks)integration-tests/README.md(1 hunks)integration-tests/configuration/test_base.yaml(1 hunks)integration-tests/src/lib.rs(1 hunks)integration-tests/tests/setup.rs(1 hunks)integration-tests/tests/sign_solana_transaction.rs(1 hunks)server/src/lib.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- integration-tests/.gitignore
🧰 Additional context used
🧬 Code graph analysis (3)
server/src/lib.rs (1)
server/src/config.rs (1)
config(149-149)
integration-tests/tests/setup.rs (1)
executors/src/metrics.rs (1)
initialize_metrics(101-104)
integration-tests/tests/sign_solana_transaction.rs (2)
integration-tests/src/lib.rs (2)
create_system_transfer(92-117)verify_signature(66-84)integration-tests/tests/setup.rs (3)
new(105-317)config(68-69)server_url(325-327)
🪛 markdownlint-cli2 (0.18.1)
integration-tests/README.md
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
186-186: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
276-276: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (3)
integration-tests/README.md (2)
140-142: Verify support for environment-specific configuration files.The example references
TEST_ENVIRONMENT=stagingto usetest_staging.yamlinstead oftest_local.yaml, but the configuration loading logic (lines 203-215) does not show how this environment variable is used. Please verify that the test setup actually supports this override pattern, or clarify the expected behavior.
1-315: Excellent integration test documentation.The README provides comprehensive setup instructions, clear configuration examples, multiple test scenario descriptions, and solid troubleshooting guidance. The structure is logical and covers the full lifecycle from prerequisites through code coverage. This should significantly reduce friction for developers running integration tests.
integration-tests/tests/setup.rs (1)
249-263:EngineServerStategets a fresh Redis connection per server
redis_client.get_connection_manager().await?returns a new manager; fine—but the lifetime ties to the local variable. Ensure it lives across tests by storing inside anArcto avoid dropping it prematurely (sinceExecutionRouterexpects it to stay alive).Please confirm with a quick run that droppping
redis_clienthere doesn’t sever the manager connection during tests.
|
|
||
| # Thirdweb service configuration | ||
| thirdweb: | ||
| urls: | ||
| rpc: rpc.thirdweb-dev.com | ||
| bundler: bundler.thirdweb-dev.com | ||
| paymaster: bundler.thirdweb-dev.com | ||
| vault: https://d2w4ge7u2axqfk.cloudfront.net | ||
| abi_service: https://contract.thirdweb.com/abi/ | ||
| iaw_service: https://embedded-wallet.thirdweb-dev.com | ||
| secret: read_from_env | ||
| client_id: read_from_env | ||
|
|
There was a problem hiding this comment.
Do not default secrets to literal sentinel strings
Hard-coding secret: read_from_env / client_id: read_from_env in the base config risks these placeholder strings leaking into tests when env vars are unset, causing auth to silently break downstream. A safer pattern is to leave them null and let setup.rs bail early if they stay unset (mirroring the vault checks you already added).
🤖 Prompt for AI Agents
In integration-tests/configuration/test_base.yaml around lines 17 to 29, the
base config currently hard-codes secret: read_from_env and client_id:
read_from_env which can leak sentinel values into tests; change those keys to
null (or remove them entirely) so they are truly unset by default and allow
setup.rs to detect missing values and fail early; ensure the YAML retains the
keys with value null (or is absent) and update any comments if needed to
indicate these must be provided via environment before running tests.
integration-tests/README.md
Outdated
| cd /Users/d4mr/work/thirdweb/engine-core | ||
| cargo test -p engine-integration-tests | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove absolute local path from documentation.
Line 121 contains a developer-specific absolute path (/Users/d4mr/work/thirdweb/engine-core) that should not appear in published documentation. Use a generic or relative path placeholder instead.
Apply this diff:
### Run all integration tests
```bash
-cd /Users/d4mr/work/thirdweb/engine-core
cargo test -p engine-integration-tests
<details>
<summary>🤖 Prompt for AI Agents</summary>
In integration-tests/README.md around lines 121 to 123, replace the
developer-specific absolute path "/Users/d4mr/work/thirdweb/engine-core" with a
generic or relative path placeholder (for example "./engine-core" or
"") so the docs are not publishing local machine paths;
update the command block to use the chosen generic/relative path and keep the
subsequent test command unchanged.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| if let Some(index) = signer_index && index < transaction.signatures.len() { | ||
| // Check if the signature is not default (i.e., has been signed) | ||
| let sig = &transaction.signatures[index]; | ||
| return sig != &Signature::default(); | ||
| } | ||
|
|
||
| false |
There was a problem hiding this comment.
Rust 1.78 requires if let ... else syntax
if let Some(index) = signer_index && ... isn’t valid until Rust 1.79 (let_chains). CI on stable 1.78 will fail. Rewrite with nested if let Some(index) = signer_index { if index < ... }.
🤖 Prompt for AI Agents
In integration-tests/src/lib.rs around lines 77 to 83, the code uses `if let
Some(index) = signer_index && index < transaction.signatures.len()` which relies
on the let_chains feature not available in Rust 1.78; rewrite using a nested
conditional: first `if let Some(index) = signer_index {` then inside that block
check `if index < transaction.signatures.len() {` and perform the signature
check and return, otherwise fall through to return false. Ensure braces and
returns match the original control flow.
| // Create vault client | ||
| let vault_client = vault_sdk::VaultClient::builder(config.thirdweb.urls.vault.clone()) | ||
| .build() | ||
| .await | ||
| .context("Failed to create Vault client")?; | ||
|
|
||
| info!("Vault client initialized: {}", config.thirdweb.urls.vault); | ||
|
|
||
| // Setup components using config | ||
| let chains = Arc::new(ThirdwebChainService { | ||
| secret_key: config.thirdweb.secret.clone(), | ||
| client_id: config.thirdweb.client_id.clone(), | ||
| bundler_base_url: config.thirdweb.urls.bundler.clone(), | ||
| paymaster_base_url: config.thirdweb.urls.paymaster.clone(), | ||
| rpc_base_url: config.thirdweb.urls.rpc.clone(), | ||
| }); | ||
|
|
||
| let iaw_client = IAWClient::new(&config.thirdweb.urls.iaw_service) | ||
| .context("Failed to create IAW client")?; | ||
|
|
||
| let kms_client_cache: KmsClientCache = moka::future::Cache::builder() | ||
| .max_capacity(100) | ||
| .time_to_live(Duration::from_secs(60 * 60)) | ||
| .time_to_idle(Duration::from_secs(60 * 30)) | ||
| .build(); | ||
|
|
||
| let signer = Arc::new(UserOpSigner { | ||
| vault_client: vault_client.clone(), | ||
| iaw_client: iaw_client.clone(), | ||
| }); | ||
|
|
||
| let eoa_signer = Arc::new(EoaSigner::new(vault_client.clone(), iaw_client.clone())); | ||
| let solana_signer = Arc::new(SolanaSigner::new(vault_client.clone(), iaw_client)); | ||
|
|
||
| // Setup Redis | ||
| let redis_client = twmq::redis::Client::open(config.redis.url.as_str()) | ||
| .context("Failed to connect to Redis")?; | ||
|
|
There was a problem hiding this comment.
Injecting VaultClient without admin auth header
vault_sdk::VaultClient::builder(...).build() returns an unauthenticated client; you never call .with_access_token() before handing it to signers. Later requests will fail with 401 unless the SDK transparently reuses the admin key (it doesn’t). Please set the admin key on the client (or the signer constructors) before using it.
| let recent_blockhash = Hash::default(); // In real test, fetch from RPC | ||
|
|
||
| info!("Creating transaction with fee payer: {}", engine_pubkey); | ||
| let transaction = create_system_transfer( | ||
| engine_pubkey, // Fee payer (engine wallet) | ||
| recipient, | ||
| 1_000_000, // 0.001 SOL | ||
| recent_blockhash, | ||
| )?; |
There was a problem hiding this comment.
Using Hash::default() breaks transaction validation
Mainnet/devnet validators reject the default (all-zero) blockhash; RPC will treat the transaction as expired. Fetch a recent blockhash from RPC (e.g., via the new Solana RPC cache or client) before signing so the signed tx is realistic.
🤖 Prompt for AI Agents
integration-tests/tests/sign_solana_transaction.rs around lines 76 to 84: the
test currently uses Hash::default() which produces an all-zero blockhash that
validators reject; replace this with a real recent blockhash fetched from RPC
(e.g., use your Solana RPC client or the new RPC cache API to call
get_latest_blockhash or get_recent_blockhash before creating the transaction),
propagate/await the result as needed, handle RPC errors (fail the test or unwrap
with a clear message), and pass the fetched blockhash into
create_system_transfer so the signed transaction is valid.
| pub use chains::ThirdwebChainService; | ||
| pub use config::{ | ||
| EngineConfig, MonitoringConfig, QueueConfig, RedisConfig, ServerConfig, SolanaConfig, | ||
| SolanRpcConfigData, ThirdwebConfig, ThirdwebUrls, | ||
| }; | ||
| pub use execution_router::ExecutionRouter; | ||
| pub use http::server::{EngineServer, EngineServerState}; | ||
| pub use queue::manager::QueueManager; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Re-exporting EngineServerState exposes internal fields
Making EngineServerState public API means any internal struct changes (like adding the new Solana fields) become semver-breaking. Unless you intend to stabilize that surface, prefer re-exporting only EngineServer or providing accessor helpers for tests. Otherwise downstream crates may start constructing EngineServerState directly and lock us into the current layout.
🤖 Prompt for AI Agents
In server/src/lib.rs around lines 8 to 15, the public re-export of
EngineServerState exposes internal struct layout and risks semver breaks; remove
or stop re-exporting EngineServerState (leave only EngineServer public) and
instead provide minimal, stable accessor functions or explicit helper APIs in
the http::server module for any fields tests or downstream code need; update
callers to use the accessors or import the type from the crate-internal module
behind a cfg(test) or keep the type private to prevent downstream construction
and preserve internal freedom to change the struct.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
integration-tests/src/lib.rs (1)
74-78: Fix let_chains syntax incompatible with Rust 1.78.As flagged in the previous review, this
if let ... && ...syntax requires Rust 1.79'slet_chainsfeature. Since CI runs on stable 1.78, this will cause compilation failure.Apply this diff to use nested conditionals:
- if let Some(index) = signer_index && index < transaction.signatures.len() { - // Check if the signature is not default (i.e., has been signed) - let sig = &transaction.signatures[index]; - return sig != &Signature::default(); + if let Some(index) = signer_index { + if index < transaction.signatures.len() { + // Check if the signature is not default (i.e., has been signed) + let sig = &transaction.signatures[index]; + return sig != &Signature::default(); + } }
🧹 Nitpick comments (3)
integration-tests/README.md (2)
111-112: Wrap bare URLs in markdown link syntax.Bare URLs should be wrapped in markdown links for better formatting and accessibility. Apply this diff to fix lines 111–112 and line 275:
-✅ **`thirdweb.secret_key`** - Get from https://thirdweb.com/dashboard -✅ **`thirdweb.client_id`** - Get from https://thirdweb.com/dashboard +✅ **`thirdweb.secret_key`** - Get from [thirdweb.com/dashboard](https://thirdweb.com/dashboard) +✅ **`thirdweb.client_id`** - Get from [thirdweb.com/dashboard](https://thirdweb.com/dashboard)And for line 275:
-- Get credentials from https://thirdweb.com/dashboard +- Get credentials from [thirdweb.com/dashboard](https://thirdweb.com/dashboard)Also applies to: 275-275
185-196: Add language specifier to file tree code block.The fenced code block for the file structure (lines 185–196) should include a language identifier. Use
textsince it's a directory tree:-``` +```text integration-tests/ ├── configuration/integration-tests/Cargo.toml (1)
39-39: Consider removingonce_cellexternal dependency.
once_cellis largely superseded bystd::sync::OnceLock(available since Rust 1.70). If the project supports recent Rust tooling and doesn't need the additional features provided by theonce_cellcrate (e.g., lazy_static), consider removing this dependency and using the standard library instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
integration-tests/Cargo.toml(1 hunks)integration-tests/README.md(1 hunks)integration-tests/src/lib.rs(1 hunks)integration-tests/tests/sign_solana_transaction.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- integration-tests/tests/sign_solana_transaction.rs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
integration-tests/README.md
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
185-185: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
275-275: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (5)
integration-tests/README.md (1)
120-122: ✅ Resolved: Absolute path removed.The developer-specific absolute path has been successfully removed from the test execution command. The documentation now uses a generic cargo command appropriate for all developers.
integration-tests/src/lib.rs (3)
26-37: Previous issue resolved - proper helper function now used.The manual instruction construction issue flagged in the previous review has been correctly addressed. The code now uses
spl_token_2022_interface::instruction::transfer_checkedwith proper parameters (amount, decimals, accounts), ensuring the correct instruction layout and Token-2022 compatibility.
84-86: LGTM!Clean and straightforward helper function with appropriate error handling.
89-114: LGTM!The system transfer helper follows the same clean pattern as the SPL token transfer function, using the appropriate
solana_system_interface::instruction::transferhelper and correctly initializing the VersionedTransaction structure.integration-tests/Cargo.toml (1)
4-4: Edition "2024" is valid and supported.Rust edition "2024" is a valid and supported edition as of 2025. No action is required.
| engine-solana-core = { path = "../solana-core" } | ||
| engine-core = { path = "../core" } | ||
| engine-executors = { path = "../executors" } | ||
| thirdweb-core = { path = "../thirdweb-core" } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that all local path dependencies exist.
Lines 33–36 specify local path dependencies pointing to sibling directories (../solana-core, ../core, ../executors, ../thirdweb-core). Confirm that these relative paths are correct and the corresponding crates exist in the repository before merging.
🏁 Script executed:
#!/bin/bash
# Description: Verify that all local path dependencies exist in the repository.
# Check each path dependency
paths=(
"solana-core"
"core"
"executors"
"thirdweb-core"
)
for path in "${paths[@]}"; do
if [ -d "../$path" ]; then
echo "✓ Found: ../$path"
else
echo "✗ Missing: ../$path"
fi
done
# Also verify from repo root
echo ""
echo "Verifying from repository root:"
fd -t d 'Cargo.toml$' -x dirname {} \; | head -20Length of output: 129
I need to map the actual repository structure to verify the path dependencies. Let me run a comprehensive check from the repository root.
🏁 Script executed:
#!/bin/bash
# Description: Map repository structure and locate all crates
echo "=== Repository structure from root ==="
fd -t d -L 1 | sort
echo ""
echo "=== Location of integration-tests ==="
fd -t d 'integration-tests$' -x dirname {} \;
echo ""
echo "=== All Cargo.toml files in repository ==="
fd -t f 'Cargo.toml$' | sort
echo ""
echo "=== Checking if sibling crates exist at root level ==="
for crate in solana-core core executors thirdweb-core; do
if [ -d "$crate" ] && [ -f "$crate/Cargo.toml" ]; then
echo "✓ Found crate: $crate/"
else
echo "✗ Missing crate: $crate/"
fi
doneLength of output: 506
The relative paths in lines 33–36 are incorrect and will fail to resolve.
integration-tests/Cargo.toml is located at the repository root level, not in a subdirectory. The crates solana-core, core, executors, and thirdweb-core are all sibling directories at the same root level. Using relative paths with ../ will attempt to navigate outside the repository.
Correct the paths to point to the sibling directories. If this is a Rust workspace, you can either:
- Remove the
../prefix and reference the crates directly (if configured as workspace members) - Use workspace member references instead of path dependencies
Verify the workspace configuration in the root Cargo.toml and update lines 33–36 accordingly.
🤖 Prompt for AI Agents
In integration-tests/Cargo.toml around lines 33–36 the path dependencies use
"../" which incorrectly points outside the repo; update each dependency to
reference the sibling crate directories (remove the "../" prefix so paths are
"solana-core", "core", "executors", "thirdweb-core") or, alternatively, remove
the path dependencies and add these crates as workspace members in the root
Cargo.toml; after changing, verify the root workspace configuration and run
cargo build to confirm resolution.
SolanaSignerto the engine core for improved Solana transaction handling.base64,bincode,solana-client, andsolana-sdkas dependencies inCargo.toml.solana_signerand added new route for signing Solana transactions.Summary by CodeRabbit
New Features
Tests
Documentation
Chores