From 0122cd4a50a06221c84c5872ca549f20aae29383 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Thu, 1 Jun 2023 12:20:03 -0700 Subject: [PATCH] Ledger Object wrapper for Checks This commit includes type-aliases for read-only and writable checks. It also formats all files as per git-clang-format convention --- src/ripple/app/tx/impl/CancelCheck.cpp | 29 ++++--- src/ripple/app/tx/impl/CancelCheck.h | 1 + src/ripple/app/tx/impl/CreateCheck.cpp | 2 +- src/ripple/ledger/ApplyView.h | 18 ++-- src/ripple/protocol/AcctRoot.h | 69 ++++++++++----- src/ripple/protocol/Checks.h | 104 +++++++++++++++++++++++ src/ripple/protocol/Indexes.h | 6 +- src/ripple/protocol/Keylet.h | 15 ++++ src/ripple/protocol/LedgerEntryWrapper.h | 30 ++++--- src/ripple/protocol/impl/Indexes.cpp | 4 +- 10 files changed, 214 insertions(+), 64 deletions(-) create mode 100644 src/ripple/protocol/Checks.h diff --git a/src/ripple/app/tx/impl/CancelCheck.cpp b/src/ripple/app/tx/impl/CancelCheck.cpp index 55caf0381f0..6786d04693c 100644 --- a/src/ripple/app/tx/impl/CancelCheck.cpp +++ b/src/ripple/app/tx/impl/CancelCheck.cpp @@ -53,8 +53,8 @@ CancelCheck::preflight(PreflightContext const& ctx) TER CancelCheck::preclaim(PreclaimContext const& ctx) { - auto const sleCheck = ctx.view.readSLE(keylet::check(ctx.tx[sfCheckID])); - if (!sleCheck) + auto const check = ctx.view.read(keylet::check(ctx.tx[sfCheckID])); + if (!check) { JLOG(ctx.j.warn()) << "Check does not exist."; return tecNO_ENTRY; @@ -62,7 +62,7 @@ CancelCheck::preclaim(PreclaimContext const& ctx) using duration = NetClock::duration; using timepoint = NetClock::time_point; - auto const optExpiry = (*sleCheck)[~sfExpiration]; + auto const optExpiry = check->getCheckExpiration(); // Expiration is defined in terms of the close time of the parent // ledger, because we definitively know the time that it closed but @@ -74,8 +74,8 @@ CancelCheck::preclaim(PreclaimContext const& ctx) // If the check is not yet expired, then only the creator or the // destination may cancel the check. AccountID const acctId{ctx.tx[sfAccount]}; - if (acctId != (*sleCheck)[sfAccount] && - acctId != (*sleCheck)[sfDestination]) + if (acctId != check->getCheckCreator() && + acctId != check->getCheckRecipient()) { JLOG(ctx.j.warn()) << "Check is not expired and canceler is " "neither check source nor destination."; @@ -88,34 +88,35 @@ CancelCheck::preclaim(PreclaimContext const& ctx) TER CancelCheck::doApply() { - auto const sleCheck = view().peekSLE(keylet::check(ctx_.tx[sfCheckID])); - if (!sleCheck) + std::optional check = + view().peek(keylet::check(ctx_.tx[sfCheckID])); + if (!check) { // Error should have been caught in preclaim. JLOG(j_.warn()) << "Check does not exist."; return tecNO_ENTRY; } - AccountID const srcId{sleCheck->getAccountID(sfAccount)}; - AccountID const dstId{sleCheck->getAccountID(sfDestination)}; + AccountID const srcId{check->getCheckCreator()}; + AccountID const dstId{check->getCheckRecipient()}; auto viewJ = ctx_.app.journal("View"); // If the check is not written to self (and it shouldn't be), remove the // check from the destination account root. if (srcId != dstId) { - std::uint64_t const page{(*sleCheck)[sfDestinationNode]}; + std::uint64_t const page{check->getDestinationNode()}; if (!view().dirRemove( - keylet::ownerDir(dstId), page, sleCheck->key(), true)) + keylet::ownerDir(dstId), page, check->key(), true)) { JLOG(j_.fatal()) << "Unable to delete check from destination."; return tefBAD_LEDGER; } } { - std::uint64_t const page{(*sleCheck)[sfOwnerNode]}; + std::uint64_t const page{check->getOwnerNode()}; if (!view().dirRemove( - keylet::ownerDir(srcId), page, sleCheck->key(), true)) + keylet::ownerDir(srcId), page, check->key(), true)) { JLOG(j_.fatal()) << "Unable to delete check from owner."; return tefBAD_LEDGER; @@ -127,7 +128,7 @@ CancelCheck::doApply() adjustOwnerCount(view(), *srcAcctRoot, -1, viewJ); // Remove check from ledger. - view().erase(sleCheck); + view().erase(*check); return tesSUCCESS; } diff --git a/src/ripple/app/tx/impl/CancelCheck.h b/src/ripple/app/tx/impl/CancelCheck.h index a7e0f6e5d93..3d221333f1b 100644 --- a/src/ripple/app/tx/impl/CancelCheck.h +++ b/src/ripple/app/tx/impl/CancelCheck.h @@ -21,6 +21,7 @@ #define RIPPLE_TX_CANCELCHECK_H_INCLUDED #include +#include namespace ripple { diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index 72fe7a00db2..802ec04fe0b 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -181,7 +181,7 @@ CreateCheck::doApply() // Note that we use the value from the sequence or ticket as the // Check sequence. For more explanation see comments in SeqProxy.h. std::uint32_t const seq = ctx_.tx.getSeqProxy().value(); - Keylet const checkKeylet = keylet::check(account_, seq); + ChecksKeylet const checkKeylet = keylet::check(account_, seq); auto sleCheck = std::make_shared(checkKeylet); sleCheck->setAccountID(sfAccount, account_); diff --git a/src/ripple/ledger/ApplyView.h b/src/ripple/ledger/ApplyView.h index f7578411a23..5047b4297dc 100644 --- a/src/ripple/ledger/ApplyView.h +++ b/src/ripple/ledger/ApplyView.h @@ -201,9 +201,11 @@ class ApplyView : public ReadView erase(std::shared_ptr const& sle) = 0; template - requires(std::is_convertible_v< - decltype(std::declval().slePtr()), - std::shared_ptr const&>) void erase(T& wrapper) + requires(std::is_convertible_v< + decltype(std::declval().slePtr()), + std::shared_ptr const&>) + void + erase(T& wrapper) { erase(wrapper.slePtr()); } @@ -249,9 +251,11 @@ class ApplyView : public ReadView update(std::shared_ptr const& sle) = 0; template - requires(std::is_convertible_v< - decltype(std::declval().slePtr()), - std::shared_ptr const&>) void update(T& wrapper) + requires(std::is_convertible_v< + decltype(std::declval().slePtr()), + std::shared_ptr const&>) + void + update(T& wrapper) { update(wrapper.slePtr()); } @@ -343,7 +347,7 @@ class ApplyView : public ReadView std::optional dirInsert( Keylet const& directory, - Keylet const& key, + KeyletBase const& key, std::function const&)> const& describe) { return dirAdd(false, directory, key.key, describe); diff --git a/src/ripple/protocol/AcctRoot.h b/src/ripple/protocol/AcctRoot.h index c9e27fd4b3e..a4688d9c5ab 100644 --- a/src/ripple/protocol/AcctRoot.h +++ b/src/ripple/protocol/AcctRoot.h @@ -69,7 +69,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setSequence(std::uint32_t seq) requires Writable + setSequence(std::uint32_t seq) + requires Writable { wrapped_->at(sfSequence) = seq; } @@ -81,7 +82,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setBalance(STAmount const& amount) requires Writable + setBalance(STAmount const& amount) + requires Writable { wrapped_->at(sfBalance) = amount; } @@ -93,7 +95,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setOwnerCount(std::uint32_t newCount) requires Writable + setOwnerCount(std::uint32_t newCount) + requires Writable { wrapped_->at(sfOwnerCount) = newCount; } @@ -105,7 +108,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setPreviousTxnID(uint256 prevTxID) requires Writable + setPreviousTxnID(uint256 prevTxID) + requires Writable { wrapped_->at(sfPreviousTxnID) = prevTxID; } @@ -117,7 +121,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setPreviousTxnLgrSeq(std::uint32_t prevTxLgrSeq) requires Writable + setPreviousTxnLgrSeq(std::uint32_t prevTxLgrSeq) + requires Writable { wrapped_->at(sfPreviousTxnLgrSeq) = prevTxLgrSeq; } @@ -129,13 +134,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setAccountTxnID(uint256 const& newAcctTxnID) requires Writable + setAccountTxnID(uint256 const& newAcctTxnID) + requires Writable { this->setOptional(sfAccountTxnID, newAcctTxnID); } void - clearAccountTxnID() requires Writable + clearAccountTxnID() + requires Writable { this->clearOptional(sfAccountTxnID); } @@ -147,13 +154,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setRegularKey(AccountID const& newRegKey) requires Writable + setRegularKey(AccountID const& newRegKey) + requires Writable { this->setOptional(sfRegularKey, newRegKey); } void - clearRegularKey() requires Writable + clearRegularKey() + requires Writable { this->clearOptional(sfRegularKey); } @@ -165,7 +174,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setEmailHash(uint128 const& newEmailHash) requires Writable + setEmailHash(uint128 const& newEmailHash) + requires Writable { this->setOrClearBaseUintIfZero(sfEmailHash, newEmailHash); } @@ -177,7 +187,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setWalletLocator(uint256 const& newWalletLocator) requires Writable + setWalletLocator(uint256 const& newWalletLocator) + requires Writable { this->setOrClearBaseUintIfZero(sfWalletLocator, newWalletLocator); } @@ -195,7 +206,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setMessageKey(Blob const& newMessageKey) requires Writable + setMessageKey(Blob const& newMessageKey) + requires Writable { this->setOrClearVLIfEmpty(sfMessageKey, newMessageKey); } @@ -207,13 +219,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setTransferRate(std::uint32_t newTransferRate) requires Writable + setTransferRate(std::uint32_t newTransferRate) + requires Writable { this->setOptional(sfTransferRate, newTransferRate); } void - clearTransferRate() requires Writable + clearTransferRate() + requires Writable { this->clearOptional(sfTransferRate); } @@ -225,7 +239,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setDomain(Blob const& newDomain) requires Writable + setDomain(Blob const& newDomain) + requires Writable { this->setOrClearVLIfEmpty(sfDomain, newDomain); } @@ -237,13 +252,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setTickSize(std::uint8_t newTickSize) requires Writable + setTickSize(std::uint8_t newTickSize) + requires Writable { this->setOptional(sfTickSize, newTickSize); } void - clearTickSize() requires Writable + clearTickSize() + requires Writable { this->clearOptional(sfTickSize); } @@ -255,13 +272,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setTicketCount(std::uint32_t newTicketCount) requires Writable + setTicketCount(std::uint32_t newTicketCount) + requires Writable { this->setOptional(sfTicketCount, newTicketCount); } void - clearTicketCount() requires Writable + clearTicketCount() + requires Writable { this->clearOptional(sfTicketCount); } @@ -273,13 +292,15 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setNFTokenMinter(AccountID const& newMinter) requires Writable + setNFTokenMinter(AccountID const& newMinter) + requires Writable { this->setOptional(sfNFTokenMinter, newMinter); } void - clearNFTokenMinter() requires Writable + clearNFTokenMinter() + requires Writable { this->clearOptional(sfNFTokenMinter); } @@ -291,7 +312,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setMintedNFTokens(std::uint32_t newMintedCount) requires Writable + setMintedNFTokens(std::uint32_t newMintedCount) + requires Writable { this->setOptional(sfMintedNFTokens, newMintedCount); } @@ -303,7 +325,8 @@ class AcctRootImpl final : public LedgerEntryWrapper } void - setBurnedNFTokens(std::uint32_t newBurnedCount) requires Writable + setBurnedNFTokens(std::uint32_t newBurnedCount) + requires Writable { this->setOptional(sfBurnedNFTokens, newBurnedCount); } diff --git a/src/ripple/protocol/Checks.h b/src/ripple/protocol/Checks.h new file mode 100644 index 00000000000..ba165eb105e --- /dev/null +++ b/src/ripple/protocol/Checks.h @@ -0,0 +1,104 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_PROTOCOL_CHECKS_H_INCLUDED +#define RIPPLE_PROTOCOL_CHECKS_H_INCLUDED + +#include +#include +#include + +namespace ripple { + +template +class ChecksImpl final : public LedgerEntryWrapper +{ +private: + using Base = LedgerEntryWrapper; + using SleT = typename Base::SleT; + using Base::wrapped_; + + ChecksImpl(std::shared_ptr&& w) : Base(std::move(w)) + { + } + + // Friend declarations of factory functions. + // + // For classes that contain factories we must declare the entire class + // as a friend unless the class declaration is visible at this point. + friend class ReadView; + friend class ApplyView; + +public: + // Conversion operator from ChecksImpl to ChecksImpl. + operator ChecksImpl() const + { + return ChecksImpl( + std::const_pointer_cast>( + wrapped_)); + } + + uint256 + key() const + { + return wrapped_->key(); + } + + auto + getCheckExpiration() const + { + return wrapped_->at(~sfExpiration); + } + + // Keshava: This function is identical to the function + // STObject::getAccountID ? + // should I piggy-back on that function instead? + AccountID + getCheckCreator() const + { + return wrapped_->at(sfAccount); + } + + AccountID + getCheckRecipient() const + { + return wrapped_->at(sfDestination); + } + + // Keshava: The below two functions refer to a page in a directory of an + // account. What is an appropriate name? These function names are cryptic + // :(( + std::uint64_t + getDestinationNode() const + { + return wrapped_->at(sfDestinationNode); + } + + std::uint64_t + getOwnerNode() const + { + return wrapped_->at(sfOwnerNode); + } +}; + +using Checks = ChecksImpl; +using ChecksRd = ChecksImpl; +} // namespace ripple + +#endif // RIPPLE_PROTOCOL_CHECKS_H_INCLUDED diff --git a/src/ripple/protocol/Indexes.h b/src/ripple/protocol/Indexes.h index e7170eff7cc..38bac928cf5 100644 --- a/src/ripple/protocol/Indexes.h +++ b/src/ripple/protocol/Indexes.h @@ -172,13 +172,13 @@ signers(AccountID const& account) noexcept; /** A Check */ /** @{ */ -Keylet +ChecksKeylet check(AccountID const& id, std::uint32_t seq) noexcept; -inline Keylet +inline ChecksKeylet check(uint256 const& key) noexcept { - return {ltCHECK, key}; + return ChecksKeylet(key); } /** @} */ diff --git a/src/ripple/protocol/Keylet.h b/src/ripple/protocol/Keylet.h index c34b484a150..05c4cb85ae9 100644 --- a/src/ripple/protocol/Keylet.h +++ b/src/ripple/protocol/Keylet.h @@ -95,6 +95,21 @@ static_assert(std::is_move_assignable_v); static_assert(std::is_nothrow_destructible_v); #endif +template +class ChecksImpl; + +struct ChecksKeylet final : public KeyletBase +{ + template + using TWrapped = ChecksImpl; + + using KeyletBase::check; + + explicit ChecksKeylet(uint256 const& key) : KeyletBase(ltCHECK, key) + { + } +}; + template class AcctRootImpl; diff --git a/src/ripple/protocol/LedgerEntryWrapper.h b/src/ripple/protocol/LedgerEntryWrapper.h index fa082a62d4a..0f60740cc67 100644 --- a/src/ripple/protocol/LedgerEntryWrapper.h +++ b/src/ripple/protocol/LedgerEntryWrapper.h @@ -66,9 +66,8 @@ class LedgerEntryWrapper // Helper functions that are useful to some derived classes. template void - setOptional( - SF const& field, - T const& value) requires Writable&& std::is_base_of_v + setOptional(SF const& field, T const& value) + requires Writable && std::is_base_of_v { if (!wrapped_->isFieldPresent(field)) wrapped_->makeFieldPresent(field); @@ -78,8 +77,8 @@ class LedgerEntryWrapper template void - clearOptional( - SF const& field) requires Writable&& std::is_base_of_v + clearOptional(SF const& field) + requires Writable && std::is_base_of_v { if (wrapped_->isFieldPresent(field)) wrapped_->makeFieldAbsent(field); @@ -96,10 +95,8 @@ class LedgerEntryWrapper template void - setOrClearBaseUintIfZero( - SF const& field, - base_uint const& - value) requires Writable&& std::is_base_of_v + setOrClearBaseUintIfZero(SF const& field, base_uint const& value) + requires Writable && std::is_base_of_v { if (value.signum() == 0) return clearOptional(field); @@ -111,7 +108,8 @@ class LedgerEntryWrapper } void - setOrClearVLIfEmpty(SF_VL const& field, Blob const& value) requires Writable + setOrClearVLIfEmpty(SF_VL const& field, Blob const& value) + requires Writable { if (value.empty()) return clearOptional(field); @@ -137,7 +135,8 @@ class LedgerEntryWrapper } [[nodiscard]] std::shared_ptr const& - slePtr() requires Writable + slePtr() + requires Writable { return wrapped_; } @@ -155,19 +154,22 @@ class LedgerEntryWrapper } void - replaceAllFlags(std::uint32_t newFlags) requires Writable + replaceAllFlags(std::uint32_t newFlags) + requires Writable { wrapped_->at(sfFlags) = newFlags; } void - setFlag(std::uint32_t flagsToSet) requires Writable + setFlag(std::uint32_t flagsToSet) + requires Writable { replaceAllFlags(flags() | flagsToSet); } void - clearFlag(std::uint32_t flagsToClear) requires Writable + clearFlag(std::uint32_t flagsToClear) + requires Writable { replaceAllFlags(flags() & ~flagsToClear); } diff --git a/src/ripple/protocol/impl/Indexes.cpp b/src/ripple/protocol/impl/Indexes.cpp index 2cc80ff41e6..2a40c9e65ba 100644 --- a/src/ripple/protocol/impl/Indexes.cpp +++ b/src/ripple/protocol/impl/Indexes.cpp @@ -277,10 +277,10 @@ signers(AccountID const& account) noexcept return signers(account, 0); } -Keylet +ChecksKeylet check(AccountID const& id, std::uint32_t seq) noexcept { - return {ltCHECK, indexHash(LedgerNameSpace::CHECK, id, seq)}; + return ChecksKeylet(indexHash(LedgerNameSpace::CHECK, id, seq)); } Keylet