Compare commits

..

1 Commits

Author SHA1 Message Date
Bart
ce2a02e828 refactor: Apply various minor improvements and corrections 2026-04-29 06:37:51 -04:00
10 changed files with 34 additions and 90 deletions

View File

@@ -20,10 +20,6 @@ removeTokenOffersWithLimit(
Keylet const& directory,
std::size_t maxDeletableOffers);
/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
TER
notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
/** Finds the specified token in the owner's token directory. */
std::optional<STObject>
findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

View File

@@ -19,7 +19,6 @@ XRPL_FIX (Cleanup3_2_0, Supported::no, VoteBehavior::DefaultNo
XRPL_FEATURE(MPTokensV2, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (Security3_1_3, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (BatchInnerSigs, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(PermissionDelegationV1_1, Supported::no, VoteBehavior::DefaultNo)

View File

@@ -81,12 +81,9 @@ setCurrentThreadNameImpl(std::string_view name)
{
// truncate and set the thread name.
char boundedName[maxThreadNameLength + 1];
std::snprintf(
boundedName,
sizeof(boundedName),
"%.*s",
static_cast<int>(maxThreadNameLength),
name.data()); // NOLINT(bugprone-suspicious-stringview-data-usage)
auto const boundedSize = name.size() < maxThreadNameLength ? name.size() : maxThreadNameLength;
name.copy(boundedName, boundedSize);
boundedName[boundedSize] = '\0';
pthread_setname_np(pthread_self(), boundedName);

View File

@@ -621,33 +621,6 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
return deletedOffersCount;
}
TER
notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
{
std::size_t totalOffers = 0;
{
Dir const buys(view, keylet::nft_buys(nftokenID));
for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
{
totalOffers += iter.page_size();
if (totalOffers > maxDeletableTokenOfferEntries)
return tefTOO_BIG;
}
}
{
Dir const sells(view, keylet::nft_sells(nftokenID));
for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
{
totalOffers += iter.page_size();
if (totalOffers > maxDeletableTokenOfferEntries)
return tefTOO_BIG;
}
}
return tesSUCCESS;
}
bool
deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
{

View File

@@ -68,15 +68,12 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
{
// Before fixExpiredNFTokenOfferRemoval amendment, expired
// offers caused tecEXPIRED in preclaim, leaving them on ledger
// forever. After the amendment, we allow expired offers to
// reach doApply() where they get deleted and tecEXPIRED is
// returned.
if (!ctx.view.rules().enabled(fixExpiredNFTokenOfferRemoval))
// Before fixSecurity3_1_3 amendment, expired offers caused tecEXPIRED in preclaim,
// leaving them on ledger forever. After the amendment, we allow expired offers to
// reach doApply() where they get deleted and tecEXPIRED is returned.
if (!ctx.view.rules().enabled(fixSecurity3_1_3))
return {nullptr, tecEXPIRED};
// Amendment enabled: return the expired offer to be handled in
// doApply
// Amendment enabled: return the expired offer to be handled in doApply.
}
if ((*offerSLE)[sfAmount].negative())
@@ -450,10 +447,9 @@ NFTokenAcceptOffer::doApply()
auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]);
auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]);
// With fixExpiredNFTokenOfferRemoval amendment, check for expired offers
// and delete them, returning tecEXPIRED. This ensures expired offers
// are properly cleaned up from the ledger.
if (view().rules().enabled(fixExpiredNFTokenOfferRemoval))
// With fixSecurity3_1_3 amendment, check for expired offers and delete them, returning
// tecEXPIRED. This ensures expired offers are properly cleaned up from the ledger.
if (view().rules().enabled(fixSecurity3_1_3))
{
bool foundExpired = false;

View File

@@ -1096,10 +1096,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
// The buy offer must not have expired.
// NOTE: this is only a preclaim check with the
// fixExpiredNFTokenOfferRemoval amendment disabled.
// fixSecurity3_1_3 amendment disabled.
env(token::acceptBuyOffer(alice, buyerExpOfferIndex), ter(tecEXPIRED));
env.close();
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
buyerCount--;
}
@@ -1117,12 +1117,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
// The sell offer must not have expired.
// NOTE: this is only a preclaim check with the
// fixExpiredNFTokenOfferRemoval amendment disabled.
// fixSecurity3_1_3 amendment disabled.
env(token::acceptSellOffer(buyer, aliceExpOfferIndex), ter(tecEXPIRED));
env.close();
// Alice's count is decremented by one when the expired offer is
// removed.
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
aliceCount--;
}
@@ -3101,10 +3101,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
// No one can accept an expired sell offer.
env(token::acceptSellOffer(buyer, offer1), ter(tecEXPIRED));
// With fixExpiredNFTokenOfferRemoval amendment, the first accept
// With fixSecurity3_1_3 amendment, the first accept
// attempt deletes the expired offer. Without the amendment,
// the offer remains and we can try to accept it again.
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
// After amendment: offer was deleted by first accept attempt
minterCount--;
@@ -3123,7 +3123,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, minter) == minterCount);
BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
if (!features[fixExpiredNFTokenOfferRemoval])
if (!features[fixSecurity3_1_3])
{
// Before amendment: expired offer still exists and needs to be
// cancelled
@@ -3189,10 +3189,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
// An expired buy offer cannot be accepted.
env(token::acceptBuyOffer(minter, offer1), ter(tecEXPIRED));
// With fixExpiredNFTokenOfferRemoval amendment, the first accept
// With fixSecurity3_1_3 amendment, the first accept
// attempt deletes the expired offer. Without the amendment,
// the offer remains and we can try to accept it again.
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
// After amendment: offer was deleted by first accept attempt
buyerCount--;
@@ -3211,7 +3211,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, minter) == minterCount);
BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
if (!features[fixExpiredNFTokenOfferRemoval])
if (!features[fixSecurity3_1_3])
{
// Before amendment: expired offer still exists and can be
// cancelled
@@ -3288,7 +3288,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
env.close();
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
// With amendment: expired offers are deleted
minterCount--;
@@ -3298,7 +3298,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, minter) == minterCount);
BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
// The buy offer was deleted, so no need to cancel it
// The sell offer still exists, so we can cancel it
@@ -3377,7 +3377,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
if (features[fixExpiredNFTokenOfferRemoval])
if (features[fixSecurity3_1_3])
{
// After amendment: expired offers were deleted during broker
// attempt
@@ -3463,7 +3463,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
// The expired offers are still in the ledger.
BEAST_EXPECT(ownerCount(env, issuer) == 0);
if (!features[fixExpiredNFTokenOfferRemoval])
if (!features[fixSecurity3_1_3])
{
// Before amendment: expired offers still exist in ledger
BEAST_EXPECT(ownerCount(env, minter) == 2);
@@ -7190,7 +7190,7 @@ public:
{
testWithFeats(
allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT -
fixExpiredNFTokenOfferRemoval);
fixSecurity3_1_3);
}
};
@@ -7227,7 +7227,7 @@ class NFTokenWOExpiredOfferRemoval_test : public NFTokenBaseUtil_test
void
run() override
{
testWithFeats(allFeatures - fixExpiredNFTokenOfferRemoval);
testWithFeats(allFeatures - fixSecurity3_1_3);
}
};

