Prevent handling triggers for failed transactions #2511

tilacog · 2021-06-01T17:56:55Z

Resolves #2409

Handle Non-Final blocks
Handle Final blocks
- Fetch existing transaction receipts from database
- Fetch existing transaction receipts from ethereum client
Add stopwatch metrics
Add manifest version switch

This PR adds a verification step to call-handler execution that filters off traces with failed transactions.

For non-final blocks, we determine failure by inspecting each transaction receipt and gas usage, which are already present in program memory, so it should be fast.

For final blocks, we search the database once for each block. If that data is absent, we call an external API once for each transaction receipt.

graph/src/components/ethereum/types.rs

chain/ethereum/src/ethereum_adapter.rs

graph/src/components/ethereum/types.rs

leoyvens · 2021-06-28T13:23:10Z

chain/ethereum/src/ethereum_adapter.rs

+            chain_store
+                .transaction_statuses_in_block_range(&(from..to))
+                .expect("failed to fetch failed transactions in the database")
+        };


You can do an early return with a once stream that emits an error https://docs.rs/futures/0.1.31/futures/stream/fn.once.html.

Alternatively, we can log the error and move on as if we had nothing cached.

leoyvens · 2021-06-28T14:04:44Z

store/postgres/src/chain_store.rs

+        for receipt in receipts.into_iter() {
+            match (receipt.status, receipt.gas_used) {
+                (Some(_status), _) => {
+                    statuses.insert(receipt.transaction_hash, receipt.is_sucessful());


We could replace is_successful() with just !status.is_zero()

leoyvens · 2021-06-28T14:09:01Z

store/postgres/src/chain_store.rs

+        let mut statuses = HashMap::new();
+        let mut pending = HashMap::new();
+        for receipt in receipts.into_iter() {
+            match (receipt.status, receipt.gas_used) {


If gas_used is None this should return an error.

chain/ethereum/src/adapter.rs

chain/ethereum/src/chain.rs

chain/ethereum/src/ethereum_adapter.rs

tilacog · 2021-07-06T20:57:21Z

chain/ethereum/src/ethereum_adapter.rs

+        Ok(Some(receipt)) => Ok(receipt),
+        Ok(None) => bail!("Could not find transaction receipt"),
+        Err(error) => bail!("Failed to fetch transaction receipt: {}", error),


I'm considering the absence of a receipt to be an error here.

chain/ethereum/src/ethereum_adapter.rs

tilacog · 2021-07-06T21:32:13Z

chain/ethereum/src/ethereum_adapter.rs

+    // Filter call triggers from unsuccessful transactions
+    block.trigger_data.retain(|trigger| {
+        if let EthereumTrigger::Call(call_trigger) = trigger {
+            // Unwrap: We already checked that those values exist
+            transaction_success[&call_trigger.transaction_hash.unwrap()]
+        } else {
+            // We are not filtering other types of triggers
+            true
+        }
+    });
+    Ok(block)


It's important to note that a block with empty triggers might be returned from this.
I'm not sure if I should drop the whole block in those cases.

Returning an empty block is fine.

chain/ethereum/src/ethereum_adapter.rs

graph/src/components/ethereum/types.rs

graph/src/components/transaction_receipt.rs

tilacog · 2021-07-06T21:46:42Z

graph/src/components/transaction_receipt.rs

+    let query = TransactionReceiptQuery {
+        schema_name: chain_name,
+        block_hash: block_hash.as_bytes(),
+    };


I'm still not sure if chain_name maps strictly to schema_name
I'll have that checked.

graph/src/data/subgraph/mod.rs

chain/ethereum/Cargo.toml

chain/ethereum/src/chain.rs

chain/ethereum/src/ethereum_adapter.rs

store/postgres/src/chain_store.rs

chain/ethereum/src/ethereum_adapter.rs

leoyvens · 2021-07-08T16:59:29Z

chain/ethereum/src/ethereum_adapter.rs

+    // Filter call triggers from unsuccessful transactions
+    block.trigger_data.retain(|trigger| {
+        if let EthereumTrigger::Call(call_trigger) = trigger {
+            // Unwrap: We already checked that those values exist
+            transaction_success[&call_trigger.transaction_hash.unwrap()]
+        } else {
+            // We are not filtering other types of triggers
+            true
+        }
+    });
+    Ok(block)


Returning an empty block is fine.

graph/src/components/ethereum/types.rs

tilacog · 2021-07-09T22:30:15Z

Please don't merge yet.
I've to check this w/ a live subgraph first.

leoyvens · 2021-07-16T18:03:15Z

graph/src/data/subgraph/mod.rs

@@ -656,7 +656,7 @@ impl UnresolvedMapping {
 pub struct UnifiedMappingApiVersion(Option<Version>);

 impl UnifiedMappingApiVersion {
-    pub fn equal_or_greater_than(&self, other_version: &'static Version) -> bool {
+    pub fn equal_or_greater_than(&self, other_version: &Version) -> bool {


the 'static here doesn't work anymore?

tilacog requested a review from leoyvens June 1, 2021 17:56

tilacog commented Jun 1, 2021

View reviewed changes

graph/src/components/ethereum/types.rs Outdated Show resolved Hide resolved

tilacog commented Jun 1, 2021

View reviewed changes

chain/ethereum/src/ethereum_adapter.rs Show resolved Hide resolved

tilacog force-pushed the tiago/dont-handle-failed-transactions branch from 4d43c22 to 91f10a0 Compare June 2, 2021 14:17

leoyvens reviewed Jun 2, 2021

View reviewed changes

graph/src/components/ethereum/types.rs Outdated Show resolved Hide resolved

tilacog force-pushed the tiago/dont-handle-failed-transactions branch from 3ca48da to 2659d6a Compare June 4, 2021 19:12

tilacog force-pushed the tiago/dont-handle-failed-transactions branch 8 times, most recently from a18a0dc to c3922f1 Compare June 16, 2021 18:00

leoyvens reviewed Jun 28, 2021

View reviewed changes

tilacog force-pushed the tiago/dont-handle-failed-transactions branch 2 times, most recently from 2eede27 to b89879f Compare July 5, 2021 22:36