Dust Off & Upgrade

[bh2smith]

User description

Just taking a moment to brush the dust of this sweet sweet EVM event indexer. This PR should address all open security alerts.

---

PR Type

enhancement, dependencies

---

Description

• Introduced new utility functions and structures in common.rs to handle event descriptors and SQL operations, enhancing code modularity and reusability.
• Refactored postgres.rs and sqlite.rs to utilize new utility functions, improving error handling and reducing code duplication.
• Updated Cargo.toml to upgrade several dependencies, ensuring compatibility with newer versions and leveraging new features.
• Improved documentation by clarifying comments in the indexer module.

---

Changes walkthrough 📝

	Relevant files
Enhancement	common.rs Introduce event handling utilities and validation functions src/database/common.rs Added a new PreparedEvent struct to handle event descriptors and SQL statements. Implemented check_descriptor function to validate event fields against descriptors. Added event_exists_with_same_signature function to check for existing events with the same signature. Introduced push_sql_value function to manage SQL value insertion. +74/-0    mod.rs Update module imports and improve EventBlock implementation src/database/mod.rs Added common module import. Updated EventBlock implementation to use a more generic lifetime. +2/-1      postgres.rs Refactor Postgres event handling with improved error management src/database/postgres.rs Integrated new functions from common for event validation and SQL value handling. Refactored PreparedEvent to use generic types for SQL statements. Replaced unwrap calls with ? for error handling. +24/-55  sqlite.rs Refactor SQLite event handling with improved error management src/database/sqlite.rs Integrated new functions from common for event validation and SQL value handling. Refactored PreparedEvent to use generic types for SQL statements. Replaced unwrap calls with ? for error handling. +17/-60
Documentation	mod.rs Improve comment clarity in indexer module                                src/indexer/mod.rs Corrected a comment for clarity regarding block indexing. +1/-1
Dependencies	Cargo.toml Upgrade dependencies for improved compatibility and features Cargo.toml Upgraded rusqlite dependency to version 0.34.0. Updated tokio and tokio-postgres to newer versions. Bumped chrono and hex-literal to latest versions. +6/-6

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[mintbase-codium-pr-agent]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The function check_descriptor at lines 23-33 uses a loop to compare event field types with descriptor types and returns an error if they don't match. This approach is correct, but the error message "event field {i} doesn't match event descriptor" does not provide enough context about what specifically is mismatched, which could make debugging more difficult. Consider enhancing the error message to include more details about the expected and actual types. Possible Bug The function event_exists_with_same_signature at lines 42-57 checks if an event with the same name has a different signature in the database and returns an error if so. However, the error message uses {name} which might not be replaced correctly because it's inside a string literal without formatting applied. This could lead to confusing error messages in the logs.

[mintbase-codium-pr-agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Replace unwrap() with ? for error propagation Replace the unwrap() calls with proper error handling to avoid potential runtime panics. Using ? operator will propagate the error, allowing for cleaner error handling in the calling functions. src/database/postgres.rs [398-400] -let block_number = i64::try_from(*block_number).unwrap(); -let log_index = i64::try_from(*log_index).unwrap(); -let transaction_index = i64::try_from(*transaction_index).unwrap(); +let block_number = i64::try_from(*block_number)?; +let log_index = i64::try_from(*log_index)?; +let transaction_index = i64::try_from(*transaction_index)?; Suggestion importance[1-10]: 9 Why: The suggestion correctly replaces unwrap() with ?, which is a safer approach to handle potential conversion errors and prevent runtime panics. This change aligns with the existing PR code diff, which already implements this improvement.	9
	Replace unwrap() with ? for error propagation in type conversions Replace the try_into().unwrap() calls with ? to handle potential conversion errors gracefully. src/database/sqlite.rs [440-443] -let block_number = ToSqlOutput::Owned(SqlValue::Integer((*block_number).try_into().unwrap())); -let log_index = ToSqlOutput::Owned(SqlValue::Integer((*log_index).try_into().unwrap())); -let transaction_index = ToSqlOutput::Owned(SqlValue::Integer((*transaction_index).try_into().unwrap())); +let block_number = ToSqlOutput::Owned(SqlValue::Integer((*block_number).try_into()?)); +let log_index = ToSqlOutput::Owned(SqlValue::Integer((*log_index).try_into()?)); +let transaction_index = ToSqlOutput::Owned(SqlValue::Integer((*transaction_index).try_into()?)); Suggestion importance[1-10]: 9 Why: The suggestion correctly replaces unwrap() with ?, improving error handling by propagating errors instead of causing potential panics. This aligns with the changes made in the PR code diff.	9
Enhancement	Improve error message for type mismatch in event fields Implement a more detailed error message that includes the expected and actual types when the event field does not match the descriptor, to aid in debugging. src/database/common.rs [28-29] if value.kind() != kind.field.kind { - return Err(anyhow!("event field {i} doesn't match event descriptor")); + return Err(anyhow!("event field {i} expected type {:?} but got type {:?}", kind.field.kind, value.kind())); } Suggestion importance[1-10]: 8 Why: The suggestion enhances the error message by including expected and actual types, which aids in debugging. This is a valuable improvement for developers, making error messages more informative.	8
	Add error handling for the write! macro Consider handling the potential error from the write! macro to ensure that any issues with writing to the string are properly managed. src/database/postgres.rs [487-512] -write!(&mut sql, "CREATE TABLE IF NOT EXISTS {} (", table.name)?; -write!(&mut sql, "{FIXED_COLUMNS}, ")?; -write!(&mut sql, "{ARRAY_COLUMN}, ")?; -write!(&mut sql, "{}", column.name)?; -write!(&mut sql, " {type_} NOT NULL, ")?; -write!(&mut sql, "PRIMARY KEY({primary_key}));")?; +write!(&mut sql, "CREATE TABLE IF NOT EXISTS {} (", table.name).map_err(|e| anyhow!("Failed to write SQL: {}", e))?; +write!(&mut sql, "{FIXED_COLUMNS}, ").map_err(|e| anyhow!("Failed to write SQL: {}", e))?; +write!(&mut sql, "{ARRAY_COLUMN}, ").map_err(|e| anyhow!("Failed to write SQL: {}", e))?; +write!(&mut sql, "{}", column.name).map_err(|e| anyhow!("Failed to write SQL: {}", e))?; +write!(&mut sql, " {type_} NOT NULL, ").map_err(|e| anyhow!("Failed to write SQL: {}", e))?; +write!(&mut sql, "PRIMARY KEY({primary_key}));").map_err(|e| anyhow!("Failed to write SQL: {}", e))?; Suggestion importance[1-10]: 7 Why: The suggestion adds error handling to the write! macro, which is a good practice to ensure robustness. However, this is a minor enhancement and not critical, as the existing code already uses the ? operator for error propagation.	7
	Make the chrono dependency optional Consider specifying the optional attribute for the chrono dependency to allow users to opt-in or out based on their needs. This can be particularly useful for libraries to reduce the number of mandatory dependencies. Cargo.toml [28] -chrono = { version = "0.4.40", default-features = false, features = ["std"] } +chrono = { version = "0.4.40", default-features = false, features = ["std"], optional = true } Suggestion importance[1-10]: 7 Why: Making the chrono dependency optional can improve flexibility for users who may not need it, reducing unnecessary dependencies. However, this change is not critical and depends on the specific use case of the library.	7
Best practice	Lock the serde dependency to a specific minor version Consider locking the serde dependency to a more specific minor version to avoid potential breaking changes in future updates, ensuring more predictable behavior and compatibility. Cargo.toml [17] -serde = { version = "1", features = ["derive"] } +serde = { version = "1.0.130", features = ["derive"] } Suggestion importance[1-10]: 8 Why: Locking the serde dependency to a specific minor version can prevent unexpected breaking changes, ensuring stability and predictability, which is a good practice for maintaining compatibility.	8
Maintainability	Specify a version range for hex-literal to allow minor updates For the hex-literal dependency, consider specifying a more specific version range instead of pinning to a single version to allow for minor updates and patches, which can include important fixes and improvements. Cargo.toml [31] -hex-literal = "1.0.0" +hex-literal = "^1.0.0" Suggestion importance[1-10]: 7 Why: Allowing minor updates for hex-literal can provide important fixes and improvements without risking major breaking changes, enhancing maintainability.	7
Possible issue	Update the time feature version used in tokio-postgres Update the tokio-postgres dependency to use the latest compatible version of time instead of the older 0.3 version, which may not be compatible with other dependencies or may lack recent improvements and bug fixes. Cargo.toml [25] -tokio-postgres = { version = "0.7.13", features = ["with-time-0_3"] } +tokio-postgres = { version = "0.7.13", features = ["with-time"] } Suggestion importance[1-10]: 6 Why: Updating the time feature to a more recent version could improve compatibility and include recent improvements, but it may also introduce compatibility issues with existing code if not tested properly.	6