diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index c4c2f9227b..d2da173345 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -339,17 +339,36 @@ Payment::checkGranularSemantics( bool const accountIsHolder = accountIsLow ? rawBalance > beast::kZero : rawBalance < beast::kZero; + bool const mayIssue = + heldGranularPermissions.contains(PaymentMint) && destLimit > beast::kZero; + // PaymentMint requires the destination to be the holder and the account to be the // issuer. destLimit > 0: destination is willing to hold account's IOUs (account is the // issuer). !accountIsHolder: DirectStepI will issue, not redeem. - if (heldGranularPermissions.contains(PaymentMint) && destLimit > beast::kZero && - !accountIsHolder) + if (mayIssue && !accountIsHolder) return tesSUCCESS; // PaymentBurn requires the source account to be the holder and the destination to be // the issuer. accountIsHolder: DirectStepI will redeem, not issue. if (heldGranularPermissions.contains(PaymentBurn) && accountIsHolder) - return tesSUCCESS; + { + if (view.rules().enabled(fixCleanup3_4_0)) + { + // Redeeming stops at the balance held; beyond that the payment engine + // crosses zero and issues the account's own IOUs, which is a mint. So with + // only PaymentBurn we must check the amount against the balance held. The + // granular template forbids sfPaths, tfPartialPayment and a cross-asset + // sfSendMax, so this is a single direct step, sfAmount is what the + // trustline is debited. + STAmount const held = accountIsLow ? rawBalance : -rawBalance; + if (dstAmount <= held || mayIssue) + return tesSUCCESS; + } + else + { + return tesSUCCESS; + } + } return terNO_DELEGATE_PERMISSION; }); diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index 3ff90c2a8f..5414269333 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -1144,6 +1144,88 @@ class Delegate_test : public beast::unit_test::Suite env.require(Balance(gw, aliceUSD(-20))); } + // PaymentBurn must not exceed the balance the account holds. Redeeming past + // zero makes the payment engine issue the account's own IOUs, which is a mint. + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const gw{"gateway"}; + auto const gwUSD = gw["USD"]; + auto const aliceUSD = alice["USD"]; + + env.fund(XRP(10000), alice, bob, gw); + env.trust(gwUSD(200), alice); + env.close(); + + env(pay(gw, alice, gwUSD(50))); + env.close(); + env.require(Balance(alice, gwUSD(50))); + + // gw accepts alice-issued USD, so the engine has issuing liquidity + // available once the trustline reaches zero. + env(trust(gw, aliceUSD(200))); + env.close(); + + env(delegate::set(alice, bob, {"PaymentBurn"})); + env.close(); + + if (!features[fixCleanup3_4_0]) + { + // Pre-fixCleanup3_4_0: the balance direction alone authorizes the payment, so it + // redeems alice's 50 and then mints 50 alice-issued USD. + env(pay(alice, gw, gwUSD(100)), delegate::As(bob)); + env.require(Balance(alice, gwUSD(-50))); + env.require(Balance(gw, aliceUSD(50))); + } + else + { + // Post-fixCleanup3_4_0: Rejected because it exceeds what alice holds. + env(pay(alice, gw, gwUSD(100)), delegate::As(bob), Ter(terNO_DELEGATE_PERMISSION)); + env.require(Balance(alice, gwUSD(50))); + env.require(Balance(gw, aliceUSD(-50))); + + // Allowed because it is less than what alice holds. + env(pay(alice, gw, gwUSD(20)), delegate::As(bob)); + env.require(Balance(alice, gwUSD(30))); + env.close(); + + // Exactly what alice holds: allowed, and settles at zero. + env(pay(alice, gw, gwUSD(30)), delegate::As(bob)); + env.require(Balance(alice, gwUSD(0))); + env.close(); + + // Nothing left to burn: rejected. + env(pay(alice, gw, gwUSD(1)), delegate::As(bob), Ter(terNO_DELEGATE_PERMISSION)); + env.require(Balance(gw, aliceUSD(0))); + } + } + + // A delegate holding both PaymentMint and PaymentBurn may cross zero. + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const gw{"gateway"}; + auto const gwUSD = gw["USD"]; + auto const aliceUSD = alice["USD"]; + + env.fund(XRP(10000), alice, bob, gw); + env.trust(gwUSD(200), alice); + env.close(); + + env(pay(gw, alice, gwUSD(50))); + env(trust(gw, aliceUSD(200))); + env.close(); + + env(delegate::set(alice, bob, {"PaymentBurn", "PaymentMint"})); + env.close(); + + env(pay(alice, gw, gwUSD(100)), delegate::As(bob)); + env.require(Balance(alice, gwUSD(-50))); + env.require(Balance(gw, aliceUSD(50))); + } + // Test invalid fields or flags not allowed in granular permission template { Env env(*this, features); @@ -2916,6 +2998,7 @@ class Delegate_test : public beast::unit_test::Suite testAccountDelete(); testDelegateTransaction(); testPaymentGranular(all); + testPaymentGranular(all - fixCleanup3_4_0); testTrustSetGranular(); testAccountSetGranular(); testMPTokenIssuanceSetGranular();