Skip to content

Commit

Permalink
EmptyNodeCapabilities for chains that don't discriminate nodes (#3985)
Browse files Browse the repository at this point in the history
* chain(all), graph: EmptyNodeCapabilities

* chain-ethereum: fix PartialOrd for NodeCapabilities

The Ord and PartialOrd implementations of NodeCapabilities currently
break trait rules: `NodeCapabilities {true, false}` is both greater
than and less than (at the same time) `NodeCapabilities {false, true}`.
It'd be nice to get this fixed before we get some obscure bug during
comparisons, or a panic due to std assuming transitivity during
sorts.
  • Loading branch information
neysofu authored Dec 8, 2022
1 parent 92d160c commit a18d80c
Show file tree
Hide file tree
Showing 18 changed files with 112 additions and 191 deletions.
5 changes: 2 additions & 3 deletions chain/arweave/src/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::capabilities::NodeCapabilities;
use crate::{data_source::DataSource, Chain};
use graph::blockchain as bc;
use graph::prelude::*;
Expand Down Expand Up @@ -26,8 +25,8 @@ impl bc::TriggerFilter<Chain> for TriggerFilter {
transaction_filter.extend(ArweaveTransactionFilter::from_data_sources(data_sources));
}

fn node_capabilities(&self) -> NodeCapabilities {
NodeCapabilities {}
fn node_capabilities(&self) -> bc::EmptyNodeCapabilities<Chain> {
bc::EmptyNodeCapabilities::default()
}

fn extend_with_template(
Expand Down
37 changes: 0 additions & 37 deletions chain/arweave/src/capabilities.rs

This file was deleted.

11 changes: 7 additions & 4 deletions chain/arweave/src/chain.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use graph::blockchain::{Block, BlockchainKind};
use graph::blockchain::{Block, BlockchainKind, EmptyNodeCapabilities};
use graph::cheap_clone::CheapClone;
use graph::data::subgraph::UnifiedMappingApiVersion;
use graph::firehose::{FirehoseEndpoint, FirehoseEndpoints};
Expand All @@ -20,7 +20,6 @@ use prost::Message;
use std::sync::Arc;

use crate::adapter::TriggerFilter;
use crate::capabilities::NodeCapabilities;
use crate::data_source::{DataSourceTemplate, UnresolvedDataSourceTemplate};
use crate::runtime::RuntimeAdapter;
use crate::trigger::{self, ArweaveTrigger};
Expand Down Expand Up @@ -82,7 +81,7 @@ impl Blockchain for Chain {

type TriggerFilter = crate::adapter::TriggerFilter;

type NodeCapabilities = crate::capabilities::NodeCapabilities;
type NodeCapabilities = EmptyNodeCapabilities<Self>;

fn triggers_adapter(
&self,
Expand Down Expand Up @@ -116,7 +115,11 @@ impl Blockchain for Chain {
unified_api_version: UnifiedMappingApiVersion,
) -> Result<Box<dyn BlockStream<Self>>, Error> {
let adapter = self
.triggers_adapter(&deployment, &NodeCapabilities {}, unified_api_version)
.triggers_adapter(
&deployment,
&EmptyNodeCapabilities::default(),
unified_api_version,
)
.unwrap_or_else(|_| panic!("no adapter for network {}", self.name));

let firehose_endpoint = self.firehose_endpoints.random()?;
Expand Down
1 change: 0 additions & 1 deletion chain/arweave/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod adapter;
mod capabilities;
mod chain;
mod codec;
mod data_source;
Expand Down
5 changes: 2 additions & 3 deletions chain/cosmos/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::collections::HashSet;
use prost::Message;
use prost_types::Any;

use crate::capabilities::NodeCapabilities;
use crate::{data_source::DataSource, Chain};
use graph::blockchain as bc;
use graph::firehose::EventTypeFilter;
Expand All @@ -25,8 +24,8 @@ impl bc::TriggerFilter<Chain> for TriggerFilter {
self.block_filter.extend_from_data_sources(data_sources);
}

fn node_capabilities(&self) -> NodeCapabilities {
NodeCapabilities {}
fn node_capabilities(&self) -> bc::EmptyNodeCapabilities<Chain> {
bc::EmptyNodeCapabilities::default()
}

fn extend_with_template(
Expand Down
33 changes: 0 additions & 33 deletions chain/cosmos/src/capabilities.rs

This file was deleted.

13 changes: 8 additions & 5 deletions chain/cosmos/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@ use graph::{
FirehoseMapper as FirehoseMapperTrait, TriggersAdapter as TriggersAdapterTrait,
},
firehose_block_stream::FirehoseBlockStream,
Block as _, BlockHash, BlockPtr, Blockchain, BlockchainKind, IngestorError,
RuntimeAdapter as RuntimeAdapterTrait,
Block as _, BlockHash, BlockPtr, Blockchain, BlockchainKind, EmptyNodeCapabilities,
IngestorError, RuntimeAdapter as RuntimeAdapterTrait,
},
components::store::DeploymentLocator,
firehose::{self, FirehoseEndpoint, FirehoseEndpoints, ForkStep},
prelude::{async_trait, o, BlockNumber, ChainStore, Error, Logger, LoggerFactory},
};
use prost::Message;

use crate::capabilities::NodeCapabilities;
use crate::data_source::{
DataSource, DataSourceTemplate, EventOrigin, UnresolvedDataSource, UnresolvedDataSourceTemplate,
};
Expand Down Expand Up @@ -80,7 +79,7 @@ impl Blockchain for Chain {

type TriggerFilter = TriggerFilter;

type NodeCapabilities = NodeCapabilities;
type NodeCapabilities = EmptyNodeCapabilities<Self>;

fn is_refetch_block_required(&self) -> bool {
false
Expand Down Expand Up @@ -113,7 +112,11 @@ impl Blockchain for Chain {
unified_api_version: UnifiedMappingApiVersion,
) -> Result<Box<dyn BlockStream<Self>>, Error> {
let adapter = self
.triggers_adapter(&deployment, &NodeCapabilities {}, unified_api_version)
.triggers_adapter(
&deployment,
&EmptyNodeCapabilities::default(),
unified_api_version,
)
.unwrap_or_else(|_| panic!("no adapter for network {}", self.name));

let firehose_endpoint = self.firehose_endpoints.random()?;
Expand Down
1 change: 0 additions & 1 deletion chain/cosmos/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod adapter;
mod capabilities;
pub mod chain;
pub mod codec;
mod data_source;
Expand Down
40 changes: 19 additions & 21 deletions chain/ethereum/src/capabilities.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use anyhow::Error;
use graph::impl_slog_value;
use std::cmp::Ordering;
use std::collections::BTreeSet;
use std::fmt;
use std::str::FromStr;
use std::{
cmp::{Ord, Ordering, PartialOrd},
collections::BTreeSet,
};

use crate::DataSource;

Expand All @@ -15,28 +13,28 @@ pub struct NodeCapabilities {
pub traces: bool,
}

// Take all NodeCapabilities fields into account when ordering
// A NodeCapabilities instance is considered equal or greater than another
// if all of its fields are equal or greater than the other
impl Ord for NodeCapabilities {
fn cmp(&self, other: &Self) -> Ordering {
match (
/// Two [`NodeCapabilities`] can only be compared if one is the subset of the
/// other. No [`Ord`] (i.e. total order) implementation is applicable.
impl PartialOrd for NodeCapabilities {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
product_order([
self.archive.cmp(&other.archive),
self.traces.cmp(&other.traces),
) {
(Ordering::Greater, Ordering::Greater) => Ordering::Greater,
(Ordering::Greater, Ordering::Equal) => Ordering::Greater,
(Ordering::Equal, Ordering::Greater) => Ordering::Greater,
(Ordering::Equal, Ordering::Equal) => Ordering::Equal,
(Ordering::Less, _) => Ordering::Less,
(_, Ordering::Less) => Ordering::Less,
}
])
}
}

impl PartialOrd for NodeCapabilities {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
/// Defines a [product order](https://en.wikipedia.org/wiki/Product_order) over
/// an array of [`Ordering`].
fn product_order<const N: usize>(cmps: [Ordering; N]) -> Option<Ordering> {
if cmps.iter().all(|c| c.is_eq()) {
Some(Ordering::Equal)
} else if cmps.iter().all(|c| c.is_le()) {
Some(Ordering::Less)
} else if cmps.iter().all(|c| c.is_ge()) {
Some(Ordering::Greater)
} else {
None
}
}

Expand Down
12 changes: 9 additions & 3 deletions chain/ethereum/src/network.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use anyhow::{anyhow, Context};
use graph::cheap_clone::CheapClone;
use graph::prelude::rand::{self, seq::IteratorRandom};
use std::cmp::Ordering;
use std::collections::HashMap;
use std::sync::Arc;

Expand Down Expand Up @@ -134,9 +135,14 @@ impl EthereumNetworks {

pub fn sort(&mut self) {
for adapters in self.networks.values_mut() {
adapters
.adapters
.sort_by_key(|adapter| adapter.capabilities)
adapters.adapters.sort_by(|a, b| {
a.capabilities
.partial_cmp(&b.capabilities)
// We can't define a total ordering over node capabilities,
// so incomparable items are considered equal and end up
// near each other.
.unwrap_or(Ordering::Equal)
})
}
}

Expand Down
5 changes: 2 additions & 3 deletions chain/near/src/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::HashSet;

use crate::capabilities::NodeCapabilities;
use crate::data_source::PartialAccounts;
use crate::{data_source::DataSource, Chain};
use graph::blockchain as bc;
Expand Down Expand Up @@ -29,8 +28,8 @@ impl bc::TriggerFilter<Chain> for TriggerFilter {
receipt_filter.extend(NearReceiptFilter::from_data_sources(data_sources));
}

fn node_capabilities(&self) -> NodeCapabilities {
NodeCapabilities {}
fn node_capabilities(&self) -> bc::EmptyNodeCapabilities<Chain> {
bc::EmptyNodeCapabilities::default()
}

fn extend_with_template(
Expand Down
37 changes: 0 additions & 37 deletions chain/near/src/capabilities.rs

This file was deleted.

12 changes: 8 additions & 4 deletions chain/near/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use graph::{
FirehoseMapper as FirehoseMapperTrait, TriggersAdapter as TriggersAdapterTrait,
},
firehose_block_stream::FirehoseBlockStream,
BlockHash, BlockPtr, Blockchain, IngestorError, RuntimeAdapter as RuntimeAdapterTrait,
BlockHash, BlockPtr, Blockchain, EmptyNodeCapabilities, IngestorError,
RuntimeAdapter as RuntimeAdapterTrait,
},
components::store::DeploymentLocator,
firehose::{self as firehose, ForkStep},
Expand All @@ -21,7 +22,6 @@ use prost::Message;
use std::sync::Arc;

use crate::adapter::TriggerFilter;
use crate::capabilities::NodeCapabilities;
use crate::data_source::{DataSourceTemplate, UnresolvedDataSourceTemplate};
use crate::runtime::RuntimeAdapter;
use crate::trigger::{self, NearTrigger};
Expand All @@ -46,7 +46,11 @@ impl BlockStreamBuilder<Chain> for NearStreamBuilder {
unified_api_version: UnifiedMappingApiVersion,
) -> Result<Box<dyn BlockStream<Chain>>> {
let adapter = chain
.triggers_adapter(&deployment, &NodeCapabilities {}, unified_api_version)
.triggers_adapter(
&deployment,
&EmptyNodeCapabilities::default(),
unified_api_version,
)
.unwrap_or_else(|_| panic!("no adapter for network {}", chain.name));

let firehose_endpoint = chain.firehose_endpoints.random()?;
Expand Down Expand Up @@ -140,7 +144,7 @@ impl Blockchain for Chain {

type TriggerFilter = crate::adapter::TriggerFilter;

type NodeCapabilities = crate::capabilities::NodeCapabilities;
type NodeCapabilities = EmptyNodeCapabilities<Chain>;

fn triggers_adapter(
&self,
Expand Down
1 change: 0 additions & 1 deletion chain/near/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod adapter;
mod capabilities;
mod chain;
pub mod codec;
mod data_source;
Expand Down
Loading

0 comments on commit a18d80c

Please sign in to comment.