test(katana-rpc): get starknet pending block #2772
Conversation
WalkthroughOhayo, sensei! This pull request introduces two new asynchronous test functions in the StarkNet testing suite. The first function, Changes
Possibly related PRs
🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
crates/katana/rpc/rpc/tests/starknet.rs (2)
956-1056: Ohayo! Well-structured test with comprehensive coverage, sensei!The test effectively verifies pending block behavior across different scenarios. However, consider these improvements:
- Replace
.unwrap()calls with proper error handling using?operator for better test maintenance.- Consider extracting the verification logic into helper functions to reduce code duplication across the three block types.
Example helper function:
async fn verify_pending_block<T>( block: &T, expected_txs: &[Felt], expected_parent_hash: Felt, ) where T: HasTransactions + HasParentHash, { assert_eq!(block.transactions().len(), expected_txs.len()); assert_eq!(block.parent_hash(), expected_parent_hash); // Add transaction verification logic }
1060-1148: Consider improving error handling and test organization.Similar to the previous test:
- Replace
.unwrap()calls with?operator- Consider extracting common verification logic into helper functions
- Add assertions to verify block numbers in instant mode
Example improvement:
// Replace unwraps with proper error handling let res = contract.transfer(&recipient, &amount) .send() .await?; dojo_utils::TransactionWaiter::new(res.transaction_hash, &provider) .await?;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
crates/katana/rpc/rpc/tests/starknet.rs(2 hunks)
🔇 Additional comments (2)
crates/katana/rpc/rpc/tests/starknet.rs (2)
26-28: Ohayo! The new imports look good, sensei!
The pending block types are properly imported and will be used effectively in the new test functions.
1058-1059: Excellent documentation of instant mining mode behavior, sensei!
The comment clearly explains the expected behavior, which helps future maintainers understand the test's purpose.
kariy
force-pushed
the
katana/pending-block-test
branch
from
01310d4 to
ca65ff7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
crates/katana/core/src/backend/gas_oracle.rs (2)
286-286: Consider documenting the external assumption in the ignore attribute.The
#[ignore]attribute should include more specific information about what external assumption is required.-#[ignore = "Requires external assumption"] +#[ignore = "Requires external L1 node at eth.merkle.io"]
Line range hint
287-324: Test could be more robust with additional scenarios.While the test covers basic functionality, consider:
- The 9-second sleep might be too short for real-world block times
- Missing error case testing (e.g., unavailable L1 provider)
- No validation of STRK price updates
Consider adding error case testing:
+#[tokio::test] +#[ignore = "Requires external L1 node"] +async fn test_gas_oracle_error_cases() { + let url = Url::parse("https://invalid-url.example.com").expect("Invalid URL"); + let oracle = L1GasOracle::sampled(url.clone()); + let shared_prices = match &oracle { + L1GasOracle::Sampled(sampled) => sampled.prices.clone(), + _ => panic!("Expected sampled oracle"), + }; + + let mut worker = GasOracleWorker::new(shared_prices.clone(), url); + let result = worker.update_once().await; + assert!(result.is_err(), "Should fail with invalid URL"); +}crates/katana/rpc/rpc/tests/starknet.rs (2)
956-1056: Ohayo sensei! Consider adding error case testing.The test thoroughly covers the happy path but could benefit from testing error scenarios:
- Invalid block IDs
- Network issues
- Race conditions
Consider adding error case tests:
+#[tokio::test] +async fn fetch_pending_blocks_error_cases() { + let config = get_default_test_config(SequencingConfig { no_mining: true, ..Default::default() }); + let sequencer = TestSequencer::start(config).await; + let provider = sequencer.provider(); + + // Test invalid block ID + let block_id = BlockId::Number(u64::MAX); + let result = provider.get_block_with_txs(block_id).await; + assert!(result.is_err(), "Should fail with invalid block number"); +}
1059-1148: Consider adding more assertions for block metadata.While the test covers the core functionality well, it could benefit from additional assertions:
- Block timestamps
- Block numbers
- Gas prices
Add more assertions to strengthen the test:
if let MaybePendingBlockWithTxs::Block(block) = block_with_txs { assert_eq!(block.transactions.len(), 1); assert_eq!(block.parent_hash, latest_block_hash); assert_eq!(*block.transactions[0].transaction_hash(), res.transaction_hash); + assert!(block.timestamp > 0, "Block timestamp should be set"); + assert_eq!(block.block_number, latest_block_hash_n_num.block_number + 1); } else { panic!("expected pending block with transactions") }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
crates/katana/core/src/backend/gas_oracle.rs(1 hunks)crates/katana/rpc/rpc/tests/starknet.rs(2 hunks)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2772 +/- ##
==========================================
+ Coverage 55.57% 55.67% +0.09%
==========================================
Files 434 434
Lines 54536 54536
==========================================
+ Hits 30311 30364 +53
+ Misses 24225 24172 -53 ☔ View full report in Codecov by Sentry. |
ref #2763
Summary by CodeRabbit
New Features
Tests
fetch_pending_blocksandfetch_pending_blocks_in_instant_modetests for pending block verification.test_gas_oracleto assess the functionality of the gas oracle.