feat: Don't store every block number in block indices DB

barretenberg/cpp/src/barretenberg/crypto/merkle_tree/lmdb_store/lmdb_tree_store.cpp

@@ -161,15 +161,7 @@ void LMDBTreeStore::write_block_index_data(const block_number_t& blockNumber,
         msgpack::unpack((const char*)data.data(), data.size()).get().convert(payload);
     }
 
-    // Double check it's not already present (it shouldn't be)
-    // We then add the block number and sort
-    // Sorting shouldn't be necessary as we add blocks in ascending order, but we will make sure
-    // Sorting here and when we unwind blocks means that looking up the block number for an index becomes O(1)
-    // These lookups are much more frequent than adds or deletes so we take the hit here
-    if (!payload.contains(blockNumber)) {
-        payload.blockNumbers.emplace_back(blockNumber);
-        payload.sort();
-    }
+    payload.add_block(blockNumber);
 
     // Write the new payload back down
     msgpack::sbuffer buffer;
@@ -189,11 +181,11 @@ bool LMDBTreeStore::find_block_for_index(const index_t& index, block_number_t& b
     }
     BlockIndexPayload payload;
     msgpack::unpack((const char*)data.data(), data.size()).get().convert(payload);
-    if (payload.blockNumbers.empty()) {
+    if (payload.is_empty()) {
         return false;
     }
     // The block numbers are sorted so we simply return the lowest
-    blockNumber = payload.blockNumbers[0];
+    blockNumber = payload.get_min_block_number();
     return true;
 }
 
