feat(torii): check all contracts deployment on startup - #3134
Conversation
WalkthroughOhayo, sensei! The changes introduce a new asynchronous function, Changes
Sequence Diagram(s)sequenceDiagram
participant R as Runner
participant J as JsonRpcClient
participant V as verify_contracts_deployed
participant BC as Blockchain
R->>J: Instantiate JsonRpcClient
R->>V: Call verify_contracts_deployed(contracts)
V->>J: Query contract info at pending BlockTag
J-->>V: Return contract data or error
V-->>R: Return list of undeployed contracts
alt Undeployed contracts found
R-->>R: Error returned with undeployed contract details
else All contracts deployed
R->>R: Continue execution
end
Suggested reviewers
🪧 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
🧹 Nitpick comments (1)
crates/torii/runner/src/lib.rs (1)
354-370: Solid implementation of contract verification, but consider a few enhancements.The function effectively checks if contracts are deployed by attempting to retrieve their class at the pending block tag. However, a few improvements could make this even better:
- Consider logging the specific errors to help diagnose deployment issues
- For a large number of contracts, parallel checks might be more efficient
async fn verify_contracts_deployed( provider: &JsonRpcClient<HttpTransport>, contracts: &[Contract], ) -> anyhow::Result<Vec<Contract>> { let mut undeployed = Vec::new(); for contract in contracts { - match provider.get_class_at(BlockId::Tag(BlockTag::Pending), contract.address).await { - Ok(_) => continue, - Err(_) => { - undeployed.push(contract.clone()); - } - } + match provider.get_class_at(BlockId::Tag(BlockTag::Pending), contract.address).await { + Ok(_) => continue, + Err(err) => { + tracing::debug!( + target: crate::constants::LOG_TARGET, + address = %contract.address, + error = %err, + "Contract not deployed" + ); + undeployed.push(contract.clone()); + } + } } Ok(undeployed) }For larger contract sets, you might consider using
futures::future::join_allfor parallel verification, sensei!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/torii/runner/src/lib.rs(3 hunks)
🔇 Additional comments (2)
crates/torii/runner/src/lib.rs (2)
27-29: Ohayo! Nice addition of necessary imports for contract verification.The new imports for
BlockId,BlockTag, andProviderare necessary for the contract deployment verification functionality. Good choice, sensei!
98-108: Early verification of contract deployment - smart approach, sensei!Moving the provider creation earlier and adding contract deployment verification before proceeding with other initialization steps is an excellent defensive programming practice. This will prevent runtime issues by failing fast if any contracts are not properly deployed.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3134 +/- ##
==========================================
- Coverage 55.74% 55.72% -0.03%
==========================================
Files 443 443
Lines 62750 62770 +20
==========================================
- Hits 34983 34976 -7
- Misses 27767 27794 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/runner/src/lib.rs (1)
354-370: Clean and effective implementation, sensei!The new
verify_contracts_deployedfunction is well-structured and focused. It:
- Accepts a provider and contracts slice
- Queries each contract's class at the pending block
- Collects undeployed contracts into a vector
- Returns the result
A small suggestion to make error handling more informative:
- match provider.get_class_at(BlockId::Tag(BlockTag::Pending), contract.address).await { - Ok(_) => continue, - Err(_) => { + match provider.get_class_at(BlockId::Tag(BlockTag::Pending), contract.address).await { + Ok(_) => continue, + Err(err) => { + tracing::debug!( + target: LOG_TARGET, + "Contract at address {} not deployed: {:?}", + contract.address, + err + ); undeployed.push(*contract); } }This would log the specific error for each undeployed contract, which could help with troubleshooting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
crates/torii/runner/src/lib.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
crates/torii/runner/src/lib.rs (2)
27-27: Ohayo, sensei! Nice work on the necessary imports.The additions of
BlockId,BlockTagfrom thestarknet::core::typesmodule andProviderfromstarknet::providerssupport the new contract deployment verification functionality. These imports are precisely what's needed for the new feature.Also applies to: 29-29
98-108: Great improvement to startup validation, sensei!The code now creates an Arc-wrapped JsonRpcClient and verifies all contracts are deployed before proceeding with the application setup. This is excellent defensive programming that will prevent runtime issues when contracts aren't available. The error message clearly indicates which contracts are missing, making debugging easier.
glihm
left a comment
glihm
left a comment
There was a problem hiding this comment.
This may slow down a bit the startup based on the provider and number of contracts, but for sure better to know that the contracts are actually there before working on them.
2 participants
#3122
Summary by CodeRabbit
Summary by CodeRabbit