Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address rare corruption of NFTokenPage linked list #4945

Merged
merged 17 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ target_sources (rippled PRIVATE
src/ripple/app/tx/impl/DID.cpp
src/ripple/app/tx/impl/Escrow.cpp
src/ripple/app/tx/impl/InvariantCheck.cpp
src/ripple/app/tx/impl/LedgerStateFix.cpp
src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
src/ripple/app/tx/impl/NFTokenBurn.cpp
src/ripple/app/tx/impl/NFTokenCancelOffer.cpp
Expand Down Expand Up @@ -831,6 +832,7 @@ if (tests)
src/test/app/DNS_test.cpp
src/test/app/Escrow_test.cpp
src/test/app/FeeVote_test.cpp
src/test/app/FixNFTokenPageLinks_test.cpp
src/test/app/Flow_test.cpp
src/test/app/Freeze_test.cpp
src/test/app/HashRouter_test.cpp
Expand Down Expand Up @@ -991,6 +993,7 @@ if (tests)
src/test/jtx/impl/invoice_id.cpp
src/test/jtx/impl/jtx_json.cpp
src/test/jtx/impl/last_ledger_sequence.cpp
src/test/jtx/impl/ledgerStateFix.cpp
src/test/jtx/impl/memo.cpp
src/test/jtx/impl/multisign.cpp
src/test/jtx/impl/offer.cpp
Expand Down
49 changes: 47 additions & 2 deletions src/ripple/app/tx/impl/InvariantCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,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;
Expand Down Expand Up @@ -588,11 +592,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
Expand Down Expand Up @@ -633,6 +663,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;
}

Expand Down
2 changes: 2 additions & 0 deletions src/ripple/app/tx/impl/InvariantCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ class ValidNFTokenPage
bool badSort_ = false;
bool badURI_ = false;
bool invalidSize_ = false;
bool deletedFinalPage_ = false;
bool deletedLink_ = false;

public:
void
Expand Down
98 changes: 98 additions & 0 deletions src/ripple/app/tx/impl/LedgerStateFix.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
//------------------------------------------------------------------------------
/*
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 <ripple/app/tx/impl/LedgerStateFix.h>

#include <ripple/app/tx/impl/details/NFTokenUtils.h>
#include <ripple/ledger/View.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/Protocol.h>
#include <ripple/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 AccountDelete is one owner reserve.
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved
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
57 changes: 57 additions & 0 deletions src/ripple/app/tx/impl/LedgerStateFix.h
Original file line number Diff line number Diff line change
@@ -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 <ripple/app/tx/impl/Transactor.h>
#include <ripple/basics/Log.h>
#include <ripple/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
3 changes: 3 additions & 0 deletions src/ripple/app/tx/impl/applySteps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <ripple/app/tx/impl/DeleteOracle.h>
#include <ripple/app/tx/impl/DepositPreauth.h>
#include <ripple/app/tx/impl/Escrow.h>
#include <ripple/app/tx/impl/LedgerStateFix.h>
#include <ripple/app/tx/impl/NFTokenAcceptOffer.h>
#include <ripple/app/tx/impl/NFTokenBurn.h>
#include <ripple/app/tx/impl/NFTokenCancelOffer.h>
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading