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

fail when ens rainbow not present #4219

Merged
merged 1 commit into from
Dec 7, 2022
Merged
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
2 changes: 2 additions & 0 deletions graph/src/components/store/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub trait EnsLookup: Send + Sync + 'static {
/// Find the reverse of keccak256 for `hash` through looking it up in the
/// rainbow table.
fn find_name(&self, hash: &str) -> Result<Option<String>, StoreError>;
// Check if the rainbow table is filled.
fn is_table_empty(&self) -> Result<bool, StoreError>;
}

/// An entry point for all operations that require access to the node's storage
Expand Down
4 changes: 4 additions & 0 deletions runtime/wasm/src/host_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,10 @@ impl<C: Blockchain> HostExports<C> {
Ok(self.ens_lookup.find_name(hash)?)
}

pub(crate) fn is_ens_data_empty(&self) -> Result<bool, anyhow::Error> {
Ok(self.ens_lookup.is_table_empty()?)
}

pub(crate) fn log_log(
&self,
logger: &Logger,
Expand Down
6 changes: 6 additions & 0 deletions runtime/wasm/src/module/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1689,6 +1689,12 @@ impl<C: Blockchain> WasmInstanceContext<C> {

let hash: String = asc_get(self, hash_ptr, gas)?;
let name = self.ctx.host_exports.ens_name_by_hash(&*hash)?;
if name.is_none() && self.ctx.host_exports.is_ens_data_empty()? {
return Err(anyhow!(
"Missing ENS data: see https://github.com/graphprotocol/ens-rainbow"
)
.into());
}

// map `None` to `null`, and `Some(s)` to a runtime string
name.map(|name| asc_new(self, &*name, gas).map_err(Into::into))
Expand Down
12 changes: 12 additions & 0 deletions store/postgres/src/primary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,18 @@ impl<'a> Connection<'a> {
.map_err(|e| anyhow!("error looking up ens_name for hash {}: {}", hash, e).into())
}

pub fn is_ens_table_empty(&self) -> Result<bool, StoreError> {
use ens_names as dsl;

dsl::table
.select(dsl::name)
.limit(1)
.get_result::<String>(self.conn.as_ref())
.optional()
.map(|r| r.is_none())
.map_err(|e| anyhow!("error if ens table is empty: {}", e).into())
}

pub fn record_active_copy(&self, src: &Site, dst: &Site) -> Result<(), StoreError> {
use active_copies as cp;

Expand Down
52 changes: 48 additions & 4 deletions store/postgres/src/subgraph_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use diesel::{
};
use std::{
collections::{BTreeMap, HashMap},
sync::{Arc, Mutex},
sync::{atomic::AtomicU8, Arc, Mutex},
};
use std::{fmt, io::Write};
use std::{iter::FromIterator, time::Duration};
Expand Down Expand Up @@ -1147,23 +1147,67 @@ impl SubgraphStoreInner {
}
}

const STATE_ENS_NOT_CHECKED: u8 = 0;
const STATE_ENS_EMPTY: u8 = 1;
const STATE_ENS_NOT_EMPTY: u8 = 2;

/// EnsLookup reads from a rainbow table store in postgres that needs to be manually
/// loaded. To avoid unnecessary database roundtrips, the empty table check is lazy
/// and will not be retried. Once the table is checked, any subsequent calls will
/// just used the stored result.
struct EnsLookup {
primary: ConnectionPool,
// In order to keep the struct lock free, we'll use u8 for the status:
// 0 - Not Checked
// 1 - Checked - empty
// 2 - Checked - non empty
state: AtomicU8,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This caching has the downside of requiring a graph node reset after the names are loaded.

I don't think this is worth it as an optimization, the fn is_ens_data_empty query is cheap and it is run only if the name lookup returned empty.

Copy link
Collaborator

@lutter lutter Dec 6, 2022

Choose a reason for hiding this comment

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

I had discussed this with @mangas and wanted that to be cached so that subgraphs that run against populated tables that do have a lot of failed lookups don't pay an additional penalty, however small.

IMHO, requiring a node restart after loading that table isn't a big deal. Loading that table takes a while anyway (~ 10 minutes from my notes) If they have a subgraph that failed because of the table being empty, they'll most likely do that anyway to restart the subgraph.

}

impl EnsLookup {
pub fn new(pool: ConnectionPool) -> Self {
Self {
primary: pool,
state: AtomicU8::new(STATE_ENS_NOT_CHECKED),
}
}

fn is_table_empty(pool: &ConnectionPool) -> Result<bool, StoreError> {
let conn = pool.get()?;
primary::Connection::new(conn).is_ens_table_empty()
}
}

impl EnsLookupTrait for EnsLookup {
fn find_name(&self, hash: &str) -> Result<Option<String>, StoreError> {
let conn = self.primary.get()?;
primary::Connection::new(conn).find_ens_name(hash)
}

fn is_table_empty(&self) -> Result<bool, StoreError> {
match self.state.load(std::sync::atomic::Ordering::SeqCst) {
STATE_ENS_NOT_CHECKED => {}
STATE_ENS_EMPTY => return Ok(true),
STATE_ENS_NOT_EMPTY => return Ok(false),
_ => unreachable!("unsupported state"),
}

let is_empty = Self::is_table_empty(&self.primary)?;
let new_state = match is_empty {
true => STATE_ENS_EMPTY,
false => STATE_ENS_NOT_EMPTY,
};
self.state
.store(new_state, std::sync::atomic::Ordering::SeqCst);

Ok(is_empty)
}
}

#[async_trait::async_trait]
impl SubgraphStoreTrait for SubgraphStore {
fn ens_lookup(&self) -> Arc<dyn EnsLookupTrait> {
Arc::new(EnsLookup {
primary: self.mirror.primary().clone(),
})
Arc::new(EnsLookup::new(self.mirror.primary().clone()))
}

// FIXME: This method should not get a node_id
Expand Down