@@ -217,7 +209,7 @@ void LMDBTreeStore::delete_block_index(const index_t& sizeAtBlock,
     payload.delete_block(blockNumber);
 
     // if it's now empty, delete it
-    if (payload.blockNumbers.empty()) {
+    if (payload.is_empty()) {
         tx.delete_value(key, *_indexToBlockDatabase);
         return;
     }

barretenberg/cpp/src/barretenberg/crypto/merkle_tree/lmdb_store/lmdb_tree_store.hpp

@@ -1,4 +1,5 @@
 #pragma once
+#include "barretenberg/common/log.hpp"
 #include "barretenberg/common/serialize.hpp"
 #include "barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp"
 #include "barretenberg/crypto/merkle_tree/lmdb_store/callbacks.hpp"
@@ -15,6 +16,7 @@
 #include <cstdint>
 #include <optional>
 #include <ostream>
+#include <stdexcept>
 #include <string>
 #include <typeinfo>
 #include <unordered_map>
@@ -63,39 +65,90 @@ struct BlockIndexPayload {
 
     bool operator==(const BlockIndexPayload& other) const { return blockNumbers == other.blockNumbers; }
 
-    void sort() { std::sort(blockNumbers.begin(), blockNumbers.end()); }
+    bool is_empty() const { return blockNumbers.empty(); }
+
+    block_number_t get_min_block_number() { return blockNumbers[0]; }
 
     bool contains(const block_number_t& blockNumber)
     {
-        auto it = std::lower_bound(blockNumbers.begin(), blockNumbers.end(), blockNumber);
-        if (it == blockNumbers.end()) {
-            // The block was not found, we can return
+        if (is_empty()) {
             return false;
         }
-        return *it == blockNumber;
+        if (blockNumbers.size() == 1) {
+            return blockNumbers[0] == blockNumber;
+        }
+        return blockNumber >= blockNumbers[0] && blockNumber <= blockNumbers[1];
     }
 
     void delete_block(const block_number_t& blockNumber)
     {
+        // Shouldn't be possible, but no need to do anything here
         if (blockNumbers.empty()) {
             return;
         }
-        // shuffle the block number down, removing the one we want to remove and then pop the end item
-        auto it = std::lower_bound(blockNumbers.begin(), blockNumbers.end(), blockNumber);
-        if (it == blockNumbers.end()) {
-            // The block was not found, we can return
+
+        // If the size is 1, the blocknumber must match that in index 0, if it does remove it
+        if (blockNumbers.size() == 1) {
+            if (blockNumbers[0] == blockNumber) {
+                blockNumbers.pop_back();
+            }
             return;
         }
-        // It could be a block higher than the one we are looking for
-        if (*it != blockNumber) {
+
+        // we have 2 entries, we must verify that the block number is equal to the item in index 1
+        if (blockNumbers[1] != blockNumber) {
+            throw std::runtime_error(format("Unable to delete block number ",
+                                            blockNumber,
+                                            " for retrieval by index, current max block number at that index: ",
+                                            blockNumbers[1]));
+        }
+        // It is equal, decrement it, we know that the new block number must have been added previously
+        --blockNumbers[1];
+
+        // We have modified the high block. If it is now equal to the low block then pop it
+        if (blockNumbers[0] == blockNumbers[1]) {
+            blockNumbers.pop_back();
+        }
+    }
+
+    void add_block(const block_number_t& blockNumber)
+    {
+        // If empty, just add the block number
+        if (blockNumbers.empty()) {
+            blockNumbers.emplace_back(blockNumber);
+            return;
+        }
+
+        // If the size is 1, then we must be adding the block 1 larger than that in index 0
+        if (blockNumbers.size() == 1) {
+            if (blockNumber != blockNumbers[0] + 1) {
+                // We can't accept a block number for this index that does not immediately follow the block before
+                throw std::runtime_error(format("Unable to store block number ",
+                                                blockNumber,
+                                                " for retrieval by index, current max block number at that index: ",
+                                                blockNumbers[0]));
+            }
+            blockNumbers.emplace_back(blockNumber);
             return;
         }
-        // we have found our block, shuffle blocks after this one down
-        auto readIt = it + 1;
-        while (readIt != blockNumbers.end()) {
-            *it++ = *readIt++;
+
+        // Size must be 2 here, if larger, this is an error
+        if (blockNumbers.size() != 2) {
+            throw std::runtime_error(format("Unable to store block number ",
+                                            blockNumber,
+                                            " for retrieval by index, block numbers is of invalid size: ",
+                                            blockNumbers.size()));
+        }
+
+        // If the size is 2, then we must be adding the block 1 larger than that in index 1
+        if (blockNumber != blockNumbers[1] + 1) {
+            // We can't accept a block number for this index that does not immediately follow the block before
+            throw std::runtime_error(format("Unable to store block number ",
+                                            blockNumber,
+                                            " for retrieval by index, current max block number at that index: ",
+                                            blockNumbers[1]));
         }
-        blockNumbers.pop_back();
+        blockNumbers[1] = blockNumber;
     }
 };
 /**

barretenberg/cpp/src/barretenberg/crypto/merkle_tree/lmdb_store/lmdb_tree_store.test.cpp

@@ -273,11 +273,10 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_by_index)
                                        BlockAndIndex{ .blockNumber = 5, .index = 130 } };
     LMDBTreeStore store(_directory, "DB1", _mapSize, _maxReaders);
     {
-        // write all of the blocks. we will write them in reverse order
+        // write all of the blocks.
         LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
-        for (int i = int(blocks.size()) - 1; i >= 0; i--) {
+        for (auto block : blocks) {
             // the arg is block size so add 1
-            const BlockAndIndex& block = blocks[size_t(i)];
             store.write_block_index_data(block.blockNumber, block.index + 1, *transaction);
         }
         transaction->commit();
@@ -409,14 +408,24 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
     {
         // write all of the blocks. we will write them in reverse order
         LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
-        for (int i = int(blocks.size()) - 1; i >= 0; i--) {
+        for (auto block : blocks) {
             // the arg is block size so add 1
-            const BlockAndIndex& block = blocks[size_t(i)];
             store.write_block_index_data(block.blockNumber, block.index + 1, *transaction);
         }
         transaction->commit();
     }
 
+    {
+        // we can't add a duplicate block at an index if it is not the next block number
+        LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
+        // the arg is block size so add 1
+        EXPECT_THROW(store.write_block_index_data(3, 60 + 1, *transaction), std::runtime_error);
+        EXPECT_THROW(store.write_block_index_data(6, 60 + 1, *transaction), std::runtime_error);
+        EXPECT_THROW(store.write_block_index_data(1, 25 + 1, *transaction), std::runtime_error);
+        EXPECT_THROW(store.write_block_index_data(3, 25 + 1, *transaction), std::runtime_error);
+        transaction->abort();
+    }
+
     {
         // read back some blocks and check them
         LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();
@@ -435,11 +444,12 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
     }
 
     {
-        // delete block 2 at index 60
+        // attempting to delete block 2 at index 60 should fail as it is not the last block in the series at index 60
         LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
         // the arg is block size so add 1
-        store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction);
-        transaction->commit();
+        EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction),
+                     std::runtime_error);
+        transaction->abort();
     }
 
     {
@@ -449,9 +459,9 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
         EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
         EXPECT_EQ(readBack, 1);
 
-        // should be the new lowest block at this index
+        // should still be the lowest block at this index
         EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
-        EXPECT_EQ(readBack, 3);
+        EXPECT_EQ(readBack, 2);
 
         EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
         EXPECT_EQ(readBack, 5);
@@ -463,9 +473,9 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
         // try and delete blocks that don't exist at index 60
         LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
         // the arg is block size so add 1
-        store.delete_block_index(blocks[1].index + 1, 2, *transaction);
-        store.delete_block_index(blocks[1].index + 1, 5, *transaction);
-        transaction->commit();
+        EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, 2, *transaction), std::runtime_error);
+        EXPECT_THROW(store.delete_block_index(blocks[1].index + 1, 5, *transaction), std::runtime_error);
+        transaction->abort();
     }
 
     {
@@ -475,9 +485,8 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
         EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
         EXPECT_EQ(readBack, 1);
 
-        // should be the new lowest block at this index
         EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
-        EXPECT_EQ(readBack, 3);
+        EXPECT_EQ(readBack, 2);
 
         EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
         EXPECT_EQ(readBack, 5);
@@ -486,14 +495,40 @@ TEST_F(LMDBTreeStoreTest, can_write_and_retrieve_block_numbers_with_duplicate_in
     }
 
     {
-        // delete 2 more blocks
+        // delete the last 2 blocks at index 60
         LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
         // the arg is block size so add 1
         store.delete_block_index(blocks[3].index + 1, blocks[3].blockNumber, *transaction);
         store.delete_block_index(blocks[2].index + 1, blocks[2].blockNumber, *transaction);
         transaction->commit();
     }
 
+    {
+        // check the blocks again
+        LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();
+        block_number_t readBack = 0;
+        EXPECT_TRUE(store.find_block_for_index(5, readBack, *transaction));
+        EXPECT_EQ(readBack, 1);
+
+        EXPECT_TRUE(store.find_block_for_index(30, readBack, *transaction));
+        EXPECT_EQ(readBack, 2);
+
+        EXPECT_TRUE(store.find_block_for_index(82, readBack, *transaction));
+        EXPECT_EQ(readBack, 5);
+    }
+
+    {
+        // delete the last final block at index 60
+        LMDBTreeWriteTransaction::Ptr transaction = store.create_write_transaction();
+        // the arg is block size so add 1
+        // Only one block remains at index 60, try and delete one that doesn't exist, it should do nothing
+        store.delete_block_index(blocks[3].index + 1, blocks[3].blockNumber, *transaction);
+
+        // Now delete the last block
+        store.delete_block_index(blocks[1].index + 1, blocks[1].blockNumber, *transaction);
+        transaction->commit();
+    }
+
     {
         // check the blocks again
         LMDBTreeReadTransaction::Ptr transaction = store.create_read_transaction();