Skip to content

Commit

Permalink
Refactor get<T>(). Minor refactoring to be consistent with develop.
Browse files Browse the repository at this point in the history
  • Loading branch information
gregtatcam committed Jul 22, 2024
1 parent 5d7e181 commit 5e9f164
Show file tree
Hide file tree
Showing 14 changed files with 159 additions and 31 deletions.
3 changes: 2 additions & 1 deletion include/xrpl/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 78;
static constexpr std::size_t numFeatures = 79;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -370,6 +370,7 @@ extern uint256 const featureNFTokenMintOffer;
extern uint256 const fixReducedOffersV2;
extern uint256 const fixEnforceNFTokenTrustline;
extern uint256 const fixInnerObjTemplate2;
extern uint256 const featureInvariantsV1_1;
extern uint256 const featureMPTokensV1;

} // namespace ripple
Expand Down
21 changes: 21 additions & 0 deletions include/xrpl/protocol/Indexes.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <xrpl/protocol/STXChainBridge.h>
#include <xrpl/protocol/Serializer.h>
#include <xrpl/protocol/UintTypes.h>
#include <xrpl/protocol/jss.h>
#include <cstdint>

namespace ripple {
Expand Down Expand Up @@ -336,6 +337,26 @@ getTicketIndex(AccountID const& account, std::uint32_t uSequence);
uint256
getTicketIndex(AccountID const& account, SeqProxy ticketSeq);

template <class... keyletParams>
struct keyletDesc
{
std::function<Keylet(keyletParams...)> function;
Json::StaticString expectedLEName;
bool includeInTests;
};

// This list should include all of the keylet functions that take a single
// AccountID parameter.
std::array<keyletDesc<AccountID const&>, 6> const directAccountKeylets{
{{&keylet::account, jss::AccountRoot, false},
{&keylet::ownerDir, jss::DirectoryNode, true},
{&keylet::signers, jss::SignerList, true},
// It's normally impossible to create an item at nftpage_min, but
// test it anyway, since the invariant checks for it.
{&keylet::nftpage_min, jss::NFTokenPage, true},
{&keylet::nftpage_max, jss::NFTokenPage, true},
{&keylet::did, jss::DID, true}}};

uint192
getMptID(AccountID const& account, std::uint32_t sequence);

Expand Down
11 changes: 3 additions & 8 deletions include/xrpl/protocol/STEitherAmount.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,7 @@ get(auto&& amount)
if constexpr (std::is_lvalue_reference_v<decltype(amount)>)
return amount.template get<T>();
else
{
T a = amount.template get<T>();
return a;
}
return std::remove_reference_t<T>(amount.template get<T>());
}
else if constexpr (std::is_same_v<TAmnt, std::optional<STEitherAmount>>)
{
Expand All @@ -179,10 +176,8 @@ get(auto&& amount)
if constexpr (std::is_lvalue_reference_v<decltype(amount)>)
return amount.operator STEitherAmount().template get<T>();
else
{
T a = amount.operator STEitherAmount().template get<T>();
return a;
}
return std::remove_reference_t<T>(
amount.operator STEitherAmount().template get<T>());
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion include/xrpl/protocol/TxMeta.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class TxMeta
}

void
setDeliveredAmount(STAmount const& delivered)
setDeliveredAmount(STEitherAmount const& delivered)
{
mDelivered = delivered;
}
Expand Down
6 changes: 0 additions & 6 deletions include/xrpl/protocol/UintTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ class NodeIDTag
explicit NodeIDTag() = default;
};

class MPTTag
{
public:
explicit MPTTag() = default;
};

} // namespace detail

