From 21b8d7f3529a8fa14de799da49c362f25285e130 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=BAc=C3=A1s=20Meier?= <lucas@cronokirby.com>
Date: Wed, 24 Jul 2024 15:20:51 -0700
Subject: [PATCH] Tweak block indexing to use height as the primary key (#4757)

Every block has a unique height, we almost always want to join with
other tables based on block height, the serial ID has no meaning: ergo,
we should get rid of the id, and just use height as the primary key.

There was no index on height either, which would have made joining on it
slow.

Also removed unwraps and expects and as in favor of actual errors.

## Checklist before requesting a review

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > Indexer changes
---
 crates/bin/pindexer/src/block.rs | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/crates/bin/pindexer/src/block.rs b/crates/bin/pindexer/src/block.rs
index 221fcfb188..6f3aa3b676 100644
--- a/crates/bin/pindexer/src/block.rs
+++ b/crates/bin/pindexer/src/block.rs
@@ -1,3 +1,4 @@
+use anyhow::anyhow;
 use cometindex::{async_trait, sqlx, AppView, ContextualizedEvent, PgTransaction};
 use penumbra_proto::{core::component::sct::v1 as pb, event::ProtoEvent};
 use sqlx::{types::chrono::DateTime, PgPool};
@@ -16,9 +17,8 @@ impl AppView for Block {
             // table name is module path + struct name
             "
 CREATE TABLE IF NOT EXISTS block_details (
-    id SERIAL PRIMARY KEY,
+    height BIGINT PRIMARY KEY,
     root BYTEA NOT NULL,
-    height INT8 NOT NULL,
     timestamp TIMESTAMPTZ NOT NULL
 );
 ",
@@ -39,7 +39,9 @@ CREATE TABLE IF NOT EXISTS block_details (
         _src_db: &PgPool,
     ) -> Result<(), anyhow::Error> {
         let pe = pb::EventBlockRoot::from_event(event.as_ref())?;
-        let timestamp = pe.timestamp.expect("Block has no timestamp");
+        let timestamp = pe
+            .timestamp
+            .ok_or(anyhow!("block at height {} has no timestamp", pe.height))?;
 
         sqlx::query(
             "
@@ -47,8 +49,11 @@ CREATE TABLE IF NOT EXISTS block_details (
             VALUES ($1, $2, $3)
             ",
         )
-        .bind(pe.height as i64)
-        .bind(DateTime::from_timestamp(timestamp.seconds, timestamp.nanos as u32).unwrap())
+        .bind(i64::try_from(pe.height)?)
+        .bind(
+            DateTime::from_timestamp(timestamp.seconds, u32::try_from(timestamp.nanos)?)
+                .ok_or(anyhow!("failed to convert timestamp"))?,
+        )
         .bind(pe.root.unwrap().inner)
         .execute(dbtx.as_mut())
         .await?;