View File

@@ -1201,7 +1201,7 @@ class LedgerEntry_test : public beast::unit_test::suite
checkErrorValue(
jrr[jss::result],
"malformedAuthorizedCredentials",
"Invalid field 'authorized_credentials', not array.");
"Invalid field 'authorized_credentials', not array of objects.");
}
{
@@ -1219,7 +1219,7 @@ class LedgerEntry_test : public beast::unit_test::suite
checkErrorValue(
jrr[jss::result],
"malformedAuthorizedCredentials",
"Invalid field 'authorized_credentials', not array.");
"Invalid field 'authorized_credentials', not array of objects.");
}
{

View File

@@ -161,11 +161,7 @@ ValidatorSite::load(
{
try
{
// This is not super efficient, but it doesn't happen often.
bool found = std::ranges::any_of(
sites_, [&uri](auto const& site) { return site.loadedResource->uri == uri; });
if (!found)
sites_.emplace_back(uri);
sites_.emplace_back(uri);
}
catch (std::exception const& e)
{
@@ -226,17 +222,7 @@ ValidatorSite::setTimer(
std::lock_guard<std::mutex> const& site_lock,
std::lock_guard<std::mutex> const& state_lock)
{
if (!sites_.empty() && //
std::ranges::all_of(
sites_, [](auto const& site) { return site.lastRefreshStatus.has_value(); }))
{
// If all of the sites have been handled at least once (including
// errors and timeouts), call missingSite, which will load the cache
// files for any lists that are still unavailable.
missingSite(site_lock);
}
auto const next = std::ranges::min_element(
auto next = std::ranges::min_element(
sites_, [](Site const& a, Site const& b) { return a.nextRefresh < b.nextRefresh; });
if (next != sites_.end())
@@ -347,7 +333,7 @@ ValidatorSite::onRequestTimeout(std::size_t siteIdx, error_code const& ec)
// processes a network error. Usually, this function runs first,
// but on extremely rare occasions, the response handler can run
// first, which will leave activeResource empty.
auto& site = sites_[siteIdx];
auto const& site = sites_[siteIdx];
if (site.activeResource)
{
JLOG(j_.warn()) << "Request for " << site.activeResource->uri << " took too long";
@@ -355,9 +341,6 @@ ValidatorSite::onRequestTimeout(std::size_t siteIdx, error_code const& ec)
else
JLOG(j_.error()) << "Request took too long, but a response has "
"already been processed";
if (!site.lastRefreshStatus)
site.lastRefreshStatus.emplace(
Site::Status{clock_type::now(), ListDisposition::invalid, "timeout"});
}
std::lock_guard const lock_state{state_mutex_};

View File

@@ -2639,7 +2639,7 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetObjectByHash> const& m)
{
fee_.update(
Resource::feeModerateBurdenPeer,
" Reply limit reached. Truncating reply.");
"Reply limit reached. Truncating reply.");
break;
}
}

View File

@@ -267,7 +267,7 @@ parseAuthorizeCredentials(Json::Value const& jv)
if (!jo.isObject())
{
return LedgerEntryHelpers::invalidFieldError(
"malformedAuthorizedCredentials", jss::authorized_credentials, "array");
"malformedAuthorizedCredentials", jss::authorized_credentials, "array of objects");
}
if (auto const value = LedgerEntryHelpers::hasRequired(