/** Directory is an index into the directory of offer books.
Expand Down
3 changes: 3 additions & 0 deletions src/libxrpl/protocol/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,9 @@ REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::De
REGISTER_FIX (fixReducedOffersV2, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixEnforceNFTokenTrustline, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixInnerObjTemplate2, Supported::yes, VoteBehavior::DefaultNo);
// InvariantsV1_1 will be changes to Supported::yes when all the
// invariants expected to be included under it are complete.
REGISTER_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo);
REGISTER_FEATURE(MPTokensV1, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
Expand Down
6 changes: 2 additions & 4 deletions src/libxrpl/protocol/TxMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ TxMeta::TxMeta(
mNodes = *dynamic_cast<STArray*>(&obj.getField(sfAffectedNodes));

if (obj.isFieldPresent(sfDeliveredAmount))
setDeliveredAmount(
get<STAmount>(obj.getFieldAmount(sfDeliveredAmount)));
setDeliveredAmount(obj.getFieldAmount(sfDeliveredAmount));
}

TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, STObject const& obj)
Expand All @@ -61,8 +60,7 @@ TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, STObject const& obj)
mNodes = *affectedNodes;

if (obj.isFieldPresent(sfDeliveredAmount))
setDeliveredAmount(
get<STAmount>(obj.getFieldAmount(sfDeliveredAmount)));
setDeliveredAmount(obj.getFieldAmount(sfDeliveredAmount));
}

TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, Blob const& vec)
Expand Down
6 changes: 3 additions & 3 deletions src/xrpld/app/misc/FeeVoteImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,9 @@ FeeVoteImpl::doVoting(
}

// choose our positions
// TODO: Use structured binding once LLVM issue
// https://github.com/llvm/llvm-project/issues/48582
// is fixed.
// TODO: Use structured binding once LLVM 16 is the minimum supported
// version. See also: https://github.com/llvm/llvm-project/issues/48582
// https://github.com/llvm/llvm-project/commit/127bf44385424891eb04cff8e52d3f157fc2cb7c
auto const baseFee = baseFeeVote.getVotes();
auto const baseReserve = baseReserveVote.getVotes();
auto const incReserve = incReserveVote.getVotes();
Expand Down
4 changes: 2 additions & 2 deletions src/xrpld/app/paths/Credit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ creditLimit(
AccountID const& issuer,
Currency const& currency)
{
STAmount result(Issue{currency, account});
STAmount result({currency, account});

auto sleRippleState = view.read(keylet::line(account, issuer, currency));

Expand Down Expand Up @@ -64,7 +64,7 @@ creditBalance(
AccountID const& issuer,
Currency const& currency)
{
STAmount result(Issue{currency, account});
STAmount result({currency, account});

auto sleRippleState = view.read(keylet::line(account, issuer, currency));

Expand Down
2 changes: 1 addition & 1 deletion src/xrpld/app/paths/PathRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ PathRequest::findPaths(
}();

STAmount saMaxAmount = saSendMax.value_or(
STAmount(Issue{issue.currency, sourceAccount}, 1u, 0, true));
STAmount({issue.currency, sourceAccount}, 1u, 0, true));

JLOG(m_journal.debug())
<< iIdentifier << " Paths found, calling rippleCalc";
Expand Down
7 changes: 3 additions & 4 deletions src/xrpld/app/paths/Pathfinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,9 @@ Pathfinder::Pathfinder(
, mSrcCurrency(uSrcCurrency)
, mSrcIssuer(uSrcIssuer)
, mSrcAmount(srcAmount.value_or(STAmount(
Issue{
uSrcCurrency,
uSrcIssuer.value_or(
isXRP(uSrcCurrency) ? xrpAccount() : uSrcAccount)},
{uSrcCurrency,
uSrcIssuer.value_or(
isXRP(uSrcCurrency) ? xrpAccount() : uSrcAccount)},
1u,
0,
true)))
Expand Down
85 changes: 85 additions & 0 deletions src/xrpld/app/tx/detail/InvariantCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,91 @@ AccountRootsNotDeleted::finalize(

//------------------------------------------------------------------------------

void
AccountRootsDeletedClean::visitEntry(
bool isDelete,
std::shared_ptr<SLE const> const& before,
std::shared_ptr<SLE const> const&)
{
if (isDelete && before && before->getType() == ltACCOUNT_ROOT)
accountsDeleted_.emplace_back(before);
}

bool
AccountRootsDeletedClean::finalize(
STTx const& tx,
TER const result,
XRPAmount const,
ReadView const& view,
beast::Journal const& j)
{
// Always check for objects in the ledger, but to prevent differing
// transaction processing results, however unlikely, only fail if the
// feature is enabled. Enabled, or not, though, a fatal-level message will
// be logged
bool const enforce = view.rules().enabled(featureInvariantsV1_1);

auto const objectExists = [&view, enforce, &j](auto const& keylet) {
if (auto const sle = view.read(keylet))
{
// Finding the object is bad
auto const typeName = [&sle]() {
auto item =
LedgerFormats::getInstance().findByType(sle->getType());

if (item != nullptr)
return item->getName();
return std::to_string(sle->getType());
}();

JLOG(j.fatal())
<< "Invariant failed: account deletion left behind a "
<< typeName << " object";
(void)enforce;
assert(enforce);
return true;
}
return false;
};

for (auto const& accountSLE : accountsDeleted_)
{
auto const accountID = accountSLE->getAccountID(sfAccount);
// Simple types
for (auto const& [keyletfunc, _, __] : directAccountKeylets)
{
if (objectExists(std::invoke(keyletfunc, accountID)) && enforce)
return false;
}

{
// NFT pages. ntfpage_min and nftpage_max were already explicitly
// checked above as entries in directAccountKeylets. This uses
// view.succ() to check for any NFT pages in between the two
// endpoints.
Keylet const first = keylet::nftpage_min(accountID);
Keylet const last = keylet::nftpage_max(accountID);

std::optional<uint256> key = view.succ(first.key, last.key.next());

// current page
if (key && objectExists(Keylet{ltNFTOKEN_PAGE, *key}) && enforce)
return false;
}

// Keys directly stored in the AccountRoot object
if (auto const ammKey = accountSLE->at(~sfAMMID))
{
if (objectExists(keylet::amm(*ammKey)) && enforce)
return false;
}
}

return true;
}

//------------------------------------------------------------------------------

void
LedgerEntryTypesMatch::visitEntry(
bool,
Expand Down
31 changes: 31 additions & 0 deletions src/xrpld/app/tx/detail/InvariantCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,36 @@ class AccountRootsNotDeleted
beast::Journal const&);
};

/**
* @brief Invariant: a deleted account must not have any objects left
*
* We iterate all deleted account roots, and ensure that there are no
* objects left that are directly accessible with that account's ID.
*
* There should only be one deleted account, but that's checked by
* AccountRootsNotDeleted. This invariant will handle multiple deleted account
* roots without a problem.
*/
class AccountRootsDeletedClean
{
std::vector<std::shared_ptr<SLE const>> accountsDeleted_;

public:
void
visitEntry(
bool,
std::shared_ptr<SLE const> const&,
std::shared_ptr<SLE const> const&);

bool
finalize(
STTx const&,
TER const,
XRPAmount const,
ReadView const&,
beast::Journal const&);
};

/**
* @brief Invariant: An account XRP balance must be in XRP and take a value
* between 0 and INITIAL_XRP drops, inclusive.
Expand Down Expand Up @@ -448,6 +478,7 @@ class ValidMPTIssuance
using InvariantChecks = std::tuple<
TransactionFeeCheck,
AccountRootsNotDeleted,
AccountRootsDeletedClean,
LedgerEntryTypesMatch,
XRPBalanceChecks,
XRPNotCreated,
Expand Down
3 changes: 2 additions & 1 deletion src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
return tecNFTOKEN_BUY_SELL_MISMATCH;

// The two offers being brokered must be for the same asset:
if (!(*bo)[sfAmount].sameIssue((*so)[sfAmount]))
if (get<STAmount>((*bo)[sfAmount]).issue() !=
get<STAmount>((*so)[sfAmount]).issue())
return tecNFTOKEN_BUY_SELL_MISMATCH;

// The two offers may not form a loop. A broker may not sell the
Expand Down

0 comments on commit 5e9f164

Please sign in to comment.