Address rare corruption of NFTokenPage linked list (#4945)

* Add fixNFTokenPageLinks amendment:

It was discovered that under rare circumstances the links between
NFTokenPages could be removed.  If this happens, then the
account_objects and account_nfts RPC commands under-report the
NFTokens owned by an account.

The fixNFTokenPageLinks amendment does the following to address
the problem:

- It fixes the underlying problem so no further broken links
  should be created.
- It adds Invariants so, if such damage were introduced in the
  future, an invariant would stop it.
- It adds a new FixLedgerState transaction that repairs
  directories that were damaged in this fashion.
- It adds unit tests for all of it.
This commit is contained in:
Scott Schurr
2024-08-07 15:14:19 -07:00
committed by GitHub
parent 0a331ea72e
commit c19a88fee9
22 changed files with 2056 additions and 133 deletions

View File

@@ -612,6 +612,10 @@ ValidNFTokenPage::visitEntry(
static constexpr uint256 const& pageBits = nft::pageMask;
static constexpr uint256 const accountBits = ~pageBits;
if ((before && before->getType() != ltNFTOKEN_PAGE) ||
(after && after->getType() != ltNFTOKEN_PAGE))
return;
auto check = [this, isDelete](std::shared_ptr<SLE const> const& sle) {
uint256 const account = sle->key() & accountBits;
uint256 const hiLimit = sle->key() & pageBits;
@@ -673,11 +677,37 @@ ValidNFTokenPage::visitEntry(
}
};
if (before && before->getType() == ltNFTOKEN_PAGE)
if (before)
{
check(before);
if (after && after->getType() == ltNFTOKEN_PAGE)
// While an account's NFToken directory contains any NFTokens, the last
// NFTokenPage (with 96 bits of 1 in the low part of the index) should
// never be deleted.
if (isDelete && (before->key() & nft::pageMask) == nft::pageMask &&
before->isFieldPresent(sfPreviousPageMin))
{
deletedFinalPage_ = true;
}
}
if (after)
check(after);
if (!isDelete && before && after)
{
// If the NFTokenPage
// 1. Has a NextMinPage field in before, but loses it in after, and
// 2. This is not the last page in the directory
// Then we have identified a corruption in the links between the
// NFToken pages in the NFToken directory.
if ((before->key() & nft::pageMask) != nft::pageMask &&
before->isFieldPresent(sfNextPageMin) &&
!after->isFieldPresent(sfNextPageMin))
{
deletedLink_ = true;
}
}
}
bool
@@ -718,6 +748,21 @@ ValidNFTokenPage::finalize(
return false;
}
if (view.rules().enabled(fixNFTokenPageLinks))
{
if (deletedFinalPage_)
{
JLOG(j.fatal()) << "Invariant failed: Last NFT page deleted with "
"non-empty directory.";
return false;
}
if (deletedLink_)
{
JLOG(j.fatal()) << "Invariant failed: Lost NextMinPage link.";
return false;
}
}
return true;
}

View File

@@ -367,6 +367,8 @@ class ValidNFTokenPage
bool badSort_ = false;
bool badURI_ = false;
bool invalidSize_ = false;
bool deletedFinalPage_ = false;
bool deletedLink_ = false;
public:
void

View File

@@ -0,0 +1,99 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2024 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.
*/
//==============================================================================
#include <xrpld/app/tx/detail/LedgerStateFix.h>
#include <xrpld/app/tx/detail/NFTokenUtils.h>
#include <xrpld/ledger/View.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/TxFlags.h>
namespace ripple {
NotTEC
LedgerStateFix::preflight(PreflightContext const& ctx)
{
if (!ctx.rules.enabled(fixNFTokenPageLinks))
return temDISABLED;
if (ctx.tx.getFlags() & tfUniversalMask)
return temINVALID_FLAG;
if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
return ret;
switch (ctx.tx[sfLedgerFixType])
{
case FixType::nfTokenPageLink:
if (!ctx.tx.isFieldPresent(sfOwner))
return temINVALID;
break;
default:
return tefINVALID_LEDGER_FIX_TYPE;
}
return preflight2(ctx);
}
XRPAmount
LedgerStateFix::calculateBaseFee(ReadView const& view, STTx const& tx)
{
// The fee required for LedgerStateFix is one owner reserve, just like
// the fee for AccountDelete.
return view.fees().increment;
}
TER
LedgerStateFix::preclaim(PreclaimContext const& ctx)
{
switch (ctx.tx[sfLedgerFixType])
{
case FixType::nfTokenPageLink: {
AccountID const owner{ctx.tx[sfOwner]};
if (!ctx.view.read(keylet::account(owner)))
return tecOBJECT_NOT_FOUND;
return tesSUCCESS;
}
}
// preflight is supposed to verify that only valid FixTypes get to preclaim.
return tecINTERNAL;
}
TER
LedgerStateFix::doApply()
{
switch (ctx_.tx[sfLedgerFixType])
{
case FixType::nfTokenPageLink:
if (!nft::repairNFTokenDirectoryLinks(view(), ctx_.tx[sfOwner]))
return tecFAILED_PROCESSING;
return tesSUCCESS;
}
// preflight is supposed to verify that only valid FixTypes get to doApply.
return tecINTERNAL;
}
} // namespace ripple

View File

@@ -0,0 +1,57 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2024 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_TX_LEDGER_STATE_FIX_H_INCLUDED
#define RIPPLE_TX_LEDGER_STATE_FIX_H_INCLUDED
#include <xrpld/app/tx/detail/Transactor.h>
#include <xrpl/basics/Log.h>
#include <xrpl/protocol/Indexes.h>
namespace ripple {
class LedgerStateFix : public Transactor
{
public:
enum FixType : std::uint16_t {
nfTokenPageLink = 1,
};
static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
explicit LedgerStateFix(ApplyContext& ctx) : Transactor(ctx)
{
}
static NotTEC
preflight(PreflightContext const& ctx);
static XRPAmount
calculateBaseFee(ReadView const& view, STTx const& tx);
static TER
preclaim(PreclaimContext const& ctx);
TER
doApply() override;
};
} // namespace ripple
#endif

View File

