Skip to content

Commit 067273c

Browse files
Kassaking7Kassaking7
committed
fix & add tests
1 parent 4529c5d commit 067273c

File tree

2 files changed

+123
-10
lines changed

2 files changed

+123
-10
lines changed

‎src/libxrpl/tx/transactors/delegate/DelegateSet.cpp‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,13 @@ DelegateSet::doApply()
105105

106106
// Add to authorized account's owner directory so the object can be found
107107
// and cleaned up when the authorized account is deleted.
108-
if (ctx_.view().rules().enabled(featurePermissionDelegationV1_1))
109-
{
110-
auto const destPage = ctx_.view().dirInsert(
111-
keylet::ownerDir(authAccount), delegateKey, describeOwnerDir(authAccount));
108+
auto const destPage = ctx_.view().dirInsert(
109+
keylet::ownerDir(authAccount), delegateKey, describeOwnerDir(authAccount));
112110

113-
if (!destPage)
114-
return tecDIR_FULL; // LCOV_EXCL_LINE
111+
if (!destPage)
112+
return tecDIR_FULL; // LCOV_EXCL_LINE
115113

116-
(*sle)[sfDestinationNode] = *destPage;
117-
}
114+
(*sle)[sfDestinationNode] = *destPage;
118115

119116
ctx_.view().insert(sle);
120117
adjustOwnerCount(ctx_.view(), sleOwner, 1, ctx_.journal);
@@ -128,8 +125,8 @@ DelegateSet::deleteDelegate(ApplyView& view, std::shared_ptr<SLE> const& sle, be
128125
if (!sle)
129126
return tecINTERNAL; // LCOV_EXCL_LINE
130127

131-
AccountID const delegator = (*sle)[sfAccount];
132-
AccountID const delegatee = (*sle)[sfAuthorize];
128+
auto const delegator = (*sle)[sfAccount];
129+
auto const delegatee = (*sle)[sfAuthorize];
133130

134131
// Remove from delegating account's owner directory
135132
if (!view.dirRemove(keylet::ownerDir(delegator), (*sle)[sfOwnerNode], sle->key(), false))

‎src/test/app/Delegate_test.cpp‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,121 @@ class Delegate_test : public beast::unit_test::suite
510510
!hasKey(xrpl::Dir(*env.closed(), keylet::ownerDir(alice.id())), delegateKey.key));
511511
BEAST_EXPECT(env.balance(carol) == carolBalance + bobBalance - deleteFee);
512512
}
513+
514+
// Multiple delegators -> same delegatee: when the delegatee (bob)
515+
// deletes his account, ALL Delegate objects (from alice and carol)
516+
// must be cleaned up from every delegator's directory.
517+
{
518+
Env env(*this);
519+
Account alice{"alice"};
520+
Account bob{"bob"};
521+
Account carol{"carol"};
522+
Account dave{"dave"};
523+
env.fund(XRP(100000), alice, bob, carol, dave);
524+
env.close();
525+
526+
// Both alice and carol delegate to bob
527+
env(delegate::set(alice, bob, {"Payment"}));
528+
env(delegate::set(carol, bob, {"EscrowCreate"}));
529+
env.close();
530+
531+
auto const aliceBobKey = keylet::delegate(alice.id(), bob.id());
532+
auto const carolBobKey = keylet::delegate(carol.id(), bob.id());
533+
534+
auto hasKey = [](xrpl::Dir const& dir, uint256 const& key) {
535+
return std::find_if(dir.begin(), dir.end(), [&](auto const& sle) {
536+
return sle->key() == key;
537+
}) != dir.end();
538+
};
539+
540+
// Both Delegate objects exist and are in bob's directory
541+
BEAST_EXPECT(env.closed()->exists(aliceBobKey));
542+
BEAST_EXPECT(env.closed()->exists(carolBobKey));
543+
BEAST_EXPECT(
544+
hasKey(xrpl::Dir(*env.closed(), keylet::ownerDir(bob.id())), aliceBobKey.key));
545+
BEAST_EXPECT(
546+
hasKey(xrpl::Dir(*env.closed(), keylet::ownerDir(bob.id())), carolBobKey.key));
547+
548+
for (std::uint32_t i = 0; i < 256; ++i)
549+
env.close();
550+
551+
auto const bobBalance = env.balance(bob);
552+
auto const daveBalance = env.balance(dave);
553+
554+
auto const deleteFee = drops(env.current()->fees().increment);
555+
env(acctdelete(bob, dave), fee(deleteFee));
556+
env.close();
557+
558+
// bob's account and directory are gone
559+
BEAST_EXPECT(!env.closed()->exists(keylet::account(bob.id())));
560+
BEAST_EXPECT(!env.closed()->exists(keylet::ownerDir(bob.id())));
561+
562+
// Both Delegate objects are erased
563+
BEAST_EXPECT(!env.closed()->exists(aliceBobKey));
564+
BEAST_EXPECT(!env.closed()->exists(carolBobKey));
565+
566+
// alice's and carol's directories no longer reference the objects
567+
BEAST_EXPECT(
568+
!hasKey(xrpl::Dir(*env.closed(), keylet::ownerDir(alice.id())), aliceBobKey.key));
569+
BEAST_EXPECT(
570+
!hasKey(xrpl::Dir(*env.closed(), keylet::ownerDir(carol.id())), carolBobKey.key));
571+
572+
BEAST_EXPECT(env.balance(dave) == daveBalance + bobBalance - deleteFee);
573+
}
574+
}
575+
576+
void
577+
testAuthorizedAccountDeleteLocksReserve()
578+
{
579+
testcase("test authorized account deletion locks delegator reserve");
580+
using namespace jtx;
581+
582+
// Scenario: Alice delegates to Bob. Bob deletes his account.
583+
// Without the bidirectional fix, the Delegate entry would remain in
584+
// Alice's directory, locking her reserve and preventing her from
585+
// deleting her own account. With the fix, Bob's AccountDelete cleans
586+
// up both sides so Alice can freely delete her account afterwards.
587+
Env env(*this);
588+
Account alice{"alice"};
589+
Account bob{"bob"};
590+
Account carol{"carol"};
591+
env.fund(XRP(100000), alice, bob, carol);
592+
env.close();
593+
594+
// Alice creates a delegate entry authorizing Bob
595+
env(delegate::set(alice, bob, {"Payment"}));
596+
env.close();
597+
598+
BEAST_EXPECT(env.closed()->exists(keylet::delegate(alice.id(), bob.id())));
599+
600+
auto const sleAlice = env.closed()->read(keylet::account(alice.id()));
601+
BEAST_EXPECT(sleAlice);
602+
BEAST_EXPECT(sleAlice->getFieldU32(sfOwnerCount) == 1);
603+
604+
for (std::uint32_t i = 0; i < 256; ++i)
605+
env.close();
606+
607+
// Bob deletes his account
608+
auto const bobDeleteFee = drops(env.current()->fees().increment);
609+
env(acctdelete(bob, carol), fee(bobDeleteFee));
610+
env.close();
611+
612+
BEAST_EXPECT(!env.closed()->exists(keylet::account(bob.id())));
613+
614+
// Bob's AccountDelete cleans up the Delegate entry from both sides.
615+
BEAST_EXPECT(!env.closed()->exists(keylet::delegate(alice.id(), bob.id())));
616+
617+
auto const sleAlice2 = env.closed()->read(keylet::account(alice.id()));
618+
BEAST_EXPECT(sleAlice2);
619+
BEAST_EXPECT(sleAlice2->getFieldU32(sfOwnerCount) == 0);
620+
621+
// Alice can now delete her own account — the Delegate entry no longer
622+
// blocks her ownerCount.
623+
auto const aliceDeleteFee = drops(env.current()->fees().increment);
624+
env(acctdelete(alice, carol), fee(aliceDeleteFee));
625+
env.close();
626+
627+
BEAST_EXPECT(!env.closed()->exists(keylet::account(alice.id())));
513628
}
514629

515630
void
@@ -1817,6 +1932,7 @@ class Delegate_test : public beast::unit_test::suite
18171932
testFee();
18181933
testSequence();
18191934
testAccountDelete();
1935+
testAuthorizedAccountDeleteLocksReserve();
18201936
testDelegateTransaction();
18211937
testPaymentGranular(all);
18221938
testTrustSetGranular();

0 commit comments

Comments
 (0)