[FOLD] Address code review feedback

src/ripple/app/ledger/Ledger.cpp

@@ -927,9 +927,12 @@ saveValidatedLedger(
  return true;
  }
 
- auto res = dynamic_cast<SQLiteDatabase*>(&app.getRelationalDatabase())
+ auto const res = dynamic_cast<SQLiteDatabase*>(&app.getRelationalDatabase())
  ->saveValidatedLedger(ledger, current);
 
+ if (!res)
+ Throw<std::runtime_error>("Failed to get relational database");
+
  // Clients can now trust the database for
  // information about this ledger sequence.
  app.pendingSaves().finishWork(seq);
@@ -1162,9 +1165,11 @@ flatFetchTransactions(ReadView const& ledger, Application& app)
  return {};
  }
 
- auto nodestoreHashes =
- dynamic_cast<PostgresDatabase*>(&app.getRelationalDatabase())
- ->getTxHashes(ledger.info().seq);
+ auto const db = dynamic_cast<PostgresDatabase*>(&app.getRelationalDatabase());
+ if (!db)
+ Throw<std::runtime_error>("Failed to get relational database");
+
+ auto nodestoreHashes = db->getTxHashes(ledger.info().seq);
 
  return flatFetchTransactions(app, nodestoreHashes);
 }

src/ripple/app/ledger/impl/LedgerMaster.cpp

@@ -275,6 +275,7 @@ LedgerMaster::getValidatedLedgerAge()
  return static_cast<PostgresDatabase*>(&app_.getRelationalDatabase())
  ->getValidatedLedgerAge();
 #endif
+
  std::chrono::seconds valClose{mValidLedgerSign.load()};
  if (valClose == 0s)
  {

src/ripple/app/main/Application.cpp

@@ -869,7 +869,7 @@ class ApplicationImp : public Application, public BasicApp
  //--------------------------------------------------------------------------
 
  bool
- initRDBMS()
+ initRelationalDatabase()
  {
  assert(mWalletDB.get() == nullptr);
 
@@ -1225,7 +1225,7 @@ ApplicationImp::setup()
  if (!config_->standalone())
  timeKeeper_->run(config_->SNTP_SERVERS);
 
- if (!initRDBMS() || !initNodeStore())
+ if (!initRelationalDatabase() || !initNodeStore())
  return false;
 
  if (shardStore_)

src/ripple/app/main/DBInit.h

@@ -72,8 +72,9 @@ inline constexpr std::array<char const*, 5> LgrDBInit{
 // Transaction database holds transactions and public keys
 inline constexpr auto TxDBName{"transaction.db"};
 
-// Omitting the explicit template parameters
-// seemed to caused undefined behaviour
+
+// In C++17 omitting the explicit template parameters caused
+// a crash
 inline constexpr std::array<char const*, 4> TxDBPragma
 {
  "PRAGMA page_size=4096;", "PRAGMA journal_size_limit=1582080;",
@@ -127,8 +128,9 @@ inline constexpr std::array<char const*, 8> TxDBInit{
 // The Ledger Meta database maps ledger hashes to shard indexes
 inline constexpr auto LgrMetaDBName{"ledger_meta.db"};
 
-// Omitting the explicit template parameters
-// seemed to caused undefined behaviour
+
+// In C++17 omitting the explicit template parameters caused
+// a crash
 inline constexpr std::array<char const*, 4> LgrMetaDBPragma
 {
  "PRAGMA page_size=4096;", "PRAGMA journal_size_limit=1582080;",
@@ -161,8 +163,8 @@ inline constexpr std::array<char const*, 3> LgrMetaDBInit{
 // Transaction Meta database maps transaction IDs to shard indexes
 inline constexpr auto TxMetaDBName{"transaction_meta.db"};
 
-// Omitting the explicit template parameters
-// seemed to caused undefined behaviour
+// In C++17 omitting the explicit template parameters caused
+// a crash
 inline constexpr std::array<char const*, 4> TxMetaDBPragma
 {
  "PRAGMA page_size=4096;", "PRAGMA journal_size_limit=1582080;",

src/ripple/app/misc/NetworkOPs.cpp

@@ -3298,6 +3298,8 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo)
 #ifdef RIPPLED_REPORTING
  if (app_.config().reporting())
  {
+ // Use a dynamic_cast to return DatabaseType::None
+ // on failure.
  if (dynamic_cast<PostgresDatabase*>(&app_.getRelationalDatabase()))
  {
  return DatabaseType::Postgres;
@@ -3306,13 +3308,17 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo)
  }
  else
  {
+ // Use a dynamic_cast to return DatabaseType::None
+ // on failure.
  if (dynamic_cast<SQLiteDatabase*>(&app_.getRelationalDatabase()))
  {
  return DatabaseType::Sqlite;
  }
  return DatabaseType::None;
  }
 #else
+ // Use a dynamic_cast to return DatabaseType::None
+ // on failure.
  if (dynamic_cast<SQLiteDatabase*>(&app_.getRelationalDatabase()))
  {
  return DatabaseType::Sqlite;

src/ripple/app/misc/SHAMapStoreImp.cpp

@@ -620,18 +620,17 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
  if (health())
  return;
 
- SQLiteDatabase* iface =
+ SQLiteDatabase* const db =
  dynamic_cast<SQLiteDatabase*>(&app_.getRelationalDatabase());
 
+ if (!db)
+ Throw<std::runtime_error>("Failed to get relational database");
+
  clearSql(
  lastRotated,
  "Ledgers",
- [&iface]() -> std::optional<LedgerIndex> {
- return iface->getMinLedgerSeq();
- },
- [&iface](LedgerIndex min) -> void {
- iface->deleteBeforeLedgerSeq(min);
- });
+ [db]() -> std::optional<LedgerIndex> { return db->getMinLedgerSeq(); },
+ [db](LedgerIndex min) -> void { db->deleteBeforeLedgerSeq(min); });
  if (health())
  return;
 
@@ -641,23 +640,23 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
  clearSql(
  lastRotated,
  "Transactions",
- [&iface]() -> std::optional<LedgerIndex> {
- return iface->getTransactionsMinLedgerSeq();
+ [&db]() -> std::optional<LedgerIndex> {
+ return db->getTransactionsMinLedgerSeq();
  },
- [&iface](LedgerIndex min) -> void {
- iface->deleteTransactionsBeforeLedgerSeq(min);
+ [&db](LedgerIndex min) -> void {
+ db->deleteTransactionsBeforeLedgerSeq(min);
  });
  if (health())
  return;
 
  clearSql(
  lastRotated,
  "AccountTransactions",
- [&iface]() -> std::optional<LedgerIndex> {
- return iface->getAccountTransactionsMinLedgerSeq();
+ [&db]() -> std::optional<LedgerIndex> {
+ return db->getAccountTransactionsMinLedgerSeq();
  },
- [&iface](LedgerIndex min) -> void {
- iface->deleteAccountTransactionsBeforeLedgerSeq(min);
+ [&db](LedgerIndex min) -> void {
+ db->deleteAccountTransactionsBeforeLedgerSeq(min);
  });
  if (health())
  return;

src/ripple/app/misc/impl/Transaction.cpp

@@ -133,8 +133,15 @@ Transaction::load(
 Transaction::Locator
 Transaction::locate(uint256 const& id, Application& app)
 {
- return dynamic_cast<PostgresDatabase*>(&app.getRelationalDatabase())
- ->locateTransaction(id);
+ auto const db =
+ dynamic_cast<PostgresDatabase*>(&app.getRelationalDatabase());
+
+ if (!db)
+ {
+ Throw<std::runtime_error>("Failed to get relational database");
+ }
+
+ return db->locateTransaction(id);
 }
 
 std::variant<
@@ -146,8 +153,14 @@ Transaction::load(
  std::optional<ClosedInterval<uint32_t>> const& range,
  error_code_i& ec)
 {
- return dynamic_cast<SQLiteDatabase*>(&app.getRelationalDatabase())
- ->getTransaction(id, range, ec);
+ auto const db = dynamic_cast<SQLiteDatabase*>(&app.getRelationalDatabase());
+
+ if (!db)
+ {
+ Throw<std::runtime_error>("Failed to get relational database");
+ }
+
+ return db->getTransaction(id, range, ec);
 }
 
 // options 1 to include the date of the transaction

src/ripple/app/rdb/README.md

@@ -21,7 +21,7 @@ backend=sqlite
 
 ## Source Files
 
-The Relational Database Interface consists of the following directory structure:
+The Relational Database Interface consists of the following directory structure (as of November 2021):
 
 ```
 src/ripple/app/rdb/
@@ -76,7 +76,7 @@ src/ripple/app/rdb/
 
 ## Classes
 
-The abstract class `RelationalDatabase` is the primary class of the Relational Database Interface and is defined in the eponymous header file. This class has provides a static method `init()` which, when invoked, creates a concrete instance of a derived class whose type is specified by the system configuration. All other methods in the class are virtual. Presently there exist two classes that derive from `RelationalDatabase`, namely `SQLiteDatabase` and `PostgresDatabase`.
+The abstract class `RelationalDatabase` is the primary class of the Relational Database Interface and is defined in the eponymous header file. This class provides a static method `init()` which, when invoked, creates a concrete instance of a derived class whose type is specified by the system configuration. All other methods in the class are virtual. Presently there exist two classes that derive from `RelationalDatabase`, namely `SQLiteDatabase` and `PostgresDatabase`.
 
 ## Database Methods
 

src/ripple/app/rdb/UnitaryShard.h

@@ -68,7 +68,7 @@ makeShardIncompleteLedgerDBs(
  * @param lgrdb Session with the ledger databases.
  * @param ledger Ledger to save.
  * @param index Index of the shard that owns the ledger.
- * @param stop Link to an atomic flag that can stop the process if raised.
+ * @param stop Reference to an atomic flag that can stop the process if raised.
  * @param j Journal
  * @return True if the ledger was successfully saved.
  */
@@ -140,7 +140,7 @@ selectAcquireDBLedgerSeqsHash(soci::session& session, std::uint32_t index);
  * @param ledger Ledger to save into the database.
  * @param index Shard index.
  * @param lastSeq Last acquired ledger sequence.
- * @param seqs Current set or acquired ledger sequences if it's not empty.
+ * @param seqs Current set of acquired ledger sequences if it's not empty.
  */
 void
 updateAcquireDB(

src/ripple/app/rdb/Wallet.h

@@ -26,7 +26,6 @@
 #include <ripple/core/DatabaseCon.h>
 #include <ripple/overlay/PeerReservationTable.h>
 #include <ripple/peerfinder/impl/Store.h>
-#include <boost/filesystem.hpp>
 
 namespace ripple {
 

src/ripple/app/rdb/backend/detail/Node.h

@@ -233,7 +233,7 @@ getHashesByIndex(
  * @param maxSeq Maximum ledger sequence.
  * @param j Journal.
  * @return Map which points sequence number of found ledger to the struct
- * LedgerHashPair which contauns ledger hash and its parent hash.
+ * LedgerHashPair which contains ledger hash and its parent hash.
  */
 std::map<LedgerIndex, LedgerHashPair>
 getHashesByIndex(

src/ripple/app/rdb/backend/impl/PostgresDatabase.cpp

@@ -46,7 +46,7 @@ using AccountTxResult = RelationalDatabase::AccountTxResult;
 using TxnsData = RelationalDatabase::AccountTxs;
 using TxnsDataBinary = RelationalDatabase::MetaTxsList;
 
-class PostgresDatabaseImp : public PostgresDatabase
+class PostgresDatabaseImp final : public PostgresDatabase
 {
 public:
  PostgresDatabaseImp(