[FOLD] Address additional review comments

src/ripple/app/consensus/RCLValidations.cpp

@@ -90,8 +90,11 @@ RCLValidationsPolicy::flush(hash_map<PublicKey, RCLValidation>&& remaining)
             doStaleWrite(sl);
         }
 
-        // Handle the case where flush() is called while a queuedWrite
-        // is already in progress.
+        // In the case when a prior asynchronous doStaleWrite was scheduled, 
+        // this loop will block until all validations have been flushed.
+        // This ensures that all validations are written upon return from
+        // this function.
+
         while (staleWriting_)
         {
             ScopedUnlockType sul(staleLock_);
@@ -100,17 +103,17 @@ RCLValidationsPolicy::flush(hash_map<PublicKey, RCLValidation>&& remaining)
     }
 }
 
-// NOTE: doStaleWrite() must be called with mLock *locked*.  The passed
+// NOTE: doStaleWrite() must be called with staleLock_ *locked*.  The passed
 // ScopedLockType& acts as a reminder to future maintainers.
 void
 RCLValidationsPolicy::doStaleWrite(ScopedLockType&)
 {
-    std::string insVal(
+    static const std::string insVal(
         "INSERT INTO Validations "
         "(InitialSeq, LedgerSeq, LedgerHash,NodePubKey,SignTime,RawData) "
         "VALUES (:initialSeq, :ledgerSeq, "
         ":ledgerHash,:nodePubKey,:signTime,:rawData);");
-    std::string findSeq(
+    static const std::string findSeq(
         "SELECT LedgerSeq FROM Ledgers WHERE Ledgerhash=:ledgerHash;");
 
     assert(staleWriting_);
@@ -199,11 +202,11 @@ handleNewValidation(Application& app,
     bool shouldRelay = false;
 
     // only add trusted or listed
-    if (val->isTrusted() || pubKey)
+    if (pubKey)
     {
         using AddOutcome = RCLValidations::AddOutcome;
 
-        AddOutcome res = validations.add(*pubKey, val);
+        AddOutcome const res = validations.add(*pubKey, val);
 
         // This is a duplicate validation
         if (res == AddOutcome::repeat)
@@ -239,10 +242,11 @@ handleNewValidation(Application& app,
     {
         JLOG(j.debug()) << "Val for " << hash << " from "
                     << toBase58(TokenType::TOKEN_NODE_PUBLIC, signer)
-                    << " not added UNtrustesd/";
+                    << " not added UNtrusted/";
     }
 
-    // FIXME: This never forwards untrusted validations, from @JoelKatz:
+    // This currently never forwards untrusted validations, though we may
+    // reconsider in the future. From @JoelKatz:
     // The idea was that we would have a certain number of validation slots with
     // priority going to validators we trusted. Remaining slots might be
     // allocated to validators that were listed by publishers we trusted but

src/ripple/app/consensus/RCLValidations.h

@@ -56,13 +56,15 @@ class RCLValidation
     }
 
     /// Validated ledger's sequence number (0 if none)
-    std::size_t
+    std::uint32_t
     seq() const
     {
-        return val_->getFieldU32(sfLedgerSequence);
+        if(auto res = (*val_)[~sfLedgerSequence])
+            return *res;
+        return 0;
     }
 
-    /// Validation ledger's signing time
+    /// Validation's signing time
     NetClock::time_point
     signTime() const
     {
@@ -97,30 +99,28 @@ class RCLValidation
         return val_->isTrusted();
     }
 
-    /// Set the prior ledger hash this validation is replacing
+    /// Set the prior ledger hash this validation is following
     void
     setPreviousLedgerID(uint256 const& hash)
     {
         val_->setPreviousHash(hash);
     }
 
-    /// Check whether the given hash matches this validaiton's prior hash
+    /// Check whether the given hash matches this validation's prior hash
     bool
     isPreviousLedgerID(uint256 const& hash) const
     {
         return val_->isPreviousHash(hash);
     }
 
-    /// Get the load fee of the validation of it exists
-    boost::optional<std::uint64_t>
+    /// Get the load fee of the validation if it exists
+    boost::optional<std::uint32_t>
     loadFee() const
     {
-        if (val_->isFieldPresent(sfLoadFee))
-            return val_->getFieldU32(sfLoadFee);
-        return boost::none;
+        return ~(*val_)[~sfLoadFee];
     }
 
-    /// Extract the underlying STValidation being wrapepd
+    /// Extract the underlying STValidation being wrapped
     STValidation::pointer
     unwrap() const
     {
@@ -129,7 +129,7 @@ class RCLValidation
 
 };
 
-/** Implements the StalePolicy policy class for adapating Validations in the RCL
+/** Implements the StalePolicy policy class for adapting Validations in the RCL
 
     Manages storing and writing stale RCLValidations to the sqlite DB.
 */
@@ -172,10 +172,10 @@ class RCLValidationsPolicy
     void
     onStale(RCLValidation&& v);
 
-     /** Flush current validations to disk before shutdown.
+    /** Flush current validations to disk before shutdown.
 
-         @param remaining The remaining validations to flush
-      */
+        @param remaining The remaining validations to flush
+    */
     void
     flush(hash_map<PublicKey, RCLValidation> && remaining);
 };
@@ -192,7 +192,7 @@ using RCLValidations =
     2. Add the validation to the set of validations if current.
     3. If new and trusted, send the validation to the ledgerMaster.
 
-    @param app Application object containing validations and ledgerMesger
+    @param app Application object containing validations and ledgerMaster
     @param val The validation to add
     @param source Name associated with validation used in logging
 

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

@@ -790,15 +790,15 @@ LedgerMaster::checkAccept (
         app_.getOrderBookDB().setup(ledger);
     }
 
-    std::uint64_t const base = app_.getFeeTrack().getLoadBase();
+    std::uint32_t const base = app_.getFeeTrack().getLoadBase();
     auto fees = app_.getValidations().fees (ledger->info().hash, base);
     {
         auto fees2 = app_.getValidations().fees (
             ledger->info(). parentHash, base);
         fees.reserve (fees.size() + fees2.size());
         std::copy (fees2.begin(), fees2.end(), std::back_inserter(fees));
     }
-    std::uint64_t fee;
+    std::uint32_t fee;
     if (! fees.empty())
     {
         std::sort (fees.begin(), fees.end());