@@ -429,10 +429,48 @@ removeToken(
return tesSUCCESS;
}
// The page is empty, so we can just unlink it and then remove it.
if (prev)
{
// Make our previous page point to our next page:
// With fixNFTokenPageLinks...
// The page is empty and there is a prev. If the last page of the
// directory is empty then we need to:
// 1. Move the contents of the previous page into the last page.
// 2. Fix up the link from prev's previous page.
// 3. Fix up the owner count.
// 4. Erase the previous page.
if (view.rules().enabled(fixNFTokenPageLinks) &&
((curr->key() & nft::pageMask) == pageMask))
{
// Copy all relevant information from prev to curr.
curr->peekFieldArray(sfNFTokens) = prev->peekFieldArray(sfNFTokens);
if (auto const prevLink = prev->at(~sfPreviousPageMin))
{
curr->at(sfPreviousPageMin) = *prevLink;
// Also fix up the NextPageMin link in the new Previous.
auto const newPrev = loadPage(curr, sfPreviousPageMin);
newPrev->at(sfNextPageMin) = curr->key();
view.update(newPrev);
}
else
{
curr->makeFieldAbsent(sfPreviousPageMin);
}
adjustOwnerCount(
view,
view.peek(keylet::account(owner)),
-1,
beast::Journal{beast::Journal::getNullSink()});
view.update(curr);
view.erase(prev);
return tesSUCCESS;
}
// The page is empty and not the last page, so we can just unlink it
// and then remove it.
if (next)
prev->setFieldH256(sfNextPageMin, next->key());
else
@@ -637,6 +675,124 @@ deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
return true;
}
bool
repairNFTokenDirectoryLinks(ApplyView& view, AccountID const& owner)
{
bool didRepair = false;
auto const last = keylet::nftpage_max(owner);
std::shared_ptr<SLE> page = view.peek(Keylet(
ltNFTOKEN_PAGE,
view.succ(keylet::nftpage_min(owner).key, last.key.next())
.value_or(last.key)));
if (!page)
return didRepair;
if (page->key() == last.key)
{
// There's only one page in this entire directory. There should be
// no links on that page.
bool const nextPresent = page->isFieldPresent(sfNextPageMin);
bool const prevPresent = page->isFieldPresent(sfPreviousPageMin);
if (nextPresent || prevPresent)
{
didRepair = true;
if (prevPresent)
page->makeFieldAbsent(sfPreviousPageMin);
if (nextPresent)
page->makeFieldAbsent(sfNextPageMin);
view.update(page);
}
return didRepair;
}
// First page is not the same as last page. The first page should not
// contain a previous link.
if (page->isFieldPresent(sfPreviousPageMin))
{
didRepair = true;
page->makeFieldAbsent(sfPreviousPageMin);
view.update(page);
}
std::shared_ptr<SLE> nextPage;
while (
(nextPage = view.peek(Keylet(
ltNFTOKEN_PAGE,
view.succ(page->key().next(), last.key.next())
.value_or(last.key)))))
{
if (!page->isFieldPresent(sfNextPageMin) ||
page->getFieldH256(sfNextPageMin) != nextPage->key())
{
didRepair = true;
page->setFieldH256(sfNextPageMin, nextPage->key());
view.update(page);
}
if (!nextPage->isFieldPresent(sfPreviousPageMin) ||
nextPage->getFieldH256(sfPreviousPageMin) != page->key())
{
didRepair = true;
nextPage->setFieldH256(sfPreviousPageMin, page->key());
view.update(nextPage);
}
if (nextPage->key() == last.key)
// We need special handling for the last page.
break;
page = nextPage;
}
// When we arrive here, nextPage should have the same index as last.
// If not, then that's something we need to fix.
if (!nextPage)
{
// It turns out that page is the last page for this owner, but
// that last page does not have the expected final index. We need
// to move the contents of the current last page into a page with the
// correct index.
//
// The owner count does not need to change because, even though
// we're adding a page, we'll also remove the page that used to be
// last.
didRepair = true;
nextPage = std::make_shared<SLE>(last);
// Copy all relevant information from prev to curr.
nextPage->peekFieldArray(sfNFTokens) = page->peekFieldArray(sfNFTokens);
if (auto const prevLink = page->at(~sfPreviousPageMin))
{
nextPage->at(sfPreviousPageMin) = *prevLink;
// Also fix up the NextPageMin link in the new Previous.
auto const newPrev = view.peek(Keylet(ltNFTOKEN_PAGE, *prevLink));
if (!newPrev)
Throw<std::runtime_error>(
"NFTokenPage directory for " + to_string(owner) +
" cannot be repaired. Unexpected link problem.");
newPrev->at(sfNextPageMin) = nextPage->key();
view.update(newPrev);
}
view.erase(page);
view.insert(nextPage);
return didRepair;
}
assert(nextPage);
if (nextPage->isFieldPresent(sfNextPageMin))
{
didRepair = true;
nextPage->makeFieldAbsent(sfNextPageMin);
view.update(nextPage);
}
return didRepair;
}
NotTEC
tokenOfferCreatePreflight(
AccountID const& acctID,

View File

@@ -95,6 +95,13 @@ removeToken(
bool
deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer);
/** Repairs the links in an NFTokenPage directory.
Returns true if a repair took place, otherwise false.
*/
bool
repairNFTokenDirectoryLinks(ApplyView& view, AccountID const& owner);
bool
compareTokens(uint256 const& a, uint256 const& b);

View File

@@ -38,6 +38,7 @@
#include <xrpld/app/tx/detail/DeleteOracle.h>
#include <xrpld/app/tx/detail/DepositPreauth.h>
#include <xrpld/app/tx/detail/Escrow.h>
#include <xrpld/app/tx/detail/LedgerStateFix.h>
#include <xrpld/app/tx/detail/NFTokenAcceptOffer.h>
#include <xrpld/app/tx/detail/NFTokenBurn.h>
#include <xrpld/app/tx/detail/NFTokenCancelOffer.h>
@@ -97,6 +98,8 @@ with_txn_type(TxType txnType, F&& f)
return f.template operator()<EscrowFinish>();
case ttESCROW_CANCEL:
return f.template operator()<EscrowCancel>();
case ttLEDGER_STATE_FIX:
return f.template operator()<LedgerStateFix>();
case ttPAYCHAN_CLAIM:
return f.template operator()<PayChanClaim>();
case ttPAYCHAN_CREATE: