Skip to content

Commit 1e10c68

Browse files
mangasmangas
committed
code review updates
1 parent a2acdaa commit 1e10c68

File tree

14 files changed

+80
-71
lines changed

14 files changed

+80
-71
lines changed

‎chain/ethereum/src/chain.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ impl Chain {
402402
hash: &BlockHash,
403403
) -> Result<Option<(String, BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError>
404404
{
405-
self.chain_store.block_number(hash).await
405+
self.chain_store.block_pointer(hash).await
406406
}
407407

408408
// TODO: This is only used to build the block stream which could prolly

‎graph/src/blockchain/mock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ impl ChainStore for MockChainStore {
546546
fn confirm_block_hash(&self, _number: BlockNumber, _hash: &BlockHash) -> Result<usize, Error> {
547547
unimplemented!()
548548
}
549-
async fn block_number(
549+
async fn block_pointer(
550550
&self,
551551
_hash: &BlockHash,
552552
) -> Result<Option<(String, BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError>

‎graph/src/blockchain/types.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -591,16 +591,22 @@ impl TryFrom<Option<String>> for BlockTime {
591591

592592
fn try_from(ts: Option<String>) -> Result<Self, Self::Error> {
593593
match ts {
594-
Some(str) => return BlockTime::from_str(&str),
594+
Some(str) => return BlockTime::from_hex_str(&str),
595595
None => return Ok(BlockTime::NONE),
596596
};
597597
}
598598
}
599599

600-
impl FromStr for BlockTime {
601-
type Err = ParseIntError;
600+
impl BlockTime {
601+
/// A timestamp from a long long time ago used to indicate that we don't
602+
/// have a timestamp
603+
pub const NONE: Self = Self::MIN;
604+
605+
pub const MAX: Self = Self(Timestamp::MAX);
602606

603-
fn from_str(ts: &str) -> Result<Self, Self::Err> {
607+
pub const MIN: Self = Self(Timestamp(DateTime::from_timestamp_nanos(0)));
608+
609+
pub fn from_hex_str(ts: &str) -> Result<Self, ParseIntError> {
604610
let (radix, idx) = if ts.starts_with("0x") {
605611
(16, 2)
606612
} else {
@@ -609,16 +615,6 @@ impl FromStr for BlockTime {
609615

610616
u64::from_str_radix(&ts[idx..], radix).map(|ts| BlockTime::since_epoch(ts as i64, 0))
611617
}
612-
}
613-
614-
impl BlockTime {
615-
// /// A timestamp from a long long time ago used to indicate that we don't
616-
// /// have a timestamp
617-
pub const NONE: Self = Self::MIN;
618-
619-
pub const MAX: Self = Self(Timestamp::MAX);
620-
621-
pub const MIN: Self = Self(Timestamp(DateTime::from_timestamp_nanos(0)));
622618

623619
/// Construct a block time that is the given number of seconds and
624620
/// nanoseconds after the Unix epoch

‎graph/src/components/store/traits.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ pub trait ChainStore: ChainHeadStore {
576576
/// Currently, the timestamp is only returned if it's present in the top level block. This format is
577577
/// depends on the chain and the implementation of Blockchain::Block for the specific chain.
578578
/// eg: {"block": { "timestamp": 123123123 } }
579-
async fn block_number(
579+
async fn block_pointer(
580580
&self,
581581
hash: &BlockHash,
582582
) -> Result<Option<(String, BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError>;
@@ -665,7 +665,7 @@ pub trait QueryStore: Send + Sync {
665665
/// Returns the blocknumber, timestamp and the parentHash. Timestamp depends on the chain block type
666666
/// and can have multiple formats, it can also not be prevent. For now this is only available
667667
/// for EVM chains both firehose and rpc.
668-
async fn block_number_with_timestamp_and_parent_hash(
668+
async fn block_pointer(
669669
&self,
670670
block_hash: &BlockHash,
671671
) -> Result<Option<(BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError>;

‎graph/src/data_source/offchain.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl DataSource {
216216
data_source::MappingTrigger::Offchain(trigger.clone()),
217217
self.mapping.handler.clone(),
218218
BlockPtr::new(Default::default(), self.creation_block.unwrap_or(0)),
219-
BlockTime::MIN,
219+
BlockTime::NONE,
220220
))
221221
}
222222

‎graphql/src/runner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ where
111111
let latest_block = match store.block_ptr().await.ok().flatten() {
112112
Some(block) => Some(LatestBlockInfo {
113113
timestamp: store
114-
.block_number_with_timestamp_and_parent_hash(&block.hash)
114+
.block_pointer(&block.hash)
115115
.await
116116
.ok()
117117
.flatten()

‎graphql/src/store/resolver.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ impl StoreResolver {
186186
let (timestamp, parent_hash) = if lookup_needed(field) {
187187
match self
188188
.store
189-
.block_number_with_timestamp_and_parent_hash(&block_ptr.hash)
189+
.block_pointer(&block_ptr.hash)
190190
.await
191191
.map_err(Into::<QueryExecutionError>::into)?
192192
{

‎node/src/manager/commands/rewind.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ async fn block_ptr(
4444
None => bail!("can not find chain store for {}", chain),
4545
Some(store) => store,
4646
};
47-
if let Some((_, number, _, _)) = chain_store.block_number(&block_ptr_to.hash).await? {
47+
if let Some((_, number, _, _)) = chain_store.block_pointer(&block_ptr_to.hash).await? {
4848
if number != block_ptr_to.number {
4949
bail!(
5050
"the given hash is for block number {} but the command specified block number {}",

‎store/postgres/src/block_store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ impl BlockStore {
391391
if create {
392392
store.create(&ident)?;
393393
}
394-
store.ensure_up_to_date(&ident)?;
394+
store.migrate(&ident)?;
395395

396396
let store = Arc::new(store);
397397
self.stores

‎store/postgres/src/chain_store.rs

Lines changed: 54 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ mod data {
669669
let pointers_query = if overwrite {
670670
format!(
671671
"insert into {pointers_table}(hash, number, parent_hash, timestamp) \
672-
values ($1, $2, $3, $5) \
672+
values ($1, $2, $3, $4) \
673673
on conflict(hash) \
674674
do update set number = $2, parent_hash = $3, timestamp = $5;
675675
",
@@ -678,7 +678,7 @@ mod data {
678678
} else {
679679
format!(
680680
"insert into {pointers_table}(hash, number, parent_hash, timestamp) \
681-
values ($1, $2, $3, $5) \
681+
values ($1, $2, $3, $4) \
682682
on conflict(hash) do nothing;
683683
",
684684
pointers_table = block_pointers.qname,
@@ -688,15 +688,15 @@ mod data {
688688
let blocks_query = if overwrite {
689689
format!(
690690
"insert into {blocks_table}(hash, data) \
691-
values ($1, $4) \
691+
values ($1, $2) \
692692
on conflict(hash) \
693-
do update set data = $4;",
693+
do update set data = $2;",
694694
blocks_table = blocks.qname,
695695
)
696696
} else {
697697
format!(
698698
"insert into {blocks_table}(hash, data) \
699-
values ($1, $4) \
699+
values ($1, $2) \
700700
on conflict(hash) do nothing;",
701701
blocks_table = blocks.qname,
702702
)
@@ -705,22 +705,18 @@ mod data {
705705
conn.transaction(move |conn| {
706706
let data = data;
707707
sql_query(blocks_query)
708+
.bind::<Bytea, _>(hash.as_slice())
709+
.bind::<Jsonb, _>(&data)
710+
.execute(conn)
711+
.map_err(StoreError::from)?;
712+
713+
sql_query(pointers_query)
708714
.bind::<Bytea, _>(hash.as_slice())
709715
.bind::<BigInt, _>(number)
710716
.bind::<Bytea, _>(parent_hash.as_slice())
711-
.bind::<Jsonb, _>(&data)
717+
.bind::<Timestamptz, _>(&block.timestamp())
712718
.execute(conn)
713719
.map_err(StoreError::from)
714-
.and_then(|_| {
715-
sql_query(pointers_query)
716-
.bind::<Bytea, _>(hash.as_slice())
717-
.bind::<BigInt, _>(number)
718-
.bind::<Bytea, _>(parent_hash.as_slice())
719-
.bind::<Jsonb, _>(data)
720-
.bind::<Timestamptz, _>(&block.timestamp())
721-
.execute(conn)
722-
.map_err(StoreError::from)
723-
})
724720
})?;
725721
}
726722
};
@@ -894,16 +890,41 @@ mod data {
894890
.execute(conn)
895891
.map_err(Error::from)
896892
}
897-
Storage::Private(Schema { block_pointers, .. }) => {
898-
let query = format!(
893+
Storage::Private(Schema {
894+
block_pointers,
895+
blocks,
896+
..
897+
}) => {
898+
let blocks_query = format!(
899+
"
900+
DELETE FROM {block_table}
901+
WHERE hash in (
902+
SELECT hash
903+
FROM {ptrs_table}
904+
WHERE number = $1 AND hash != $2
905+
)
906+
",
907+
block_table = blocks.qname,
908+
ptrs_table = block_pointers.qname
909+
);
910+
let ptrs_query = format!(
899911
"delete from {} where number = $1 and hash != $2",
900912
block_pointers.qname
901913
);
902-
sql_query(query)
903-
.bind::<BigInt, _>(number)
904-
.bind::<Bytea, _>(hash.as_slice())
905-
.execute(conn)
906-
.map_err(Error::from)
914+
915+
conn.transaction(|conn| {
916+
sql_query(blocks_query)
917+
.bind::<BigInt, _>(number)
918+
.bind::<Bytea, _>(hash.as_slice())
919+
.execute(conn)
920+
.map_err(Error::from)?;
921+
922+
sql_query(ptrs_query)
923+
.bind::<BigInt, _>(number)
924+
.bind::<Bytea, _>(hash.as_slice())
925+
.execute(conn)
926+
.map_err(Error::from)
927+
})
907928
}
908929
}
909930
}
@@ -971,12 +992,7 @@ mod data {
971992
Some((number, ts, parent_hash)) => {
972993
let number = BlockNumber::try_from(number)
973994
.map_err(|e| StoreError::QueryExecutionError(e.to_string()))?;
974-
Ok(Some((
975-
number,
976-
ts,
977-
// crate::chain_store::try_parse_timestamp(ts)?,
978-
parent_hash.map(|h| BlockHash::from(h)),
979-
)))
995+
Ok(Some((number, ts, parent_hash.map(|h| BlockHash::from(h)))))
980996
}
981997
}
982998
}
@@ -1006,14 +1022,11 @@ mod data {
10061022
})
10071023
.collect::<Vec<_>>()
10081024
}
1009-
Storage::Private(Schema { block_pointers, .. }) => {
1010-
// let hashes: Vec<_> = hashes.into_iter().map(|hash| &hash.0).collect();
1011-
block_pointers
1012-
.table()
1013-
.select((block_pointers.hash(), block_pointers.number()))
1014-
.filter(block_pointers.hash().eq_any(hashes))
1015-
.load::<(BlockHash, i64)>(conn)?
1016-
}
1025+
Storage::Private(Schema { block_pointers, .. }) => block_pointers
1026+
.table()
1027+
.select((block_pointers.hash(), block_pointers.number()))
1028+
.filter(block_pointers.hash().eq_any(hashes))
1029+
.load::<(BlockHash, i64)>(conn)?,
10171030
};
10181031

10191032
let pairs = pairs
@@ -1403,7 +1416,8 @@ mod data {
14031416
"
14041417
DELETE FROM {blocks}
14051418
WHERE hash in (
1406-
SELECT FROM {block_ptrs}
1419+
SELECT hash
1420+
FROM {block_ptrs}
14071421
WHERE hash = any($1) AND number > 0;
14081422
);
14091423
DELETE FROM {block_ptrs} WHERE hash = any($1) AND number > 0;
@@ -1998,7 +2012,7 @@ impl ChainStore {
19982012
Ok(())
19992013
}
20002014

2001-
pub(crate) fn ensure_up_to_date(&self, _ident: &ChainIdentifier) -> Result<(), Error> {
2015+
pub(crate) fn migrate(&self, _ident: &ChainIdentifier) -> Result<(), Error> {
20022016
let mut conn = self.get_conn()?;
20032017
// no version (which implicitly is version 1) to version 2 upgrade.
20042018
self.storage.split_block_cache_update(&mut conn)?;
@@ -2683,7 +2697,7 @@ impl ChainStoreTrait for ChainStore {
26832697
.confirm_block_hash(&mut conn, &self.chain, number, hash)
26842698
}
26852699

2686-
async fn block_number(
2700+
async fn block_pointer(
26872701
&self,
26882702
hash: &BlockHash,
26892703
) -> Result<Option<(String, BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError>

‎store/postgres/src/query_store.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl QueryStoreTrait for QueryStore {
7070
async fn block_ptr(&self) -> Result<Option<BlockPtr>, StoreError> {
7171
self.store.block_ptr(self.site.cheap_clone()).await
7272
}
73-
async fn block_number_with_timestamp_and_parent_hash(
73+
async fn block_pointer(
7474
&self,
7575
block_hash: &BlockHash,
7676
) -> Result<Option<(BlockNumber, Option<BlockTime>, Option<BlockHash>)>, StoreError> {
@@ -82,7 +82,7 @@ impl QueryStoreTrait for QueryStore {
8282
// database the blocks on the main chain that we consider final
8383
let subgraph_network = self.network_name();
8484
self.chain_store
85-
.block_number(block_hash)
85+
.block_pointer(block_hash)
8686
.await?
8787
.map(|(network_name, number, timestamp, parent_hash)| {
8888
if network_name == subgraph_network {
@@ -101,7 +101,7 @@ impl QueryStoreTrait for QueryStore {
101101
&self,
102102
block_hash: &BlockHash,
103103
) -> Result<Option<BlockNumber>, StoreError> {
104-
self.block_number_with_timestamp_and_parent_hash(block_hash)
104+
self.block_pointer(block_hash)
105105
.await
106106
.map(|opt| opt.map(|(number, _, _)| number))
107107
}

‎store/test-store/src/block_store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ lazy_static! {
5858
pub const NO_PARENT: &str = "0000000000000000000000000000000000000000000000000000000000000000";
5959
/// The parts of an Ethereum block that are interesting for these tests:
6060
/// the block number, hash, and the hash of the parent block
61-
#[derive(Default, Clone, Debug, PartialEq)]
61+
#[derive(Clone, Debug, PartialEq)]
6262
pub struct FakeBlock {
6363
pub number: BlockNumber,
6464
pub hash: String,

‎store/test-store/tests/graphql/query.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ lazy_static! {
5353
/// The id of the sole publisher in the test data
5454
static ref PUB1: IdVal = IdType::Bytes.parse("0xb1");
5555
/// The chain we actually put into the chain store, blocks 0 to 3
56-
// static ref CHAIN: Vec<FakeBlock> = vec![GENESIS_BLOCK.clone(), BLOCK_ONE.clone(), BLOCK_TWO.clone(), BLOCK_THREE.clone()];
5756
static ref CHAIN: Vec<FakeBlock> = vec![GENESIS_BLOCK.clone(), BLOCK_ONE.clone(), BLOCK_TWO.clone(), BLOCK_THREE.clone()];
5857
/// The known block pointers for blocks 0 to 3 from the chain plus a
5958
/// nonexistent block 4

‎store/test-store/tests/postgres/store.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,7 @@ fn parse_timestamp() {
16931693
.expect("fake chain store");
16941694

16951695
let (_network, number, timestamp, _) = chain_store
1696-
.block_number(&BLOCK_THREE_TIMESTAMP.block_hash())
1696+
.block_pointer(&BLOCK_THREE_TIMESTAMP.block_hash())
16971697
.await
16981698
.expect("block_number to return correct number and timestamp")
16991699
.unwrap();
@@ -1727,7 +1727,7 @@ fn parse_timestamp_firehose() {
17271727
.expect("fake chain store");
17281728

17291729
let (_network, number, timestamp, _) = chain_store
1730-
.block_number(&BLOCK_THREE_TIMESTAMP_FIREHOSE.block_hash())
1730+
.block_pointer(&BLOCK_THREE_TIMESTAMP_FIREHOSE.block_hash())
17311731
.await
17321732
.expect("block_number to return correct number and timestamp")
17331733
.unwrap();
@@ -1761,7 +1761,7 @@ fn parse_null_timestamp() {
17611761
.expect("fake chain store");
17621762

17631763
let (_network, number, timestamp, _) = chain_store
1764-
.block_number(&BLOCK_THREE_NO_TIMESTAMP.block_hash())
1764+
.block_pointer(&BLOCK_THREE_NO_TIMESTAMP.block_hash())
17651765
.await
17661766
.expect("block_number to return correct number and timestamp")
17671767
.unwrap();

0 commit comments

Comments
 (0)