Skip to content

Commit 37c377a

Browse files
dangell7dangell7
authored
Fix: EscrowTokenV1 (#5571)
* resolves an accounting inconsistency in MPT escrows where transfer fees were not properly handled when unlocking escrowed tokens.
1 parent bd182c0 commit 37c377a

File tree

5 files changed

+113
-10
lines changed

5 files changed

+113
-10
lines changed

‎include/xrpl/protocol/detail/features.macro‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
// If you add an amendment here, then do not forget to increment `numFeatures`
3333
// in include/xrpl/protocol/Feature.h.
3434

35+
XRPL_FIX (TokenEscrowV1, Supported::yes, VoteBehavior::DefaultNo)
3536
XRPL_FIX (DelegateV1_1, Supported::no, VoteBehavior::DefaultNo)
3637
XRPL_FIX (PriceOracleOrder, Supported::no, VoteBehavior::DefaultNo)
3738
XRPL_FIX (MPTDeliveredAmount, Supported::no, VoteBehavior::DefaultNo)

‎src/test/app/EscrowToken_test.cpp‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,6 +3501,10 @@ struct EscrowToken_test : public beast::unit_test::suite
35013501
BEAST_EXPECT(
35023502
transferRate.value == std::uint32_t(1'000'000'000 * 1.25));
35033503

3504+
BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 125);
3505+
BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == 125);
3506+
BEAST_EXPECT(env.balance(gw, MPT) == MPT(20'000));
3507+
35043508
// bob can finish escrow
35053509
env(escrow::finish(bob, alice, seq1),
35063510
escrow::condition(escrow::cb1),
@@ -3510,6 +3514,15 @@ struct EscrowToken_test : public beast::unit_test::suite
35103514

35113515
BEAST_EXPECT(env.balance(alice, MPT) == preAlice - delta);
35123516
BEAST_EXPECT(env.balance(bob, MPT) == MPT(10'100));
3517+
3518+
auto const escrowedWithFix =
3519+
env.current()->rules().enabled(fixTokenEscrowV1) ? 0 : 25;
3520+
auto const outstandingWithFix =
3521+
env.current()->rules().enabled(fixTokenEscrowV1) ? MPT(19'975)
3522+
: MPT(20'000);
3523+
BEAST_EXPECT(mptEscrowed(env, alice, MPT) == escrowedWithFix);
3524+
BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == escrowedWithFix);
3525+
BEAST_EXPECT(env.balance(gw, MPT) == outstandingWithFix);
35133526
}
35143527

35153528
// test locked rate: cancel
@@ -3554,6 +3567,60 @@ struct EscrowToken_test : public beast::unit_test::suite
35543567

35553568
BEAST_EXPECT(env.balance(alice, MPT) == preAlice);
35563569
BEAST_EXPECT(env.balance(bob, MPT) == preBob);
3570+
BEAST_EXPECT(env.balance(gw, MPT) == MPT(20'000));
3571+
BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 0);
3572+
BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == 0);
3573+
}
3574+
3575+
// test locked rate: issuer is destination
3576+
{
3577+
Env env{*this, features};
3578+
auto const baseFee = env.current()->fees().base;
3579+
auto const alice = Account("alice");
3580+
auto const bob = Account("bob");
3581+
auto const gw = Account("gw");
3582+
3583+
MPTTester mptGw(env, gw, {.holders = {alice, bob}});
3584+
mptGw.create(
3585+
{.transferFee = 25000,
3586+
.ownerCount = 1,
3587+
.holderCount = 0,
3588+
.flags = tfMPTCanEscrow | tfMPTCanTransfer});
3589+
mptGw.authorize({.account = alice});
3590+
mptGw.authorize({.account = bob});
3591+
auto const MPT = mptGw["MPT"];
3592+
env(pay(gw, alice, MPT(10'000)));
3593+
env(pay(gw, bob, MPT(10'000)));
3594+
env.close();
3595+
3596+
// alice can create escrow w/ xfer rate
3597+
auto const preAlice = env.balance(alice, MPT);
3598+
auto const seq1 = env.seq(alice);
3599+
auto const delta = MPT(125);
3600+
env(escrow::create(alice, gw, MPT(125)),
3601+
escrow::condition(escrow::cb1),
3602+
escrow::finish_time(env.now() + 1s),
3603+
fee(baseFee * 150));
3604+
env.close();
3605+
auto const transferRate = escrow::rate(env, alice, seq1);
3606+
BEAST_EXPECT(
3607+
transferRate.value == std::uint32_t(1'000'000'000 * 1.25));
3608+
3609+
BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 125);
3610+
BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == 125);
3611+
BEAST_EXPECT(env.balance(gw, MPT) == MPT(20'000));
3612+
3613+
// bob can finish escrow
3614+
env(escrow::finish(gw, alice, seq1),
3615+
escrow::condition(escrow::cb1),
3616+
escrow::fulfillment(escrow::fb1),
3617+
fee(baseFee * 150));
3618+
env.close();
3619+
3620+
BEAST_EXPECT(env.balance(alice, MPT) == preAlice - delta);
3621+
BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 0);
3622+
BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == 0);
3623+
BEAST_EXPECT(env.balance(gw, MPT) == MPT(19'875));
35573624
}
35583625
}
35593626

@@ -3878,6 +3945,7 @@ struct EscrowToken_test : public beast::unit_test::suite
38783945
FeatureBitset const all{testable_amendments()};
38793946
testIOUWithFeats(all);
38803947
testMPTWithFeats(all);
3948+
testMPTWithFeats(all - fixTokenEscrowV1);
38813949
}
38823950
};
38833951

‎src/xrpld/app/tx/detail/Escrow.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,8 +1007,13 @@ escrowUnlockApplyHelper<MPTIssue>(
10071007
// compute balance to transfer
10081008
finalAmt = amount.value() - xferFee;
10091009
}
1010-
1011-
return rippleUnlockEscrowMPT(view, sender, receiver, finalAmt, journal);
1010+
return rippleUnlockEscrowMPT(
1011+
view,
1012+
sender,
1013+
receiver,
1014+
finalAmt,
1015+
view.rules().enabled(fixTokenEscrowV1) ? amount : finalAmt,
1016+
journal);
10121017
}
10131018

10141019
TER

‎src/xrpld/ledger/View.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,8 @@ rippleUnlockEscrowMPT(
719719
ApplyView& view,
720720
AccountID const& uGrantorID,
721721
AccountID const& uGranteeID,
722-
STAmount const& saAmount,
722+
STAmount const& netAmount,
723+
STAmount const& grossAmount,
723724
beast::Journal j);
724725

725726
/** Calls static accountSendIOU if saAmount represents Issue.

‎src/xrpld/ledger/detail/View.cpp‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3006,11 +3006,17 @@ rippleUnlockEscrowMPT(
30063006
ApplyView& view,
30073007
AccountID const& sender,
30083008
AccountID const& receiver,
3009-
STAmount const& amount,
3009+
STAmount const& netAmount,
3010+
STAmount const& grossAmount,
30103011
beast::Journal j)
30113012
{
3012-
auto const issuer = amount.getIssuer();
3013-
auto const mptIssue = amount.get<MPTIssue>();
3013+
if (!view.rules().enabled(fixTokenEscrowV1))
3014+
XRPL_ASSERT(
3015+
netAmount == grossAmount,
3016+
"ripple::rippleUnlockEscrowMPT : netAmount == grossAmount");
3017+
3018+
auto const& issuer = netAmount.getIssuer();
3019+
auto const& mptIssue = netAmount.get<MPTIssue>();
30143020
auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
30153021
auto sleIssuance = view.peek(mptID);
30163022
if (!sleIssuance)
@@ -3031,7 +3037,7 @@ rippleUnlockEscrowMPT(
30313037
} // LCOV_EXCL_STOP
30323038

30333039
auto const locked = sleIssuance->getFieldU64(sfLockedAmount);
3034-
auto const redeem = amount.mpt().value();
3040+
auto const redeem = grossAmount.mpt().value();
30353041

30363042
// Underflow check for subtraction
30373043
if (!canSubtract(
@@ -3064,7 +3070,7 @@ rippleUnlockEscrowMPT(
30643070
} // LCOV_EXCL_STOP
30653071

30663072
auto current = sle->getFieldU64(sfMPTAmount);
3067-
auto delta = amount.mpt().value();
3073+
auto delta = netAmount.mpt().value();
30683074

30693075
// Overflow check for addition
30703076
if (!canAdd(STAmount(mptIssue, current), STAmount(mptIssue, delta)))
@@ -3082,7 +3088,7 @@ rippleUnlockEscrowMPT(
30823088
{
30833089
// Decrease the Issuance OutstandingAmount
30843090
auto const outstanding = sleIssuance->getFieldU64(sfOutstandingAmount);
3085-
auto const redeem = amount.mpt().value();
3091+
auto const redeem = netAmount.mpt().value();
30863092

30873093
// Underflow check for subtraction
30883094
if (!canSubtract(
@@ -3126,7 +3132,7 @@ rippleUnlockEscrowMPT(
31263132
} // LCOV_EXCL_STOP
31273133

31283134
auto const locked = sle->getFieldU64(sfLockedAmount);
3129-
auto const delta = amount.mpt().value();
3135+
auto const delta = grossAmount.mpt().value();
31303136

31313137
// Underflow check for subtraction
31323138
if (!canSubtract(STAmount(mptIssue, locked), STAmount(mptIssue, delta)))
@@ -3144,6 +3150,28 @@ rippleUnlockEscrowMPT(
31443150
sle->setFieldU64(sfLockedAmount, newLocked);
31453151
view.update(sle);
31463152
}
3153+
3154+
// Note: The gross amount is the amount that was locked, the net
3155+
// amount is the amount that is being unlocked. The difference is the fee
3156+
// that was charged for the transfer. If this difference is greater than
3157+
// zero, we need to update the outstanding amount.
3158+
auto const diff = grossAmount.mpt().value() - netAmount.mpt().value();
3159+
if (diff != 0)
3160+
{
3161+
auto const outstanding = sleIssuance->getFieldU64(sfOutstandingAmount);
3162+
// Underflow check for subtraction
3163+
if (!canSubtract(
3164+
STAmount(mptIssue, outstanding), STAmount(mptIssue, diff)))
3165+
{ // LCOV_EXCL_START
3166+
JLOG(j.error())
3167+
<< "rippleUnlockEscrowMPT: insufficient outstanding amount for "
3168+
<< mptIssue.getMptID() << ": " << outstanding << " < " << diff;
3169+
return tecINTERNAL;
3170+
} // LCOV_EXCL_STOP
3171+
3172+
sleIssuance->setFieldU64(sfOutstandingAmount, outstanding - diff);
3173+
view.update(sleIssuance);
3174+
}
31473175
return tesSUCCESS;
31483176
}
31493177

0 commit comments

Comments
 (0)