Merge pull request #761

01e0a69 Revert "Merge pull request #749" (Riccardo Spagni)

1 files changed, 155 insertions, 937 deletions

diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h
index 3e0ca141e..3396b8c20 100644
--- a/src/blockchain_db/blockchain_db.h
+++ b/src/blockchain_db/blockchain_db.h
@@ -38,20 +38,18 @@
 #include "cryptonote_core/difficulty.h"
 #include "cryptonote_core/hardfork.h"
 
-/** \file
- * Cryptonote Blockchain Database Interface
+/* DB Driver Interface
  *
  * The DB interface is a store for the canonical block chain.
  * It serves as a persistent storage for the blockchain.
  *
- * For the sake of efficiency, a concrete implementation may also
+ * For the sake of efficiency, the reference implementation will also
  * store some blockchain data outside of the blocks, such as spent
  * transfer key images, unspent transaction outputs, etc.
  *
- * Examples are as follows:
- *
  * Transactions are duplicated so that we don't have to fetch a whole block
- * in order to fetch a transaction from that block.
+ * in order to fetch a transaction from that block.  If this is deemed
+ * unnecessary later, this can change.
  *
  * Spent key images are duplicated outside of the blocks so it is quick
  * to verify an output hasn't already been spent
@@ -59,34 +57,100 @@
  * Unspent transaction outputs are duplicated to quickly gather random
  * outputs to use for mixins
  *
+ * IMPORTANT:
+ * A concrete implementation of this interface should populate these
+ * duplicated members!  It is possible to have a partial implementation
+ * of this interface call to private members of the interface to be added
+ * later that will then populate as needed.
+ *
+ * General:
+ *   open()
+ *   is_open()
+ *   close()
+ *   sync()
+ *   reset()
+ *
+ *   Lock and unlock provided for reorg externally, and for block
+ *   additions internally, this way threaded reads are completely fine
+ *   unless the blockchain is changing.
+ *   bool lock()
+ *   unlock()
+ *
+ * vector<str>   get_filenames()
+ *
+ * Blocks:
+ *   bool        block_exists(hash)
+ *   height      add_block(block, block_size, cumulative_difficulty, coins_generated, transactions)
+ *   block       get_block(hash)
+ *   height      get_block_height(hash)
+ *   header      get_block_header(hash)
+ *   block       get_block_from_height(height)
+ *   size_t      get_block_size(height)
+ *   difficulty  get_block_cumulative_difficulty(height)
+ *   uint64_t    get_block_already_generated_coins(height)
+ *   uint64_t    get_block_timestamp(height)
+ *   uint64_t    get_top_block_timestamp()
+ *   hash        get_block_hash_from_height(height)
+ *   blocks      get_blocks_range(height1, height2)
+ *   hashes      get_hashes_range(height1, height2)
+ *   hash        top_block_hash()
+ *   block       get_top_block()
+ *   height      height()
+ *   void        pop_block(block&, tx_list&)
+ *
+ * Transactions:
+ *   bool        tx_exists(hash)
+ *   uint64_t    get_tx_unlock_time(hash)
+ *   tx          get_tx(hash)
+ *   uint64_t    get_tx_count()
+ *   tx_list     get_tx_list(hash_list)
+ *   height      get_tx_block_height(hash)
+ *
+ * Outputs:
+ *   uint64_t    get_num_outputs(amount)
+ *   pub_key     get_output_key(amount, index)
+ *   hash,index  get_output_tx_and_index_from_global(index)
+ *   hash,index  get_output_tx_and_index(amount, index)
+ *   vec<uint64> get_tx_output_indices(tx_hash)
+ *
+ *
+ * Spent Output Key Images:
+ *   bool        has_key_image(key_image)
+ *
+ * Exceptions:
+ *   DB_ERROR -- generic
+ *   DB_OPEN_FAILURE
+ *   DB_CREATE_FAILURE
+ *   DB_SYNC_FAILURE
+ *   BLOCK_DNE
+ *   BLOCK_PARENT_DNE
+ *   BLOCK_EXISTS
+ *   BLOCK_INVALID -- considering making this multiple errors
+ *   TX_DNE
+ *   TX_EXISTS
+ *   OUTPUT_DNE
+ *   OUTPUT_EXISTS
+ *   KEY_IMAGE_EXISTS
  */
 
 namespace cryptonote
 {
 
-/** a pair of <transaction hash, output index>, typedef for convenience */
+// typedef for convenience
 typedef std::pair<crypto::hash, uint64_t> tx_out_index;
 
 #pragma pack(push, 1)
-
-/**
- * @brief a struct containing output metadata
- */
 struct output_data_t
 {
-  crypto::public_key pubkey;       //!< the output's public key (for spend verification)
-  uint64_t           unlock_time;  //!< the output's unlock time (or height)
-  uint64_t           height;       //!< the height of the block which created the output
+  crypto::public_key pubkey;
+  uint64_t unlock_time;
+  uint64_t height;
 };
 #pragma pack(pop)
 
 /***********************************
  * Exception Definitions
  ***********************************/
-
-/**
- * @brief A base class for BlockchainDB exceptions
- */
 class DB_EXCEPTION : public std::exception
 {
   private:
@@ -104,9 +168,6 @@ class DB_EXCEPTION : public std::exception
     }
 };
 
