From 69bb2be446e3cc24c694c0835b48bd2ecd3d119e Mon Sep 17 00:00:00 2001 From: Crypto Brad Garlinghouse Date: Tue, 19 Jul 2022 18:51:31 +0000 Subject: [PATCH] Introduce amendment to handle trustlines to self: Trustlines must be between two different accounts but two trustlines exist where an account extends trust to itself. They were created in the early days, likely because of bugs that have been fixed. The new fixTrustLinesToSelf amendment will remove those trustlines when it activates. --- src/ripple/app/tx/impl/Change.cpp | 87 ++++++++++++++++++++++++++++ src/ripple/app/tx/impl/Change.h | 3 + src/ripple/app/tx/impl/SetTrust.cpp | 53 ++++++++++------- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + 5 files changed, 124 insertions(+), 23 deletions(-) diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index bd66d7d58..93ed1a04f 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -23,9 +23,11 @@ #include #include #include +#include #include #include #include +#include namespace ripple { @@ -120,6 +122,88 @@ Change::preCompute() assert(account_ == beast::zero); } +void +Change::activateTrustLinesToSelfFix() +{ + JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code starting"; + + auto removeTrustLineToSelf = [this](Sandbox& sb, uint256 id) { + auto tl = sb.peek(keylet::child(id)); + + if (tl == nullptr) + { + JLOG(j_.warn()) << id << ": Unable to locate trustline"; + return true; + } + + if (tl->getType() != ltRIPPLE_STATE) + { + JLOG(j_.warn()) << id << ": Unexpected type " + << static_cast(tl->getType()); + return true; + } + + auto const& lo = tl->getFieldAmount(sfLowLimit); + auto const& hi = tl->getFieldAmount(sfHighLimit); + + if (lo != hi) + { + JLOG(j_.warn()) << id << ": Trustline doesn't meet requirements"; + return true; + } + + if (auto const page = tl->getFieldU64(sfLowNode); !sb.dirRemove( + keylet::ownerDir(lo.getIssuer()), page, tl->key(), false)) + { + JLOG(j_.error()) << id << ": failed to remove low entry from " + << toBase58(lo.getIssuer()) << ":" << page + << " owner directory"; + return false; + } + + if (auto const page = tl->getFieldU64(sfHighNode); !sb.dirRemove( + keylet::ownerDir(hi.getIssuer()), page, tl->key(), false)) + { + JLOG(j_.error()) << id << ": failed to remove high entry from " + << toBase58(hi.getIssuer()) << ":" << page + << " owner directory"; + return false; + } + + if (tl->getFlags() & lsfLowReserve) + adjustOwnerCount( + sb, sb.peek(keylet::account(lo.getIssuer())), -1, j_); + + if (tl->getFlags() & lsfHighReserve) + adjustOwnerCount( + sb, sb.peek(keylet::account(hi.getIssuer())), -1, j_); + + sb.erase(tl); + + JLOG(j_.warn()) << "Successfully deleted trustline " << id; + + return true; + }; + + using namespace std::literals; + + Sandbox sb(&view()); + + if (removeTrustLineToSelf( + sb, + uint256{ + "2F8F21EFCAFD7ACFB07D5BB04F0D2E18587820C7611305BB674A64EAB0FA71E1"sv}) && + removeTrustLineToSelf( + sb, + uint256{ + "326035D5C0560A9DA8636545DD5A1B0DFCFF63E68D491B5522B767BB00564B1A"sv})) + { + JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code " + "executed successfully"; + sb.apply(ctx_.rawView()); + } +} + TER Change::applyAmendment() { @@ -196,6 +280,9 @@ Change::applyAmendment() amendments.push_back(amendment); amendmentObject->setFieldV256(sfAmendments, amendments); + if (amendment == fixTrustLinesToSelf) + activateTrustLinesToSelfFix(); + ctx_.app.getAmendmentTable().enable(amendment); if (!ctx_.app.getAmendmentTable().isSupported(amendment)) diff --git a/src/ripple/app/tx/impl/Change.h b/src/ripple/app/tx/impl/Change.h index acd21837e..0ee7067b3 100644 --- a/src/ripple/app/tx/impl/Change.h +++ b/src/ripple/app/tx/impl/Change.h @@ -56,6 +56,9 @@ public: preclaim(PreclaimContext const& ctx); private: + void + activateTrustLinesToSelfFix(); + TER applyAmendment(); diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 5f268f2c2..23af19c7b 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -104,19 +104,27 @@ SetTrust::preclaim(PreclaimContext const& ctx) auto const currency = saLimitAmount.getCurrency(); auto const uDstAccountID = saLimitAmount.getIssuer(); - if (id == uDstAccountID) + if (ctx.view.rules().enabled(fixTrustLinesToSelf)) { - // Prevent trustline to self from being created, - // unless one has somehow already been created - // (in which case doApply will clean it up). - auto const sleDelete = - ctx.view.read(keylet::line(id, uDstAccountID, currency)); - - if (!sleDelete) - { - JLOG(ctx.j.trace()) - << "Malformed transaction: Can not extend credit to self."; + if (id == uDstAccountID) return temDST_IS_SRC; + } + else + { + if (id == uDstAccountID) + { + // Prevent trustline to self from being created, + // unless one has somehow already been created + // (in which case doApply will clean it up). + auto const sleDelete = + ctx.view.read(keylet::line(id, uDstAccountID, currency)); + + if (!sleDelete) + { + JLOG(ctx.j.trace()) + << "Malformed transaction: Can not extend credit to self."; + return temDST_IS_SRC; + } } } @@ -183,18 +191,19 @@ SetTrust::doApply() auto viewJ = ctx_.app.journal("View"); - if (account_ == uDstAccountID) + // Trust lines to self are impossible but because of the old bug there are + // two on 19-02-2022. This code was here to allow those trust lines to be + // deleted. The fixTrustLinesToSelf fix amendment will remove them when it + // enables so this code will no longer be needed. + if (!view().rules().enabled(fixTrustLinesToSelf) && + account_ == uDstAccountID) { - // The only purpose here is to allow a mistakenly created - // trust line to oneself to be deleted. If no such trust - // lines exist now, why not remove this code and simply - // return an error? - SLE::pointer sleDelete = - view().peek(keylet::line(account_, uDstAccountID, currency)); - - JLOG(j_.warn()) << "Clearing redundant line."; - - return trustDelete(view(), sleDelete, account_, uDstAccountID, viewJ); + return trustDelete( + view(), + view().peek(keylet::line(account_, uDstAccountID, currency)), + account_, + uDstAccountID, + viewJ); } SLE::pointer sleDst = view().peek(keylet::account(uDstAccountID)); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index b46e4d588..fc3256d16 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,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 = 50; +static constexpr std::size_t numFeatures = 51; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -337,6 +337,7 @@ extern uint256 const featureExpandedSignerList; extern uint256 const fixNFTokenDirV1; extern uint256 const fixNFTokenNegOffer; extern uint256 const featureNonFungibleTokensV1_1; +extern uint256 const fixTrustLinesToSelf; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index fcd774ce9..e33b9efeb 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -447,6 +447,7 @@ REGISTER_FEATURE(ExpandedSignerList, Supported::yes, DefaultVote::no) REGISTER_FIX (fixNFTokenDirV1, Supported::yes, DefaultVote::no); REGISTER_FIX (fixNFTokenNegOffer, Supported::yes, DefaultVote::no); REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated.