Prefer keylets instead of naked hashes:

Entries in the ledger are located using 256-bit locators. The locators are calculated using a wide range of parameters specific to the entry whose locator we are calculating (e.g. an account's locator is derived from the account's address, whereas the locator for an offer is derived from the account and the offer sequence.) Keylets enhance type safety during lookup and make the code more robust, so this commit removes most of the earlier code, which used naked uint256 values.
XRPLF · Apr 29, 2020 · da75e5b · da75e5b
1 parent 3f15977
commit da75e5b
Show file tree

Hide file tree

Showing 23 changed files with 459 additions and 633 deletions.
diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp
@@ -1496,8 +1496,8 @@ NetworkOPsImp::getOwnerInfo(
     AccountID const& account)
 {
     Json::Value jvObjects(Json::objectValue);
-    auto uRootIndex = getOwnerDirIndex(account);
-    auto sleNode = lpLedger->read(keylet::page(uRootIndex));
+    auto root = keylet::ownerDir(account);
+    auto sleNode = lpLedger->read(keylet::page(root));
     if (sleNode)
     {
         std::uint64_t uNodeDir;
@@ -1543,7 +1543,7 @@ NetworkOPsImp::getOwnerInfo(
 
             if (uNodeDir)
             {
-                sleNode = lpLedger->read(keylet::page(uRootIndex, uNodeDir));
+                sleNode = lpLedger->read(keylet::page(root, uNodeDir));
                 assert(sleNode);
             }
         } while (uNodeDir);

diff --git a/src/ripple/app/tx/impl/CancelCheck.cpp b/src/ripple/app/tx/impl/CancelCheck.cpp
@@ -88,8 +88,7 @@ CancelCheck::preclaim(PreclaimContext const& ctx)
 TER
 CancelCheck::doApply()
 {
-    uint256 const checkId{ctx_.tx[sfCheckID]};
-    auto const sleCheck = view().peek(keylet::check(checkId));
+    auto const sleCheck = view().peek(keylet::check(ctx_.tx[sfCheckID]));
     if (!sleCheck)
     {
         // Error should have been caught in preclaim.
@@ -106,15 +105,17 @@ CancelCheck::doApply()
     if (srcId != dstId)
     {
         std::uint64_t const page{(*sleCheck)[sfDestinationNode]};
-        if (!view().dirRemove(keylet::ownerDir(dstId), page, checkId, true))
+        if (!view().dirRemove(
+                keylet::ownerDir(dstId), page, sleCheck->key(), true))
         {
             JLOG(j_.warn()) << "Unable to delete check from destination.";
             return tefBAD_LEDGER;
         }
     }
     {
         std::uint64_t const page{(*sleCheck)[sfOwnerNode]};
-        if (!view().dirRemove(keylet::ownerDir(srcId), page, checkId, true))
+        if (!view().dirRemove(
+                keylet::ownerDir(srcId), page, sleCheck->key(), true))
         {
             JLOG(j_.warn()) << "Unable to delete check from owner.";
             return tefBAD_LEDGER;

diff --git a/src/ripple/app/tx/impl/CancelOffer.cpp b/src/ripple/app/tx/impl/CancelOffer.cpp
@@ -83,11 +83,7 @@ CancelOffer::doApply()
     if (!sle)
         return tefINTERNAL;
 
-    uint256 const offerIndex(getOfferIndex(account_, offerSequence));
-
-    auto sleOffer = view().peek(keylet::offer(offerIndex));
-
-    if (sleOffer)
+    if (auto sleOffer = view().peek(keylet::offer(account_, offerSequence)))
     {
         JLOG(j_.debug()) << "Trying to cancel offer #" << offerSequence;
         return offerDelete(view(), sleOffer, ctx_.app.journal("View"));

diff --git a/src/ripple/app/tx/impl/CashCheck.cpp b/src/ripple/app/tx/impl/CashCheck.cpp
@@ -255,8 +255,7 @@ CashCheck::doApply()
     // directly on a View.
     PaymentSandbox psb(&ctx_.view());
 
-    uint256 const checkKey{ctx_.tx[sfCheckID]};
-    auto const sleCheck = psb.peek(keylet::check(checkKey));
+    auto const sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID]));
     if (!sleCheck)
     {
         JLOG(j_.fatal()) << "Precheck did not verify check's existence.";
@@ -390,7 +389,7 @@ CashCheck::doApply()
     {
         std::uint64_t const page{(*sleCheck)[sfDestinationNode]};
         if (!ctx_.view().dirRemove(
-                keylet::ownerDir(account_), page, checkKey, true))
+                keylet::ownerDir(account_), page, sleCheck->key(), true))
         {
             JLOG(j_.warn()) << "Unable to delete check from destination.";
             return tefBAD_LEDGER;
@@ -400,7 +399,7 @@ CashCheck::doApply()
     {
         std::uint64_t const page{(*sleCheck)[sfOwnerNode]};
         if (!ctx_.view().dirRemove(
-                keylet::ownerDir(srcId), page, checkKey, true))
+                keylet::ownerDir(srcId), page, sleCheck->key(), true))
         {
             JLOG(j_.warn()) << "Unable to delete check from owner.";
             return tefBAD_LEDGER;

diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp
@@ -185,8 +185,7 @@ CreateCheck::doApply()
 
     AccountID const dstAccountId{ctx_.tx[sfDestination]};
     std::uint32_t const seq{ctx_.tx.getSequence()};
-    auto sleCheck =
-        std::make_shared<SLE>(ltCHECK, getCheckIndex(account_, seq));
+    auto sleCheck = std::make_shared<SLE>(keylet::check(account_, seq));
 
     sleCheck->setAccountID(sfAccount, account_);
     sleCheck->setAccountID(sfDestination, dstAccountId);

diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp
@@ -1373,16 +1373,11 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel)
     }
 
     // We need to place the remainder of the offer into its order book.
-    auto const offer_index = getOfferIndex(account_, uSequence);
+    auto const offer_index = keylet::offer(account_, uSequence);
 
     // Add offer to owner's directory.
-    auto const ownerNode = dirAdd(
-        sb,
-        keylet::ownerDir(account_),
-        offer_index,
-        false,
-        describeOwnerDir(account_),
-        viewJ);
+    auto const ownerNode = sb.dirInsert(
+        keylet::ownerDir(account_), offer_index, describeOwnerDir(account_));
 
     if (!ownerNode)
     {
@@ -1404,29 +1399,21 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel)
     auto dir = keylet::quality(keylet::book(book), uRate);
     bool const bookExisted = static_cast<bool>(sb.peek(dir));
 
-    auto const bookNode = dirAdd(
-        sb,
-        dir,
-        offer_index,
-        true,
-        [&](SLE::ref sle) {
-            sle->setFieldH160(
-                sfTakerPaysCurrency, saTakerPays.issue().currency);
-            sle->setFieldH160(sfTakerPaysIssuer, saTakerPays.issue().account);
-            sle->setFieldH160(
-                sfTakerGetsCurrency, saTakerGets.issue().currency);
-            sle->setFieldH160(sfTakerGetsIssuer, saTakerGets.issue().account);
-            sle->setFieldU64(sfExchangeRate, uRate);
-        },
-        viewJ);
+    auto const bookNode = sb.dirAppend(dir, offer_index, [&](SLE::ref sle) {
+        sle->setFieldH160(sfTakerPaysCurrency, saTakerPays.issue().currency);
+        sle->setFieldH160(sfTakerPaysIssuer, saTakerPays.issue().account);
+        sle->setFieldH160(sfTakerGetsCurrency, saTakerGets.issue().currency);
+        sle->setFieldH160(sfTakerGetsIssuer, saTakerGets.issue().account);
+        sle->setFieldU64(sfExchangeRate, uRate);
+    });
 
     if (!bookNode)
     {
         JLOG(j_.debug()) << "final result: failed to add offer to book";
         return {tecDIR_FULL, true};
     }
 
-    auto sleOffer = std::make_shared<SLE>(ltOFFER, offer_index);
+    auto sleOffer = std::make_shared<SLE>(offer_index);
     sleOffer->setAccountID(sfAccount, account_);
     sleOffer->setFieldU32(sfSequence, uSequence);
     sleOffer->setFieldH256(sfBookDirectory, dir.key);

diff --git a/src/ripple/app/tx/impl/DepositPreauth.cpp b/src/ripple/app/tx/impl/DepositPreauth.cpp
@@ -154,11 +154,11 @@ DepositPreauth::doApply()
     }
     else
     {
-        AccountID const unauth{ctx_.tx[sfUnauthorize]};
-        uint256 const preauthIndex{getDepositPreauthIndex(account_, unauth)};
+        auto const preauth = keylet::depositPreauth(
+            account_, ctx_.tx[sfUnauthorize]);
 
-        return DepositPreauth::removeFromLedger(
-            ctx_.app, view(), preauthIndex, j_);
+        return DepositPreauth::removeFromLedger (
+            ctx_.app, view(), preauth.key, j_);
     }
     return tesSUCCESS;
 }

diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp
@@ -477,18 +477,18 @@ SetTrust::doApply()
         // Zero balance in currency.
         STAmount saBalance({currency, noAccount()});
 
-        uint256 index(getRippleStateIndex(account_, uDstAccountID, currency));
+        auto const k = keylet::line(account_, uDstAccountID, currency);
 
         JLOG(j_.trace()) << "doTrustSet: Creating ripple line: "
-                         << to_string(index);
+                         << to_string(k.key);
 
         // Create a new ripple line.
         terResult = trustCreate(
             view(),
             bHigh,
             account_,
             uDstAccountID,
-            index,
+            k.key,
             sle,
             bSetAuth,
             bSetNoRipple && !bClearNoRipple,

diff --git a/src/ripple/crypto/secure_erase.h b/src/ripple/crypto/secure_erase.h
@@ -24,16 +24,16 @@
 
 namespace ripple {
 
-/** Attempts to fill memory with zeroes.
+/** Attempts to clear the given blob of memory.
 
     The underlying implementation of this function takes pains to
-    attempt to outsmart compilers from optimizing the zeroization
+    attempt to outsmart the compiler from optimizing the clearing
     away. Please note that, despite that, remnants of content may
     remain floating around in memory as well as registers, caches
     and more.
 
-    For a comprehensive treatise on the subject of secure
-    memory clearing, see:
+    For a more in-depth discussion of the subject please see the
+    below posts by Colin Percival:
 
     http://www.daemonology.net/blog/2014-09-04-how-to-zero-a-buffer.html
     http://www.daemonology.net/blog/2014-09-06-zeroing-buffers-is-insufficient.html