feat(torii-indexer): relation between txns and contracts #3055
Conversation
|
Ohayo, sensei! WalkthroughThe changes introduce a new parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant E as Engine
participant TP as TransactionProcessor
participant DB as Database
E->>TP: process_transaction(..., contract_addresses)
TP->>DB: store_transaction(..., contract_addresses)
DB-->>TP: Acknowledge storage
TP-->>E: Return processing result
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 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: 2
🧹 Nitpick comments (1)
crates/torii/migrations/20250220094437_transactions_contract.sql (1)
1-1: Ohayo sensei, solid migration addition!
The SQL statement correctly adds a newcontract_addresscolumn to thetransactionstable. Everything looks good! As a follow-up, consider if an index on this column might be beneficial in the future if queries frequently filter by contract address.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/torii/indexer/src/engine.rs(3 hunks)crates/torii/indexer/src/processors/mod.rs(1 hunks)crates/torii/indexer/src/processors/store_transaction.rs(1 hunks)crates/torii/migrations/20250220094437_transactions_contract.sql(1 hunks)crates/torii/sqlite/src/lib.rs(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: ci
crates/torii/sqlite/src/lib.rs
[error] 557-557: Rust formatting check failed. Please run 'rustfmt' to format the code.
crates/torii/indexer/src/engine.rs
[error] 721-721: Rust formatting check failed. Please run 'rustfmt' to format the code.
🔇 Additional comments (4)
crates/torii/indexer/src/processors/store_transaction.rs (1)
21-25: Ohayo! The changes look good, sensei!The addition of the
contract_addressparameter and its usage in thestore_transactioncall are implemented correctly.crates/torii/indexer/src/processors/mod.rs (1)
102-102: Ohayo! The trait modification looks good, sensei!The addition of the
contract_addressparameter to theTransactionProcessortrait'sprocessmethod is implemented correctly.crates/torii/indexer/src/engine.rs (1)
756-768: The parameter addition looks good, sensei!The addition of the
contract_addressparameter to theprocess_transactionmethod and its usage in the processor call are implemented correctly.crates/torii/sqlite/src/lib.rs (1)
518-574: The database changes look good, sensei!The addition of the
contract_addressparameter to thestore_transactionmethod and its inclusion in the SQL insert statement are implemented correctly.🧰 Tools
🪛 GitHub Actions: ci
[error] 557-557: Rust formatting check failed. Please run 'rustfmt' to format the code.
Larkooo
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3055 +/- ##
==========================================
- Coverage 57.65% 57.64% -0.01%
==========================================
Files 439 439
Lines 59437 59448 +11
==========================================
+ Hits 34267 34270 +3
- Misses 25170 25178 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/sqlite/src/lib.rs (1)
576-583: Consider adding an index for better query performance, sensei!The new many-to-many relationship table
transaction_contractwould benefit from an index to optimize queries that look up transactions by contract address or vice versa.Add this index creation statement:
+self.executor.send(QueryMessage::other( + "CREATE INDEX IF NOT EXISTS idx_transaction_contract_lookup ON transaction_contract \ + (contract_address, transaction_hash)".to_string(), + vec![], +))?;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/torii/sqlite/src/lib.rs(4 hunks)
🔇 Additional comments (1)
crates/torii/sqlite/src/lib.rs (1)
518-518: Ohayo! The parameter addition looks good, sensei!The use of
&HashSet<Felt>is an efficient choice for storing unique contract addresses.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/sqlite/src/lib.rs (1)
575-582: Consider adding an index for better query performance.While the implementation correctly handles multiple contract addresses, adding an index on the transaction_contract table would improve query performance when looking up transactions by contract address.
Add the following index after the loop:
+ self.executor.send(QueryMessage::other( + "CREATE INDEX IF NOT EXISTS idx_transaction_contract_address ON transaction_contract \ + (contract_address)", + vec![], + ))?;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/torii/indexer/src/processors/store_transaction.rs(2 hunks)crates/torii/migrations/20250220094437_transactions_contract.sql(1 hunks)crates/torii/sqlite/src/lib.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/torii/indexer/src/processors/store_transaction.rs
- crates/torii/migrations/20250220094437_transactions_contract.sql
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: clippy
🔇 Additional comments (3)
crates/torii/sqlite/src/lib.rs (3)
1-1: LGTM, sensei!The HashSet import is correctly added alongside other collection imports.
514-520: Ohayo! The method signature changes look good!The parameter changes align with the requirements, using appropriate types for block numbers and contract addresses.
556-573: Nice work on the transaction storage, sensei!The SQL statement is properly formatted and the transaction hash cloning is necessary for the subsequent contract relationship storage.
| transaction_hash.clone(), | ||
| transaction_hash.clone(), |
There was a problem hiding this comment.
We still need the two of them? Or one should be enough?
Summary by CodeRabbit