Skip to content

graphman config check providers #5517

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

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Conversation

mangas
Copy link
Contributor

@mangas mangas commented Jun 28, 2024

  • Adds a graphman command to check the genesis information against all providers (graphman config check-providers)
  • Adds a graphman command to update the genesis block of a chain (graphman chain update-genesis BLOCK_HASH CHAIN-NAME [--force])

@mangas mangas force-pushed the filipe/graphman-config-providers branch from d259620 to 40fcc35 Compare June 28, 2024 13:19
@mangas mangas marked this pull request as ready for review June 28, 2024 15:24
@fordN fordN requested a review from incrypto32 July 8, 2024 15:52
@mangas mangas force-pushed the filipe/graphman-config-providers branch from 6a771c7 to 8773434 Compare July 9, 2024 06:07
Comment on lines 2311 to 2330
fn set_chain_identifier(&self, ident: &ChainIdentifier) -> Result<(), Error> {
use primary::chains as c;
use public::ethereum_networks as n;

let mut conn = self.pool.get()?;
diesel::update(n::table.filter(n::name.eq(&self.chain)))
.set((
n::genesis_block_hash.eq(ident.genesis_block_hash.hash_hex()),
n::net_version.eq(&ident.net_version),
))
.execute(&mut conn)?;
conn.transaction(|conn| {
diesel::update(n::table.filter(n::name.eq(&self.chain)))
.set((
n::genesis_block_hash.eq(ident.genesis_block_hash.hash_hex()),
n::net_version.eq(&ident.net_version),
))
.execute(conn)?;

diesel::update(c::table.filter(c::name.eq(&self.chain)))
.set((
c::genesis_block_hash.eq(ident.genesis_block_hash.hash_hex()),
c::net_version.eq(&ident.net_version),
))
.execute(conn)
})?;
Copy link
Member

Choose a reason for hiding this comment

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

Bit confused about the location of these tables. Isn't the chains table on the primary shard and ethereum_network on the shard of the block cache for the chain is, so in this transaction we are only updating the block cache shard right?
Don't we need to update in primary also?

Comment on lines 28 to 37
println!("Checking providers");
for (chain_id, ids) in networks.all_chain_identifiers().await.into_iter() {
let (_oks, errs): (Vec<_>, Vec<_>) = ids
.into_iter()
.map(|(provider, id)| {
id.map_err(IdentValidatorError::from)
.and_then(|id| store.check_ident(chain_id, &id).map(|_| (provider, id)))
})
.partition_result();
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpick: Felt like this bit could be made a bit more readable, feel free to leave as it is if there's time constraint

Comment on lines 151 to 157
// let mut adapters: Vec<dyn NetIdentifiable> =
// self.rpc_provider_manager.get_all_unverified(chain_id);
// adapters.extend(self.firehose_provider_manager.get_all_unverified(chain_id));
// adapters.extend(
// self.substreams_provider_manager
// .get_all_unverified(chain_id),
// );
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove these comments?

@mangas mangas force-pushed the filipe/graphman-config-providers branch 3 times, most recently from 83fc45f to e9d37bc Compare July 10, 2024 13:42
@mangas mangas force-pushed the filipe/graphman-config-providers branch from e9d37bc to 2c5d375 Compare July 10, 2024 13:44
Copy link
Contributor

@fordN fordN left a comment

Choose a reason for hiding this comment

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

👍

@mangas mangas merged commit e3aad48 into master Jul 15, 2024
7 checks passed
@mangas mangas deleted the filipe/graphman-config-providers branch July 15, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants