From 1b7c57371da9aac5b1d42666d86313431cdd6efc Mon Sep 17 00:00:00 2001
From: Jane Lusby <jlusby42@gmail.com>
Date: Fri, 30 Oct 2020 15:24:39 -0700
Subject: [PATCH] Fix format used to store transactions in sled (#1238)

## Motivation

While working on the block locator fix PR together with Henry we noticed that we'd accidentally serialized entire transactions in `tx_by_hash`, instead of serializing just the height of the block and the index of the transaction within the block, as described by the original RFC.

## Solution

We've fixed it by adding a `TransactionLocation` new type, which handles the sled format traits. We've removed the sled format impls for `Transaction` to prevent inserting the wrong data in the future. Finally we've bumped the database format to reflect the change in the format on the disk and its incompatibility with previous versions.
---
 zebra-state/src/constants.rs              |  2 +-
 zebra-state/src/sled_state.rs             | 14 +++++--
 zebra-state/src/sled_state/sled_format.rs | 45 +++++++++++++++++++----
 3 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs
index 7664f89ce..612a7b039 100644
--- a/zebra-state/src/constants.rs
+++ b/zebra-state/src/constants.rs
@@ -14,4 +14,4 @@ pub const MIN_TRASPARENT_COINBASE_MATURITY: block::Height = block::Height(100);
 pub const MAX_BLOCK_REORG_HEIGHT: block::Height =
     block::Height(MIN_TRASPARENT_COINBASE_MATURITY.0 - 1);
 
-pub const SLED_FORMAT_VERSION: u32 = 0;
+pub const SLED_FORMAT_VERSION: u32 = 1;
diff --git a/zebra-state/src/sled_state.rs b/zebra-state/src/sled_state.rs
index dc31216ea..c01541c61 100644
--- a/zebra-state/src/sled_state.rs
+++ b/zebra-state/src/sled_state.rs
@@ -1,6 +1,6 @@
 //! The primary implementation of the `zebra_state::Service` built upon sled
 
-use std::{collections::HashMap, sync::Arc};
+use std::{collections::HashMap, convert::TryInto, sync::Arc};
 
 use tracing::trace;
 use zebra_chain::transparent;
@@ -15,6 +15,8 @@ mod sled_format;
 
 use sled_format::{FromSled, IntoSled, SledDeserialize, SledSerialize};
 
+use self::sled_format::TransactionLocation;
+
 /// The finalized part of the chain state, stored in sled.
 ///
 /// This structure has two categories of methods:
@@ -250,9 +252,15 @@ impl FinalizedState {
                     }
 
                     // Index each transaction
-                    for transaction in block.transactions.iter() {
+                    for (transaction_index, transaction) in block.transactions.iter().enumerate() {
                         let transaction_hash = transaction.hash();
-                        tx_by_hash.zs_insert(transaction_hash, transaction)?;
+                        let transaction_location = TransactionLocation {
+                            height,
+                            index: transaction_index
+                                .try_into()
+                                .expect("no more than 4 billion transactions per block"),
+                        };
+                        tx_by_hash.zs_insert(transaction_hash, transaction_location)?;
 
                         // Mark all transparent inputs as spent
                         for input in transaction.inputs() {
diff --git a/zebra-state/src/sled_state/sled_format.rs b/zebra-state/src/sled_state/sled_format.rs
index df1ab83a5..c861258fd 100644
--- a/zebra-state/src/sled_state/sled_format.rs
+++ b/zebra-state/src/sled_state/sled_format.rs
@@ -6,13 +6,16 @@ use zebra_chain::{
     block::Block,
     sapling,
     serialization::{ZcashDeserialize, ZcashSerialize},
-    sprout, transaction,
-    transaction::Transaction,
-    transparent,
+    sprout, transaction, transparent,
 };
 
 use crate::BoxError;
 
+pub struct TransactionLocation {
+    pub height: block::Height,
+    pub index: u32,
+}
+
 // Helper trait for defining the exact format used to interact with sled per
 // type.
 pub trait IntoSled {
@@ -54,16 +57,42 @@ impl FromSled for Arc<Block> {
     }
 }
 
-impl IntoSled for &Arc<Transaction> {
-    type Bytes = Vec<u8>;
+impl IntoSled for TransactionLocation {
+    type Bytes = [u8; 8];
 
     fn as_bytes(&self) -> Self::Bytes {
-        self.zcash_serialize_to_vec()
-            .expect("serialization to vec doesn't fail")
+        let height_bytes = self.height.0.to_be_bytes();
+        let index_bytes = self.index.to_be_bytes();
+
+        let mut bytes = [0; 8];
+
+        bytes[0..4].copy_from_slice(&height_bytes);
+        bytes[4..8].copy_from_slice(&index_bytes);
+
+        bytes
     }
 
     fn into_ivec(self) -> sled::IVec {
-        self.as_bytes().into()
+        self.as_bytes().as_ref().into()
+    }
+}
+
+impl FromSled for TransactionLocation {
+    fn from_ivec(sled_bytes: sled::IVec) -> Result<Self, BoxError> {
+        let height = {
+            let mut bytes = [0; 4];
+            bytes.copy_from_slice(&sled_bytes[0..4]);
+            let height = u32::from_be_bytes(bytes);
+            block::Height(height)
+        };
+
+        let index = {
+            let mut bytes = [0; 4];
+            bytes.copy_from_slice(&sled_bytes[4..8]);
+            u32::from_be_bytes(bytes)
+        };
+
+        Ok(TransactionLocation { height, index })
     }
 }