add signature existence check, consume ReserveCount if pre-funded sponsoring

src/test/app/Sponsor_test.cpp

@@ -7,6 +7,8 @@
 #include <xrpl/basics/strHex.h>
 #include <xrpl/protocol/Feature.h>
 
+#include "test/jtx/sponsor.h"
+
 namespace xrpl {
 namespace test {
 
@@ -587,6 +589,12 @@ public:
             Account const sponsor2("sponsor2");
             env.fund(XRP(10000), alice, bob, sponsor1, sponsor2);
 
+            // sfSponsor provided but sfSponsorSignature not provided
+            env(sponsor::transfer(alice),
+                sponsor::as(sponsor1, tfSponsorReserve),
+                ter(temMALFORMED));
+            env.close();
+
             adjustAccountXRPBalance(
                 env, sponsor1, accountReserve(env, 2) - drops(1));
 
@@ -698,7 +706,7 @@ public:
             env.close();
         }
         {
-            // sponsor object
+            // sponsor object (co-signing)
             Env env{*this, testable_amendments()};
             Account const alice("alice");
             Account const bob("bob");
@@ -823,6 +831,134 @@ public:
             auto const sle3 = env.le(keylet::unchecked(checkId));
             BEAST_EXPECT(!sle3->isFieldPresent(sfSponsorAccount));
         }
+        {
+            // sponsor object (pre-funded + no ltSponsorship entry)
+            Env env{*this, testable_amendments()};
+            Account const alice("alice");
+            Account const bob("bob");
+            Account const sponsor1("sponsor1");
+            Account const sponsor2("sponsor2");
+            env.fund(XRP(10000), alice, bob, sponsor1, sponsor2);
+            env.close();
+
+            auto const seq = env.seq(alice);
+            env(check::create(alice, bob, XRP(1)));
+            env.close();
+
+            auto const checkId = keylet::check(alice, seq).key;
+            BEAST_EXPECT(env.le(keylet::unchecked(checkId)) != nullptr);
+
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor1, tfSponsorReserve),
+                ter(terNO_SPONSORSHIP));
+            env.close();
+
+            env(sponsor::set_reserve(sponsor2, 0, 1),
+                sponsor::sponseeAcc(alice));
+            env.close();
+
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor2, tfSponsorReserve));
+            env.close();
+
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor1, tfSponsorReserve),
+                ter(terNO_SPONSORSHIP));
+            env.close();
+        }
+        {
+            // sponsor object (pre-funded)
+            Env env{*this, testable_amendments()};
+            Account const alice("alice");
+            Account const bob("bob");
+            Account const sponsor1("sponsor1");
+            Account const sponsor2("sponsor2");
+            env.fund(XRP(10000), alice, bob, sponsor1, sponsor2);
+            env.close();
+
+            auto const seq = env.seq(alice);
+            env(check::create(alice, bob, XRP(1)));
+            env.close();
+
+            auto const checkId = keylet::check(alice, seq).key;
+            BEAST_EXPECT(env.le(keylet::unchecked(checkId)) != nullptr);
+
+            // insufficient reserve count
+            env(sponsor::set_fee(sponsor1, 0, XRP(100)),
+                sponsor::sponseeAcc(alice));
+            env.close();
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor1, tfSponsorReserve),
+                ter(tecINSUFFICIENT_RESERVE));
+            env.close();
+
+            env(sponsor::set_reserve(sponsor1, 0, 100),
+                sponsor::sponseeAcc(alice));
+            env.close();
+
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor1, tfSponsorReserve));
+            env.close();
+
+            BEAST_EXPECT(ownerCount(env, alice) == 1);
+            BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 1);
+            BEAST_EXPECT(sponsoredOwnerCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, sponsor1) == 1);
+            BEAST_EXPECT(sponsoringAccountCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, sponsor1) == 0);
+            auto const sle1 = env.le(keylet::unchecked(checkId));
+            BEAST_EXPECT(sle1->isFieldPresent(sfSponsorAccount));
+            BEAST_EXPECT(sle1->getAccountID(sfSponsorAccount) == sponsor1.id());
+            auto const sponsorSle = env.le(keylet::sponsor(sponsor1, alice));
+            BEAST_EXPECT(sponsorSle->getFieldU32(sfReserveCount) == 99);
+
+            // transfer sponsor
+            env(sponsor::set_reserve(sponsor2, 0, 100),
+                sponsor::sponseeAcc(alice));
+            env.close();
+
+            env(sponsor::transfer(alice, checkId),
+                sponsor::as(sponsor2, tfSponsorReserve));
+            env.close();
+
+            BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 1);
+            BEAST_EXPECT(sponsoredOwnerCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoredOwnerCount(env, sponsor2) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, sponsor2) == 1);
+            BEAST_EXPECT(sponsoringAccountCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, sponsor2) == 0);
+            auto const sle2 = env.le(keylet::unchecked(checkId));
+            BEAST_EXPECT(sle2->isFieldPresent(sfSponsorAccount));
+            BEAST_EXPECT(sle2->getAccountID(sfSponsorAccount) == sponsor2.id());
+            auto const sponsorSle2 = env.le(keylet::sponsor(sponsor2, alice));
+            BEAST_EXPECT(sponsorSle2->getFieldU32(sfReserveCount) == 99);
+
+            // dissolve sponsor
+            adjustAccountXRPBalance(env, alice, reserve(env, 1));
+            env(sponsor::transfer(alice, checkId));
+            env.close();
+
+            BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoredOwnerCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoredOwnerCount(env, sponsor2) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoringOwnerCount(env, sponsor2) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, alice) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, sponsor1) == 0);
+            BEAST_EXPECT(sponsoringAccountCount(env, sponsor2) == 0);
+            BEAST_EXPECT(!env.le(keylet::account(sponsor2))
+                              ->isFieldPresent(sfSponsoringOwnerCount));
+            auto const sle3 = env.le(keylet::unchecked(checkId));
+            BEAST_EXPECT(!sle3->isFieldPresent(sfSponsorAccount));
+            auto const sponsorSle3 = env.le(keylet::sponsor(sponsor2, alice));
+            BEAST_EXPECT(
+                sponsorSle3->getFieldU32(sfReserveCount) == 99);  // no change
+        }
 
         {
             // invalid transfer

src/xrpld/app/tx/detail/SponsorshipTransfer.cpp

@@ -15,6 +15,20 @@ namespace xrpl {
 NotTEC
 SponsorshipTransfer::preflight(PreflightContext const& ctx)
 {
+    // When an account sponsoring, sfSponsorSignature must be provided
+    auto const newSponsor = getTxReserveSponsorAccountID(ctx.tx);
+    bool const isObjectSponsor = ctx.tx.isFieldPresent(sfObjectID);
+
+    // both sfSponsor and sfObjectID are provided
+    bool const isNewAccountSponsor = newSponsor && !isObjectSponsor;
+
+    if (isNewAccountSponsor && !ctx.tx.isFieldPresent(sfSponsorSignature))
+    {
+        JLOG(ctx.j.debug())
+            << "preflight: sponsoring an account needs co-signing sponsor";
+        return temMALFORMED;
+    }
+
     return tesSUCCESS;
 }
 
@@ -154,7 +168,9 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx)
 
     bool const isObjectSponsor = index != std::nullopt;
 
-    auto const accSle = ctx.view.read(keylet::account(ctx.tx[sfAccount]));
+    auto const account = ctx.tx[sfAccount];
+
+    auto const accSle = ctx.view.read(keylet::account(account));
     if (!accSle)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
@@ -166,9 +182,8 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx)
 
         auto const ownerCountDelta = getLedgerEntryOwnerCount(sle);
 
