Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(torii): add transaction hash to erc_transfers table #2520

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/torii/core/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,8 @@
let mut unique_contracts = HashSet::new();
// Contract -> Cursor
for (event_idx, event) in events.iter().enumerate() {
// NOTE: erc* processors expect the event_id to be in this format to get
// transaction_hash:
let event_id =
format!("{:#064x}:{:#x}:{:#04x}", block_number, transaction_hash, event_idx);
lambda-0x marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -681,6 +683,8 @@

unique_contracts.insert(event.from_address);

// NOTE: erc* processors expect the event_id to be in this format to get
// transaction_hash:

Check warning on line 687 in crates/torii/core/src/engine.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/engine.rs#L686-L687

Added lines #L686 - L687 were not covered by tests
let event_id =
format!("{:#064x}:{:#x}:{:#04x}", block_number, *transaction_hash, event_idx);

Expand Down Expand Up @@ -899,3 +903,8 @@
MaybePendingBlockWithTxHashes::PendingBlock(block) => Ok(block.timestamp),
}
}

// event_id format: block_number:transaction_hash:event_idx
pub(crate) fn get_transaction_hash_from_event_id(event_id: &str) -> String {
event_id.split(':').nth(1).unwrap().to_string()
}

Check warning on line 910 in crates/torii/core/src/engine.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/engine.rs#L908-L910

Added lines #L908 - L910 were not covered by tests
Comment on lines +908 to +910
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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:

  1. Adding error handling for cases where the split doesn't produce enough parts.
  2. 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.

16 changes: 13 additions & 3 deletions crates/torii/core/src/processors/erc20_legacy_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use tracing::debug;

use super::EventProcessor;
use crate::engine::get_transaction_hash_from_event_id;
use crate::sql::Sql;

pub(crate) const LOG_TARGET: &str = "torii_core::processors::erc20_legacy_transfer";
Expand Down Expand Up @@ -40,18 +41,27 @@
db: &mut Sql,
_block_number: u64,
block_timestamp: u64,
_event_id: &str,
event_id: &str,
event: &Event,
) -> Result<(), Error> {
let token_address = event.from_address;
let from = event.data[0];
let to = event.data[1];
let transaction_hash = get_transaction_hash_from_event_id(event_id);

Check warning on line 50 in crates/torii/core/src/processors/erc20_legacy_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc20_legacy_transfer.rs#L50

Added line #L50 was not covered by tests

let value = U256Cainome::cairo_deserialize(&event.data, 2)?;
let value = U256::from_words(value.low, value.high);

db.handle_erc20_transfer(token_address, from, to, value, world.provider(), block_timestamp)
.await?;
db.handle_erc20_transfer(
token_address,
from,
to,
value,
world.provider(),
block_timestamp,
&transaction_hash,
)
.await?;

Check warning on line 64 in crates/torii/core/src/processors/erc20_legacy_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc20_legacy_transfer.rs#L55-L64

Added lines #L55 - L64 were not covered by tests
debug!(target: LOG_TARGET,from = ?from, to = ?to, value = ?value, "Legacy ERC20 Transfer");

Ok(())
Expand Down
16 changes: 13 additions & 3 deletions crates/torii/core/src/processors/erc20_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use tracing::debug;

use super::EventProcessor;
use crate::engine::get_transaction_hash_from_event_id;
use crate::sql::Sql;

pub(crate) const LOG_TARGET: &str = "torii_core::processors::erc20_transfer";
Expand Down Expand Up @@ -40,7 +41,7 @@
db: &mut Sql,
_block_number: u64,
block_timestamp: u64,
_event_id: &str,
event_id: &str,
event: &Event,
) -> Result<(), Error> {
let token_address = event.from_address;
Expand All @@ -49,9 +50,18 @@

let value = U256Cainome::cairo_deserialize(&event.data, 0)?;
let value = U256::from_words(value.low, value.high);
let transaction_hash = get_transaction_hash_from_event_id(event_id);

Check warning on line 53 in crates/torii/core/src/processors/erc20_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc20_transfer.rs#L53

Added line #L53 was not covered by tests

db.handle_erc20_transfer(token_address, from, to, value, world.provider(), block_timestamp)
.await?;
db.handle_erc20_transfer(
token_address,
from,
to,
value,
world.provider(),
block_timestamp,
&transaction_hash,
)
.await?;

Check warning on line 64 in crates/torii/core/src/processors/erc20_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc20_transfer.rs#L55-L64

Added lines #L55 - L64 were not covered by tests
debug!(target: LOG_TARGET,from = ?from, to = ?to, value = ?value, "ERC20 Transfer");

Ok(())
Expand Down
5 changes: 4 additions & 1 deletion crates/torii/core/src/processors/erc721_legacy_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use tracing::debug;

use super::EventProcessor;
use crate::engine::get_transaction_hash_from_event_id;
use crate::sql::Sql;

pub(crate) const LOG_TARGET: &str = "torii_core::processors::erc721_legacy_transfer";
Expand Down Expand Up @@ -40,7 +41,7 @@
db: &mut Sql,
_block_number: u64,
block_timestamp: u64,
_event_id: &str,
event_id: &str,
event: &Event,
) -> Result<(), Error> {
let token_address = event.from_address;
Expand All @@ -49,6 +50,7 @@

let token_id = U256Cainome::cairo_deserialize(&event.data, 2)?;
let token_id = U256::from_words(token_id.low, token_id.high);
let transaction_hash = get_transaction_hash_from_event_id(event_id);

Check warning on line 53 in crates/torii/core/src/processors/erc721_legacy_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc721_legacy_transfer.rs#L53

Added line #L53 was not covered by tests

db.handle_erc721_transfer(
token_address,
Expand All @@ -57,6 +59,7 @@
token_id,
world.provider(),
block_timestamp,
&transaction_hash,

Check warning on line 62 in crates/torii/core/src/processors/erc721_legacy_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc721_legacy_transfer.rs#L62

Added line #L62 was not covered by tests
)
.await?;
debug!(target: LOG_TARGET, from = ?from, to = ?to, token_id = ?token_id, "ERC721 Transfer");
Expand Down
5 changes: 4 additions & 1 deletion crates/torii/core/src/processors/erc721_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use tracing::debug;

use super::EventProcessor;
use crate::engine::get_transaction_hash_from_event_id;
use crate::sql::Sql;

pub(crate) const LOG_TARGET: &str = "torii_core::processors::erc721_transfer";
Expand Down Expand Up @@ -40,7 +41,7 @@
db: &mut Sql,
_block_number: u64,
block_timestamp: u64,
_event_id: &str,
event_id: &str,
event: &Event,
) -> Result<(), Error> {
let token_address = event.from_address;
Expand All @@ -49,6 +50,7 @@

let token_id = U256Cainome::cairo_deserialize(&event.keys, 3)?;
let token_id = U256::from_words(token_id.low, token_id.high);
let transaction_hash = get_transaction_hash_from_event_id(event_id);

Check warning on line 53 in crates/torii/core/src/processors/erc721_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc721_transfer.rs#L53

Added line #L53 was not covered by tests

db.handle_erc721_transfer(
token_address,
Expand All @@ -57,6 +59,7 @@
token_id,
world.provider(),
block_timestamp,
&transaction_hash,

Check warning on line 62 in crates/torii/core/src/processors/erc721_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/processors/erc721_transfer.rs#L62

Added line #L62 was not covered by tests
)
.await?;
debug!(target: LOG_TARGET, from = ?from, to = ?to, token_id = ?token_id, "ERC721 Transfer");
Expand Down
10 changes: 9 additions & 1 deletion crates/torii/core/src/sql/erc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
amount: U256,
provider: &P,
block_timestamp: u64,
transaction_hash: &str,

Check warning on line 28 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L28

Added line #L28 was not covered by tests
) -> Result<()> {
// contract_address
let token_id = felt_to_sql_string(&contract_address);
Expand All @@ -43,6 +44,7 @@
amount,
&token_id,
block_timestamp,
transaction_hash,

Check warning on line 47 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L47

Added line #L47 was not covered by tests
)?;

if from_address != Felt::ZERO {
Expand Down Expand Up @@ -79,6 +81,7 @@
token_id: U256,
provider: &P,
block_timestamp: u64,
transaction_hash: &str,

Check warning on line 84 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L84

Added line #L84 was not covered by tests
) -> Result<()> {
// contract_address:id
let token_id = felt_and_u256_to_sql_string(&contract_address, &token_id);
Expand All @@ -96,6 +99,7 @@
U256::from(1u8),
&token_id,
block_timestamp,
transaction_hash,

Check warning on line 102 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L102

Added line #L102 was not covered by tests
)?;

// from_address/contract_address:id
Expand Down Expand Up @@ -307,6 +311,7 @@
Ok(())
}

#[allow(clippy::too_many_arguments)]
fn store_erc_transfer_event(
&mut self,
contract_address: Felt,
Expand All @@ -315,9 +320,11 @@
amount: U256,
token_id: &str,
block_timestamp: u64,
transaction_hash: &str,

Check warning on line 323 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L323

Added line #L323 was not covered by tests
) -> Result<()> {
let insert_query = "INSERT INTO erc_transfers (contract_address, from_address, \
to_address, amount, token_id, executed_at) VALUES (?, ?, ?, ?, ?, ?)";
to_address, amount, token_id, executed_at, transaction_hash) VALUES \
(?, ?, ?, ?, ?, ?, ?)";

Check warning on line 327 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L326-L327

Added lines #L326 - L327 were not covered by tests

self.executor.send(QueryMessage::other(
insert_query.to_string(),
Expand All @@ -328,6 +335,7 @@
Argument::String(u256_to_sql_string(&amount)),
Argument::String(token_id.to_string()),
Argument::String(utc_dt_string_from_timestamp(block_timestamp)),
Argument::String(transaction_hash.to_string()),

Check warning on line 338 in crates/torii/core/src/sql/erc.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/core/src/sql/erc.rs#L338

Added line #L338 was not covered by tests
],
))?;

Expand Down
29 changes: 15 additions & 14 deletions crates/torii/graphql/src/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,25 +146,26 @@ lazy_static! {
]);

pub static ref ERC_BALANCE_TYPE_MAPPING: TypeMapping = IndexMap::from([
(Name::new("balance"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("type"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("tokenMetadata"), TypeData::Simple(TypeRef::named(ERC_TOKEN_TYPE_NAME))),
(Name::new("balance"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("type"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("tokenMetadata"), TypeData::Simple(TypeRef::named_nn(ERC_TOKEN_TYPE_NAME))),
]);

pub static ref ERC_TRANSFER_TYPE_MAPPING: TypeMapping = IndexMap::from([
(Name::new("from"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("to"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("amount"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("type"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("executedAt"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("tokenMetadata"), TypeData::Simple(TypeRef::named(ERC_TOKEN_TYPE_NAME))),
(Name::new("from"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("to"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("amount"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("type"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("executedAt"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("tokenMetadata"), TypeData::Simple(TypeRef::named_nn(ERC_TOKEN_TYPE_NAME))),
(Name::new("transactionHash"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
]);

pub static ref ERC_TOKEN_TYPE_MAPPING: TypeMapping = IndexMap::from([
(Name::new("name"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("symbol"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("tokenId"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("decimals"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("contractAddress"), TypeData::Simple(TypeRef::named(TypeRef::STRING))),
(Name::new("name"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("symbol"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("tokenId"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("decimals"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
(Name::new("contractAddress"), TypeData::Simple(TypeRef::named_nn(TypeRef::STRING))),
]);
}
6 changes: 5 additions & 1 deletion crates/torii/graphql/src/object/erc/erc_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@
t.name,
t.symbol,
t.decimals,
c.contract_type
c.contract_type,
et.transaction_hash

Check warning on line 83 in crates/torii/graphql/src/object/erc/erc_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/graphql/src/object/erc/erc_transfer.rs#L82-L83

Added lines #L82 - L83 were not covered by tests
FROM
erc_transfers et
JOIN
Expand Down Expand Up @@ -121,6 +122,7 @@
(Name::new("type"), Value::String(row.contract_type)),
(Name::new("executedAt"), Value::String(row.executed_at)),
(Name::new("tokenMetadata"), token_metadata),
(Name::new("transactionHash"), Value::String(row.transaction_hash.clone())),

Check warning on line 125 in crates/torii/graphql/src/object/erc/erc_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/graphql/src/object/erc/erc_transfer.rs#L125

Added line #L125 was not covered by tests
]))
}
"erc721" => {
Expand All @@ -143,6 +145,7 @@
(Name::new("type"), Value::String(row.contract_type)),
(Name::new("executedAt"), Value::String(row.executed_at)),
(Name::new("tokenMetadata"), token_metadata),
(Name::new("transactionHash"), Value::String(row.transaction_hash.clone())),

Check warning on line 148 in crates/torii/graphql/src/object/erc/erc_transfer.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/graphql/src/object/erc/erc_transfer.rs#L148

Added line #L148 was not covered by tests
]))
}
_ => {
Expand Down Expand Up @@ -178,4 +181,5 @@
pub symbol: String,
pub decimals: u8,
pub contract_type: String,
pub transaction_hash: String,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE erc_transfers ADD COLUMN transaction_hash TEXT NOT NULL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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:

  1. 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;
  2. 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;
  3. 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?

Loading