-/**
- * @brief A generic BlockchainDB exception
- */
 class DB_ERROR : public DB_EXCEPTION
 {
   public:
@@ -114,9 +175,7 @@ class DB_ERROR : public DB_EXCEPTION
     DB_ERROR(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when there is an error starting a DB transaction
- */
+// For distinguishing errors trying to set up a DB txn from other errors
 class DB_ERROR_TXN_START : public DB_EXCEPTION
 {
   public:
@@ -124,9 +183,6 @@ class DB_ERROR_TXN_START : public DB_EXCEPTION
     DB_ERROR_TXN_START(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when opening the BlockchainDB fails
- */
 class DB_OPEN_FAILURE : public DB_EXCEPTION
 {
   public:
@@ -134,9 +190,6 @@ class DB_OPEN_FAILURE : public DB_EXCEPTION
     DB_OPEN_FAILURE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when creating the BlockchainDB fails
- */
 class DB_CREATE_FAILURE : public DB_EXCEPTION
 {
   public:
@@ -144,9 +197,6 @@ class DB_CREATE_FAILURE : public DB_EXCEPTION
     DB_CREATE_FAILURE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when synchronizing the BlockchainDB to disk fails
- */
 class DB_SYNC_FAILURE : public DB_EXCEPTION
 {
   public:
@@ -154,9 +204,6 @@ class DB_SYNC_FAILURE : public DB_EXCEPTION
     DB_SYNC_FAILURE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a requested block does not exist
- */
 class BLOCK_DNE : public DB_EXCEPTION
 {
   public:
@@ -164,9 +211,6 @@ class BLOCK_DNE : public DB_EXCEPTION
     BLOCK_DNE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a block's parent does not exist (and it needed to)
- */
 class BLOCK_PARENT_DNE : public DB_EXCEPTION
 {
   public:
@@ -174,9 +218,6 @@ class BLOCK_PARENT_DNE : public DB_EXCEPTION
     BLOCK_PARENT_DNE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a block exists, but shouldn't, namely when adding a block
- */
 class BLOCK_EXISTS : public DB_EXCEPTION
 {
   public:
@@ -184,9 +225,6 @@ class BLOCK_EXISTS : public DB_EXCEPTION
     BLOCK_EXISTS(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when something is wrong with the block to be added
- */
 class BLOCK_INVALID : public DB_EXCEPTION
 {
   public:
@@ -194,9 +232,6 @@ class BLOCK_INVALID : public DB_EXCEPTION
     BLOCK_INVALID(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a requested transaction does not exist
- */
 class TX_DNE : public DB_EXCEPTION
 {
   public:
@@ -204,9 +239,6 @@ class TX_DNE : public DB_EXCEPTION
     TX_DNE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a transaction exists, but shouldn't, namely when adding a block
- */
 class TX_EXISTS : public DB_EXCEPTION
 {
   public:
@@ -214,9 +246,6 @@ class TX_EXISTS : public DB_EXCEPTION
     TX_EXISTS(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a requested output does not exist
- */
 class OUTPUT_DNE : public DB_EXCEPTION
 {
   public:
@@ -224,9 +253,6 @@ class OUTPUT_DNE : public DB_EXCEPTION
     OUTPUT_DNE(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when an output exists, but shouldn't, namely when adding a block
- */
 class OUTPUT_EXISTS : public DB_EXCEPTION
 {
   public:
@@ -234,9 +260,6 @@ class OUTPUT_EXISTS : public DB_EXCEPTION
     OUTPUT_EXISTS(const char* s) : DB_EXCEPTION(s) { }
 };
 
-/**
- * @brief thrown when a spent key image exists, but shouldn't, namely when adding a block
- */
 class KEY_IMAGE_EXISTS : public DB_EXCEPTION
 {
   public:
@@ -249,18 +272,6 @@ class KEY_IMAGE_EXISTS : public DB_EXCEPTION
  ***********************************/
 
 
-/**
- * @brief The BlockchainDB backing store interface declaration/contract
- *
- * This class provides a uniform interface for using BlockchainDB to store
- * a blockchain.  Any implementation of this class will also implement all
- * functions exposed here, so one can use this class without knowing what
- * implementation is being used.  Refer to each pure virtual function's
- * documentation here when implementing a BlockchainDB subclass.
- *
- * A subclass which encounters an issue should report that issue by throwing
- * a DB_EXCEPTION which adequately conveys the issue.
- */
 class BlockchainDB
 {
 private:
@@ -268,22 +279,7 @@ private:
    * private virtual members
    *********************************************************************/
 
-  /**
-   * @brief add the block and metadata to the db
-   *
-   * The subclass implementing this will add the specified block and
-   * block metadata to its backing store.  This does not include its
-   * transactions, those are added in a separate step.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param blk the block to be added
-   * @param block_size the size of the block (transactions and all)
-   * @param cumulative_difficulty the accumulated difficulty after this block
-   * @param coins_generated the number of coins generated total after this block
-   * @param blk_hash the hash of the block
-   */
+  // tells the subclass to add the block and metadata to storage
   virtual void add_block( const block& blk
                 , const size_t& block_size
                 , const difficulty_type& cumulative_difficulty
@@ -291,274 +287,84 @@ private:
                 , const crypto::hash& blk_hash
                 ) = 0;
 
-  /**
-   * @brief remove data about the top block
-   *
-   * The subclass implementing this will remove the block data from the top
-   * block in the chain.  The data to be removed is that which was added in
-   * BlockchainDB::add_block(const block& blk, const size_t& block_size, const difficulty_type& cumulative_difficulty, const uint64_t& coins_generated, const crypto::hash& blk_hash)
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
+  // tells the subclass to remove data about the top block
   virtual void remove_block() = 0;
 
-  /**
-   * @brief store the transaction and its metadata
-   *
-   * The subclass implementing this will add the specified transaction data
-   * to its backing store.  This includes only the transaction blob itself
-   * and the other data passed here, not the separate outputs of the
-   * transaction.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param blk_hash the hash of the block containing the transaction
-   * @param tx the transaction to be added
-   * @param tx_hash the hash of the transaction
-   */
+  // tells the subclass to store the transaction and its metadata
   virtual void add_transaction_data(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash& tx_hash) = 0;
 
-  /**
-   * @brief remove data about a transaction
-   *
-   * The subclass implementing this will remove the transaction data 
-   * for the passed transaction.  The data to be removed was added in
-   * add_transaction_data().  Additionally, current subclasses have behavior
-   * which requires the transaction itself as a parameter here.  Future
-   * implementations should note that this parameter is subject to be removed
-   * at a later time.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param tx_hash the hash of the transaction to be removed
-   * @param tx the transaction
-   */
+  // tells the subclass to remove data about a transaction
   virtual void remove_transaction_data(const crypto::hash& tx_hash, const transaction& tx) = 0;
 
-  /**
-   * @brief store an output
-   *
-   * The subclass implementing this will add the output data passed to its
-   * backing store in a suitable manner.  In addition, the subclass is responsible
-   * for keeping track of the global output count in some manner, so that
-   * outputs may be indexed by the order in which they were created.  In the
-   * future, this tracking (of the number, at least) should be moved to
-   * this class, as it is necessary and the same among all BlockchainDB.
-   *
-   * This data should be stored in such a manner that the only thing needed to
-   * reverse the process is the tx_out.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param tx_hash hash of the transaction the output was created by
-   * @param tx_output the output
-   * @param local_index index of the output in its transaction
-   * @param unlock_time unlock time/height of the output
-   */
+  // tells the subclass to store an output
   virtual void add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time) = 0;
 
-  /**
-   * @brief remove an output
-   *
-   * The subclass implementing this will remove all output data it stored
-   * in add_output().
-   *
-   * In addition, the subclass is responsible for correctly decrementing
-   * its global output counter (this may be automatic for some, such as using
-   * a database backend "count" feature).
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param tx_output the output to be removed
-   */
+  // tells the subclass to remove an output
   virtual void remove_output(const tx_out& tx_output) = 0;
 
-  /**
-   * @brief store a spent key
-   *
-   * The subclass implementing this will store the spent key image.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param k_image the spent key image to store
-   */
+  // tells the subclass to store a spent key
   virtual void add_spent_key(const crypto::key_image& k_image) = 0;
 
-  /**
-   * @brief remove a spent key
-   *
-   * The subclass implementing this will remove the key image.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param k_image the spent key image to remove
-   */
+  // tells the subclass to remove a spent key
   virtual void remove_spent_key(const crypto::key_image& k_image) = 0;
 
 
   /*********************************************************************
    * private concrete members
    *********************************************************************/
-  /**
-   * @brief private version of pop_block, for undoing if an add_block fails
-   *
-   * This function simply calls pop_block(block& blk, std::vector<transaction>& txs)
-   * with dummy parameters, as the returns-by-reference can be discarded.
-   */
+  // private version of pop_block, for undoing if an add_block goes tits up
   void pop_block();
 
-  /**
-   * @brief helper function for add_transactions, to add each individual transaction
-   *
-   * This function is called by add_transactions() for each transaction to be
-   * added.
-   *
-   * @param blk_hash hash of the block which has the transaction
-   * @param tx the transaction to add
-   * @param tx_hash_ptr the hash of the transaction, if already calculated
-   */
+  // helper function for add_transactions, to add each individual tx
   void add_transaction(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash* tx_hash_ptr = NULL);
 
   // helper function to remove transaction from blockchain
-  /**
-   * @brief helper function to remove transaction from the blockchain
-   *
-   * This function encapsulates aspects of removing a transaction.
-   *
-   * @param tx_hash the hash of the transaction to be removed
-   */
   void remove_transaction(const crypto::hash& tx_hash);
 
-  uint64_t num_calls = 0;  //!< a performance metric
-  uint64_t time_blk_hash = 0;  //!< a performance metric
-  uint64_t time_add_block1 = 0;  //!< a performance metric
-  uint64_t time_add_transaction = 0;  //!< a performance metric
+  uint64_t num_calls = 0;
+  uint64_t time_blk_hash = 0;
+  uint64_t time_add_block1 = 0;
+  uint64_t time_add_transaction = 0;
 
 
 protected:
 
-  mutable uint64_t time_tx_exists = 0;  //!< a performance metric
-  uint64_t time_commit1 = 0;  //!< a performance metric
-  bool m_auto_remove_logs = true;  //!< whether or not to automatically remove old logs
+  mutable uint64_t time_tx_exists = 0;
+  uint64_t time_commit1 = 0;
+  bool m_auto_remove_logs = true;
 
   HardFork* m_hardfork;
 
 public:
 
-  /**
-   * @brief An empty destructor.
-   */
+  // virtual dtor
   virtual ~BlockchainDB() { };
 
-  /**
-   * @brief reset profiling stats
-   */
+  // reset profiling stats
   void reset_stats();
 
-  /**
-   * @brief show profiling stats
-   *
-   * This function prints current performance/profiling data to whichever
-   * log file(s) are set up (possibly including stdout or stderr)
-   */
+  // show profiling stats
   void show_stats();
 
-  /**
-   * @brief open a db, or create it if necessary.
-   *
-   * This function opens an existing database or creates it if it
-   * does not exist.
-   *
-   * The subclass implementing this will handle all file opening/creation,
-   * and is responsible for maintaining its state.
-   *
-   * The parameter <filename> may not refer to a file name, necessarily, but
-   * could be an IP:PORT for a database which needs it, and so on.  Calling it
-   * <filename> is convenient and should be descriptive enough, however.
-   *
-   * For now, db_flags are
-   * specific to the subclass being instantiated.  This is subject to change,
-   * and the db_flags parameter may be deprecated.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param filename a string referring to the BlockchainDB to open
-   * @param db_flags flags relevant to how to open/use the BlockchainDB
-   */
+  // open the db at location <filename>, or create it if there isn't one.
   virtual void open(const std::string& filename, const int db_flags = 0) = 0;
 
-  /**
-   * @brief Gets the current open/ready state of the BlockchainDB
-   *
-   * @return true if open/ready, otherwise false
-   */
+  // returns true of the db is open/ready, else false
   bool is_open() const;
 
-  /**
-   * @brief close the BlockchainDB
-   *
-   * At minimum, this call ensures that further use of the BlockchainDB
-   * instance will not have effect.  In any case where it is necessary
-   * to do so, a subclass implementing this will sync with disk.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
+  // close and sync the db
   virtual void close() = 0;
 
-  /**
-   * @brief sync the BlockchainDB with disk
-   *
-   * This function should write any changes to whatever permanent backing
-   * store the subclass uses.  Example: a BlockchainDB instance which
-   * keeps the whole blockchain in RAM won't need to regularly access a
-   * disk, but should write out its state when this is called.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
+  // sync the db
   virtual void sync() = 0;
 
-  /**
-   * @brief Remove everything from the BlockchainDB
-   *
-   * This function should completely remove all data from a BlockchainDB.
-   *
-   * Use with caution!
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
+  // reset the db -- USE WITH CARE
   virtual void reset() = 0;
 
-  /**
-   * @brief get all files used by the BlockchainDB (if any)
-   *
-   * This function is largely for ease of automation, namely for unit tests.
-   *
-   * The subclass implementation should return all filenames it uses.
-   *
-   * @return a list of filenames
-   */
+  // get all files used by this db (if any)
   virtual std::vector<std::string> get_filenames() const = 0;
 
   // return the name of the folder the db's file(s) should reside in
-  /**
-   * @brief gets the name of the folder the BlockchainDB's file(s) should be in
-   *
-   * The subclass implementation should return the name of the folder in which
-   * it stores files, or an empty string if there is none.
-   *
-   * @return the name of the folder with the BlockchainDB's files, if any.
-   */
   virtual std::string get_db_name() const = 0;
 
 
@@ -566,86 +372,13 @@ public:
   // RAII-friendly and multi-read one-write friendly locking mechanism
   //
   // acquire db lock
-  /**
-   * @brief acquires the BlockchainDB lock
-   *
-   * This function is a stub until such a time as locking is implemented at
-   * this level.
-   *
-   * The subclass implementation should return true unless implementing a
-   * locking scheme of some sort, in which case it should return true upon
-   * acquisition of the lock and block until then.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @return true, unless at a future time false makes sense (timeout, etc)
-   */
   virtual bool lock() = 0;
 
   // release db lock
-  /**
-   * @brief This function releases the BlockchainDB lock
-   *
-   * The subclass, should it have implemented lock(), will release any lock
-   * held by the calling thread.  In the case of recursive locking, it should
-   * release one instance of a lock.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
   virtual void unlock() = 0;
 
-  /**
-   * @brief tells the BlockchainDB to start a new "batch" of blocks
-   *
-   * If the subclass implements a batching method of caching blocks in RAM to
-   * be added to a backing store in groups, it should start a batch which will
-   * end either when <batch_num_blocks> has been added or batch_stop() has
-   * been called.  In either case, it should end the batch and write to its
-   * backing store.
-   *
-   * If a batch is already in-progress, this function should throw a DB_ERROR.
-   * This exception may change in the future if it is deemed necessary to
-   * have a more granular exception type for this scenario.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param batch_num_blocks number of blocks to batch together
-   */
   virtual void batch_start(uint64_t batch_num_blocks=0) = 0;
-
-  /**
-   * @brief ends a batch transaction
-   *
-   * If the subclass implements batching, this function should store the
-   * batch it is currently on and mark it finished.
-   *
-   * If no batch is in-progress, this function should throw a DB_ERROR.
-   * This exception may change in the future if it is deemed necessary to
-   * have a more granular exception type for this scenario.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   */
   virtual void batch_stop() = 0;
-
-  /**
-   * @brief sets whether or not to batch transactions
-   *
-   * If the subclass implements batching, this function tells it to begin
-   * batching automatically.
-   *
-   * If the subclass implements batching and has a batch in-progress, a
-   * parameter of false should disable batching and call batch_stop() to
-   * store the current batch.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param bool batch whether or not to use batch transactions.
-   */
   virtual void set_batch_transactions(bool) = 0;
 
   virtual void block_txn_start(bool readonly=false) = 0;
@@ -655,26 +388,8 @@ public:
   virtual void set_hard_fork(HardFork* hf);
 
   // adds a block with the given metadata to the top of the blockchain, returns the new height
-  /**
-   * @brief handles the addition of a new block to BlockchainDB
-   *
-   * This function organizes block addition and calls various functions as
-   * necessary.
-   *
-   * NOTE: subclass implementations of this (or the functions it calls) need
-   * to handle undoing any partially-added blocks in the event of a failure.
-   *
-   * If any of this cannot be done, the subclass should throw the corresponding
-   * subclass of DB_EXCEPTION
-   *
-   * @param blk the block to be added
-   * @param block_size the size of the block (transactions and all)
-   * @param cumulative_difficulty the accumulated difficulty after this block
-   * @param coins_generated the number of coins generated total after this block
-   * @param txs the transactions in the block
-   *
-   * @return the height of the chain post-addition
-   */
+  // NOTE: subclass implementations of this (or the functions it calls) need
+  // to handle undoing any partially-added blocks in the event of a failure.
   virtual uint64_t add_block( const block& blk
                             , const size_t& block_size
                             , const difficulty_type& cumulative_difficulty
@@ -682,639 +397,142 @@ public:
                             , const std::vector<transaction>& txs
                             );
 
-  /**
-   * @brief checks if a block exists
-   *
-   * @param h the hash of the requested block
-   *
-   * @return true of the block exists, otherwise false
-   */
+  // return true if a block with hash <h> exists in the blockchain
   virtual bool block_exists(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief fetches the block with the given hash
-   *
-   * The subclass should return the requested block.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param h the hash to look for
-   *
-   * @return the block requested
-   */
+  // return block with hash <h>
   virtual block get_block(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief gets the height of the block with a given hash
-   *
-   * The subclass should return the requested height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param h the hash to look for
-   *
-   * @return the height
-   */
+  // return the height of the block with hash <h> on the blockchain,
+  // throw if it doesn't exist
   virtual uint64_t get_block_height(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief fetch a block header
-   *
-   * The subclass should return the block header from the block with
-   * the given hash.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param h the hash to look for
-   *
-   * @return the block header
-   */
+  // return header for block with hash <h>
   virtual block_header get_block_header(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief fetch a block by height
-   *
-   * The subclass should return the block at the given height.
-   *
-   * If the block does not exist, that is to say if the blockchain is not
-   * that high, then the subclass should throw BLOCK_DNE
-   *
-   * @param height the height to look for
-   *
-   * @return the block
-   */
+  // return block at height <height>
   virtual block get_block_from_height(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a block's timestamp
-   *
-   * The subclass should return the timestamp of the block with the
-   * given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the timestamp
-   */
+  // return timestamp of block at height <height>
   virtual uint64_t get_block_timestamp(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch the top block's timestamp
-   *
-   * The subclass should return the timestamp of the most recent block.
-   *
-   * @return the top block's timestamp
-   */
+  // return timestamp of most recent block
   virtual uint64_t get_top_block_timestamp() const = 0;
 
-  /**
-   * @brief fetch a block's size
-   *
-   * The subclass should return the size of the block with the
-   * given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the size
-   */
+  // return block size of block at height <height>
   virtual size_t get_block_size(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a block's cumulative difficulty
-   *
-   * The subclass should return the cumulative difficulty of the block with the
-   * given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the cumulative difficulty
-   */
+  // return cumulative difficulty up to and including block at height <height>
   virtual difficulty_type get_block_cumulative_difficulty(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a block's difficulty
-   *
-   * The subclass should return the difficulty of the block with the
-   * given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the difficulty
-   */
+  // return difficulty of block at height <height>
   virtual difficulty_type get_block_difficulty(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a block's already generated coins
-   *
-   * The subclass should return the total coins generated as of the block
-   * with the given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the already generated coins
-   */
+  // return number of coins generated up to and including block at height <height>
   virtual uint64_t get_block_already_generated_coins(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a block's hash
-   *
-   * The subclass should return hash of the block with the
-   * given height.
-   *
-   * If the block does not exist, the subclass should throw BLOCK_DNE
-   *
-   * @param height the height requested
-   *
-   * @return the hash
-   */
+  // return hash of block at height <height>
   virtual crypto::hash get_block_hash_from_height(const uint64_t& height) const = 0;
 
-  /**
-   * @brief fetch a list of blocks
-   *
-   * The subclass should return a vector of blocks with heights starting at
-   * h1 and ending at h2, inclusively.
-   *
-   * If the height range requested goes past the end of the blockchain,
-   * the subclass should throw BLOCK_DNE.  (current implementations simply
-   * don't catch this exception as thrown by methods called within)
-   *
-   * @param h1 the start height
-   * @param h2 the end height
-   *
-   * @return a vector of blocks
-   */
+  // return vector of blocks in range <h1,h2> of height (inclusively)
   virtual std::vector<block> get_blocks_range(const uint64_t& h1, const uint64_t& h2) const = 0;
 
-  /**
-   * @brief fetch a list of block hashes
-   *
-   * The subclass should return a vector of block hashes from blocks with
-   * heights starting at h1 and ending at h2, inclusively.
-   *
-   * If the height range requested goes past the end of the blockchain,
-   * the subclass should throw BLOCK_DNE.  (current implementations simply
-   * don't catch this exception as thrown by methods called within)
-   *
-   * @param h1 the start height
-   * @param h2 the end height
-   *
-   * @return a vector of block hashes
-   */
+  // return vector of block hashes in range <h1, h2> of height (inclusively)
   virtual std::vector<crypto::hash> get_hashes_range(const uint64_t& h1, const uint64_t& h2) const = 0;
 
-  /**
-   * @brief fetch the top block's hash
-   *
-   * The subclass should return the hash of the most recent block
-   *
-   * @return the top block's hash
-   */
+  // return the hash of the top block on the chain
   virtual crypto::hash top_block_hash() const = 0;
 
-  /**
-   * @brief fetch the top block
-   *
-   * The subclass should return most recent block
-   *
-   * @return the top block
-   */
+  // return the block at the top of the blockchain
   virtual block get_top_block() const = 0;
 
-  /**
-   * @brief fetch the current blockchain height
-   *
-   * The subclass should return the current blockchain height
-   *
-   * @return the current blockchain height
-   */
+  // return the height of the chain
   virtual uint64_t height() const = 0;
 
-
-  /**
-   * <!--
-   * TODO: Rewrite (if necessary) such that all calls to remove_* are
-   *       done in concrete members of this base class.
-   * -->
-   *
-   * @brief pops the top block off the blockchain
-   *
-   * The subclass should remove the most recent block from the blockchain,
-   * along with all transactions, outputs, and other metadata created as
-   * a result of its addition to the blockchain.  Most of this is handled
-   * by the concrete members of the base class provided the subclass correctly
-   * implements remove_* functions.
-   *
-   * The subclass should return by reference the popped block and
-   * its associated transactions
-   *
-   * @param blk return-by-reference the block which was popped
-   * @param txs return-by-reference the transactions from the popped block
-   */
+  // pops the top block off the blockchain.
+  // Returns by reference the popped block and its associated transactions
+  //
+  // IMPORTANT:
+  // When a block is popped, the transactions associated with it need to be
+  // removed, as well as outputs and spent key images associated with
+  // those transactions.
   virtual void pop_block(block& blk, std::vector<transaction>& txs);
 
 
-  /**
-   * @brief check if a transaction with a given hash exists
-   *
-   * The subclass should check if a transaction is stored which has the
-   * given hash and return true if so, false otherwise.
-   *
-   * @param h the hash to check against
-   *
-   * @return true if the transaction exists, otherwise false
-   */
+  // return true if a transaction with hash <h> exists
   virtual bool tx_exists(const crypto::hash& h) const = 0;
 
   // return unlock time of tx with hash <h>
-  /**
-   * @brief fetch a transaction's unlock time/height
-   *
-   * The subclass should return the stored unlock time for the transaction
-   * with the given hash.
-   *
-   * If no such transaction exists, the subclass should throw TX_DNE.
-   *
-   * @param h the hash of the requested transaction
-   *
-   * @return the unlock time/height
-   */
   virtual uint64_t get_tx_unlock_time(const crypto::hash& h) const = 0;
 
   // return tx with hash <h>
   // throw if no such tx exists
-  /**
-   * @brief fetches the transaction with the given hash
-   *
-   * The subclass should return the transaction stored which has the given
-   * hash.
-   *
-   * If the transaction does not exist, the subclass should throw TX_DNE.
-   *
-   * @param h the hash to look for
-   *
-   * @return the transaction with the given hash
-   */
   virtual transaction get_tx(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief fetches the total number of transactions ever
-   *
-   * The subclass should return a count of all the transactions from
-   * all blocks.
-   *
-   * @return the number of transactions in the blockchain
-   */
+  // returns the total number of transactions in all blocks
   virtual uint64_t get_tx_count() const = 0;
 
-  /**
-   * @brief fetches a list of transactions based on their hashes
-   *
-   * The subclass should attempt to fetch each transaction referred to by
-   * the hashes passed.
-   *
-   * Currently, if any of the transactions is not in BlockchainDB, the call
-   * to get_tx in the implementation will throw TX_DNE.
-   *
-   * <!-- TODO: decide if this behavior is correct for missing transactions -->
-   *
-   * @param hlist a list of hashes
-   *
-   * @return the list of transactions
-   */
+  // return list of tx with hashes <hlist>.
+  // TODO: decide if a missing hash means return empty list
+  // or just skip that hash
   virtual std::vector<transaction> get_tx_list(const std::vector<crypto::hash>& hlist) const = 0;
 
   // returns height of block that contains transaction with hash <h>
-  /**
-   * @brief fetches the height of a transaction's block
-   *
-   * The subclass should attempt to return the height of the block containing
-   * the transaction with the given hash.
-   *
-   * If the transaction cannot be found, the subclass should throw TX_DNE.
-   *
-   * @param h the hash of the transaction
-   *
-   * @return the height of the transaction's block
-   */
   virtual uint64_t get_tx_block_height(const crypto::hash& h) const = 0;
 
   // returns the total number of outputs of amount <amount>
-  /**
-   * @brief fetches the number of outputs of a given amount
-   *
-   * The subclass should return a count of outputs of the given amount,
-   * or zero if there are none.
-   *
-   * <!-- TODO: should outputs spent with a low mixin (especially 0) be
-   * excluded from the count? -->
-   *
-   * @param amount the output amount being looked up
-   *
-   * @return the number of outputs of the given amount
-   */
   virtual uint64_t get_num_outputs(const uint64_t& amount) const = 0;
 
-  /**
-   * @brief return index of the first element (should be hidden, but isn't)
-   *
-   * @return the index
-   */
+  // return index of the first element (should be hidden, but isn't)
   virtual uint64_t get_indexing_base() const { return 0; }
 
-  /**
-   * @brief get some of an output's data
-   *
-   * The subclass should return the public key, unlock time, and block height
-   * for the output with the given amount and index, collected in a struct.
-   *
-   * If the output cannot be found, the subclass should throw OUTPUT_DNE.
-   *
-   * If any of these parts cannot be found, but some are, the subclass
-   * should throw DB_ERROR with a message stating as much.
-   *
-   * @param amount the output amount
-   * @param index the output's index (indexed by amount)
-   *
-   * @return the requested output data
-   */
+  // return public key for output with global output amount <amount> and index <index>
   virtual output_data_t get_output_key(const uint64_t& amount, const uint64_t& index) = 0;
-
-  /**
-   * @brief get some of an output's data
-   *
-   * The subclass should return the public key, unlock time, and block height
-   * for the output with the given global index, collected in a struct.
-   *
-   * If the output cannot be found, the subclass should throw OUTPUT_DNE.
-   *
-   * If any of these parts cannot be found, but some are, the subclass
-   * should throw DB_ERROR with a message stating as much.
-   *
-   * @param global_index the output's index (global)
-   *
-   * @return the requested output data
-   */
   virtual output_data_t get_output_key(const uint64_t& global_index) const = 0;
 
-  /**
-   * @brief gets an output's tx hash and index
-   *
-   * The subclass should return the hash of the transaction which created the
-   * output with the global index given, as well as its index in that transaction.
-   *
-   * @param index an output's global index
-   *
-   * @return the tx hash and output index
-   */
+  // returns the tx hash associated with an output, referenced by global output index
   virtual tx_out_index get_output_tx_and_index_from_global(const uint64_t& index) const = 0;
 
-  /**
-   * @brief gets an output's tx hash and index
-   *
-   * The subclass should return the hash of the transaction which created the
-   * output with the amount and index given, as well as its index in that
-   * transaction.
-   *
-   * @param amount an output amount
-   * @param index an output's amount-specific index
-   *
-   * @return the tx hash and output index
-   */
+  // returns the transaction-local reference for the output with <amount> at <index>
+  // return type is pair of tx hash and index
   virtual tx_out_index get_output_tx_and_index(const uint64_t& amount, const uint64_t& index) = 0;
-
-  /**
-   * @brief gets some outputs' tx hashes and indices
-   *
-   * This function is a mirror of
-   * get_output_tx_and_index(const uint64_t& amount, const uint64_t& index),
-   * but for a list of outputs rather than just one.
-   *
-   * @param amount an output amount
-   * @param offsets a list of amount-specific output indices
-   * @param indices return-by-reference a list of tx hashes and output indices (as pairs)
-   */
   virtual void get_output_tx_and_index(const uint64_t& amount, const std::vector<uint64_t> &offsets, std::vector<tx_out_index> &indices) = 0;
-
-  /**
-   * @brief gets outputs' data
-   *
-   * This function is a mirror of
-   * get_output_data(const uint64_t& amount, const uint64_t& index)
-   * but for a list of outputs rather than just one.
-   *
-   * @param amount an output amount
-   * @param offsets a list of amount-specific output indices
-   * @param outputs return-by-reference a list of outputs' metadata
-   */
   virtual void get_output_key(const uint64_t &amount, const std::vector<uint64_t> &offsets, std::vector<output_data_t> &outputs) = 0;
-  
-  /*
-   * FIXME: Need to check with git blame and ask what this does to
-   * document it
-   */
+
   virtual bool can_thread_bulk_indices() const = 0;
 
-  /**
-   * @brief gets output indices (global) for a transaction's outputs
-   *
-   * The subclass should fetch the global output indices for each output
-   * in the transaction with the given hash.
-   *
-   * If the transaction does not exist, the subclass should throw TX_DNE.
-   *
-   * If an output cannot be found, the subclass should throw OUTPUT_DNE.
-   *
-   * @param h a transaction hash
-   *
-   * @return a list of global output indices
-   */
+  // return a vector of indices corresponding to the global output index for
+  // each output in the transaction with hash <h>
   virtual std::vector<uint64_t> get_tx_output_indices(const crypto::hash& h) const = 0;
-
-  /**
-   * @brief gets output indices (amount-specific) for a transaction's outputs
-   *
-   * The subclass should fetch the amount-specific output indices for each
-   * output in the transaction with the given hash.
-   *
-   * If the transaction does not exist, the subclass should throw TX_DNE.
-   *
-   * If an output cannot be found, the subclass should throw OUTPUT_DNE.
-   *
-   * @param h a transaction hash
-   *
-   * @return a list of amount-specific output indices
-   */
+  // return a vector of indices corresponding to the amount output index for
+  // each output in the transaction with hash <h>
   virtual std::vector<uint64_t> get_tx_amount_output_indices(const crypto::hash& h) const = 0;
 
-  /**
-   * @brief check if a key image is stored as spent
-   *
-   * @param img the key image to check for
-   *
-   * @return true if the image is present, otherwise false
-   */
+  // returns true if key image <img> is present in spent key images storage
   virtual bool has_key_image(const crypto::key_image& img) const = 0;
 
-  /**
-   * @brief runs a function over all key images stored
-   *
-   * The subclass should run the passed function for each key image it has
-   * stored, passing the key image as its parameter.
-   *
-   * If any call to the function returns false, the subclass should return
-   * false.  Otherwise, the subclass returns true.
-   *
-   * @param std::function fn the function to run
-   *
-   * @return false if the function returns false for any key image, otherwise true
-   */
   virtual bool for_all_key_images(std::function<bool(const crypto::key_image&)>) const = 0;
-
-  /**
-   * @brief runs a function over all blocks stored
-   *
-   * The subclass should run the passed function for each block it has
-   * stored, passing (block_height, block_hash, block) as its parameters.
-   *
-   * If any call to the function returns false, the subclass should return
-   * false.  Otherwise, the subclass returns true.
-   *
-   * The subclass should throw DB_ERROR if any of the expected values are
-   * not found.  Current implementations simply return false.
-   *
-   * @param std::function fn the function to run
-   *
-   * @return false if the function returns false for any block, otherwise true
-   */
   virtual bool for_all_blocks(std::function<bool(uint64_t, const crypto::hash&, const cryptonote::block&)>) const = 0;
-
-  /**
-   * @brief runs a function over all transactions stored
-   *
-   * The subclass should run the passed function for each transaction it has
-   * stored, passing (transaction_hash, transaction) as its parameters.
-   *
-   * If any call to the function returns false, the subclass should return
-   * false.  Otherwise, the subclass returns true.
-   *
-   * The subclass should throw DB_ERROR if any of the expected values are
-   * not found.  Current implementations simply return false.
-   *
-   * @param std::function fn the function to run
-   *
-   * @return false if the function returns false for any transaction, otherwise true
-   */
   virtual bool for_all_transactions(std::function<bool(const crypto::hash&, const cryptonote::transaction&)>) const = 0;
-
-  /**
-   * @brief runs a function over all outputs stored
-   *
-   * The subclass should run the passed function for each output it has
-   * stored, passing (amount, transaction_hash, tx_local_output_index)
-   * as its parameters.
-   *
-   * If any call to the function returns false, the subclass should return
-   * false.  Otherwise, the subclass returns true.
-   *
-   * The subclass should throw DB_ERROR if any of the expected values are
-   * not found.  Current implementations simply return false.
-   *
-   * @param std::function f the function to run
-   *
-   * @return false if the function returns false for any output, otherwise true
-   */
   virtual bool for_all_outputs(std::function<bool(uint64_t amount, const crypto::hash &tx_hash, size_t tx_idx)> f) const = 0;
 
-
-  //
   // Hard fork related storage
-  //
-
-  // FIXME: verify that this is all correct
-  //      - TW
-  /**
-   * @brief sets the height at which a hard fork has been voted to happen
-   *
-   *
-   * @param version the version voted to fork to
-   * @param height the height of the first block on the new fork
-   */
   virtual void set_hard_fork_starting_height(uint8_t version, uint64_t height) = 0;
-
-  /**
-   * @brief gets the height at which a hard fork has been voted to happen
-   *
-   * @param version the version to check
-   *
-   * @return the height at which the hard fork was accepted, if it has been,
-   * otherwise max(uint64_t)
-   */
   virtual uint64_t get_hard_fork_starting_height(uint8_t version) const = 0;
-
-  /**
-   * @brief sets which hardfork version a height is on
-   *
-   * @param height the height
-   * @param version the version
-   */
   virtual void set_hard_fork_version(uint64_t height, uint8_t version) = 0;
-
-  /**
-   * @brief checks which hardfork version a height is on
-   *
-   * @param height the height
-   *
-   * @return the version
-   */
   virtual uint8_t get_hard_fork_version(uint64_t height) const = 0;
-
-  /**
-   * @brief verify hard fork info in database
-   */
   virtual void check_hard_fork_info() = 0;
-
-  /**
-   * @brief delete hard fork info from database
-   */
   virtual void drop_hard_fork_info() = 0;
 
-  /**
-   * @brief is BlockchainDB in read-only mode?
-   *
-   * @return true if in read-only mode, otherwise false
-   */
   virtual bool is_read_only() const = 0;
 
-  // TODO: this should perhaps be (or call) a series of functions which
-  // progressively update through version updates
-  /**
-   * @brief fix up anything that may be wrong due to past bugs
-   */
+  // fix up anything that may be wrong due to past bugs
   virtual void fixup();
 
-  /**
-   * @brief set whether or not to automatically remove logs
-   *
-   * This function is only relevant for one implementation (BlockchainBDB), but
-   * is here to keep BlockchainDB users implementation-agnostic.
-   *
-   * @param auto_remove whether or not to auto-remove logs
-   */
   void set_auto_remove_logs(bool auto_remove) { m_auto_remove_logs = auto_remove; }
 
-  bool m_open;  //!< Whether or not the BlockchainDB is open/ready for use
-  mutable epee::critical_section m_synchronization_lock;  //!< A lock, currently for when BlockchainLMDB needs to resize the backing db file
-
+  bool m_open;
+  mutable epee::critical_section m_synchronization_lock;
 };  // class BlockchainDB