feat(torii): add transaction hash to erc_transfers table #2520
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2520 +/- ##
==========================================
- Coverage 69.35% 69.31% -0.04%
==========================================
Files 388 388
Lines 49988 50025 +37
==========================================
+ Hits 34667 34674 +7
- Misses 15321 15351 +30 ☔ View full report in Codecov by Sentry. |
lambda-0x
force-pushed
the
spr/main/108ca77e
branch
from
0b7b2ab
to
798ff06
Compare
lambda-0x
force-pushed
the
spr/main/02be81af
branch
from
4890761
to
dfe28fa
Compare
lambda-0x
force-pushed
the
spr/main/02be81af
branch
from
dfe28fa
to
375f1d6
Compare
WalkthroughOhayo, sensei! This pull request introduces several changes primarily in the Changes
Possibly related PRs
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: 3
🧹 Outside diff range and nitpick comments (7)
crates/torii/core/src/processors/erc721_transfer.rs (1)
Line range hint
53-62: Ohayo once more, sensei! Excellent implementation!The addition of
get_transaction_hash_from_event_idand its inclusion in thedb.handle_erc721_transfercall is spot on. This change aligns perfectly with the PR objectives to add transaction hash to the erc_transfers table.One small suggestion to consider:
You might want to add a debug log for the transaction hash, similar to how you're logging other transfer details. This could be helpful for troubleshooting. Here's a suggested addition:
debug!(target: LOG_TARGET, from = ?from, to = ?to, token_id = ?token_id, "ERC721 Transfer"); +debug!(target: LOG_TARGET, transaction_hash = ?transaction_hash, "ERC721 Transfer Transaction Hash");What do you think, sensei? Would this additional logging be useful?
crates/torii/core/src/processors/erc721_legacy_transfer.rs (2)
44-44: Signature update is on point, sensei!Removing the underscore from
_event_idaligns perfectly with its new usage in the method. Great attention to detail!Consider adding a brief comment explaining the purpose of the
event_idparameter for improved code readability.
53-53: Ohayo, sensei! Excellent addition of transaction hash handling!The new line extracting the transaction hash and its subsequent use in
db.handle_erc721_transfersignificantly improves the traceability of ERC721 transfers. This change aligns perfectly with the PR objectives.Consider adding a brief comment explaining the importance of including the transaction hash in the transfer record.
Also applies to: 62-62
crates/torii/core/src/processors/erc20_legacy_transfer.rs (1)
50-64: Ohayo, sensei! These changes are looking mighty fine!The addition of
transaction_hashand its integration into thedb.handle_erc20_transfercall align perfectly with our PR objectives. Great job on seamlessly incorporating this new piece of information!A small suggestion to enhance readability:
Consider extracting the
db.handle_erc20_transfercall into a separate line for better readability. Here's a quick example:let transfer_result = db.handle_erc20_transfer( token_address, from, to, value, world.provider(), block_timestamp, &transaction_hash, ); transfer_result.await?;This separation makes the code a bit easier on the eyes and could potentially make future modifications simpler. What do you think, sensei?
crates/torii/graphql/src/object/erc/erc_balance.rs (1)
Line range hint
83-110: Ohayo, sensei! Consider a small refactoring opportunity.While the changes improve naming consistency, there's an opportunity to reduce code duplication between the ERC20 and ERC721 logic. Consider extracting the common parts of the token metadata creation into a separate function. This could make the code more maintainable and easier to extend in the future.
For example:
fn create_token_metadata(row: &BalanceQueryResultRaw, token_id: Option<String>) -> Value { Value::Object(ValueMapping::from([ (Name::new("contractAddress"), Value::String(row.contract_address.clone())), (Name::new("name"), Value::String(row.name.clone())), (Name::new("symbol"), Value::String(row.symbol.clone())), (Name::new("tokenId"), token_id.map_or(Value::Null, Value::String)), (Name::new("decimals"), Value::String(row.decimals.to_string())), ])) }This function could then be called from both the ERC20 and ERC721 branches, reducing duplication and making the code more DRY.
crates/torii/core/src/engine.rs (2)
613-614: Ohayo, sensei! Consider centralizing the event_id format information.The comments explaining the event_id format are identical in two locations. To improve maintainability, consider:
- Creating a constant for the event_id format.
- Adding this information to the documentation of the relevant ERC processors.
This approach would reduce duplication and make it easier to update if the format changes in the future.
Also applies to: 686-687
Line range hint
613-910: Ohayo, sensei! Consider a broader refactoring for event_id handling.The changes you've made are improving the handling of event_ids and transaction hashes. To further enhance consistency and maintainability across the codebase, consider:
- Creating a dedicated
EventIdstruct that encapsulates the parsing and validation logic.- Using this struct throughout the codebase instead of raw strings for event_ids.
- Implementing methods on
EventIdfor extracting components like transaction hash, block number, etc.This approach would centralize the event_id logic, making it easier to maintain and reducing the risk of inconsistencies.
Would you like me to sketch out a basic structure for this
EventIdstruct?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- crates/torii/core/src/engine.rs (3 hunks)
- crates/torii/core/src/processors/erc20_legacy_transfer.rs (2 hunks)
- crates/torii/core/src/processors/erc20_transfer.rs (3 hunks)
- crates/torii/core/src/processors/erc721_legacy_transfer.rs (4 hunks)
- crates/torii/core/src/processors/erc721_transfer.rs (4 hunks)
- crates/torii/core/src/sql/erc.rs (7 hunks)
- crates/torii/graphql/src/mapping.rs (1 hunks)
- crates/torii/graphql/src/object/erc/erc_balance.rs (2 hunks)
- crates/torii/graphql/src/object/erc/erc_transfer.rs (3 hunks)
- crates/torii/migrations/20241011103804_add_transaction_hash_in_erc_transfers_table.sql (1 hunks)
🧰 Additional context used
🔇 Additional comments (23)
crates/torii/core/src/processors/erc20_transfer.rs (4)
10-10: Ohayo, sensei! New import looks good!The addition of
get_transaction_hash_from_event_idaligns perfectly with our mission to enhance theerc_transferstable with transaction hash information. It's like adding a secret ingredient to our code ramen!
44-44: Ohayo again, sensei! Parameter update looks sharp!Removing the underscore from
_event_idis like unveiling a hidden treasure in our code dojo. It signals that we're now putting this parameter to good use. Excellent move!
53-53: Transaction hash extraction, a true code ninja move!Ohayo, sensei! This line is the heart of our mission. By extracting the transaction hash from the event ID, we're adding a powerful new technique to our ERC20 transfer arsenal. Well done!
55-64: Ohayo once more, sensei! Method call upgrade is masterful!The
db.handle_erc20_transfercall has evolved like a Pokémon! Not only is it more readable with its multi-line format, but it now includes the crucial&transaction_hashparameter. This completes our quest to enhance theerc_transferstable. Excellent work, code samurai!crates/torii/core/src/processors/erc721_transfer.rs (2)
10-10: Ohayo, sensei! Nice import addition!The new import statement for
get_transaction_hash_from_event_idis well-placed and specific. It's great to see you're importing only what's needed for the upcoming changes.
44-44: Ohayo again, sensei! Sharp eye on that parameter!Excellent move renaming
_event_idtoevent_id. This change clearly signals that we're now using this parameter in the method. It's a small but important detail that improves code clarity.crates/torii/core/src/processors/erc721_legacy_transfer.rs (2)
10-10: Ohayo, sensei! New import looks good!The addition of
get_transaction_hash_from_event_idimport is spot-on for the new functionality. It's properly scoped and follows our naming conventions.
10-10: Ohayo, sensei! Overall, these changes are a solid improvement!The modifications to include transaction hash information in ERC721 transfer processing align perfectly with the PR objectives. They enhance traceability and data completeness for ERC721 transfers.
Here's a quick summary of the changes:
- Added import for
get_transaction_hash_from_event_id- Updated method signature to use
event_id- Extracted transaction hash from event ID
- Included transaction hash in database handling
These changes collectively contribute to a more robust and informative ERC721 transfer processing system. Great work, sensei!
Also applies to: 44-44, 53-53, 62-62
crates/torii/core/src/processors/erc20_legacy_transfer.rs (2)
10-10: Ohayo, sensei! New import looks good!The new import for
get_transaction_hash_from_event_idis well-placed and aligns with the upcoming changes in theprocessmethod.
44-44: Signature update looks sharp, sensei!The removal of the underscore prefix from
event_idnicely indicates that we're now using this parameter. It's a clean change that aligns well with the new functionality.crates/torii/graphql/src/object/erc/erc_balance.rs (2)
83-85: Ohayo, sensei! Excellent work on improving naming conventions!The changes to camelCase for object keys (
tokenId,contractAddress,tokenMetadata) in the "erc20" branch enhance code consistency and readability. This aligns well with JavaScript/TypeScript conventions, which is beneficial for GraphQL APIs.Also applies to: 91-91
100-100: Consistency across token types - well done, sensei!The changes to camelCase for object keys (
contractAddress,tokenId,tokenMetadata) in the "erc721" branch maintain consistency with the ERC20 implementation. This uniform approach across different token types enhances the overall code quality and API consistency.Also applies to: 103-103, 110-110
crates/torii/graphql/src/mapping.rs (4)
149-151: Ohayo, sensei! Great improvements toERC_BALANCE_TYPE_MAPPING!The changes look good and bring several improvements:
- All fields are now non-nullable (
TypeRef::named_nn), which is excellent for required fields.- Renaming "token_metadata" to "tokenMetadata" improves consistency with GraphQL naming conventions.
These updates enhance type safety and align better with GraphQL best practices. Nice work!
155-161: Ohayo again, sensei! Excellent updates toERC_TRANSFER_TYPE_MAPPING!These changes bring significant improvements:
- All fields are now non-nullable (
TypeRef::named_nn), enhancing type safety.- Renaming "executed_at" to "executedAt" and "token_metadata" to "tokenMetadata" improves consistency with GraphQL naming conventions.
- The addition of the "transactionHash" field (also non-nullable) aligns perfectly with the PR objective of adding transaction hash to the erc_transfers table.
These updates not only improve the type definitions but also extend the functionality as intended. Great job!
165-169: Ohayo once more, sensei! Fantastic improvements toERC_TOKEN_TYPE_MAPPING!The changes here are on point:
- All fields are now non-nullable (
TypeRef::named_nn), which greatly enhances type safety.- Renaming "token_id" to "tokenId" and "contract_address" to "contractAddress" improves consistency with GraphQL naming conventions.
These updates bring the
ERC_TOKEN_TYPE_MAPPINGin line with best practices and make it more robust. Excellent work!
149-169: Ohayo, sensei! Overall fantastic improvements to the GraphQL type mappings!The changes across
ERC_BALANCE_TYPE_MAPPING,ERC_TRANSFER_TYPE_MAPPING, andERC_TOKEN_TYPE_MAPPINGconsistently enhance the code quality:
- All fields are now non-nullable (
TypeRef::named_nn), significantly improving type safety.- Field names have been updated to camelCase, aligning with GraphQL naming conventions.
- The addition of the "transactionHash" field to
ERC_TRANSFER_TYPE_MAPPINGfulfills the PR objective.These updates not only improve the robustness of the type definitions but also enhance the overall consistency and functionality of the GraphQL schema. Great job on these changes!
crates/torii/core/src/sql/erc.rs (3)
28-28: Ohayo, sensei! The ERC20 transfer handler now includes the transaction hash. Sugoi!The addition of the
transaction_hashparameter and its usage instore_erc_transfer_eventaligns perfectly with the PR objective. The implementation looks clean and doesn't interfere with existing logic. Nicely done!Also applies to: 47-47
84-84: Ohayo again, sensei! The ERC721 transfer handler is now equipped with transaction hash powers!Just like its ERC20 counterpart, this function has been updated to include the
transaction_hashparameter and pass it along tostore_erc_transfer_event. The changes are consistent and well-implemented. Excellent work!Also applies to: 102-102
Line range hint
314-338: Ohayo once more, sensei! The ERC transfer event storage now captures the essence of transactions!The
store_erc_transfer_eventfunction has been skillfully updated to include thetransaction_hash. The SQL query and parameters have been adjusted accordingly. This change aligns perfectly with the PR objective. Excellent implementation!However, to ensure the harmony of our codebase, we should verify that all calls to this function have been updated to include the new
transaction_hashparameter.Let's use our ninja skills to check for any missed function calls:
crates/torii/graphql/src/object/erc/erc_transfer.rs (4)
82-83: Addingcontract_typeandtransaction_hashto the SELECT statementOhayo, sensei! Including
c.contract_typeandet.transaction_hashin the SELECT statement is appropriate and ensures that all necessary data is retrieved for processing ERC transfers.
107-108: Simplify contract type matching with case-insensitive comparisonConverting
row.contract_typeto lowercase before matching simplifies the logic and handles different casing consistently.
123-125: IncludeexecutedAtandtransactionHashin transfer objectsOhayo, sensei! Adding
executedAtandtransactionHashto the transfer objects enhances the data returned to clients, improving transparency and traceability of transactions.Also applies to: 146-148
184-184: ExtendTransferQueryResultRawstruct withtransaction_hashOhayo, sensei! Adding
transaction_hashto theTransferQueryResultRawstruct ensures that all relevant transaction data is captured and available for processing.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE erc_transfers ADD COLUMN transaction_hash TEXT NOT NULL; | |||
There was a problem hiding this comment.
Ohayo, sensei! Consider handling existing data when adding the new NOT NULL column.
The addition of the transaction_hash column aligns well with the PR objectives. However, adding a NOT NULL column to an existing table without a DEFAULT value might cause issues if there are existing rows in the erc_transfers table.
To address this, consider the following options:
-
If the table is empty or you're certain all existing data can be safely discarded:
TRUNCATE TABLE erc_transfers; ALTER TABLE erc_transfers ADD COLUMN transaction_hash TEXT NOT NULL;
-
If existing data needs to be preserved, use a two-step migration:
-- Step 1: Add the column as nullable ALTER TABLE erc_transfers ADD COLUMN transaction_hash TEXT; -- Step 2 (in a separate migration after data population): ALTER TABLE erc_transfers ALTER COLUMN transaction_hash SET NOT NULL;
-
If a default value is appropriate:
ALTER TABLE erc_transfers ADD COLUMN transaction_hash TEXT NOT NULL DEFAULT 'placeholder_hash';
Choose the option that best fits your data requirements and existing table state, sensei.
Would you like assistance in implementing one of these solutions or in creating a data migration script?
| pub(crate) fn get_transaction_hash_from_event_id(event_id: &str) -> String { | ||
| event_id.split(':').nth(1).unwrap().to_string() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo, sensei! Consider adding error handling to get_transaction_hash_from_event_id.
The function assumes the event_id format is always correct. To improve robustness, consider:
- Adding error handling for cases where the split doesn't produce enough parts.
- Validating the extracted transaction hash (e.g., checking if it's a valid hex string).
Here's a potential implementation:
pub(crate) fn get_transaction_hash_from_event_id(event_id: &str) -> Result<String, &'static str> {
event_id.split(':').nth(1).ok_or("Invalid event_id format").map(|s| s.to_string())
}This change would make the function more resilient to incorrect inputs.
| // contract_address:token_id | ||
| let token_id = row.token_id.split(':').collect::<Vec<&str>>(); | ||
| assert!(token_id.len() == 2); |
There was a problem hiding this comment.
Avoid using assert! for input validation
Using assert! for input validation can cause the application to panic if token_id does not split into two parts. Consider handling this scenario gracefully to prevent unexpected crashes.
Here's a suggested change:
let token_id = row.token_id.split(':').collect::<Vec<&str>>();
- assert!(token_id.len() == 2);
+ if token_id.len() != 2 {
+ warn!("Invalid token_id format: {}", row.token_id);
+ continue;
+ }📝 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.
| // contract_address:token_id | |
| let token_id = row.token_id.split(':').collect::<Vec<&str>>(); | |
| assert!(token_id.len() == 2); | |
| // contract_address:token_id | |
| let token_id = row.token_id.split(':').collect::<Vec<&str>>(); | |
| if token_id.len() != 2 { | |
| warn!("Invalid token_id format: {}", row.token_id); | |
| continue; | |
| } |
lambda-0x
force-pushed
the
spr/main/02be81af
branch
from
375f1d6
to
49fd019
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
crates/torii/core/src/sql/erc.rs (1)
Line range hint
314-338: Ohayo once more, sensei! LGTM with a small suggestion.The changes to
store_erc_transfer_eventare well-implemented, adding thetransaction_hashto the method signature, SQL query, and query parameters. This aligns perfectly with the PR objective and maintains the existing logic.A small suggestion for consistency:
Consider changing line 338 from:
Argument::String(transaction_hash.to_string()),to:
Argument::String(transaction_hash.to_owned()),This change would be more idiomatic and consistent with how other string arguments are handled in this method (e.g.,
token_id.to_string()). Both achieve the same result, butto_owned()is more commonly used when converting&strtoString.crates/torii/core/src/engine.rs (2)
686-687: Ohayo again, sensei! Good consistency in documentation.These comments mirror the ones added earlier, maintaining consistency in explaining the
event_idformat. However, if this information is repeated often, consider centralizing it in a documentation comment or constant to avoid duplication.
906-910: Ohayo, sensei! Nice utility function, but let's make it more robust.The new
get_transaction_hash_from_event_idfunction is a useful addition. However, it assumes theevent_idformat is always correct. Consider adding error handling for cases where the split doesn't produce enough parts or the transaction hash is invalid.Here's a suggestion to improve robustness:
pub(crate) fn get_transaction_hash_from_event_id(event_id: &str) -> Result<String, &'static str> { event_id.split(':').nth(1).ok_or("Invalid event_id format").map(|s| s.to_string()) }This change would make the function more resilient to incorrect inputs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- crates/torii/core/src/engine.rs (3 hunks)
- crates/torii/core/src/processors/erc20_legacy_transfer.rs (2 hunks)
- crates/torii/core/src/processors/erc20_transfer.rs (3 hunks)
- crates/torii/core/src/processors/erc721_legacy_transfer.rs (4 hunks)
- crates/torii/core/src/processors/erc721_transfer.rs (4 hunks)
- crates/torii/core/src/sql/erc.rs (7 hunks)
- crates/torii/graphql/src/mapping.rs (1 hunks)
- crates/torii/graphql/src/object/erc/erc_balance.rs (2 hunks)
- crates/torii/graphql/src/object/erc/erc_transfer.rs (3 hunks)
- crates/torii/migrations/20241011103804_add_transaction_hash_in_erc_transfers_table.sql (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/torii/core/src/processors/erc20_legacy_transfer.rs
- crates/torii/core/src/processors/erc20_transfer.rs
- crates/torii/core/src/processors/erc721_legacy_transfer.rs
- crates/torii/core/src/processors/erc721_transfer.rs
- crates/torii/graphql/src/mapping.rs
- crates/torii/graphql/src/object/erc/erc_balance.rs
- crates/torii/graphql/src/object/erc/erc_transfer.rs
- crates/torii/migrations/20241011103804_add_transaction_hash_in_erc_transfers_table.sql
🧰 Additional context used
🔇 Additional comments (4)
crates/torii/core/src/sql/erc.rs (2)
28-28: Ohayo, sensei! LGTM: Transaction hash parameter added correctly.The addition of the
transaction_hashparameter to thehandle_erc20_transfermethod and its usage in thestore_erc_transfer_eventcall align perfectly with the PR objective. The changes are minimal and focused, maintaining the existing logic while enhancing the data captured for ERC20 transfers.Also applies to: 47-47
84-84: Ohayo again, sensei! LGTM: Consistent changes for ERC721 transfers.The modifications to
handle_erc721_transfermirror those made tohandle_erc20_transfer, demonstrating excellent consistency. Thetransaction_hashparameter is added to the method signature and correctly passed tostore_erc_transfer_event, aligning perfectly with the PR objective and maintaining the existing logic.Also applies to: 102-102
crates/torii/core/src/engine.rs (2)
613-614: Ohayo, sensei! Nice addition of clarifying comments.These comments provide valuable context about the expected format of
event_idfor erc* processors. This improves code readability and maintainability.
Line range hint
1-910: Ohayo, sensei! Overall, these changes enhance the codebase.The additions to this file improve documentation and add useful functionality:
- The new comments clarify the expected
event_idformat, enhancing code readability.- The
get_transaction_hash_from_event_idfunction provides a centralized way to extract transaction hashes from event IDs.These changes align well with the PR objective of adding transaction hash to the
erc_transferstable. They lay the groundwork for consistent handling of transaction hashes across the codebase.To further improve:
- Consider centralizing the
event_idformat documentation if it's used in multiple places.- Ensure that the new
get_transaction_hash_from_event_idfunction is used consistently wherever transaction hashes are extracted from event IDs.Great work on enhancing the codebase's clarity and functionality!
commit-id:02be81af
lambda-0x
force-pushed
the
spr/main/02be81af
branch
from
49fd019
to
1ee2bb6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
crates/torii/graphql/src/object/erc/erc_transfer.rs (2)
125-125: Ohayo, sensei! LGTM with a small suggestion.The addition of the
transactionHashfield to the ERC20 transfer object is spot-on and aligns perfectly with our PR objective. Great job on maintaining consistency with GraphQL naming conventions by using camelCase!A small suggestion to optimize memory usage:
Consider removing the
.clone()call onrow.transaction_hash. Since this is the last use ofrow, you can userow.transaction_hashdirectly without cloning. Here's the suggested change:- (Name::new("transactionHash"), Value::String(row.transaction_hash.clone())), + (Name::new("transactionHash"), Value::String(row.transaction_hash)),This minor optimization can help reduce unnecessary memory allocations.
148-148: Ohayo again, sensei! LGTM with the same suggestion as before.Excellent work on adding the
transactionHashfield to the ERC721 transfer object as well. This maintains consistency across different token standards and fulfills our PR objective.As with the ERC20 case, we can make a small optimization:
Consider removing the
.clone()call onrow.transaction_hash. Since this is the last use ofrow, you can userow.transaction_hashdirectly without cloning. Here's the suggested change:- (Name::new("transactionHash"), Value::String(row.transaction_hash.clone())), + (Name::new("transactionHash"), Value::String(row.transaction_hash)),This minor change can help reduce unnecessary memory allocations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
- crates/torii/core/src/engine.rs (3 hunks)
- crates/torii/core/src/processors/erc20_legacy_transfer.rs (2 hunks)
- crates/torii/core/src/processors/erc20_transfer.rs (3 hunks)
- crates/torii/core/src/processors/erc721_legacy_transfer.rs (4 hunks)
- crates/torii/core/src/processors/erc721_transfer.rs (4 hunks)
- crates/torii/core/src/sql/erc.rs (7 hunks)
- crates/torii/graphql/src/mapping.rs (1 hunks)
- crates/torii/graphql/src/object/erc/erc_transfer.rs (4 hunks)
- crates/torii/migrations/20241011103804_add_transaction_hash_in_erc_transfers_table.sql (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- crates/torii/core/src/engine.rs
- crates/torii/core/src/processors/erc20_legacy_transfer.rs
- crates/torii/core/src/processors/erc20_transfer.rs
- crates/torii/core/src/processors/erc721_legacy_transfer.rs
- crates/torii/core/src/processors/erc721_transfer.rs
- crates/torii/core/src/sql/erc.rs
- crates/torii/migrations/20241011103804_add_transaction_hash_in_erc_transfers_table.sql
🧰 Additional context used
🔇 Additional comments (5)
crates/torii/graphql/src/object/erc/erc_transfer.rs (2)
82-83: Ohayo, sensei! LGTM: Transaction hash added to the query.The addition of
et.transaction_hashto the SELECT statement aligns perfectly with the PR objective of enhancing theerc_transferstable with transaction hash information. This change ensures that we retrieve the transaction hash along with other transfer details, providing a more comprehensive dataset for our GraphQL resolvers.
184-184: Ohayo once more, sensei! LGTM: TransferQueryResultRaw struct updated.The addition of the
transaction_hash: Stringfield to theTransferQueryResultRawstruct is spot-on. This change ensures that we can properly deserialize the transaction hash from our SQL query results, which is crucial for our PR objective. The field is correctly set as public and uses the appropriateStringtype, maintaining consistency with other fields in the struct.crates/torii/graphql/src/mapping.rs (3)
165-169: Ohayo, sensei! Confirm non-null constraints forERC_TOKEN_TYPE_MAPPINGfields.The fields
name,symbol,tokenId,decimals, andcontractAddresshave been updated to be non-nullable. Please ensure:
- All records in the
erc_tokentable have these fields populated.- Any data ingestion processes provide values for these fields.
- Client applications are aware of these fields being non-nullable to prevent any unexpected behavior.
To check for null values in these fields, execute:
#!/bin/bash # Description: Verify no null values exist in the key fields of the 'erc_token' table. psql $DATABASE_URL -c " SELECT COUNT(*) AS null_count FROM erc_token WHERE name IS NULL OR symbol IS NULL OR token_id IS NULL OR decimals IS NULL OR contract_address IS NULL; "The count should be
0, confirming all fields are properly populated.
155-161: Ohayo, sensei! Ensure data integrity for non-nullable fields and the newtransactionHash.The fields
from,to,amount,type,executedAt,tokenMetadatahave been updated to non-nullable types, and a new non-nullable fieldtransactionHashhas been added toERC_TRANSFER_TYPE_MAPPING. It's important to:
- Verify that all existing and incoming data for these fields are non-null.
- Confirm that the addition of
transactionHashis correctly handled in all parts of the application, including data ingestion and queries.To verify there are no null values and that
transactionHashis properly populated, run:Expect
null_countto be0to ensure data integrity.
149-151: Ohayo, sensei! Verify non-nullable fields inERC_BALANCE_TYPE_MAPPING.The fields
balance,type, andtokenMetadatahave been changed to non-nullable types usingTypeRef::named_nn. This means these fields must always have values. Please ensure that all data sources and database entries guarantee these fields are always populated to prevent potential null reference exceptions or runtime errors.To confirm that there are no records with null values for these fields in the existing database, you can run the following script:
The expected result is
null_countequals0, indicating no null values are present.
|
✓ Commit merged in pull request #2526 |
Stack:
Connectionabstraction to return data for ercBalance and ercTransfer query #2527idin erc_transfers #2526Summary by CodeRabbit
New Features
transaction_hashfield to theerc_transferstable for improved data capture during token transfers.Bug Fixes
Documentation
Style
Chores
transaction_hashcolumn in theerc_transferstable.