-        auto const owner =
-            getLedgerEntryOwner(ctx.view, sle, ctx.tx[sfAccount]);
-        if (!owner || owner != ctx.tx[sfAccount])
+        auto const owner = getLedgerEntryOwner(ctx.view, sle, account);
+        if (!owner || owner != account)
             return tecNO_PERMISSION;
 
         auto const& sponsorField = getLedgerEntrySponsorField(sle, *owner);
@@ -240,6 +255,37 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
+TER
+adjustReserveCount(
+    ApplyView& view,
+    AccountID const& account,
+    AccountID const& sponsor,
+    int32_t delta)
+{
+    if (delta == 0)
+        return tesSUCCESS;
+    if (delta > 0)
+        return tefINTERNAL;  // LCOV_EXCL_LINE
+    auto const sponsorKeylet = keylet::sponsor(sponsor, account);
+    auto const sponsorSle = view.peek(sponsorKeylet);
+    if (!sponsorSle)
+        return tefINTERNAL;  // LCOV_EXCL_LINE
+
+    auto const reserveCount = sponsorSle->getFieldU32(sfReserveCount);
+    int32_t const afterReserveCount = reserveCount + delta;
+
+    if (afterReserveCount < 0)
+        // already checked in preclaim()
+        return tefINTERNAL;  // LCOV_EXCL_LINE
+
+    if (afterReserveCount == 0)
+        sponsorSle->makeFieldAbsent(sfReserveCount);
+    else
+        sponsorSle->setFieldU32(sfReserveCount, afterReserveCount);
+    view.update(sponsorSle);
+    return tesSUCCESS;
+}
+
 TER
 SponsorshipTransfer::doApply()
 {
@@ -252,8 +298,19 @@ SponsorshipTransfer::doApply()
     if (!accSle)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
+    auto const setSponsorFieldU32 =
+        [](auto const& sle, auto const& field, auto const& delta) {
+            auto const newValue = sle->getFieldU32(field) + delta;
+            if (newValue == 0)
+                sle->makeFieldAbsent(field);
+            else
+                sle->setFieldU32(field, newValue);
+        };
+
     if (isObjectSponsor)
     {
+        auto const hasSignature = tx.isFieldPresent(sfSponsorSignature);
+
         // transfer object sponsor
         auto const objSle = view().peek(keylet::unchecked(*index));
         if (!objSle)
@@ -280,65 +337,51 @@ SponsorshipTransfer::doApply()
             if (auto const oldSponsorSle =
                     view().peek(keylet::account(oldSponsor)))
             {
-                auto const newCount =
-                    oldSponsorSle->getFieldU32(sfSponsoringOwnerCount) -
-                    ownerCountDelta;
-                if (newCount == 0)
-                    oldSponsorSle->makeFieldAbsent(sfSponsoringOwnerCount);
-                else
-                    oldSponsorSle->setFieldU32(
-                        sfSponsoringOwnerCount, newCount);
-
+                setSponsorFieldU32(
+                    oldSponsorSle, sfSponsoringOwnerCount, -ownerCountDelta);
                 view().update(oldSponsorSle);
             }
             else
             {
                 // update owner's sponsored count
-                auto const newCount =
-                    ownerSle->getFieldU32(sfSponsoredOwnerCount) +
-                    ownerCountDelta;
-                if (newCount == 0)
-                    ownerSle->makeFieldAbsent(sfSponsoredOwnerCount);
-                else
-                    ownerSle->setFieldU32(sfSponsoredOwnerCount, newCount);
+                setSponsorFieldU32(
+                    ownerSle, sfSponsoredOwnerCount, ownerCountDelta);
                 view().update(ownerSle);
             }
+
             // increment new sponsoring count
             auto const newSponsorSle = view().peek(keylet::account(newSponsor));
-            auto const newCount =
-                newSponsorSle->getFieldU32(sfSponsoringOwnerCount) +
-                ownerCountDelta;
-            newSponsorSle->setFieldU32(sfSponsoringOwnerCount, newCount);
+
+            setSponsorFieldU32(
+                newSponsorSle, sfSponsoringOwnerCount, ownerCountDelta);
             view().update(newSponsorSle);
 
             objSle->setAccountID(sponsorField, newSponsor);
             view().update(objSle);
+
+            if (!hasSignature)
+            {
+                // pre-funded sponsor
+                if (auto const ter = adjustReserveCount(
+                        view(), account_, newSponsor, -ownerCountDelta);
+                    !isTesSuccess(ter))
+                    return ter;
+            }
         }
         else
         {
             // dissolve object sponsor
             auto const oldSponsor = objSle->getAccountID(sfSponsorAccount);
             // decrement sponsored count
-            auto const newCount =
-                accSle->getFieldU32(sfSponsoredOwnerCount) - ownerCountDelta;
-            if (newCount == 0)
-                accSle->makeFieldAbsent(sfSponsoredOwnerCount);
-            else
-                accSle->setFieldU32(sfSponsoredOwnerCount, newCount);
+            setSponsorFieldU32(accSle, sfSponsoredOwnerCount, -ownerCountDelta);
 
             view().update(accSle);
             // decrement old sponsoring count
             if (auto const oldSponsorSle =
                     view().peek(keylet::account(oldSponsor)))
             {
-                auto const newCount =
-                    oldSponsorSle->getFieldU32(sfSponsoringOwnerCount) -
-                    ownerCountDelta;
-                if (newCount == 0)
-                    oldSponsorSle->makeFieldAbsent(sfSponsoringOwnerCount);
-                else
-                    oldSponsorSle->setFieldU32(
-                        sfSponsoringOwnerCount, newCount);
+                setSponsorFieldU32(
+                    oldSponsorSle, sfSponsoringOwnerCount, -ownerCountDelta);
                 view().update(oldSponsorSle);
             }
 
@@ -357,9 +400,8 @@ SponsorshipTransfer::doApply()
             // increment new sponsoring count
             auto const newSponsor = sponsorObj[sfAccount];
             auto const newSponsorSle = view().peek(keylet::account(newSponsor));
-            newSponsorSle->setFieldU32(
-                sfSponsoringAccountCount,
-                newSponsorSle->getFieldU32(sfSponsoringAccountCount) + 1);
+            setSponsorFieldU32(newSponsorSle, sfSponsoringAccountCount, 1);
+
             view().update(newSponsorSle);
             // decrement old sponsoring count
             if (accSle->isFieldPresent(sfSponsorAccount))
@@ -367,13 +409,7 @@ SponsorshipTransfer::doApply()
                 auto const oldSponsor = accSle->getAccountID(sfSponsorAccount);
                 auto const oldSponsorSle =
                     view().peek(keylet::account(oldSponsor));
-                auto const newCount =
-                    oldSponsorSle->getFieldU32(sfSponsoringAccountCount) - 1;
-                if (newCount == 0)
-                    oldSponsorSle->makeFieldAbsent(sfSponsoringAccountCount);
-                else
-                    oldSponsorSle->setFieldU32(
-                        sfSponsoringAccountCount, newCount);
+                setSponsorFieldU32(oldSponsorSle, sfSponsoringAccountCount, -1);
                 view().update(oldSponsorSle);
             }
             accSle->setAccountID(sfSponsorAccount, newSponsor);
@@ -386,12 +422,7 @@ SponsorshipTransfer::doApply()
             accSle->makeFieldAbsent(sfSponsorAccount);
             // decrement account sponsoring count
             auto const oldSponsorSle = view().peek(keylet::account(oldSponsor));
-            auto const newCount =
-                oldSponsorSle->getFieldU32(sfSponsoringAccountCount) - 1;
-            if (newCount == 0)
-                oldSponsorSle->makeFieldAbsent(sfSponsoringAccountCount);
-            else
-                oldSponsorSle->setFieldU32(sfSponsoringAccountCount, newCount);
+            setSponsorFieldU32(oldSponsorSle, sfSponsoringAccountCount, -1);
             view().update(oldSponsorSle);
         }
     }