From 14986cc043469ae7c596eab263cb9b97cb7b5001 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Wed, 17 Jun 2026 14:08:06 -0400 Subject: [PATCH] Sponsorship should be non-obligated for sponsee (#7552) --- .../tx/transactors/sponsor/SponsorshipSet.h | 3 + .../tx/transactors/Sponsor/SponsorshipSet.cpp | 68 ++++++++++------ .../tx/transactors/account/AccountDelete.cpp | 31 +++++-- src/test/app/Sponsor_test.cpp | 80 +++++++++++++++++-- 4 files changed, 145 insertions(+), 37 deletions(-) diff --git a/include/xrpl/tx/transactors/sponsor/SponsorshipSet.h b/include/xrpl/tx/transactors/sponsor/SponsorshipSet.h index d7e78da2ca..3eaeba6392 100644 --- a/include/xrpl/tx/transactors/sponsor/SponsorshipSet.h +++ b/include/xrpl/tx/transactors/sponsor/SponsorshipSet.h @@ -25,6 +25,9 @@ public: static TER preclaim(PreclaimContext const& ctx); + static TER + deleteSponsorship(ApplyView& view, SLE::ref sle, beast::Journal j); + TER doApply() override; diff --git a/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp b/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp index 07dd8628f3..8529ebd59b 100644 --- a/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp +++ b/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp @@ -1,7 +1,10 @@ #include +#include +#include #include #include +#include #include #include #include @@ -175,6 +178,47 @@ SponsorshipSet::preclaim(PreclaimContext const& ctx) return tesSUCCESS; } +TER +SponsorshipSet::deleteSponsorship(ApplyView& view, SLE::ref sle, beast::Journal j) +{ + if (!sle) + return tecINTERNAL; // LCOV_EXCL_LINE + + auto const sponsorAccountID = (*sle)[sfOwner]; + auto const sponseeAccountID = (*sle)[sfSponsee]; + + // The reserve for the Sponsorship object is held by the sponsor (Owner). + auto sponsorAccSle = view.peek(keylet::account(sponsorAccountID)); + if (!sponsorAccSle) + return tecINTERNAL; // LCOV_EXCL_LINE + + if (!view.dirRemove(keylet::ownerDir(sponsorAccountID), (*sle)[sfOwnerNode], sle->key(), false)) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "Unable to delete Sponsorship from sponsor."; + return tefBAD_LEDGER; + // LCOV_EXCL_STOP + } + if (!view.dirRemove( + keylet::ownerDir(sponseeAccountID), (*sle)[sfSponseeNode], sle->key(), false)) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "Unable to delete Sponsorship from sponsee."; + return tefBAD_LEDGER; + // LCOV_EXCL_STOP + } + + adjustOwnerCountObj(view, sponsorAccSle, sle, -1, j); + + // transfer feeAmount back to the sponsor + if (sle->isFieldPresent(sfFeeAmount)) + (*sponsorAccSle)[sfBalance] += sle->getFieldAmount(sfFeeAmount); + + view.erase(sle); + + return tesSUCCESS; +} + TER SponsorshipSet::doApply() { @@ -200,29 +244,7 @@ SponsorshipSet::doApply() if (!sponsorObjSle) return tecINTERNAL; // LCOV_EXCL_LINE - adjustOwnerCountObj(ctx_.view(), sponsorAccSle, sponsorObjSle, -1, ctx_.journal); - - ctx_.view().dirRemove( - keylet::ownerDir(sponsorAccountID), - (*sponsorObjSle)[sfOwnerNode], - sponsorObjSle->key(), - false); - ctx_.view().dirRemove( - keylet::ownerDir(sponseeAccountID), - (*sponsorObjSle)[sfSponseeNode], - sponsorObjSle->key(), - false); - - // transfer feeAmount from ledger entry - if (sponsorObjSle->isFieldPresent(sfFeeAmount)) - { - auto const feeAmount = sponsorObjSle->getFieldAmount(sfFeeAmount); - (*sponsorAccSle)[sfBalance] += feeAmount; - } - - ctx_.view().erase(sponsorObjSle); - - return tesSUCCESS; + return deleteSponsorship(ctx_.view(), sponsorObjSle, ctx_.journal); } auto const feeAmount = ctx_.tx[~sfFeeAmount]; diff --git a/src/libxrpl/tx/transactors/account/AccountDelete.cpp b/src/libxrpl/tx/transactors/account/AccountDelete.cpp index 231783b2cc..f3d8b4a5c6 100644 --- a/src/libxrpl/tx/transactors/account/AccountDelete.cpp +++ b/src/libxrpl/tx/transactors/account/AccountDelete.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -185,11 +186,23 @@ removeDelegateFromLedger( return DelegateSet::deleteDelegate(view, sleDel, j); } -// Return nullptr if the LedgerEntryType represents an obligation that can't -// be deleted. Otherwise return the pointer to the function that can delete -// the non-obligation +TER +removeSponsorshipFromLedger( + ServiceRegistry&, + ApplyView& view, + AccountID const&, + uint256 const&, + SLE::ref sleDel, + beast::Journal j) +{ + return SponsorshipSet::deleteSponsorship(view, sleDel, j); +} + +// Return nullptr if the object represents an obligation that can't be deleted +// during deletion of account. Otherwise return the pointer to the function +// that can delete the non-obligation. DeleterFuncPtr -nonObligationDeleter(LedgerEntryType t) +nonObligationDeleter(LedgerEntryType t, SLE::const_ref sleItem, AccountID const& account) { switch (t) { @@ -211,6 +224,12 @@ nonObligationDeleter(LedgerEntryType t) return removeCredentialFromLedger; case ltDELEGATE: return removeDelegateFromLedger; + case ltSPONSORSHIP: + // A Sponsorship lives in both the sponsor's (Owner) and the + // sponsee's owner directories, but it is an obligation only for the + // sponsor, who holds its reserve. The sponsee must remain free to + // delete its account. + return (*sleItem)[sfOwner] == account ? nullptr : removeSponsorshipFromLedger; default: return nullptr; } @@ -335,7 +354,7 @@ AccountDelete::preclaim(PreclaimContext const& ctx) LedgerEntryType const nodeType{safeCast((*sleItem)[sfLedgerEntryType])}; - if (nonObligationDeleter(nodeType) == nullptr) + if (nonObligationDeleter(nodeType, sleItem, account) == nullptr) return tecHAS_OBLIGATIONS; // We found a deletable directory entry. Count it. If we find too @@ -376,7 +395,7 @@ AccountDelete::doApply() [&](LedgerEntryType nodeType, uint256 const& dirEntry, SLE::pointer& sleItem) -> std::pair { - if (auto deleter = nonObligationDeleter(nodeType)) + if (auto deleter = nonObligationDeleter(nodeType, sleItem, accountID_)) { TER const result{deleter(ctx_.registry, view(), accountID_, dirEntry, sleItem, j_)}; diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index c5399c71ca..d3f94c2ccb 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/src/test/app/Sponsor_test.cpp @@ -5456,7 +5456,8 @@ public: Account const sponsor("sponsor"); { - // Delete Sponsor/Sponsee Account with ltSponsorship (tecHAS_OBLIGATIONS) + // A Sponsorship object blocks deletion of the sponsor, + // but NOT of the sponsee. Env env{*this, testableAmendments()}; env.fund(XRP(1000000), alice, bob, sponsor); env.close(); @@ -5467,20 +5468,83 @@ public: Ter(tesSUCCESS)); env.close(); - incLgrSeqForAccDel(env, sponsor); - auto const keylet = keylet::sponsor(sponsor, alice); - auto const sponsorObj = env.le(keylet); - BEAST_EXPECT(sponsorObj); + BEAST_EXPECT(env.le(keylet)); + // sponsor pays its own reserve here, so there is no SponsoredOwnerCount. + BEAST_EXPECT(ownerCount(env, sponsor) == 1); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, sponsor) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0); - // AccountDelete + // sponsor's sequence is the higher one, so a single call readies + // both accounts for deletion + incLgrSeqForAccDel(env, sponsor); auto const requiredFee = drops(env.current()->fees().increment); - env(acctdelete(alice, bob), Fee(requiredFee), Ter(tecHAS_OBLIGATIONS)); + + // sponsor cannot be deleted while the Sponsorship exists env(acctdelete(sponsor, bob), Fee(requiredFee), Ter(tecHAS_OBLIGATIONS)); + env.close(); + + // sponsee can be deleted + auto const sponsorBalBefore = env.balance(sponsor); + env(acctdelete(alice, bob), Fee(requiredFee), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(!env.le(keylet)); + BEAST_EXPECT(!env.le(keylet::account(alice))); + // deleting sponsee makes those counts zero. + BEAST_EXPECT(ownerCount(env, sponsor) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, sponsor) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0); + + // FeeAmount is returned to the sponsor + BEAST_EXPECT(env.balance(sponsor) == sponsorBalBefore + XRP(100)); } { - // Delete SponsoredAccount + // The SponsorshipSet transaction itself + // is sponsored by a 3rd counterparty. Test deleting sponsee. + Env env{*this, testableAmendments()}; + Account const counterparty("counterparty"); + env.fund(XRP(1000000), alice, bob, sponsor, counterparty); + env.close(); + + // sponsor creates a Sponsorship for alice; the SponsorshipSet tx is + // itself reserve-sponsored by counterparty, so the object's reserve is counterparty's. + env(sponsor::set(sponsor, 0, 100, XRP(100)), + sponsor::SponseeAcc(alice), + sponsor::As(counterparty, spfSponsorReserve), + Sig(sfSponsorSignature, counterparty), + Ter(tesSUCCESS)); + env.close(); + + auto const keylet = keylet::sponsor(sponsor, alice); + auto const obj = env.le(keylet); + BEAST_EXPECT(obj); + BEAST_EXPECT( + obj->isFieldPresent(sfSponsor) && + obj->getAccountID(sfSponsor) == counterparty.id()); + // sponsor owns the object; its reserve is sponsored by counterparty. + BEAST_EXPECT(ownerCount(env, sponsor) == 1); + BEAST_EXPECT(sponsoredOwnerCount(env, sponsor) == 1); + BEAST_EXPECT(sponsoringOwnerCount(env, counterparty) == 1); + + auto const sponsorBalBefore = env.balance(sponsor); + incLgrSeqForAccDel(env, alice); + auto const requiredFee = drops(env.current()->fees().increment); + env(acctdelete(alice, bob), Fee(requiredFee), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(!env.le(keylet)); + // deleting sponsee makes those counts zero. + BEAST_EXPECT(ownerCount(env, sponsor) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, sponsor) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, counterparty) == 0); + BEAST_EXPECT(env.balance(sponsor) == sponsorBalBefore + XRP(100)); + } + + { + // Deleting SponsoredAccount, whose account reserve is paid by a sponsor. Env env{*this, testableAmendments()}; env.memoize(alice); env.fund(XRP(1000000), bob, sponsor);