chain, graph: Use a Result type when checking for transaction success

chain/ethereum/Cargo.toml

@@ -28,6 +28,7 @@ ethabi = { git = "https://github.com/graphprotocol/ethabi.git", branch = "master
 
 # We have a couple custom patches to web3.
 web3 = { git = "https://github.com/graphprotocol/rust-web3", branch = "master" }
+itertools = "0.10.0"
 
 [dev-dependencies]
 diesel = { version = "1.4.6", features = ["postgres", "serde_json", "numeric", "r2d2"] }

chain/ethereum/src/chain.rs

@@ -338,7 +338,7 @@ impl TriggersAdapterTrait<Chain> for TriggersAdapter {
                     &filter.log,
                     &full_block.ethereum_block,
                 ));
-                triggers.append(&mut parse_call_triggers(&filter.call, &full_block));
+                triggers.append(&mut parse_call_triggers(&filter.call, &full_block)?);
                 triggers.append(&mut parse_block_triggers(filter.block.clone(), &full_block));
                 Ok(BlockWithTriggers::new(
                     WrappedBlockFinality(block),

chain/ethereum/src/ethereum_adapter.rs

@@ -1,14 +1,7 @@
+use ethabi::ParamType;
 use ethabi::Token;
 use futures::future;
 use futures::prelude::*;
-use lazy_static::lazy_static;
-use std::collections::{HashMap, HashSet};
-use std::convert::TryFrom;
-use std::iter::FromIterator;
-use std::sync::Arc;
-use std::time::Instant;
-
-use ethabi::ParamType;
 use graph::{
     blockchain::{block_stream::BlockWithTriggers, BlockPtr, IngestorError},
     prelude::{
@@ -30,6 +23,13 @@ use graph::{
     components::ethereum::*,
     prelude::web3::types::{Trace, TraceFilter, TraceFilterBuilder, H160},
 };
+use itertools::Itertools;
+use lazy_static::lazy_static;
+use std::collections::{HashMap, HashSet};
+use std::convert::TryFrom;
+use std::iter::FromIterator;
+use std::sync::Arc;
+use std::time::Instant;
 use web3::api::Web3;
 use web3::transports::batch::Batch;
 use web3::types::Filter;
@@ -1588,15 +1588,21 @@ pub(crate) fn parse_log_triggers(
 pub(crate) fn parse_call_triggers(
     call_filter: &EthereumCallFilter,
     block: &EthereumBlockWithCalls,
-) -> Vec<EthereumTrigger> {
+) -> anyhow::Result<Vec<EthereumTrigger>> {
     match &block.calls {
         Some(calls) => calls
             .iter()
             .filter(move |call| call_filter.matches(call))
-            .filter(move |call| block.transaction_for_call_succeeded(call))
-            .map(move |call| EthereumTrigger::Call(Arc::new(call.clone())))
+            .map(
+                move |call| match block.transaction_for_call_succeeded(call) {
+                    Ok(true) => Ok(Some(EthereumTrigger::Call(Arc::new(call.clone())))),
+                    Ok(false) => Ok(None),
+                    Err(e) => Err(e),
+                },
+            )
+            .filter_map_ok(|some_trigger| some_trigger)
             .collect(),
-        None => vec![],
+        None => Ok(vec![]),
     }
 }
 

graph/src/components/ethereum/types.rs

@@ -11,6 +11,8 @@ use crate::{
     prelude::{BlockNumber, DeploymentHash, EntityKey, ToEntityKey},
 };
 
+const BYZANTIUM_FORK_BLOCK_NUMBER: u64 = 4_370_000;
+
 pub type LightEthereumBlock = Block<Transaction>;
 
 pub trait LightEthereumBlockExt {
@@ -113,29 +115,48 @@ pub struct EthereumBlockWithCalls {
 
 impl EthereumBlockWithCalls {
     /// Given an `EthereumCall`, check within receipts if that transaction was successful.
-    pub fn transaction_for_call_succeeded(&self, call: &EthereumCall) -> bool {
-        // return true if call does not have a transaction hash.
-        // TODO: should we return `false` instead?
-        let call_transaction_hash = match call.transaction_hash {
-            Some(transaction_hash) => transaction_hash,
-            None => return true,
-        };
+    pub fn transaction_for_call_succeeded(&self, call: &EthereumCall) -> anyhow::Result<bool> {
+        let call_transaction_hash = call.transaction_hash.ok_or(anyhow::anyhow!(
+            "failed to find a transaction for this call"
+        ))?;
 
-        // TODO: if there is no transaction receipt, should we return `true` instead?
-        let receipt = match self
+        let receipt = self
             .ethereum_block
             .transaction_receipts
             .iter()
             .find(|txn| txn.transaction_hash == call_transaction_hash)
-        {
-            Some(receipt) => receipt,
-            None => return false,
-        };
+            .ok_or(anyhow::anyhow!(
+                "failed to find the receipt for this transaction"
+            ))?;
+
+        let transaction = self
+            .ethereum_block
+            .block
+            .transaction_for_call(&call)
+            .ok_or(anyhow::anyhow!(
+                "failed to find the transaction for this call"
+            ))?;
+
+        let pre_byzantium = self
+            .ethereum_block
+            .block
+            .number
+            .ok_or(anyhow::anyhow!("Pending block number"))?
+            .as_u64()
+            < BYZANTIUM_FORK_BLOCK_NUMBER;
+
+        let used_all_gas = receipt
+            .gas_used
+            .ok_or(anyhow::anyhow!("Running in light client mode)"))?
+            >= transaction.gas;
+
+        if pre_byzantium && used_all_gas {
+            return Ok(false);
+        }
 
-        // TODO: should we handle None differently than Some(0)?
         match receipt.status {
-            Some(x) if x == web3::types::U64::from(1) => true,
-            Some(_) | None => false,
+            Some(x) if x == web3::types::U64::from(1) => Ok(true),
+            Some(_) | None => Ok(false),
         }
     }
 }