From 74f9edef079b8682969e3cd16715ad91334127a3 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Tue, 31 Mar 2020 02:20:59 -0700 Subject: [PATCH] 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. --- src/ripple/app/misc/NetworkOPs.cpp | 6 +- src/ripple/app/tx/impl/CancelCheck.cpp | 9 +- src/ripple/app/tx/impl/CancelOffer.cpp | 6 +- src/ripple/app/tx/impl/CashCheck.cpp | 7 +- src/ripple/app/tx/impl/CreateCheck.cpp | 3 +- src/ripple/app/tx/impl/CreateOffer.cpp | 35 +-- src/ripple/app/tx/impl/DepositPreauth.cpp | 6 +- src/ripple/app/tx/impl/SetTrust.cpp | 6 +- src/ripple/crypto/secure_erase.h | 8 +- src/ripple/ledger/impl/View.cpp | 302 +++++++++--------- src/ripple/overlay/impl/ProtocolVersion.cpp | 1 - src/ripple/protocol/Indexes.h | 310 +++++++------------ src/ripple/protocol/LedgerFormats.h | 32 +- src/ripple/protocol/impl/Indexes.cpp | 327 +++++++++----------- src/ripple/rpc/handlers/LedgerEntry.cpp | 14 +- src/ripple/rpc/impl/RPCHelpers.cpp | 6 +- src/test/app/AccountDelete_test.cpp | 2 +- src/test/app/AccountTxPaging_test.cpp | 4 +- src/test/app/Check_test.cpp | 8 +- src/test/ledger/Directory_test.cpp | 2 +- src/test/ledger/Invariants_test.cpp | 26 +- src/test/rpc/AccountTx_test.cpp | 4 +- src/test/rpc/LedgerRPC_test.cpp | 4 +- 23 files changed, 477 insertions(+), 651 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 359f2cd153..29c8654502 100644 --- 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 index a074d9f87c..1b62d9f720 100644 --- 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,7 +105,8 @@ 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; @@ -114,7 +114,8 @@ CancelCheck::doApply() } { 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 index 16815707b2..a7a766537e 100644 --- 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 index 6fa5c8aeff..753ecf8a77 100644 --- 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 index ed3ad0122c..edba73c705 100644 --- 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(ltCHECK, getCheckIndex(account_, seq)); + auto sleCheck = std::make_shared(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 index b39cb41449..f0fa487cba 100644 --- 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,21 +1399,13 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) auto dir = keylet::quality(keylet::book(book), uRate); bool const bookExisted = static_cast(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) { @@ -1426,7 +1413,7 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) return {tecDIR_FULL, true}; } - auto sleOffer = std::make_shared(ltOFFER, offer_index); + auto sleOffer = std::make_shared(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 index cf85e3c598..61a5539d12 100644 --- 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_); + 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 index afacee9a6b..5c60f4ed24 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -477,10 +477,10 @@ 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( @@ -488,7 +488,7 @@ SetTrust::doApply() 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 index 6df172f0a1..d7eceab87a 100644 --- 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 diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 0ce76b4d0c..911f99b3f2 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -952,51 +952,13 @@ rippleCredit( assert(uSenderID != uReceiverID); bool const bSenderHigh = uSenderID > uReceiverID; - uint256 const uIndex = - getRippleStateIndex(uSenderID, uReceiverID, currency); - auto const sleRippleState = view.peek(keylet::line(uIndex)); - - TER terResult; + auto const index = keylet::line(uSenderID, uReceiverID, currency); assert(!isXRP(uSenderID) && uSenderID != noAccount()); assert(!isXRP(uReceiverID) && uReceiverID != noAccount()); - if (!sleRippleState) - { - STAmount const saReceiverLimit({currency, uReceiverID}); - STAmount saBalance{saAmount}; - - saBalance.setIssuer(noAccount()); - - JLOG(j.debug()) << "rippleCredit: " - "create line: " - << to_string(uSenderID) << " -> " - << to_string(uReceiverID) << " : " - << saAmount.getFullText(); - - auto const sleAccount = view.peek(keylet::account(uReceiverID)); - if (!sleAccount) - return tefINTERNAL; - - bool const noRipple = (sleAccount->getFlags() & lsfDefaultRipple) == 0; - - terResult = trustCreate( - view, - bSenderHigh, - uSenderID, - uReceiverID, - uIndex, - sleAccount, - false, - noRipple, - false, - saBalance, - saReceiverLimit, - 0, - 0, - j); - } - else + // If the line exists, modify it accordingly. + if (auto const sleRippleState = view.peek(index)) { STAmount saBalance = sleRippleState->getFieldAmount(sfBalance); @@ -1018,7 +980,8 @@ rippleCredit( std::uint32_t const uFlags(sleRippleState->getFieldU32(sfFlags)); bool bDelete = false; - // YYY Could skip this if rippling in reverse. + // FIXME This NEEDS to be cleaned up and simplified. It's impossible + // for anyone to understand. if (saBefore > beast::zero // Sender balance was positive. && saBalance <= beast::zero @@ -1066,21 +1029,49 @@ rippleCredit( if (bDelete) { - terResult = trustDelete( + return trustDelete( view, sleRippleState, bSenderHigh ? uReceiverID : uSenderID, !bSenderHigh ? uReceiverID : uSenderID, j); } - else - { - view.update(sleRippleState); - terResult = tesSUCCESS; - } + + view.update(sleRippleState); + return tesSUCCESS; } - return terResult; + STAmount const saReceiverLimit({currency, uReceiverID}); + STAmount saBalance{saAmount}; + + saBalance.setIssuer(noAccount()); + + JLOG(j.debug()) << "rippleCredit: " + "create line: " + << to_string(uSenderID) << " -> " << to_string(uReceiverID) + << " : " << saAmount.getFullText(); + + auto const sleAccount = view.peek(keylet::account(uReceiverID)); + if (!sleAccount) + return tefINTERNAL; + + bool const noRipple = (sleAccount->getFlags() & lsfDefaultRipple) == 0; + + return trustCreate( + view, + bSenderHigh, + uSenderID, + uReceiverID, + index.key, + sleAccount, + false, + noRipple, + false, + saBalance, + saReceiverLimit, + 0, + 0, + j); } // Send regardless of limits. @@ -1309,81 +1300,80 @@ issueIOU( << amount.getFullText(); bool bSenderHigh = issue.account > account; - uint256 const index = - getRippleStateIndex(issue.account, account, issue.currency); - auto state = view.peek(keylet::line(index)); - if (!state) + auto const index = keylet::line(issue.account, account, issue.currency); + + if (auto state = view.peek(index)) { - // NIKB TODO: The limit uses the receiver's account as the issuer and - // this is unnecessarily inefficient as copying which could be avoided - // is now required. Consider available options. - STAmount const limit({issue.currency, account}); - STAmount final_balance = amount; + STAmount final_balance = state->getFieldAmount(sfBalance); - final_balance.setIssuer(noAccount()); + if (bSenderHigh) + final_balance.negate(); // Put balance in sender terms. - auto const receiverAccount = view.peek(keylet::account(account)); - if (!receiverAccount) - return tefINTERNAL; + STAmount const start_balance = final_balance; - bool noRipple = (receiverAccount->getFlags() & lsfDefaultRipple) == 0; + final_balance -= amount; - return trustCreate( - view, - bSenderHigh, - issue.account, - account, - index, - receiverAccount, - false, - noRipple, - false, - final_balance, - limit, - 0, - 0, - j); - } - - STAmount final_balance = state->getFieldAmount(sfBalance); - - if (bSenderHigh) - final_balance.negate(); // Put balance in sender terms. - - STAmount const start_balance = final_balance; - - final_balance -= amount; - - auto const must_delete = updateTrustLine( - view, - state, - bSenderHigh, - issue.account, - start_balance, - final_balance, - j); - - view.creditHook(issue.account, account, amount, start_balance); - - if (bSenderHigh) - final_balance.negate(); - - // Adjust the balance on the trust line if necessary. We do this even if we - // are going to delete the line to reflect the correct balance at the time - // of deletion. - state->setFieldAmount(sfBalance, final_balance); - if (must_delete) - return trustDelete( + auto const must_delete = updateTrustLine( view, state, - bSenderHigh ? account : issue.account, - bSenderHigh ? issue.account : account, + bSenderHigh, + issue.account, + start_balance, + final_balance, j); - view.update(state); + view.creditHook(issue.account, account, amount, start_balance); - return tesSUCCESS; + if (bSenderHigh) + final_balance.negate(); + + // Adjust the balance on the trust line if necessary. We do this even if + // we are going to delete the line to reflect the correct balance at the + // time of deletion. + state->setFieldAmount(sfBalance, final_balance); + if (must_delete) + return trustDelete( + view, + state, + bSenderHigh ? account : issue.account, + bSenderHigh ? issue.account : account, + j); + + view.update(state); + + return tesSUCCESS; + } + + // NIKB TODO: The limit uses the receiver's account as the issuer and + // this is unnecessarily inefficient as copying which could be avoided + // is now required. Consider available options. + STAmount const limit({issue.currency, account}); + STAmount final_balance = amount; + + final_balance.setIssuer(noAccount()); + + auto const receiverAccount = view.peek(keylet::account(account)); + if (!receiverAccount) + return tefINTERNAL; + + bool noRipple = (receiverAccount->getFlags() & lsfDefaultRipple) == 0; + + return trustCreate( + view, + bSenderHigh, + issue.account, + account, + index.key, + receiverAccount, + false, + noRipple, + false, + final_balance, + limit, + 0, + 0, + j); } TER @@ -1406,56 +1396,54 @@ redeemIOU( << amount.getFullText(); bool bSenderHigh = account > issue.account; - uint256 const index = - getRippleStateIndex(account, issue.account, issue.currency); - auto state = view.peek(keylet::line(index)); - if (!state) + if (auto state = + view.peek(keylet::line(account, issue.account, issue.currency))) { - // In order to hold an IOU, a trust line *MUST* exist to track the - // balance. If it doesn't, then something is very wrong. Don't try - // to continue. - JLOG(j.fatal()) << "redeemIOU: " << to_string(account) - << " attempts to redeem " << amount.getFullText() - << " but no trust line exists!"; + STAmount final_balance = state->getFieldAmount(sfBalance); - return tefINTERNAL; + if (bSenderHigh) + final_balance.negate(); // Put balance in sender terms. + + STAmount const start_balance = final_balance; + + final_balance -= amount; + + auto const must_delete = updateTrustLine( + view, state, bSenderHigh, account, start_balance, final_balance, j); + + view.creditHook(account, issue.account, amount, start_balance); + + if (bSenderHigh) + final_balance.negate(); + + // Adjust the balance on the trust line if necessary. We do this even if + // we are going to delete the line to reflect the correct balance at the + // time of deletion. + state->setFieldAmount(sfBalance, final_balance); + + if (must_delete) + { + return trustDelete( + view, + state, + bSenderHigh ? issue.account : account, + bSenderHigh ? account : issue.account, + j); + } + + view.update(state); + return tesSUCCESS; } - STAmount final_balance = state->getFieldAmount(sfBalance); + // In order to hold an IOU, a trust line *MUST* exist to track the + // balance. If it doesn't, then something is very wrong. Don't try + // to continue. + JLOG(j.fatal()) << "redeemIOU: " << to_string(account) + << " attempts to redeem " << amount.getFullText() + << " but no trust line exists!"; - if (bSenderHigh) - final_balance.negate(); // Put balance in sender terms. - - STAmount const start_balance = final_balance; - - final_balance -= amount; - - auto const must_delete = updateTrustLine( - view, state, bSenderHigh, account, start_balance, final_balance, j); - - view.creditHook(account, issue.account, amount, start_balance); - - if (bSenderHigh) - final_balance.negate(); - - // Adjust the balance on the trust line if necessary. We do this even if we - // are going to delete the line to reflect the correct balance at the time - // of deletion. - state->setFieldAmount(sfBalance, final_balance); - - if (must_delete) - { - return trustDelete( - view, - state, - bSenderHigh ? issue.account : account, - bSenderHigh ? account : issue.account, - j); - } - - view.update(state); - return tesSUCCESS; + return tefINTERNAL; } TER diff --git a/src/ripple/overlay/impl/ProtocolVersion.cpp b/src/ripple/overlay/impl/ProtocolVersion.cpp index 6f6d13c8f6..cb35cd5b21 100644 --- a/src/ripple/overlay/impl/ProtocolVersion.cpp +++ b/src/ripple/overlay/impl/ProtocolVersion.cpp @@ -42,7 +42,6 @@ constexpr ProtocolVersion const supportedProtocolList[] }; // clang-format on - // This ugly construct ensures that supportedProtocolList is sorted in strictly // ascending order and doesn't contain any duplicates. // FIXME: With C++20 we can use std::is_sorted with an appropriate comparator diff --git a/src/ripple/protocol/Indexes.h b/src/ripple/protocol/Indexes.h index b8b1bef6c6..95c52805a6 100644 --- a/src/ripple/protocol/Indexes.h +++ b/src/ripple/protocol/Indexes.h @@ -32,130 +32,58 @@ namespace ripple { -// get the index of the node that holds the last 256 ledgers -uint256 -getLedgerHashIndex(); +/** Keylet computation funclets. -// Get the index of the node that holds the set of 256 ledgers that includes -// this ledger's hash (or the first ledger after it if it's not a multiple -// of 256). -uint256 -getLedgerHashIndex(std::uint32_t desiredLedgerIndex); + 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.) -// get the index of the node that holds the enabled amendments -uint256 -getLedgerAmendmentIndex(); + To enhance type safety during lookup and make the code more robust, we use + keylets, which contain not only the locator of the object but also the type + of the object being referenced. -// get the index of the node that holds the fee schedule -uint256 -getLedgerFeeIndex(); - -uint256 -getGeneratorIndex(AccountID const& uGeneratorID); - -uint256 -getBookBase(Book const& book); - -uint256 -getOfferIndex(AccountID const& account, std::uint32_t uSequence); - -uint256 -getOwnerDirIndex(AccountID const& account); - -uint256 -getDirNodeIndex(uint256 const& uDirRoot, const std::uint64_t uNodeIndex); - -uint256 -getQualityIndex(uint256 const& uBase, const std::uint64_t uNodeDir = 0); - -uint256 -getQualityNext(uint256 const& uBase); - -// VFALCO This name could be better -std::uint64_t -getQuality(uint256 const& uBase); - -uint256 -getTicketIndex(AccountID const& account, std::uint32_t uSequence); - -uint256 -getRippleStateIndex( - AccountID const& a, - AccountID const& b, - Currency const& currency); - -uint256 -getRippleStateIndex(AccountID const& a, Issue const& issue); - -uint256 -getSignerListIndex(AccountID const& account); - -uint256 -getCheckIndex(AccountID const& account, std::uint32_t uSequence); - -uint256 -getDepositPreauthIndex(AccountID const& owner, AccountID const& preauthorized); - -//------------------------------------------------------------------------------ - -/* VFALCO TODO - For each of these operators that take just the uin256 and - only attach the LedgerEntryType, we can comment out that - operator to see what breaks, and those call sites are - candidates for having the Keylet either passed in as a - parameter, or having a data member that stores the keylet. + These functions each return a type-specific keylet. */ - -/** Keylet computation funclets. */ namespace keylet { /** AccountID root */ -struct account_t -{ - explicit account_t() = default; +Keylet +account(AccountID const& id) noexcept; - Keylet - operator()(AccountID const& id) const; -}; -static account_t const account{}; - -/** The amendment table */ -struct amendments_t -{ - explicit amendments_t() = default; - - Keylet - operator()() const; -}; -static amendments_t const amendments{}; +/** The index of the amendment table */ +Keylet const& +amendments() noexcept; /** Any item that can be in an owner dir. */ Keylet -child(uint256 const& key); +child(uint256 const& key) noexcept; -/** Skip list */ -struct skip_t -{ - explicit skip_t() = default; +/** The index of the "short" skip list - Keylet - operator()() const; + The "short" skip list is a node (at a fixed index) that holds the hashes + of ledgers since the last flag ledger. It will contain, at most, 256 hashes. +*/ +Keylet const& +skip() noexcept; - Keylet - operator()(LedgerIndex ledger) const; -}; -static skip_t const skip{}; +/** The index of the long skip for a particular ledger range. -/** The ledger fees */ -struct fees_t -{ - explicit fees_t() = default; + The "long" skip list is a node that holds the hashes of (up to) 256 flag + ledgers. - // VFALCO This could maybe be constexpr - Keylet - operator()() const; -}; -static fees_t const fees{}; + It can be used to efficiently skip back to any ledger using only two hops: + the first hop gets the "long" skip list for the ledger it wants to retrieve + and uses it to get the hash of the flag ledger whose short skip list will + contain the hash of the requested ledger. +*/ +Keylet +skip(LedgerIndex ledger) noexcept; + +/** The (fixed) index of the object containing the ledger fees. */ +Keylet const& +fees() noexcept; /** The beginning of an order book */ struct book_t @@ -167,53 +95,42 @@ struct book_t }; static book_t const book{}; -/** A trust line */ -struct line_t +/** The index of a trust line for a given currency + + Note that a trustline is *shared* between two accounts (commonly referred + to as the issuer and the holder); if Alice sets up a trust line to Bob for + BTC, and Bob trusts Alice for BTC, here is only a single BTC trust line + between them. + * */ +/** @{ */ +Keylet +line( + AccountID const& id0, + AccountID const& id1, + Currency const& currency) noexcept; + +inline Keylet +line(AccountID const& id, Issue const& issue) noexcept { - explicit line_t() = default; - - Keylet - operator()( - AccountID const& id0, - AccountID const& id1, - Currency const& currency) const; - - Keylet - operator()(AccountID const& id, Issue const& issue) const; - - Keylet - operator()(uint256 const& key) const - { - return {ltRIPPLE_STATE, key}; - } -}; -static line_t const line{}; + return line(id, issue.account, issue.currency); +} +/** @} */ /** An offer from an account */ -struct offer_t +/** @{ */ +Keylet +offer(AccountID const& id, std::uint32_t seq) noexcept; + +inline Keylet +offer(uint256 const& key) noexcept { - explicit offer_t() = default; - - Keylet - operator()(AccountID const& id, std::uint32_t seq) const; - - Keylet - operator()(uint256 const& key) const - { - return {ltOFFER, key}; - } -}; -static offer_t const offer{}; + return {ltOFFER, key}; +} +/** @} */ /** The initial directory page for a specific quality */ -struct quality_t -{ - explicit quality_t() = default; - - Keylet - operator()(Keylet const& k, std::uint64_t q) const; -}; -static quality_t const quality{}; +Keylet +quality(Keylet const& k, std::uint64_t q) noexcept; /** The directory for the next lower quality */ struct next_t @@ -242,88 +159,81 @@ struct ticket_t static ticket_t const ticket{}; /** A SignerList */ -struct signers_t -{ - explicit signers_t() = default; - - Keylet - operator()(AccountID const& id) const; - - Keylet - operator()(uint256 const& key) const - { - return {ltSIGNER_LIST, key}; - } -}; -static signers_t const signers{}; +Keylet +signers(AccountID const& account) noexcept; /** A Check */ -struct check_t +/** @{ */ +Keylet +check(AccountID const& id, std::uint32_t seq) noexcept; + +inline Keylet +check(uint256 const& key) noexcept { - explicit check_t() = default; - - Keylet - operator()(AccountID const& id, std::uint32_t seq) const; - - Keylet - operator()(uint256 const& key) const - { - return {ltCHECK, key}; - } -}; -static check_t const check{}; + return {ltCHECK, key}; +} +/** @} */ /** A DepositPreauth */ -struct depositPreauth_t +/** @{ */ +Keylet +depositPreauth(AccountID const& owner, AccountID const& preauthorized) noexcept; + +inline Keylet +depositPreauth(uint256 const& key) noexcept { - explicit depositPreauth_t() = default; - - Keylet - operator()(AccountID const& owner, AccountID const& preauthorized) const; - - Keylet - operator()(uint256 const& key) const - { - return {ltDEPOSIT_PREAUTH, key}; - } -}; -static depositPreauth_t const depositPreauth{}; + return {ltDEPOSIT_PREAUTH, key}; +} +/** @} */ //------------------------------------------------------------------------------ /** Any ledger entry */ Keylet -unchecked(uint256 const& key); +unchecked(uint256 const& key) noexcept; /** The root page of an account's directory */ Keylet -ownerDir(AccountID const& id); +ownerDir(AccountID const& id) noexcept; /** A page in a directory */ /** @{ */ Keylet -page(uint256 const& root, std::uint64_t index); -Keylet -page(Keylet const& root, std::uint64_t index); -/** @} */ +page(uint256 const& root, std::uint64_t index = 0) noexcept; -// DEPRECATED inline Keylet -page(uint256 const& key) +page(Keylet const& root, std::uint64_t index = 0) noexcept { - return {ltDIR_NODE, key}; + assert(root.type == ltDIR_NODE); + return page(root.key, index); } +/** @} */ /** An escrow entry */ Keylet -escrow(AccountID const& source, std::uint32_t seq); +escrow(AccountID const& src, std::uint32_t seq) noexcept; /** A PaymentChannel */ Keylet -payChan(AccountID const& source, AccountID const& dst, std::uint32_t seq); +payChan(AccountID const& src, AccountID const& dst, std::uint32_t seq) noexcept; } // namespace keylet +// Everything below is deprecated and should be removed in favor of keylets: + +uint256 +getBookBase(Book const& book); + +uint256 +getQualityNext(uint256 const& uBase); + +// VFALCO This name could be better +std::uint64_t +getQuality(uint256 const& uBase); + +uint256 +getTicketIndex(AccountID const& account, std::uint32_t uSequence); + } // namespace ripple #endif diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index d65aee0e46..bee6a467ad 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -88,37 +88,9 @@ enum LedgerEntryType { // No longer used or supported. Left here to prevent accidental // reassignment of the ledger type. - ltNICKNAME = 'n', + ltNICKNAME [[deprecated]] = 'n', - ltNotUsed01 = 'c', -}; - -/** - @ingroup protocol -*/ -// Used as a prefix for computing ledger indexes (keys). -enum LedgerNameSpace { - spaceAccount = 'a', - spaceDirNode = 'd', - spaceGenerator = 'g', - spaceRipple = 'r', - spaceOffer = 'o', // Entry for an offer. - spaceOwnerDir = 'O', // Directory of things owned by an account. - spaceBookDir = 'B', // Directory of order books. - spaceContract = 'c', - spaceSkipList = 's', - spaceEscrow = 'u', - spaceAmendment = 'f', - spaceFee = 'e', - spaceTicket = 'T', - spaceSignerList = 'S', - spaceXRPUChannel = 'x', - spaceCheck = 'C', - spaceDepositPreauth = 'p', - - // No longer used or supported. Left here to reserve the space and - // avoid accidental reuse of the space. - spaceNickname = 'n', + ltNotUsed01 [[deprecated]] = 'c', }; /** diff --git a/src/ripple/protocol/impl/Indexes.cpp b/src/ripple/protocol/impl/Indexes.cpp index b5e8052ea0..6df08f90d2 100644 --- a/src/ripple/protocol/impl/Indexes.cpp +++ b/src/ripple/protocol/impl/Indexes.cpp @@ -20,97 +20,75 @@ #include #include #include +#include #include namespace ripple { -// get the index of the node that holds the last 256 ledgers -uint256 -getLedgerHashIndex() -{ - return sha512Half(std::uint16_t(spaceSkipList)); -} +/** Type-specific prefix for calculating ledger indices. -// Get the index of the node that holds the set of 256 ledgers that includes -// this ledger's hash (or the first ledger after it if it's not a multiple -// of 256). -uint256 -getLedgerHashIndex(std::uint32_t desiredLedgerIndex) -{ - return sha512Half( - std::uint16_t(spaceSkipList), std::uint32_t(desiredLedgerIndex >> 16)); -} -// get the index of the node that holds the enabled amendments -uint256 -getLedgerAmendmentIndex() -{ - return sha512Half(std::uint16_t(spaceAmendment)); -} + The identifier for a given object within the ledger is calculated based + on some object-specific parameters. To ensure that different types of + objects have different indices, even if they happen to use the same set + of parameters, we use "tagged hashing" by adding a type-specific prefix. -// get the index of the node that holds the fee schedule -uint256 -getLedgerFeeIndex() -{ - return sha512Half(std::uint16_t(spaceFee)); -} + @note These values are part of the protocol and *CANNOT* be arbitrarily + changed. If they were, on-ledger objects may no longer be able to + be located or addressed. -uint256 -getGeneratorIndex(AccountID const& uGeneratorID) + Additions to this list are OK, but changing existing entries to + assign them a different values should never be needed. + + Entries that are removed should be moved to the bottom of the enum + and marked as [[deprecated]] to prevent accidental reuse. +*/ +enum class LedgerNameSpace : std::uint16_t { + ACCOUNT = 'a', + DIR_NODE = 'd', + TRUST_LINE = 'r', + OFFER = 'o', + OWNER_DIR = 'O', + BOOK_DIR = 'B', + SKIP_LIST = 's', + ESCROW = 'u', + AMENDMENTS = 'f', + FEE_SETTINGS = 'e', + TICKET = 'T', + SIGNER_LIST = 'S', + XRP_PAYMENT_CHANNEL = 'x', + CHECK = 'C', + DEPOSIT_PREAUTH = 'p', + + // No longer used or supported. Left here to reserve the space + // to avoid accidental reuse. + CONTRACT [[deprecated]] = 'c', + GENERATOR [[deprecated]] = 'g', + NICKNAME [[deprecated]] = 'n', +}; + +template +static uint256 +indexHash(LedgerNameSpace space, Args const&... args) { - return sha512Half(std::uint16_t(spaceGenerator), uGeneratorID); + return sha512Half(safe_cast(space), args...); } uint256 getBookBase(Book const& book) { assert(isConsistent(book)); - // Return with quality 0. - return getQualityIndex(sha512Half( - std::uint16_t(spaceBookDir), + + auto const index = indexHash( + LedgerNameSpace::BOOK_DIR, book.in.currency, book.out.currency, book.in.account, - book.out.account)); -} + book.out.account); -uint256 -getOfferIndex(AccountID const& account, std::uint32_t uSequence) -{ - return sha512Half( - std::uint16_t(spaceOffer), account, std::uint32_t(uSequence)); -} + // Return with quality 0. + auto k = keylet::quality({ltDIR_NODE, index}, 0); -uint256 -getOwnerDirIndex(AccountID const& account) -{ - return sha512Half(std::uint16_t(spaceOwnerDir), account); -} - -uint256 -getDirNodeIndex(uint256 const& uDirRoot, const std::uint64_t uNodeIndex) -{ - if (uNodeIndex == 0) - return uDirRoot; - - return sha512Half( - std::uint16_t(spaceDirNode), uDirRoot, std::uint64_t(uNodeIndex)); -} - -uint256 -getQualityIndex(uint256 const& uBase, const std::uint64_t uNodeDir) -{ - // Indexes are stored in big endian format: they print as hex as stored. - // Most significant bytes are first. Least significant bytes represent - // adjacent entries. We place uNodeDir in the 8 right most bytes to be - // adjacent. Want uNodeDir in big endian format so ++ goes to the next - // entry for indexes. - uint256 uNode(uBase); - - // TODO(tom): there must be a better way. - // VFALCO [base_uint] This assumes a certain storage format - ((std::uint64_t*)uNode.end())[-1] = boost::endian::native_to_big(uNodeDir); - - return uNode; + return k.key; } uint256 @@ -130,51 +108,8 @@ getQuality(uint256 const& uBase) uint256 getTicketIndex(AccountID const& account, std::uint32_t uSequence) { - return sha512Half( - std::uint16_t(spaceTicket), account, std::uint32_t(uSequence)); -} - -uint256 -getRippleStateIndex( - AccountID const& a, - AccountID const& b, - Currency const& currency) -{ - if (a < b) - return sha512Half(std::uint16_t(spaceRipple), a, b, currency); - return sha512Half(std::uint16_t(spaceRipple), b, a, currency); -} - -uint256 -getRippleStateIndex(AccountID const& a, Issue const& issue) -{ - return getRippleStateIndex(a, issue.account, issue.currency); -} - -uint256 -getSignerListIndex(AccountID const& account) -{ - // We are prepared for there to be multiple SignerLists in the future, - // but we don't have them yet. In anticipation of multiple SignerLists - // We supply a 32-bit ID to locate the SignerList. Until we actually - // *have* multiple signer lists, we can default that ID to zero. - return sha512Half( - std::uint16_t(spaceSignerList), - account, - std::uint32_t(0)); // 0 == default SignerList ID. -} - -uint256 -getCheckIndex(AccountID const& account, std::uint32_t uSequence) -{ - return sha512Half( - std::uint16_t(spaceCheck), account, std::uint32_t(uSequence)); -} - -uint256 -getDepositPreauthIndex(AccountID const& owner, AccountID const& preauthorized) -{ - return sha512Half(std::uint16_t(spaceDepositPreauth), owner, preauthorized); + return indexHash( + LedgerNameSpace::TICKET, account, std::uint32_t(uSequence)); } //------------------------------------------------------------------------------ @@ -182,39 +117,49 @@ getDepositPreauthIndex(AccountID const& owner, AccountID const& preauthorized) namespace keylet { Keylet -account_t::operator()(AccountID const& id) const +account(AccountID const& id) noexcept { - return {ltACCOUNT_ROOT, sha512Half(std::uint16_t(spaceAccount), id)}; + return {ltACCOUNT_ROOT, indexHash(LedgerNameSpace::ACCOUNT, id)}; } Keylet -child(uint256 const& key) +child(uint256 const& key) noexcept { return {ltCHILD, key}; } -Keylet -skip_t::operator()() const +Keylet const& +skip() noexcept { - return {ltLEDGER_HASHES, getLedgerHashIndex()}; + static Keylet const ret{ + ltLEDGER_HASHES, indexHash(LedgerNameSpace::SKIP_LIST)}; + return ret; } Keylet -skip_t::operator()(LedgerIndex ledger) const +skip(LedgerIndex ledger) noexcept { - return {ltLEDGER_HASHES, getLedgerHashIndex(ledger)}; + return { + ltLEDGER_HASHES, + indexHash( + LedgerNameSpace::SKIP_LIST, + std::uint32_t(static_cast(ledger) >> 16))}; } -Keylet -amendments_t::operator()() const +Keylet const& +amendments() noexcept { - return {ltAMENDMENTS, getLedgerAmendmentIndex()}; + static Keylet const ret{ + ltAMENDMENTS, indexHash(LedgerNameSpace::AMENDMENTS)}; + return ret; } -Keylet -fees_t::operator()() const +Keylet const& +fees() noexcept { - return {ltFEE_SETTINGS, getLedgerFeeIndex()}; + static Keylet const ret{ + ltFEE_SETTINGS, indexHash(LedgerNameSpace::FEE_SETTINGS)}; + return ret; } Keylet @@ -224,31 +169,56 @@ book_t::operator()(Book const& b) const } Keylet -line_t::operator()( +line( AccountID const& id0, AccountID const& id1, - Currency const& currency) const + Currency const& currency) noexcept { - return {ltRIPPLE_STATE, getRippleStateIndex(id0, id1, currency)}; + // There is code in SetTrust that calls us with id0 == id1, to allow users + // to locate and delete such "weird" trustlines. If we remove that code, we + // could enable this assert: + // assert(id0 != id1); + + // A trust line is shared between two accounts; while we typically think + // of this as an "issuer" and a "holder" the relationship is actually fully + // bidirectional. + // + // So that we can generate a unique ID for a trust line, regardess of which + // side of the line we're looking at, we define a "canonical" order for the + // two accounts (smallest then largest) and hash them in that order: + auto const accounts = std::minmax(id0, id1); + + return { + ltRIPPLE_STATE, + indexHash( + LedgerNameSpace::TRUST_LINE, + accounts.first, + accounts.second, + currency)}; } Keylet -line_t::operator()(AccountID const& id, Issue const& issue) const +offer(AccountID const& id, std::uint32_t seq) noexcept { - return {ltRIPPLE_STATE, getRippleStateIndex(id, issue)}; + return {ltOFFER, indexHash(LedgerNameSpace::OFFER, id, seq)}; } Keylet -offer_t::operator()(AccountID const& id, std::uint32_t seq) const -{ - return {ltOFFER, getOfferIndex(id, seq)}; -} - -Keylet -quality_t::operator()(Keylet const& k, std::uint64_t q) const +quality(Keylet const& k, std::uint64_t q) noexcept { assert(k.type == ltDIR_NODE); - return {ltDIR_NODE, getQualityIndex(k.key, q)}; + + // Indexes are stored in big endian format: they print as hex as stored. + // Most significant bytes are first and the least significant bytes + // represent adjacent entries. We place the quality, in big endian format, + // in the 8 right most bytes; this way, incrementing goes to the next entry + // for indexes. + uint256 x = k.key; + + // FIXME This is ugly and we can and should do better... + ((std::uint64_t*)x.end())[-1] = boost::endian::native_to_big(q); + + return {ltDIR_NODE, x}; } Keylet @@ -264,74 +234,71 @@ ticket_t::operator()(AccountID const& id, std::uint32_t seq) const return {ltTICKET, getTicketIndex(id, seq)}; } -Keylet -signers_t::operator()(AccountID const& id) const +// This function is presently static, since it's never accessed from anywhere +// else. If we ever support multiple pages of signer lists, this would be the +// keylet used to locate them. +static Keylet +signers(AccountID const& account, std::uint32_t page) noexcept { - return {ltSIGNER_LIST, getSignerListIndex(id)}; + return { + ltSIGNER_LIST, indexHash(LedgerNameSpace::SIGNER_LIST, account, page)}; } Keylet -check_t::operator()(AccountID const& id, std::uint32_t seq) const +signers(AccountID const& account) noexcept { - return {ltCHECK, getCheckIndex(id, seq)}; + return signers(account, 0); } Keylet -depositPreauth_t::operator()( - AccountID const& owner, - AccountID const& preauthorized) const +check(AccountID const& id, std::uint32_t seq) noexcept { - return {ltDEPOSIT_PREAUTH, getDepositPreauthIndex(owner, preauthorized)}; + return {ltCHECK, indexHash(LedgerNameSpace::CHECK, id, seq)}; +} + +Keylet +depositPreauth(AccountID const& owner, AccountID const& preauthorized) noexcept +{ + return { + ltDEPOSIT_PREAUTH, + indexHash(LedgerNameSpace::DEPOSIT_PREAUTH, owner, preauthorized)}; } //------------------------------------------------------------------------------ Keylet -unchecked(uint256 const& key) +unchecked(uint256 const& key) noexcept { return {ltANY, key}; } Keylet -ownerDir(AccountID const& id) +ownerDir(AccountID const& id) noexcept { - return {ltDIR_NODE, getOwnerDirIndex(id)}; + return {ltDIR_NODE, indexHash(LedgerNameSpace::OWNER_DIR, id)}; } Keylet -page(uint256 const& key, std::uint64_t index) +page(uint256 const& key, std::uint64_t index) noexcept { - return {ltDIR_NODE, getDirNodeIndex(key, index)}; + if (index == 0) + return {ltDIR_NODE, key}; + + return {ltDIR_NODE, indexHash(LedgerNameSpace::DIR_NODE, key, index)}; } Keylet -page(Keylet const& root, std::uint64_t index) +escrow(AccountID const& src, std::uint32_t seq) noexcept { - assert(root.type == ltDIR_NODE); - return page(root.key, index); + return {ltESCROW, indexHash(LedgerNameSpace::ESCROW, src, seq)}; } Keylet -escrow(AccountID const& source, std::uint32_t seq) +payChan(AccountID const& src, AccountID const& dst, std::uint32_t seq) noexcept { - sha512_half_hasher h; - using beast::hash_append; - hash_append(h, std::uint16_t(spaceEscrow)); - hash_append(h, source); - hash_append(h, seq); - return {ltESCROW, static_cast(h)}; -} - -Keylet -payChan(AccountID const& source, AccountID const& dst, std::uint32_t seq) -{ - sha512_half_hasher h; - using beast::hash_append; - hash_append(h, std::uint16_t(spaceXRPUChannel)); - hash_append(h, source); - hash_append(h, dst); - hash_append(h, seq); - return {ltPAYCHAN, static_cast(h)}; + return { + ltPAYCHAN, + indexHash(LedgerNameSpace::XRP_PAYMENT_CHANNEL, src, dst, seq)}; } } // namespace keylet diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index 95f23a4be7..a521d2724c 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -144,7 +144,7 @@ doLedgerEntry(RPC::JsonContext& context) context.params[jss::directory][jss::dir_root] .asString()); - uNodeIndex = getDirNodeIndex(uDirRoot, uSubIndex); + uNodeIndex = keylet::page(uDirRoot, uSubIndex).key; } } else if (context.params[jss::directory].isMember(jss::owner)) @@ -158,8 +158,8 @@ doLedgerEntry(RPC::JsonContext& context) } else { - uint256 uDirRoot = getOwnerDirIndex(*ownerID); - uNodeIndex = getDirNodeIndex(uDirRoot, uSubIndex); + uNodeIndex = + keylet::page(keylet::ownerDir(*ownerID), uSubIndex).key; } } else @@ -216,8 +216,10 @@ doLedgerEntry(RPC::JsonContext& context) if (!id) jvResult[jss::error] = "malformedAddress"; else - uNodeIndex = getOfferIndex( - *id, context.params[jss::offer][jss::seq].asUInt()); + uNodeIndex = + keylet::offer( + *id, context.params[jss::offer][jss::seq].asUInt()) + .key; } } else if (context.params.isMember(jss::payment_channel)) @@ -260,7 +262,7 @@ doLedgerEntry(RPC::JsonContext& context) } else { - uNodeIndex = getRippleStateIndex(*id1, *id2, uCurrency); + uNodeIndex = keylet::line(*id1, *id2, uCurrency).key; } } } diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 5eb999ba5c..1104caa59f 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -101,12 +101,12 @@ getAccountObjects( std::uint32_t const limit, Json::Value& jvResult) { - auto const rootDirIndex = getOwnerDirIndex(account); + auto const root = keylet::ownerDir(account); auto found = false; if (dirIndex.isZero()) { - dirIndex = rootDirIndex; + dirIndex = root.key; found = true; } @@ -166,7 +166,7 @@ getAccountObjects( if (nodeIndex == 0) return true; - dirIndex = getDirNodeIndex(rootDirIndex, nodeIndex); + dirIndex = keylet::page(root, nodeIndex).key; dir = ledger.read({ltDIR_NODE, dirIndex}); if (!dir) return true; diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 54534f3f0d..5e571fce6b 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -317,7 +317,7 @@ public: // alice writes a check to becky. Until that check is cashed or // canceled it will prevent alice's and becky's accounts from being // deleted. - uint256 const checkId{getCheckIndex(alice, env.seq(alice))}; + uint256 const checkId = keylet::check(alice, env.seq(alice)).key; env(check::create(alice, becky, XRP(1))); env.close(); diff --git a/src/test/app/AccountTxPaging_test.cpp b/src/test/app/AccountTxPaging_test.cpp index 76ea1f7545..ec2c66809d 100644 --- a/src/test/app/AccountTxPaging_test.cpp +++ b/src/test/app/AccountTxPaging_test.cpp @@ -710,11 +710,11 @@ class AccountTxPaging_test : public beast::unit_test::suite // Check { - uint256 const aliceCheckId{getCheckIndex(alice, env.seq(alice))}; + auto const aliceCheckId = keylet::check(alice, env.seq(alice)).key; env(check::create(alice, gw, XRP(300)), sig(alie)); auto txn = env.tx(); - uint256 const gwCheckId{getCheckIndex(gw, env.seq(gw))}; + auto const gwCheckId = keylet::check(gw, env.seq(gw)).key; env(check::create(gw, alice, XRP(200))); env.close(); diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index dcd516777c..b555c7d3a7 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -85,6 +85,12 @@ public: class Check_test : public beast::unit_test::suite { + static uint256 + getCheckIndex(AccountID const& account, std::uint32_t uSequence) + { + return keylet::check(account, uSequence).key; + } + // Helper function that returns the Checks on an account. static std::vector> checksOnAccount(test::jtx::Env& env, test::jtx::Account account) @@ -1064,7 +1070,7 @@ class Check_test : public beast::unit_test::suite env(trust(truster, iou(1000)), inOrOut(pct)); env.close(); - uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; + uint256 const chkId = getCheckIndex(alice, env.seq(alice)); env(check::create(alice, bob, USD(10))); env.close(); diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index ed10224983..39d319ad9b 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -83,7 +83,7 @@ struct Directory_test : public beast::unit_test::suite auto alice = Account("alice"); auto bob = Account("bob"); - testcase("Directory Ordering"); + testcase("Directory Ordering (with 'SortedDirectories' amendment)"); Env env(*this); env.fund(XRP(10000000), alice, gw); diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp index 53b35e7717..4b0271e7ef 100644 --- a/src/test/ledger/Invariants_test.cpp +++ b/src/test/ledger/Invariants_test.cpp @@ -187,12 +187,16 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; + // make a dummy escrow ledger entry, then change the type to an // unsupported value so that the valid type invariant check // will fail. auto sleNew = std::make_shared( keylet::escrow(A1, (*sle)[sfSequence] + 2)); - sleNew->type_ = ltNICKNAME; + + // We don't use ltNICKNAME directly since it's marked deprecated + // to prevent accidental use elsewhere. + sleNew->type_ = static_cast('n'); ac.view().insert(sleNew); return true; }); @@ -207,9 +211,8 @@ class Invariants_test : public beast::unit_test::suite {{"an XRP trust line was created"}}, [](Account const& A1, Account const& A2, ApplyContext& ac) { // create simple trust SLE with xrp currency - auto index = getRippleStateIndex(A1, A2, xrpIssue().currency); - auto const sleNew = - std::make_shared(ltRIPPLE_STATE, index); + auto const sleNew = std::make_shared( + keylet::line(A1, A2, xrpIssue().currency)); ac.view().insert(sleNew); return true; }); @@ -308,9 +311,8 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; - auto const offer_index = - getOfferIndex(A1.id(), (*sle)[sfSequence]); - auto sleNew = std::make_shared(ltOFFER, offer_index); + auto sleNew = std::make_shared( + keylet::offer(A1.id(), (*sle)[sfSequence])); sleNew->setAccountID(sfAccount, A1.id()); sleNew->setFieldU32(sfSequence, (*sle)[sfSequence]); sleNew->setFieldAmount(sfTakerPays, XRP(-1)); @@ -325,9 +327,8 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; - auto const offer_index = - getOfferIndex(A1.id(), (*sle)[sfSequence]); - auto sleNew = std::make_shared(ltOFFER, offer_index); + auto sleNew = std::make_shared( + keylet::offer(A1.id(), (*sle)[sfSequence])); sleNew->setAccountID(sfAccount, A1.id()); sleNew->setFieldU32(sfSequence, (*sle)[sfSequence]); sleNew->setFieldAmount(sfTakerPays, A1["USD"](10)); @@ -343,9 +344,8 @@ class Invariants_test : public beast::unit_test::suite auto const sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; - auto const offer_index = - getOfferIndex(A1.id(), (*sle)[sfSequence]); - auto sleNew = std::make_shared(ltOFFER, offer_index); + auto sleNew = std::make_shared( + keylet::offer(A1.id(), (*sle)[sfSequence])); sleNew->setAccountID(sfAccount, A1.id()); sleNew->setFieldU32(sfSequence, (*sle)[sfSequence]); sleNew->setFieldAmount(sfTakerPays, XRP(10)); diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 1655391dbe..1ef9daffb7 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -385,10 +385,10 @@ class AccountTx_test : public beast::unit_test::suite // Check { - uint256 const aliceCheckId{getCheckIndex(alice, env.seq(alice))}; + auto const aliceCheckId = keylet::check(alice, env.seq(alice)).key; env(check::create(alice, gw, XRP(300)), sig(alie)); - uint256 const gwCheckId{getCheckIndex(gw, env.seq(gw))}; + auto const gwCheckId = keylet::check(gw, env.seq(gw)).key; env(check::create(gw, alice, XRP(200))); env.close(); diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index b0d84e429e..ee007e32aa 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -353,7 +353,7 @@ class LedgerRPC_test : public beast::unit_test::suite env.fund(XRP(10000), alice); env.close(); - uint256 const checkId{getCheckIndex(env.master, env.seq(env.master))}; + auto const checkId = keylet::check(env.master, env.seq(env.master)); env(check::create(env.master, alice, XRP(100))); env.close(); @@ -362,7 +362,7 @@ class LedgerRPC_test : public beast::unit_test::suite { // Request a check. Json::Value jvParams; - jvParams[jss::check] = to_string(checkId); + jvParams[jss::check] = to_string(checkId.key); jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc( "json", "ledger_entry", to_string(jvParams))[jss::result];