From 3ac0265f0727aca0a4b0a763857154da9f49562c Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Wed, 11 Sep 2024 11:58:27 -0400
Subject: [PATCH] Add section documenting the safety of txindex upon
 reorganizations

---
 processor/bitcoin/src/scan.rs    |  8 ++++----
 processor/bitcoin/src/txindex.rs | 30 ++++++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/processor/bitcoin/src/scan.rs b/processor/bitcoin/src/scan.rs
index b3d3a6dc..6d7fab88 100644
--- a/processor/bitcoin/src/scan.rs
+++ b/processor/bitcoin/src/scan.rs
@@ -16,7 +16,7 @@ use serai_client::networks::bitcoin::Address;
 use serai_db::Get;
 use primitives::OutputType;
 
-use crate::{db, hash_bytes};
+use crate::hash_bytes;
 
 const KEY_DST: &[u8] = b"Serai Bitcoin Processor Key Offset";
 static BRANCH_BASE_OFFSET: LazyLock<<Secp256k1 as Ciphersuite>::F> =
@@ -62,9 +62,9 @@ pub(crate) fn presumed_origin(getter: &impl Get, tx: &Transaction) -> Option<Add
   for input in &tx.input {
     let txid = hash_bytes(input.previous_output.txid.to_raw_hash());
     let vout = input.previous_output.vout;
-    if let Some(address) = Address::new(ScriptBuf::from_bytes(
-      db::ScriptPubKey::get(getter, txid, vout).expect("unknown output being spent by input"),
-    )) {
+    if let Some(address) =
+      Address::new(crate::txindex::script_pubkey_for_on_chain_output(getter, txid, vout))
+    {
       return Some(address);
     }
   }
diff --git a/processor/bitcoin/src/txindex.rs b/processor/bitcoin/src/txindex.rs
index 63d5072c..2d3f1cd6 100644
--- a/processor/bitcoin/src/txindex.rs
+++ b/processor/bitcoin/src/txindex.rs
@@ -14,13 +14,27 @@
   This task builds that index.
 */
 
-use serai_db::{DbTxn, Db};
+use bitcoin_serai::bitcoin::ScriptBuf;
+
+use serai_db::{Get, DbTxn, Db};
 
 use primitives::task::ContinuallyRan;
 use scanner::ScannerFeed;
 
 use crate::{db, rpc::Rpc, hash_bytes};
 
+pub(crate) fn script_pubkey_for_on_chain_output(
+  getter: &impl Get,
+  txid: [u8; 32],
+  vout: u32,
+) -> ScriptBuf {
+  // We index every single output on the blockchain, so this shouldn't be possible
+  ScriptBuf::from_bytes(
+    db::ScriptPubKey::get(getter, txid, vout)
+      .expect("requested script public key for unknown output"),
+  )
+}
+
 pub(crate) struct TxIndexTask<D: Db>(Rpc<D>);
 
 #[async_trait::async_trait]
@@ -40,6 +54,18 @@ impl<D: Db> ContinuallyRan for TxIndexTask<D> {
         Rpc::<D>::CONFIRMATIONS
       ))?;
 
+    /*
+      `finalized_block_number` is the latest block number minus confirmations. The blockchain may
+      undetectably re-organize though, as while the scanner will maintain an index of finalized
+      blocks and panics on reorganization, this runs prior to the scanner and that index.
+
+      A reorganization of `CONFIRMATIONS` blocks is still an invariant. Even if that occurs, this
+      saves the script public keys *by the transaction hash an output index*. Accordingly, it isn't
+      invalidated on reorganization. The only risk would be if the new chain reorganized to
+      include a transaction to Serai which we didn't index the parents of. If that happens, we'll
+      panic when we scan the transaction, causing the invariant to be detected.
+    */
+
     let finalized_block_number_in_db = db::LatestBlockToYieldAsFinalized::get(&self.0.db);
     let next_block = finalized_block_number_in_db.map_or(0, |block| block + 1);
 
@@ -63,7 +89,7 @@ impl<D: Db> ContinuallyRan for TxIndexTask<D> {
 
       let mut txn = self.0.db.txn();
 
-      for tx in &block.txdata[1 ..] {
+      for tx in &block.txdata {
         let txid = hash_bytes(tx.compute_txid().to_raw_hash());
         for (o, output) in tx.output.iter().enumerate() {
           let o = u32::try_from(o).unwrap();