Basic integration test #34
Conversation
WalkthroughThis pull request refines the command‐line interface, core functionality, and testing suite. It updates CLI command aliases and field options, modifies function signatures to take ownership instead of references, and simplifies error handling. Additionally, default implementations and new configuration fields are introduced for plugin management. Integration tests and helper functions have been added, and dependency versions have been updated with new dependencies included while removing an unused one. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Simnet
participant PluginManager
participant TUI
User->>CLI: Invoke Simnet command
CLI->>Simnet: Start simulation with config
Simnet->>PluginManager: Load and initialize plugins
PluginManager-->>Simnet: Acknowledge plugin loading
Simnet->>PluginManager: Send transaction data
PluginManager-->>Simnet: Return processed transaction info
Simnet->>TUI: Emit TransactionProcessed event
TUI->>TUI: Update successful_transactions counter
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/core/src/types.rs (1)
26-36: Consider externalizing hardcoded URLs.The default implementations contain hardcoded URLs which could make testing in different environments challenging.
Consider moving these to configuration constants or environment variables:
+const DEFAULT_MAINNET_URL: &str = "https://api.mainnet-beta.solana.com"; impl Default for SimnetConfig { fn default() -> Self { Self { - remote_rpc_url: "https://api.mainnet-beta.solana.com".to_string(), + remote_rpc_url: std::env::var("SOLANA_RPC_URL") + .unwrap_or_else(|_| DEFAULT_MAINNET_URL.to_string()), slot_time: 0, // ... rest of the implementation } } } impl Default for RpcConfig { fn default() -> Self { Self { - remote_rpc_url: "https://api.mainnet-beta.solana.com".to_string(), + remote_rpc_url: std::env::var("SOLANA_RPC_URL") + .unwrap_or_else(|_| DEFAULT_MAINNET_URL.to_string()), // ... rest of the implementation } } }Also applies to: 51-59
crates/core/src/rpc/full.rs (1)
501-503: Consider implementing essential RPC methods for integration testing.Several methods are marked as
unimplemented!()which might be required for basic integration testing. Based on the PR objectives mentioning integration test development, consider implementing at least:
minimum_ledger_slotget_signatures_for_addressget_stake_minimum_delegationget_recent_prioritization_feesAlso applies to: 608-609, 672-674, 680-682
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/cli/src/cli/mod.rs(2 hunks)crates/core/src/lib.rs(1 hunks)crates/core/src/rpc/full.rs(1 hunks)crates/core/src/types.rs(3 hunks)crates/core/tests/integration.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: run_cargo_checks
🔇 Additional comments (6)
crates/core/src/lib.rs (1)
26-32: LGTM! Simplified error handling.The direct return of the awaited result is more idiomatic and reduces unnecessary wrapping.
crates/core/src/types.rs (1)
3-9: LGTM! Well-structured default implementation for RunloopTriggerMode.The Clock variant as default is a sensible choice for the trigger mode.
crates/core/tests/integration.rs (1)
9-33: LGTM! Well-structured test for simulation readiness.The test effectively verifies that the simulation can start and reach a ready state.
crates/cli/src/cli/mod.rs (1)
77-78: LGTM! Better short flag choice for host option.Changing from '-h' to '-o' is a good choice as '-h' is conventionally reserved for help.
crates/core/src/rpc/full.rs (2)
620-636: LGTM! Clean error handling in get_latest_blockhash.The implementation has been simplified to use the
?operator for error propagation, making the code more concise and readable.
622-623: Address the TODO comment about block height.The comment suggests uncertainty about returning the correct block height. This should be verified to ensure proper functionality.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for block height usage in the codebase rg -A 5 "block_height" --type rustLength of output: 2635
Clarify the Block Height Source
It appears that the latest block height is consistently retrieved via
state_reader.epoch_info.block_height(as seen in both full and minimal RPC implementations). While this usage is coherent across the codebase, the TODO comment rightly points out that we should double-check whether this value indeed represents the "last valid block height" as intended—especially when integration tests and real-world scenarios are considered.
- Confirm with integration tests or design docs that using
epoch_info.block_heightaligns with the definition of “last valid block height.”- If discrepancies are found during testing, adjust the computation or add clarifying comments to document any differences between the design intent and the current implementation.
| match test_rx.recv_timeout(Duration::from_millis(1000)) { | ||
| Ok(_) => (), | ||
| Err(_) => panic!("not enough ticks"), | ||
| } |
There was a problem hiding this comment.
I think it makes sense to me. As we go, we should create as many helpers as possible, to make testing easy and boring.
@MicaiahReid, is there anything specific you'd like to see?
Do we want to setup https://crates.io/crates/test-case ?
There was a problem hiding this comment.
For unit testing, I'd love to see each rpc endpoint tested with a few edge cases. test-case is great for that, when we get there.
For integration tests, I'd love to see something like:
- start surfpool
- get some vm data and assert pre-condition
- send some transaction(s)
- get some vm data and assert post-conditions
If we have easy interfaces for starting surfpool, creating/sending txs, and calling the rpc to get the data, we probably don't need to go overboard with test_case for that. We'd have to have very generic interfaces for the post conditions.
Some other considerations. It probably is not the best idea to point to a real mainnet rpc for tests? Or does that matter.
We don't want to have flaky tests if an rpc is slow or we're being rate limited.
This makes me wonder if we could eventually pull off a few things:
- surfpool for CI
- surfpool without an upstream
- surfpool pointing to surfpool as its upstream
With this, for our test suite, we could:
- start sp with no upstream
- do some writes to update the state
- start another sp, pointing to sp1
- continue with the integration tests
There was a problem hiding this comment.
surfpool pointing to surfpool as its upstream
🤯 I love it. that upstream could return some known hard coded data, so that we test that the "fetch remote fallbacks" are kicking in when expected.
crates/core/tests/integration.rs
Outdated
| } | ||
| }); | ||
|
|
||
| match test_rx.recv_timeout(Duration::from_millis(1000)) { |
There was a problem hiding this comment.
Hmm, I wonder if this will be flaky in CI. The race condition here seems like it could be problematic, right?
There was a problem hiding this comment.
I was worried the test would hand on forever (and consume CI credits) if it failed. I can increase that value to reduce chances of problems
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
crates/core/Cargo.toml (1)
41-68: 💡 Verification agent🧩 Analysis chain
Verify Solana dependency versions.
All Solana dependencies are pinned to version 2.1.10. While version pinning is good for stability, we should verify if this is the latest stable version suitable for testing.
🌐 Web query:
What is the latest stable version of Solana SDK?💡 Result:
The latest stable versions of Solana SDK components are:
JavaScript/TypeScript (
@solana/web3.js)
Version 2.0.0 (released November 7, 2024)[1][6]:
Recommended for general use after dropping its Release Candidate label[1]. This major update introduced new APIs and fixes for program error decoding and RPC subscriptions[1].Version 1.x maintenance branch:
Latest patch is v1.98.0[6], though developers are advised to upgrade to v2.x where possible[1].Rust Ecosystem
solana-sdkcrate: v2.2.0 (updated February 7, 2025)[9]solana-programcrate: v2.2.0 (updated February 7, 2025)[10]Network Client Tools
- Mainnet Beta validators: Using v1.18.x series (latest v1.18.26 as of October 2024)[3], though this repo is being phased out in favor of Agave[3].
A security incident occurred in December 2024 affecting
@solana/web3.jsversions 1.95.6-1.95.7[1]. All developers should ensure they're using at least v1.95.8 if remaining on the 1.x branch[1].Citations:
- 1: https://github.com/solana-labs/solana-web3.js/releases
- 2: https://pub.dev/packages/solana/versions
- 3: https://github.com/solana-labs/solana/releases
- 4: https://solana.com/docs/programs/rust
- 5: https://docs.rs/solana-sdk/latest/solana_sdk/
- 6: https://www.npmjs.com/package/@solana/web3.js
- 7: https://pypi.org/project/solana-sdk/
- 8: https://solana.com/docs/intro/installation
- 9: https://crates.io/crates/solana-sdk
- 10: https://crates.io/crates/solana-program
Update Solana Dependency Versions
The web query confirms that the latest stable version of the
solana-sdkcrate is v2.2.0, while the dependencies incrates/core/Cargo.tomlare pinned to 2.1.10. Consider upgrading these dependencies to ensure compatibility with the latest stable releases.
- File:
crates/core/Cargo.toml(lines 41-68)- Action: Update Solana ecosystem dependencies from
2.1.10to2.2.0
🧹 Nitpick comments (4)
crates/core/src/simnet/mod.rs (3)
285-287: Channel initialization for plugin data is appropriate.
Usingunbounded::<(Transaction, TransactionMetadata)>()ensures the system can send unlimited plugin data messages. Confirm usage patterns to avoid memory bloat if the sender outpaces the consumer.
288-383: Ensure proper error handling and resource cleanup in plugin thread.
The code spawns a thread to load multiple plugins, returning errors if configuration files or libraries fail to load. Ensure that any errors are appropriately communicated back to the main thread or logs. Also confirm that memory from dynamically loaded plugins (viaBox::from_raw(plugin_raw)) is safely released upon shutdown.Consider capturing the returned
Resultfrom the thread for better error reporting in the caller, rather than returning errors that are possibly ignored.
526-527: Ignoring the result fromtry_sendcould mask errors.
Usingtry_sendwithout handling the result may miss channel-related problems (e.g., full channel, closed receiver). Consider logging or otherwise handling send failures.crates/cli/src/cli/simnet/mod.rs (1)
204-207: Consider adding cleanup logic before breaking.While the loop break is implemented correctly, consider adding cleanup logic (e.g., resource cleanup) before breaking.
loop { if do_stop_loop.load(Ordering::Relaxed) { + // Perform any necessary cleanup + info!(ctx.expect_logger(), "Shutting down gracefully..."); break; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/release.yaml(1 hunks)crates/cli/Cargo.toml(1 hunks)crates/cli/src/cli/mod.rs(3 hunks)crates/cli/src/cli/simnet/mod.rs(4 hunks)crates/core/Cargo.toml(2 hunks)crates/core/src/lib.rs(1 hunks)crates/core/src/rpc/full.rs(2 hunks)crates/core/src/simnet/mod.rs(5 hunks)crates/core/src/types.rs(3 hunks)rust-toolchain(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- rust-toolchain
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/core/src/rpc/full.rs
- crates/core/src/lib.rs
🔇 Additional comments (18)
.github/workflows/release.yaml (1)
88-88: Verify Rust toolchain version compatibility.Let's confirm that version 1.84.0 is a stable release and check for any potential compatibility issues.
Also, let's verify if this version is consistently specified across the project:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for Rust version specifications across the project # Look for rust-toolchain files echo "Checking rust-toolchain files:" fd -t f "rust-toolchain" # Look for Rust version specifications in other configuration files echo -e "\nChecking other configuration files:" rg -l "1\.84\.0|rust" .github/ rust-toolchain*Length of output: 318
🌐 Web query:
What is the latest stable version of Rust? When was version 1.84.0 released and is it stable?💡 Result:
The latest stable version of Rust as of February 12, 2025 is 1.84.1[2][4][6].
Rust 1.84.0 was released on January 9, 2025[1][3][5][9] and was originally part of the stable channel[3][5]. However, it has since been superseded by 1.84.1 (released January 30, 2025)[6], which addressed several regressions in compiler diagnostics, incremental rebuilds, and performance issues related to trait solving[6][7].
Key details about version lineage:
- Stable channel: Currently at 1.84.x series (latest: 1.84.1)[2][4]
- Beta channel: Testing 1.85 (scheduled for February 20 release)[2][4]
- Nightly channel: Developing 1.86 features[2][4]
The original 1.84 release introduced three major features:
- Cargo resolver awareness of Minimum Supported Rust Version (MRSV)[3]
- Progress in next-generation trait solver implementation[3][7]
- New pointer provenance checking APIs[3]
While 1.84.x remains supported[5], developers should update to 1.84.1 using
rustup update stablefor optimal stability and performance[6]. The next scheduled stable release (version 1.85) is expected in February[2][4].Citations:
- 1: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html
- 2: https://www.whatrustisit.com
- 3: https://devclass.com/2025/01/14/rust-1-84-released-including-name-change-to-wasi-target-that-may-break-toolchains/
- 4: https://releases.rs
- 5: https://endoflife.date/rust
- 6: https://blog.rust-lang.org/2025/01/30/Rust-1.84.1.html
- 7: https://github.com/rust-lang/rust/releases
- 8: https://blog.rust-lang.org
- 9: https://forum.devtalk.com/t/announcing-rust-1-84-0/183445
- 10: https://doc.rust-lang.org/book/ch01-01-installation.html
Rust Toolchain Version Update Verified
- Stability: Rust 1.84.0 was released on January 9, 2025, as part of the stable channel.
- Consistency: The version is used in the GitHub Actions workflows (both in
release.yamlandrust.yml) as well as the repository’srust-toolchainfile.- Consideration: Note that while 1.84.0 is stable, the latest stable release is now 1.84.1. Ensure that using 1.84.0 meets all dependency and performance expectations.
crates/core/src/simnet/mod.rs (5)
1-3: Imports look valid and clear.
These new imports fromagave_geyser_plugin_interfaceappear correct and consistent with the usage below.
12-14: Geyser plugin manager imports are consistent.
The additions fromsolana_geyser_plugin_managermatch the subsequent usage for managing plugins.
18-24: Additional Solana transaction imports are correct.
These imports (LoadedAddresses,SanitizedTransaction, etc.) align with the usage of transaction sanitization and status metadata.Also applies to: 29-31, 34-34
218-219: Use of unsafe function pointer alias warrants caution.
DefiningPluginConstructoras anunsafe fn() -> *mut dyn GeyserPluginis valid for dynamic plugin loading but requires careful memory safety handling. Consider additional runtime checks or wrapping it in a safe abstraction to mitigate accidental misuses.
389-389: Slot time assignment is straightforward.
Copyingslot_timefromsimnet_configis consistent with the logic. No immediate concerns.crates/core/src/types.rs (5)
2-2: New import is clear.
PathBufis needed for plugin configuration paths.
4-10: Default derivation onRunloopTriggerMode.
MarkingClockas the default variant is correct, and derivingDefaultensures convenient usage.
12-17:plugin_config_pathaddition forSurfpoolConfig.
The new field accommodates multiple plugin paths. Ensure all code that initializesSurfpoolConfigsets or handlesplugin_config_pathcorrectly.
28-39:Defaultimplementation forSimnetConfigis valid.
Setting the remote RPC URL, slot time, airdrop tokens, and addresses to zero or empty is a reasonable baseline.
53-61:Defaultimplementation forRpcConfigis clear.
Using localhost and default port 8899 is a practical choice.crates/cli/src/cli/mod.rs (3)
57-57: Reduced command aliases.
Removing"run"from the alias list clarifies CLI usage and avoids confusion with other commands.
77-77: Updated short flag for host.
Switching from-hto-ohelps avoid collisions with other flags (commonly-hfor help).
109-111: Newplugin_config_pathfield.
Allowing multiple plugins to be loaded via-gis a logical extension. Confirm that the user experiences no unexpected behavior if multiple paths are specified.crates/cli/src/cli/simnet/mod.rs (3)
82-86: LGTM! Plugin configuration handling looks good.The implementation correctly maps plugin configuration paths to PathBuf, maintaining proper ownership semantics.
197-202: Good improvement in Ctrl-C handling.The implementation uses AtomicBool for thread-safe state management, ensuring clean shutdown when Ctrl-C is received.
97-97: Verify ownership transfer in start_simnet call.The removal of the reference operator suggests ownership transfer. Please ensure this aligns with the updated function signature in the core library.
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for start_simnet function definition ast-grep --pattern 'fn start_simnet($config: SurfpoolConfig, $_, $_)'Length of output: 71
Action: Manually verify the signature of
start_simnetin the core library.Our initial search for the function’s definition returned no matches, so we couldn’t automatically confirm whether its first parameter accepts an owned
SurfpoolConfigor a borrowed reference. This inconclusive result means you should manually inspect the definition ofstart_simnet(or its relevant imports) in the core library to ensure that passingconfigby value aligns with its updated signature.
- Verify in the core library that the first parameter of
start_simnetis defined to take ownership rather than a reference.- Confirm that the removal of the reference operator in the call from
crates/cli/src/cli/simnet/mod.rsis intentional and consistent with the new signature.crates/core/Cargo.toml (1)
73-76: Good addition of plugin-related dependencies.The addition of geyser plugin interface and manager dependencies aligns well with the new plugin configuration feature.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
crates/core/tests/integration.rs (1)
25-49: Add timeout to prevent test from hanging.The test could potentially hang if the Ready event is never received. Consider adding a timeout using
recv_timeoutsimilar to the approach used intest_simnet_ticks.- match simnet_events_rx.recv() { + match simnet_events_rx.recv_timeout(Duration::from_secs(10)) { Ok(SimnetEvent::Ready) => println!("Simnet is ready"), - e => panic!("Expected Ready event: {e:?}"), + Ok(e) => panic!("Expected Ready event: {e:?}"), + Err(e) => panic!("Timeout waiting for Ready event: {e:?}"), }crates/cli/src/cli/simnet/mod.rs (1)
82-87: Validate plugin configuration paths.The code collects plugin paths without validation. Consider checking that the paths exist and are readable before proceeding.
plugin_config_path: cmd .plugin_config_path .iter() - .map(|f| PathBuf::from(f)) + .map(|f| { + let path = PathBuf::from(f); + if !path.exists() { + return Err(format!("Plugin config path does not exist: {}", f)); + } + if !path.is_file() { + return Err(format!("Plugin config path is not a file: {}", f)); + } + Ok(path) + }) + .collect::<Result<Vec<_>, String>>()?crates/cli/src/tui/simnet.rs (1)
275-278: Consider tracking failed transactions.The code only tracks successful transactions. Consider tracking failed transactions as well for better monitoring.
struct App { // ... successful_transactions: u32, + failed_transactions: u32, // ... } // In the event handler: - SimnetEvent::TransactionProcessed(_dt, _transaction) => { - app.successful_transactions += 1; + SimnetEvent::TransactionProcessed(_dt, _meta, err) => { + match err { + None => app.successful_transactions += 1, + Some(_) => app.failed_transactions += 1, + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/cli/src/cli/simnet/mod.rs(5 hunks)crates/cli/src/tui/simnet.rs(1 hunks)crates/core/Cargo.toml(2 hunks)crates/core/src/simnet/mod.rs(7 hunks)crates/core/tests/integration.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/core/Cargo.toml
| #[tokio::test] | ||
| async fn test_simnet_ticks() { | ||
| let config = SurfpoolConfig { | ||
| simnet: SimnetConfig { | ||
| slot_time: 1, | ||
| ..SimnetConfig::default() | ||
| }, | ||
| ..SurfpoolConfig::default() | ||
| }; | ||
|
|
||
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | ||
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | ||
| let (test_tx, test_rx) = unbounded(); | ||
|
|
||
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | ||
| let future = start(config, simnet_events_tx, simnet_commands_rx); | ||
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | ||
| panic!("{e:?}"); | ||
| } | ||
| }); | ||
|
|
||
| let _ = hiro_system_kit::thread_named("ticks").spawn(move || { | ||
| let mut ticks = 0; | ||
| loop { | ||
| match simnet_events_rx.recv() { | ||
| Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1, | ||
| _ => (), | ||
| } | ||
|
|
||
| if ticks > 100 { | ||
| let _ = test_tx.send(true); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| match test_rx.recv_timeout(Duration::from_secs(10)) { | ||
| Ok(_) => (), | ||
| Err(_) => panic!("not enough ticks"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic numbers into named constants.
The test uses magic numbers for the tick count and timeout duration. These should be constants to improve maintainability and make the test's expectations clear.
+const EXPECTED_TICK_COUNT: usize = 100;
+const TICK_TEST_TIMEOUT_SECS: u64 = 10;
#[tokio::test]
async fn test_simnet_ticks() {
// ...
- if ticks > 100 {
+ if ticks > EXPECTED_TICK_COUNT {
let _ = test_tx.send(true);
}
// ...
- match test_rx.recv_timeout(Duration::from_secs(10)) {
+ match test_rx.recv_timeout(Duration::from_secs(TICK_TEST_TIMEOUT_SECS)) {Clean up the tick counting thread.
The thread spawned for counting ticks is never cleaned up, which could lead to resource leaks. Consider using a scope to ensure the thread is properly joined.
+ let scope = std::thread::scope(|s| {
- let _ = hiro_system_kit::thread_named("ticks").spawn(move || {
+ s.spawn(move || {
let mut ticks = 0;
loop {
match simnet_events_rx.recv() {
Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1,
_ => (),
}
if ticks > EXPECTED_TICK_COUNT {
let _ = test_tx.send(true);
+ break;
}
}
});
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[tokio::test] | |
| async fn test_simnet_ticks() { | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| slot_time: 1, | |
| ..SimnetConfig::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let (test_tx, test_rx) = unbounded(); | |
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| let _ = hiro_system_kit::thread_named("ticks").spawn(move || { | |
| let mut ticks = 0; | |
| loop { | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1, | |
| _ => (), | |
| } | |
| if ticks > 100 { | |
| let _ = test_tx.send(true); | |
| } | |
| } | |
| }); | |
| match test_rx.recv_timeout(Duration::from_secs(10)) { | |
| Ok(_) => (), | |
| Err(_) => panic!("not enough ticks"), | |
| } | |
| } | |
| const EXPECTED_TICK_COUNT: usize = 100; | |
| const TICK_TEST_TIMEOUT_SECS: u64 = 10; | |
| #[tokio::test] | |
| async fn test_simnet_ticks() { | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| slot_time: 1, | |
| ..SimnetConfig::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let (test_tx, test_rx) = unbounded(); | |
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| let scope = std::thread::scope(|s| { | |
| s.spawn(move || { | |
| let mut ticks = 0; | |
| loop { | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1, | |
| _ => (), | |
| } | |
| if ticks > EXPECTED_TICK_COUNT { | |
| let _ = test_tx.send(true); | |
| break; | |
| } | |
| } | |
| }); | |
| }); | |
| match test_rx.recv_timeout(Duration::from_secs(TICK_TEST_TIMEOUT_SECS)) { | |
| Ok(_) => (), | |
| Err(_) => panic!("not enough ticks"), | |
| } | |
| } |
crates/core/tests/integration.rs
Outdated
| } | ||
| } | ||
|
|
||
| let minimal_client = | ||
| http::connect::<MinimalClient>(format!("http://{bind_host}:{bind_port}").as_str()) | ||
| .await | ||
| .expect("Failed to connect to Surfpool"); | ||
| let full_client = | ||
| http::connect::<FullClient>(format!("http://{bind_host}:{bind_port}").as_str()) | ||
| .await | ||
| .expect("Failed to connect to Surfpool"); | ||
|
|
||
| let recent_blockhash = full_client | ||
| .get_latest_blockhash(None) | ||
| .await | ||
| .map(|r| { | ||
| Hash::from_str(r.value.blockhash.as_str()).expect("Failed to deserialize blockhash") | ||
| }) | ||
| .expect("Failed to get blockhash"); | ||
|
|
||
| let balances = join_all( | ||
| airdrop_addresses | ||
| .iter() | ||
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | ||
| ) | ||
| .await | ||
| .into_iter() | ||
| .collect::<Result<Vec<_>, _>>() | ||
| .expect("Failed to fetch balances"); | ||
|
|
||
| assert!( | ||
| balances.iter().all(|b| b.value == airdrop_token_amount), | ||
| "All addresses did not receive the airdrop" | ||
| ); | ||
|
|
||
| let _transfers = join_all(airdrop_keypairs.iter().map(|kp| { | ||
| let msg = Message::new_with_blockhash( | ||
| &[system_instruction::transfer( | ||
| &kp.pubkey(), | ||
| &airdrop_addresses[0], | ||
| airdrop_token_amount / 2, | ||
| )], | ||
| Some(&kp.pubkey()), | ||
| &recent_blockhash, | ||
| ); | ||
|
|
||
| let Ok(tx) = VersionedTransaction::try_new( | ||
| VersionedMessage::Legacy(msg), | ||
| &vec![kp.insecure_clone()], | ||
| ) else { | ||
| return Box::pin(future::err(Error::invalid_params("tx"))); | ||
| }; | ||
|
|
||
| let Ok(encoded) = bincode::serialize(&tx) else { | ||
| return Box::pin(future::err(Error::invalid_params("encoded"))); | ||
| }; | ||
| let data = bs58::encode(encoded).into_string(); | ||
|
|
||
| Box::pin(future::ready(Ok(full_client.send_transaction(data, None)))) | ||
| })) | ||
| .await | ||
| .into_iter() | ||
| .collect::<Result<Vec<_>, _>>() | ||
| .expect("Transfers failed"); | ||
|
|
||
| // Wait for all transactions to be received | ||
| // let mut processed = 0; | ||
| // loop { | ||
| // match simnet_events_rx.recv() { | ||
| // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | ||
| // _ => (), | ||
| // } | ||
|
|
||
| // if processed == airdrop_addresses.len() { | ||
| // break; | ||
| // } | ||
| // } | ||
| sleep(Duration::from_secs(2)).await; // TODO: solve flakyness | ||
|
|
||
| let final_balances = join_all( | ||
| airdrop_addresses | ||
| .iter() | ||
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | ||
| ) | ||
| .await | ||
| .into_iter() | ||
| .collect::<Result<Vec<_>, _>>() | ||
| .expect("Failed to fetch final balances"); | ||
|
|
||
| assert!( | ||
| final_balances.iter().enumerate().all(|(i, b)| { | ||
| println!("{}", b.value); | ||
| if i == 0 { | ||
| b.value > airdrop_token_amount | ||
| } else { | ||
| b.value < airdrop_token_amount / 2 | ||
| } | ||
| }), // TODO: compute fee | ||
| "Some transfers failed" | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic numbers into named constants and improve test structure.
The test uses magic numbers for amounts and array sizes, and could benefit from better structure and cleanup.
+const TEST_KEYPAIR_COUNT: usize = 10;
+const TEST_AIRDROP_AMOUNT: u64 = LAMPORTS_PER_SOL;
+const TEST_TRANSFER_AMOUNT_DIVISOR: u64 = 2;
+const TEST_RPC_PORT: u16 = 8899;
#[tokio::test]
async fn test_simnet_some_sol_transfers() {
- let airdrop_keypairs = (0..10).map(|_| Keypair::new()).collect::<Vec<_>>();
+ let airdrop_keypairs = (0..TEST_KEYPAIR_COUNT)
+ .map(|_| Keypair::new())
+ .collect::<Vec<_>>();
let airdrop_addresses: Vec<Pubkey> = airdrop_keypairs.iter().map(|kp| kp.pubkey()).collect();
- let airdrop_token_amount = LAMPORTS_PER_SOL;
let bind_host = "127.0.0.1";
- let bind_port = 8899;
+ let bind_port = TEST_RPC_PORT;
let config = SurfpoolConfig {
simnet: SimnetConfig {
slot_time: 1,
airdrop_addresses: airdrop_addresses.clone(),
- airdrop_token_amount,
+ airdrop_token_amount: TEST_AIRDROP_AMOUNT,
..SimnetConfig::default()
},
// ...
};
// ... rest of the test ...
let _transfers = join_all(airdrop_keypairs.iter().map(|kp| {
let msg = Message::new_with_blockhash(
&[system_instruction::transfer(
&kp.pubkey(),
&airdrop_addresses[0],
- airdrop_token_amount / 2,
+ TEST_AIRDROP_AMOUNT / TEST_TRANSFER_AMOUNT_DIVISOR,
)],
// ...
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[tokio::test] | |
| async fn test_simnet_some_sol_transfers() { | |
| let airdrop_keypairs = (0..10).map(|_| Keypair::new()).collect::<Vec<_>>(); | |
| let airdrop_addresses: Vec<Pubkey> = airdrop_keypairs.iter().map(|kp| kp.pubkey()).collect(); | |
| let airdrop_token_amount = LAMPORTS_PER_SOL; | |
| let bind_host = "127.0.0.1"; | |
| let bind_port = 8899; | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| slot_time: 1, | |
| airdrop_addresses: airdrop_addresses.clone(), | |
| airdrop_token_amount, | |
| ..SimnetConfig::default() | |
| }, | |
| rpc: RpcConfig { | |
| bind_host: bind_host.to_string(), | |
| bind_port, | |
| ..Default::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| loop { | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::Ready) => break, | |
| _ => (), | |
| } | |
| } | |
| let minimal_client = | |
| http::connect::<MinimalClient>(format!("http://{bind_host}:{bind_port}").as_str()) | |
| .await | |
| .expect("Failed to connect to Surfpool"); | |
| let full_client = | |
| http::connect::<FullClient>(format!("http://{bind_host}:{bind_port}").as_str()) | |
| .await | |
| .expect("Failed to connect to Surfpool"); | |
| let recent_blockhash = full_client | |
| .get_latest_blockhash(None) | |
| .await | |
| .map(|r| { | |
| Hash::from_str(r.value.blockhash.as_str()).expect("Failed to deserialize blockhash") | |
| }) | |
| .expect("Failed to get blockhash"); | |
| let balances = join_all( | |
| airdrop_addresses | |
| .iter() | |
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | |
| ) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Failed to fetch balances"); | |
| assert!( | |
| balances.iter().all(|b| b.value == airdrop_token_amount), | |
| "All addresses did not receive the airdrop" | |
| ); | |
| let _transfers = join_all(airdrop_keypairs.iter().map(|kp| { | |
| let msg = Message::new_with_blockhash( | |
| &[system_instruction::transfer( | |
| &kp.pubkey(), | |
| &airdrop_addresses[0], | |
| airdrop_token_amount / 2, | |
| )], | |
| Some(&kp.pubkey()), | |
| &recent_blockhash, | |
| ); | |
| let Ok(tx) = VersionedTransaction::try_new( | |
| VersionedMessage::Legacy(msg), | |
| &vec![kp.insecure_clone()], | |
| ) else { | |
| return Box::pin(future::err(Error::invalid_params("tx"))); | |
| }; | |
| let Ok(encoded) = bincode::serialize(&tx) else { | |
| return Box::pin(future::err(Error::invalid_params("encoded"))); | |
| }; | |
| let data = bs58::encode(encoded).into_string(); | |
| Box::pin(future::ready(Ok(full_client.send_transaction(data, None)))) | |
| })) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Transfers failed"); | |
| // Wait for all transactions to be received | |
| // let mut processed = 0; | |
| // loop { | |
| // match simnet_events_rx.recv() { | |
| // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | |
| // _ => (), | |
| // } | |
| // if processed == airdrop_addresses.len() { | |
| // break; | |
| // } | |
| // } | |
| sleep(Duration::from_secs(2)).await; // TODO: solve flakyness | |
| let final_balances = join_all( | |
| airdrop_addresses | |
| .iter() | |
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | |
| ) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Failed to fetch final balances"); | |
| assert!( | |
| final_balances.iter().enumerate().all(|(i, b)| { | |
| println!("{}", b.value); | |
| if i == 0 { | |
| b.value > airdrop_token_amount | |
| } else { | |
| b.value < airdrop_token_amount / 2 | |
| } | |
| }), // TODO: compute fee | |
| "Some transfers failed" | |
| ); | |
| const TEST_KEYPAIR_COUNT: usize = 10; | |
| const TEST_AIRDROP_AMOUNT: u64 = LAMPORTS_PER_SOL; | |
| const TEST_TRANSFER_AMOUNT_DIVISOR: u64 = 2; | |
| const TEST_RPC_PORT: u16 = 8899; | |
| #[tokio::test] | |
| async fn test_simnet_some_sol_transfers() { | |
| let airdrop_keypairs = (0..TEST_KEYPAIR_COUNT) | |
| .map(|_| Keypair::new()) | |
| .collect::<Vec<_>>(); | |
| let airdrop_addresses: Vec<Pubkey> = airdrop_keypairs.iter().map(|kp| kp.pubkey()).collect(); | |
| let bind_host = "127.0.0.1"; | |
| let bind_port = TEST_RPC_PORT; | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| slot_time: 1, | |
| airdrop_addresses: airdrop_addresses.clone(), | |
| airdrop_token_amount: TEST_AIRDROP_AMOUNT, | |
| ..SimnetConfig::default() | |
| }, | |
| rpc: RpcConfig { | |
| bind_host: bind_host.to_string(), | |
| bind_port, | |
| ..Default::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| loop { | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::Ready) => break, | |
| _ => (), | |
| } | |
| } | |
| let minimal_client = | |
| http::connect::<MinimalClient>(format!("http://{bind_host}:{bind_port}").as_str()) | |
| .await | |
| .expect("Failed to connect to Surfpool"); | |
| let full_client = | |
| http::connect::<FullClient>(format!("http://{bind_host}:{bind_port}").as_str()) | |
| .await | |
| .expect("Failed to connect to Surfpool"); | |
| let recent_blockhash = full_client | |
| .get_latest_blockhash(None) | |
| .await | |
| .map(|r| { | |
| Hash::from_str(r.value.blockhash.as_str()).expect("Failed to deserialize blockhash") | |
| }) | |
| .expect("Failed to get blockhash"); | |
| let balances = join_all( | |
| airdrop_addresses | |
| .iter() | |
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | |
| ) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Failed to fetch balances"); | |
| assert!( | |
| balances.iter().all(|b| b.value == TEST_AIRDROP_AMOUNT), | |
| "All addresses did not receive the airdrop" | |
| ); | |
| let _transfers = join_all(airdrop_keypairs.iter().map(|kp| { | |
| let msg = Message::new_with_blockhash( | |
| &[system_instruction::transfer( | |
| &kp.pubkey(), | |
| &airdrop_addresses[0], | |
| TEST_AIRDROP_AMOUNT / TEST_TRANSFER_AMOUNT_DIVISOR, | |
| )], | |
| Some(&kp.pubkey()), | |
| &recent_blockhash, | |
| ); | |
| let Ok(tx) = VersionedTransaction::try_new( | |
| VersionedMessage::Legacy(msg), | |
| &vec![kp.insecure_clone()], | |
| ) else { | |
| return Box::pin(future::err(Error::invalid_params("tx"))); | |
| }; | |
| let Ok(encoded) = bincode::serialize(&tx) else { | |
| return Box::pin(future::err(Error::invalid_params("encoded"))); | |
| }; | |
| let data = bs58::encode(encoded).into_string(); | |
| Box::pin(future::ready(Ok(full_client.send_transaction(data, None)))) | |
| })) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Transfers failed"); | |
| sleep(Duration::from_secs(2)).await; // TODO: solve flakyness | |
| let final_balances = join_all( | |
| airdrop_addresses | |
| .iter() | |
| .map(|pk| minimal_client.get_balance(pk.to_string(), None)), | |
| ) | |
| .await | |
| .into_iter() | |
| .collect::<Result<Vec<_>, _>>() | |
| .expect("Failed to fetch final balances"); | |
| assert!( | |
| final_balances.iter().enumerate().all(|(i, b)| { | |
| println!("{}", b.value); | |
| if i == 0 { | |
| b.value > TEST_AIRDROP_AMOUNT | |
| } else { | |
| b.value < TEST_AIRDROP_AMOUNT / TEST_TRANSFER_AMOUNT_DIVISOR | |
| } | |
| }), | |
| "Some transfers failed" | |
| ); | |
| } |
crates/core/tests/integration.rs
Outdated
| // let mut processed = 0; | ||
| // loop { | ||
| // match simnet_events_rx.recv() { | ||
| // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | ||
| // _ => (), | ||
| // } | ||
|
|
||
| // if processed == airdrop_addresses.len() { | ||
| // break; | ||
| // } | ||
| // } | ||
| sleep(Duration::from_secs(2)).await; // TODO: solve flakyness |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Replace sleep with proper transaction confirmation wait.
Using sleep to wait for transaction processing is flaky and could lead to inconsistent test results. The commented out code shows a better approach of waiting for transaction processing events.
Uncomment and fix the transaction processing wait code:
- // let mut processed = 0;
- // loop {
- // match simnet_events_rx.recv() {
- // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1,
- // _ => (),
- // }
-
- // if processed == airdrop_addresses.len() {
- // break;
- // }
- // }
- sleep(Duration::from_secs(2)).await; // TODO: solve flakyness
+ let mut processed = 0;
+ let timeout = Duration::from_secs(10);
+ let start = std::time::Instant::now();
+ loop {
+ match simnet_events_rx.recv_timeout(timeout.saturating_sub(start.elapsed())) {
+ Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1,
+ Ok(_) => continue,
+ Err(_) => panic!("Timeout waiting for transaction processing"),
+ }
+
+ if processed == airdrop_addresses.len() {
+ break;
+ }
+ }🌐 Web query:
What is the recommended way to wait for Solana transaction confirmation in tests?
💡 Result:
To properly wait for Solana transaction confirmation in test environments, follow these key strategies based on Solana's documentation and best practices:
1. Use Test-Specific Tools
- Leverage the Solana Test Validator ([10]) for a controlled testing environment with predictable block times (~400ms slots)
- Configure your tests to use
--commitment processedwhen interacting with local test validator nodes for faster feedback loops
2. Implement Blockheight Polling
Fetch lastValidBlockHeight when creating transactions:
const {blockhash, lastValidBlockHeight} = await connection.getLatestBlockhash();Then poll using:
async function confirmTx(connection, signature) {
await connection.confirmTransaction({
signature,
blockhash,
lastValidBlockHeight
}, 'processed');
}3. Commitment Level Configuration
- Set matching commitments for blockhash fetching and confirmation:
// Fetch with same commitment used for confirmation const blockhash = await connection.getLatestBlockhash('processed'); // Send with matching preflight commitment await connection.sendTransaction(tx, {preflightCommitment: 'processed'});
4. Retry Logic Considerations
- Avoid using
skipPreflight: true([3]) even in tests to catch signature/blockhash issues early - Implement explicit retries rather than relying on RPC node retries:
const retries = 3; while (retries-- > 0) { try { return await confirmTx(connection, signature); } catch (e) { if (!e.message.includes('not found')) throw e; await sleep(1000); } }
Key Testing Considerations
- Blockhash Expiration: Tests should complete within ~60 seconds of transaction creation ([1][7])
- Finality Assurance: Use
finalizedcommitment only when testing chain reorganizations - Network Simulation: Introduce artificial delays (100-500ms) between transactions to mimic mainnet conditions
For comprehensive testing validation:
const confirmation = await connection.getSignatureStatus(signature, {
searchTransactionHistory: true
});
if (confirmation.value?.confirmationStatus === 'finalized') {
// Test assertion passed
}These methods ensure reliable transaction confirmation detection while maintaining test execution speed through proper use of commitment levels and local network emulation[1][3][7][10].
Citations:
- 1: https://solana.com/docs/advanced/confirmation
- 2: https://www.c-sharpcorner.com/article/transaction-confirmation-and-expiration-in-solana/
- 3: https://solana.com/docs/advanced/retry
- 4: https://chainstack.com/solana-transaction-commitment-levels/
- 5: https://docs.dynamic.xyz/wallets/using-wallets/solana/solana-transaction-tips
- 6: The solana-web3.js transaction confirmation logic is very broken solana-labs/solana#23949
- 7: https://omniatech.io/pages/solana-transaction-confirmation/
- 8: https://www.quicknode.com/guides/solana-development/transactions/how-to-optimize-solana-transactions
- 9: https://solana.com/docs/core/transactions
- 10: https://solana.com/developers/guides/getstarted/solana-test-validator
Replace sleep with a robust transaction confirmation mechanism
Using sleep(Duration::from_secs(2)).await for waiting on transaction confirmations is unreliable and can introduce flakiness in tests. Based on best practices from Solana’s documentation, tests should actively poll for transaction confirmation rather than using an arbitrary delay. The previously commented-out code indicates a better approach—waiting on SimnetEvent::TransactionProcessed events—but it now needs proper timeout handling and cleanup of magic numbers.
Actionable changes:
- Uncomment and enhance the event-waiting loop: Instead of using a fixed sleep duration, actively wait for events using a timeout (e.g., 10 seconds). This aligns with the recommended strategy to avoid arbitrary delays and provides immediate feedback when confirmations do not occur as expected.
- Remove magic numbers: Replace the hard-coded 2-second delay with a clearly defined timeout, ensuring the test reacts appropriately to transaction delays.
Example change:
- // let mut processed = 0;
- // loop {
- // match simnet_events_rx.recv() {
- // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1,
- // _ => (),
- // }
-
- // if processed == airdrop_addresses.len() {
- // break;
- // }
- // }
- sleep(Duration::from_secs(2)).await; // TODO: solve flakyness
+ let mut processed = 0;
+ let timeout = Duration::from_secs(10);
+ let start = Instant::now();
+ loop {
+ match simnet_events_rx.recv_timeout(timeout.saturating_sub(start.elapsed())) {
+ Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1,
+ Ok(_) => continue,
+ Err(_) => panic!("Timeout waiting for transaction processing"),
+ }
+
+ if processed == airdrop_addresses.len() {
+ break;
+ }
+ }This revised approach offers a more deterministic and maintainable strategy for processing confirmation events in your tests.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // let mut processed = 0; | |
| // loop { | |
| // match simnet_events_rx.recv() { | |
| // Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | |
| // _ => (), | |
| // } | |
| // if processed == airdrop_addresses.len() { | |
| // break; | |
| // } | |
| // } | |
| sleep(Duration::from_secs(2)).await; // TODO: solve flakyness | |
| let mut processed = 0; | |
| let timeout = Duration::from_secs(10); | |
| let start = Instant::now(); | |
| loop { | |
| match simnet_events_rx.recv_timeout(timeout.saturating_sub(start.elapsed())) { | |
| Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | |
| Ok(_) => continue, | |
| Err(_) => panic!("Timeout waiting for transaction processing"), | |
| } | |
| if processed == airdrop_addresses.len() { | |
| break; | |
| } | |
| } |
crates/cli/src/cli/simnet/mod.rs
Outdated
| let stop_loop = Arc::new(AtomicBool::new(false)); | ||
| let do_stop_loop = stop_loop.clone(); | ||
| ctrlc::set_handler(move || { | ||
| stop_loop.store(true, Ordering::Relaxed); | ||
| }) | ||
| .expect("Error setting Ctrl-C handler"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve Ctrl-C handler error handling.
The code ignores potential errors from the Ctrl-C handler setup. Consider proper error handling.
- ctrlc::set_handler(move || {
+ ctrlc::set_handler(move || {
stop_loop.store(true, Ordering::Relaxed);
- })
- .expect("Error setting Ctrl-C handler");
+ })
+ .map_err(|e| format!("Failed to set Ctrl-C handler: {}", e))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let stop_loop = Arc::new(AtomicBool::new(false)); | |
| let do_stop_loop = stop_loop.clone(); | |
| ctrlc::set_handler(move || { | |
| stop_loop.store(true, Ordering::Relaxed); | |
| }) | |
| .expect("Error setting Ctrl-C handler"); | |
| let stop_loop = Arc::new(AtomicBool::new(false)); | |
| let do_stop_loop = stop_loop.clone(); | |
| ctrlc::set_handler(move || { | |
| stop_loop.store(true, Ordering::Relaxed); | |
| }) | |
| .map_err(|e| format!("Failed to set Ctrl-C handler: {}", e))?; |
crates/core/src/simnet/mod.rs
Outdated
| let transaction = SanitizedTransaction::try_from_legacy_transaction(transaction, &HashSet::new()) | ||
| .unwrap(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for transaction sanitization.
The code unwraps the result of transaction sanitization without proper error handling.
- let transaction = SanitizedTransaction::try_from_legacy_transaction(transaction, &HashSet::new())
- .unwrap();
+ let transaction = SanitizedTransaction::try_from_legacy_transaction(transaction, &HashSet::new())
+ .map_err(|e| GeyserPluginManagerError::TransactionError(format!(
+ "Failed to sanitize transaction: {}", e
+ )))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let transaction = SanitizedTransaction::try_from_legacy_transaction(transaction, &HashSet::new()) | |
| .unwrap(); | |
| let transaction = SanitizedTransaction::try_from_legacy_transaction(transaction, &HashSet::new()) | |
| .map_err(|e| GeyserPluginManagerError::TransactionError(format!( | |
| "Failed to sanitize transaction: {}", e | |
| )))?; |
crates/core/src/simnet/mod.rs
Outdated
| let (plugin, lib) = unsafe { | ||
| let lib = Library::new(libpath) | ||
| .map_err(|e| GeyserPluginManagerError::PluginLoadError(e.to_string()))?; | ||
| let constructor: Symbol<PluginConstructor> = lib | ||
| .get(b"_create_plugin") | ||
| .map_err(|e| GeyserPluginManagerError::PluginLoadError(e.to_string()))?; | ||
| let plugin_raw = constructor(); | ||
| (Box::from_raw(plugin_raw), lib) | ||
| }; | ||
| plugin_manager.plugins.push(LoadedGeyserPlugin::new(lib, plugin, plugin_name)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve plugin loading error handling and resource cleanup.
The unsafe plugin loading code could benefit from better error handling and resource cleanup using Drop.
+struct PluginHandle {
+ lib: Library,
+ plugin: Box<dyn GeyserPlugin>,
+}
+
+impl Drop for PluginHandle {
+ fn drop(&mut self) {
+ // Plugin cleanup code here
+ }
+}
- let (plugin, lib) = unsafe {
+ let plugin_handle = unsafe {
let lib = Library::new(libpath)
.map_err(|e| GeyserPluginManagerError::PluginLoadError(e.to_string()))?;
let constructor: Symbol<PluginConstructor> = lib
.get(b"_create_plugin")
.map_err(|e| GeyserPluginManagerError::PluginLoadError(e.to_string()))?;
let plugin_raw = constructor();
- (Box::from_raw(plugin_raw), lib)
+ PluginHandle {
+ lib,
+ plugin: Box::from_raw(plugin_raw),
+ }
};
- plugin_manager.plugins.push(LoadedGeyserPlugin::new(lib, plugin, plugin_name));
+ plugin_manager.plugins.push(LoadedGeyserPlugin::new(
+ plugin_handle.lib,
+ plugin_handle.plugin,
+ plugin_name
+ ));Committable suggestion skipped: line range outside the PR's diff.
|
I wanted to implement the integration test suggested by @MicaiahReid before moving to build simple Anchor test programs but ran into an issue I can't figure out... (https://github.com/txtx/surfpool/pull/34/files#diff-a99b0f7e34db21b03fc510b5abebc18c48817d00dc8f2b32ef6f8aa866dd8021R194) |
crates/core/tests/integration.rs
Outdated
| let airdrop_addresses: Vec<Pubkey> = airdrop_keypairs.iter().map(|kp| kp.pubkey()).collect(); | ||
| let airdrop_token_amount = LAMPORTS_PER_SOL; | ||
| let bind_host = "127.0.0.1"; | ||
| let bind_port = 8899; |
There was a problem hiding this comment.
This is good for now, but for the future, we may want to find a free port. If we have a full test suite, tests will take forever without parallelization, so we'll need variable ports. I did this in a past project: https://github.com/hirosystems/chainhook/blob/4e4960897983206c376bb7d94a9ba727802ec043/components/chainhook-cli/src/service/tests/helpers/mod.rs#L25-L34
crates/core/tests/integration.rs
Outdated
| .expect("Transfers failed"); | ||
|
|
||
| // Wait for all transactions to be received | ||
| // let mut processed = 0; |
There was a problem hiding this comment.
Is there a chance that because we've awaited above, one of them has already emitted the processed event, so we're too late when we start the loop below?
Maybe we need to start that loop in a thread before sending the transfers, and the test will fail if that thread isn't closed within a certain amount of time?
There was a problem hiding this comment.
Sorry I forgot to ping: @Dodecahedr0x
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
crates/core/tests/integration.rs (2)
198-213:⚠️ Potential issueFix potential race condition in transaction processing.
The current implementation might miss transaction events that occur before the event listener is set up.
Move the transaction processing listener before sending transactions:
+ // Start listening for transaction events before sending transactions + let tx_processor = task::spawn_blocking(move || { + let mut processed = 0; + loop { + match simnet_events_rx.recv() { + Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, + _ => (), + } + + if processed == expected { + break; + } + } + }); let _transfers = join_all(airdrop_keypairs.iter().map(|kp| { // ... transfer logic ... })) .await .into_iter() .collect::<Result<Vec<_>, _>>() .expect("Transfers failed"); - // Wait for all transactions to be received - let expected = airdrop_addresses.len(); - let _ = task::spawn_blocking(move || { - let mut processed = 0; - loop { - match simnet_events_rx.recv() { - Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, - _ => (), - } - - if processed == expected { - break; - } - } - }) - .await; + // Wait for all transactions to be processed + tx_processor.await.expect("Transaction processing failed");
54-93: 🛠️ Refactor suggestionImprove test reliability and maintainability.
The test has several areas for improvement:
- Magic numbers should be constants
- Thread cleanup is missing
- Potential race condition in tick counting
Apply this diff to address the issues:
+const EXPECTED_TICK_COUNT: usize = 100; +const TICK_TEST_TIMEOUT_SECS: u64 = 10; + #[tokio::test] async fn test_simnet_ticks() { // ... config setup ... - let _ = hiro_system_kit::thread_named("ticks").spawn(move || { + let tick_counter = std::thread::scope(|s| { + s.spawn(|| { let mut ticks = 0; loop { match simnet_events_rx.recv() { Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1, _ => (), } - if ticks > 100 { + if ticks > EXPECTED_TICK_COUNT { let _ = test_tx.send(true); + break; } } - }); + }); + }); - match test_rx.recv_timeout(Duration::from_secs(10)) { + match test_rx.recv_timeout(Duration::from_secs(TICK_TEST_TIMEOUT_SECS)) { Ok(_) => (), Err(_) => panic!("not enough ticks"), }
🧹 Nitpick comments (2)
crates/core/tests/integration.rs (2)
97-101: Extract magic numbers into named constants.The test uses magic numbers for configuration values that should be constants.
Apply this diff:
+const TEST_ADDRESS_COUNT: usize = 10; +const TEST_AIRDROP_AMOUNT: u64 = LAMPORTS_PER_SOL; + #[tokio::test] async fn test_simnet_some_sol_transfers() { - let n_addresses = 10; - let airdrop_keypairs = (0..n_addresses).map(|_| Keypair::new()).collect::<Vec<_>>(); + let airdrop_keypairs = (0..TEST_ADDRESS_COUNT) + .map(|_| Keypair::new()) + .collect::<Vec<_>>(); let airdrop_addresses: Vec<Pubkey> = airdrop_keypairs.iter().map(|kp| kp.pubkey()).collect(); - let airdrop_token_amount = LAMPORTS_PER_SOL; + let airdrop_token_amount = TEST_AIRDROP_AMOUNT;
224-234: Remove debug print statement from assertion.The print statement in the assertion block should be removed or replaced with proper logging.
Apply this diff:
assert!( final_balances.iter().enumerate().all(|(i, b)| { - println!("{}", b.value); if i == 0 { b.value > airdrop_token_amount } else { b.value < airdrop_token_amount / 2 } }), // TODO: compute fee "Some transfers failed" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/core/tests/helpers.rs(1 hunks)crates/core/tests/integration.rs(1 hunks)
🔇 Additional comments (2)
crates/core/tests/helpers.rs (1)
3-12: LGTM! Well-implemented helper function.The implementation follows best practices by:
- Using port 0 to let the OS assign an ephemeral port
- Properly freeing the port by dropping the listener
- Providing descriptive error messages
crates/core/tests/integration.rs (1)
28-52: LGTM! Clean and focused test implementation.The test effectively validates the simulation readiness by:
- Preventing automatic ticks using
RunloopTriggerMode::Manual- Properly handling simulation events
- Providing clear error messages
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/core/tests/integration.rs (1)
75-87:⚠️ Potential issueFix potential thread hang in tick counting.
The tick counting thread runs in an infinite loop without any exit condition, which could cause the thread to hang indefinitely if the test fails.
let _ = hiro_system_kit::thread_named("ticks").spawn(move || { let mut ticks = 0; loop { - match simnet_events_rx.recv() { + match simnet_events_rx.recv_timeout(Duration::from_secs(10)) { Ok(SimnetEvent::ClockUpdate(_)) => ticks += 1, + Err(_) => break, // Exit if no events received within timeout _ => (), } if ticks > 100 { let _ = test_tx.send(true); + break; // Exit after sending the signal } } });
🧹 Nitpick comments (1)
crates/core/tests/integration.rs (1)
224-233: Improve balance assertions with fee calculations.The current assertions use hard-coded comparisons and don't account for transaction fees. This could make the test brittle.
+ const LAMPORTS_PER_SIGNATURE: u64 = 5000; + let transfer_amount = airdrop_token_amount / 2; + let fee_per_transfer = LAMPORTS_PER_SIGNATURE; + assert!( final_balances.iter().enumerate().all(|(i, b)| { if i == 0 { - b.value > airdrop_token_amount + // First account receives transfers from all other accounts + let expected = airdrop_token_amount + + (transfer_amount * (n_addresses - 1) as u64); + b.value == expected } else { - b.value < airdrop_token_amount / 2 + // Other accounts send transfer and pay fee + let expected = airdrop_token_amount - + transfer_amount - fee_per_transfer; + b.value == expected } - }), // TODO: compute fee + }), "Some transfers failed" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/core/tests/integration.rs(1 hunks)
🔇 Additional comments (1)
crates/core/tests/integration.rs (1)
1-27: LGTM! Well-organized imports.The imports are logically grouped and include all necessary dependencies for Solana SDK integration, JSON-RPC communication, and threading operations.
| let expected = airdrop_addresses.len(); | ||
| let _ = task::spawn_blocking(move || { | ||
| let mut processed = 0; | ||
| loop { | ||
| match simnet_events_rx.recv() { | ||
| Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | ||
| _ => (), | ||
| } | ||
|
|
||
| if processed == expected { | ||
| break; | ||
| } | ||
| } | ||
| }) | ||
| .await; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve transaction confirmation handling with timeout and error handling.
The current implementation could hang indefinitely if not all transactions are processed. Add proper timeout and error handling.
- let _ = task::spawn_blocking(move || {
+ let confirmation_handle = task::spawn_blocking(move || {
let mut processed = 0;
+ let timeout = Duration::from_secs(30);
+ let start = std::time::Instant::now();
loop {
- match simnet_events_rx.recv() {
+ match simnet_events_rx.recv_timeout(timeout.saturating_sub(start.elapsed())) {
Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1,
+ Err(_) => return Err("Timeout waiting for transaction confirmation"),
_ => (),
}
if processed == expected {
- break;
+ return Ok(());
}
}
- })
- .await;
+ });
+
+ confirmation_handle
+ .await
+ .expect("Failed to join confirmation task")
+ .expect("Failed to confirm all transactions");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let expected = airdrop_addresses.len(); | |
| let _ = task::spawn_blocking(move || { | |
| let mut processed = 0; | |
| loop { | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | |
| _ => (), | |
| } | |
| if processed == expected { | |
| break; | |
| } | |
| } | |
| }) | |
| .await; | |
| let expected = airdrop_addresses.len(); | |
| let confirmation_handle = task::spawn_blocking(move || { | |
| let mut processed = 0; | |
| let timeout = Duration::from_secs(30); | |
| let start = std::time::Instant::now(); | |
| loop { | |
| match simnet_events_rx.recv_timeout(timeout.saturating_sub(start.elapsed())) { | |
| Ok(SimnetEvent::TransactionProcessed(..)) => processed += 1, | |
| Err(_) => return Err("Timeout waiting for transaction confirmation"), | |
| _ => (), | |
| } | |
| if processed == expected { | |
| return Ok(()); | |
| } | |
| } | |
| }); | |
| confirmation_handle | |
| .await | |
| .expect("Failed to join confirmation task") | |
| .expect("Failed to confirm all transactions"); |
| #[tokio::test] | ||
| async fn test_simnet_ready() { | ||
| let config = SurfpoolConfig { | ||
| simnet: SimnetConfig { | ||
| runloop_trigger_mode: RunloopTriggerMode::Manual, // Prevent ticks | ||
| ..SimnetConfig::default() | ||
| }, | ||
| ..SurfpoolConfig::default() | ||
| }; | ||
|
|
||
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | ||
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | ||
|
|
||
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | ||
| let future = start(config, simnet_events_tx, simnet_commands_rx); | ||
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | ||
| panic!("{e:?}"); | ||
| } | ||
| }); | ||
|
|
||
| match simnet_events_rx.recv() { | ||
| Ok(SimnetEvent::Ready) => (), | ||
| e => panic!("Expected Ready event: {e:?}"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add thread cleanup to prevent resource leaks.
The test spawns a thread but doesn't ensure it's properly cleaned up, which could lead to resource leaks in the test suite.
- let _handle = hiro_system_kit::thread_named("test").spawn(move || {
+ let handle = hiro_system_kit::thread_named("test").spawn(move || {
let future = start(config, simnet_events_tx, simnet_commands_rx);
if let Err(e) = hiro_system_kit::nestable_block_on(future) {
panic!("{e:?}");
}
});
+
+ // Ensure thread cleanup after the test
+ defer! {
+ handle.join().expect("Failed to join test thread");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[tokio::test] | |
| async fn test_simnet_ready() { | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| runloop_trigger_mode: RunloopTriggerMode::Manual, // Prevent ticks | |
| ..SimnetConfig::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let _handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::Ready) => (), | |
| e => panic!("Expected Ready event: {e:?}"), | |
| } | |
| } | |
| #[tokio::test] | |
| async fn test_simnet_ready() { | |
| let config = SurfpoolConfig { | |
| simnet: SimnetConfig { | |
| runloop_trigger_mode: RunloopTriggerMode::Manual, // Prevent ticks | |
| ..SimnetConfig::default() | |
| }, | |
| ..SurfpoolConfig::default() | |
| }; | |
| let (simnet_events_tx, simnet_events_rx) = unbounded(); | |
| let (_simnet_commands_tx, simnet_commands_rx) = unbounded(); | |
| let handle = hiro_system_kit::thread_named("test").spawn(move || { | |
| let future = start(config, simnet_events_tx, simnet_commands_rx); | |
| if let Err(e) = hiro_system_kit::nestable_block_on(future) { | |
| panic!("{e:?}"); | |
| } | |
| }); | |
| // Ensure thread cleanup after the test | |
| defer! { | |
| handle.join().expect("Failed to join test thread"); | |
| } | |
| match simnet_events_rx.recv() { | |
| Ok(SimnetEvent::Ready) => (), | |
| e => panic!("Expected Ready event: {e:?}"), | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
crates/cli/src/tui/simnet.rs (1)
203-211:⚠️ Potential issueImprove error handling in the transaction processing thread.
The current implementation uses
unwrap()which could cause panics, and the thread continues running even after errors.Consider implementing proper error handling and graceful shutdown:
let _ = hiro_system_kit::thread_named("break solana").spawn(move || { + let mut consecutive_errors = 0; + const MAX_ERRORS: u32 = 3; while let Ok((message, keypair)) = rx.recv() { - let client = - RpcClient::new_with_commitment(&rpc_api_url, CommitmentConfig::processed()); - let blockhash = client.get_latest_blockhash().unwrap(); - let transaction = Transaction::new(&[keypair], message, blockhash); - let _ = client.send_transaction(&transaction).unwrap(); + match (|| -> Result<_, Box<dyn std::error::Error>> { + let client = RpcClient::new_with_commitment(&rpc_api_url, CommitmentConfig::processed()); + let blockhash = client.get_latest_blockhash()?; + let transaction = Transaction::new(&[keypair], message, blockhash); + client.send_transaction(&transaction)?; + Ok(()) + })() { + Ok(_) => consecutive_errors = 0, + Err(e) => { + eprintln!("Error processing transaction: {}", e); + consecutive_errors += 1; + if consecutive_errors >= MAX_ERRORS { + eprintln!("Too many consecutive errors, stopping transaction processing"); + break; + } + } + } } });
🧹 Nitpick comments (2)
crates/cli/src/tui/simnet.rs (2)
467-491: Enhance transaction statistics display.The UI only shows successful transactions count, which provides incomplete feedback about system health.
Consider adding failed transactions count and success rate:
let infos = vec![ Line::from(vec![ Span::styled("۬", app.colors.white), Span::styled( - format!("{} ", app.successful_transactions), + format!("{}/{} ", app.successful_transactions, app.total_transactions), app.colors.accent, ), - Span::styled("transactions processed", app.colors.white), + Span::styled(format!("transactions processed ({}% success)", + if app.total_transactions > 0 { + (app.successful_transactions as f64 / app.total_transactions as f64 * 100.0) as u32 + } else { + 0 + } + ), app.colors.white), ]), ];
347-357: Add user feedback for transaction breaking action.When the user triggers transaction breaking with 'f' or 'j', there's no feedback that the action was taken.
Add visual feedback when transaction breaking is triggered:
Char('f') | Char('j') => { // Break Solana let sender = app.breaker.as_ref().unwrap(); + app.events.push_front(( + EventType::Info, + Local::now(), + "Transaction breaking triggered".to_string(), + )); let instruction = system_instruction::transfer( &sender.pubkey(), &Pubkey::new_unique(), 100, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/cli/src/cli/simnet/mod.rs(1 hunks)crates/cli/src/tui/simnet.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/cli/src/cli/simnet/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: run_cargo_checks
| SimnetEvent::TransactionReceived(_dt, _transaction) => {} | ||
| SimnetEvent::TransactionProcessed(_dt, _meta, _err) => { | ||
| app.successful_transactions += 1; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling and logging for failed transactions.
The current implementation only tracks successful transactions but doesn't handle or log failed transactions, which could make debugging issues more difficult.
Consider modifying the TransactionProcessed handler to track both successful and failed transactions:
- SimnetEvent::TransactionProcessed(_dt, _meta, _err) => {
- app.successful_transactions += 1;
- }
+ SimnetEvent::TransactionProcessed(dt, meta, err) => {
+ if err.is_none() {
+ app.successful_transactions += 1;
+ } else {
+ app.events.push_front((
+ EventType::Failure,
+ dt,
+ format!("Transaction failed: {:?}", err),
+ ));
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SimnetEvent::TransactionReceived(_dt, _transaction) => {} | |
| SimnetEvent::TransactionProcessed(_dt, _meta, _err) => { | |
| app.successful_transactions += 1; | |
| } | |
| SimnetEvent::TransactionReceived(_dt, _transaction) => {} | |
| SimnetEvent::TransactionProcessed(dt, meta, err) => { | |
| if err.is_none() { | |
| app.successful_transactions += 1; | |
| } else { | |
| app.events.push_front(( | |
| EventType::Failure, | |
| dt, | |
| format!("Transaction failed: {:?}", err), | |
| )); | |
| } | |
| } |
starting to work on tests, created this PR to have some feedbacks on the structure
Summary by CodeRabbit
New Features
Improvements
Tests