mirror of
https://github.com/XRPLF/rippled.git
synced 2025-11-20 11:05:54 +00:00
VaultWithdraw destination account bugfix (#5572)
#5224 added (among other things) a `VaultWithdraw` transaction that allows setting the recipient of the withdrawn funds in the `Destination` transaction field. This technically turns this transaction into a payment, and in some respect the implementation does follow payment rules, e.g. enforcement of `lsfRequireDestTag` or `lsfDepositAuth`, or that MPT transfer has destination `MPToken`. However for IOUs, it missed verification that the destination account has a trust line to the asset issuer. Since the default behavior of `accountSendIOU` is to create this trust line (if missing), this is what `VaultWithdraw` currently does. This is incorrect, since the `Destination` might not be interested in holding the asset in question; this basically enables spammy transfers. This change, therefore, removes automatic creation of a trust line to the `Destination` account in `VaultWithdraw`.
This commit is contained in:
Bronek Kozicki
@@ -234,6 +234,28 @@ class Vault_test : public beast::unit_test::suite
|
||||
env(tx, ter{tecNO_PERMISSION});
|
||||
}
|
||||
|
||||
{
|
||||
testcase(prefix + " fail to withdraw to zero destination");
|
||||
auto tx = vault.withdraw(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(1000)});
|
||||
tx[sfDestination] = "0";
|
||||
env(tx, ter(temMALFORMED));
|
||||
}
|
||||
|
||||
{
|
||||
testcase(
|
||||
prefix +
|
||||
" fail to withdraw with tag but without destination");
|
||||
auto tx = vault.withdraw(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(1000)});
|
||||
tx[sfDestinationTag] = "0";
|
||||
env(tx, ter(temMALFORMED));
|
||||
}
|
||||
|
||||
if (!asset.raw().native())
|
||||
{
|
||||
testcase(
|
||||
@@ -1335,6 +1357,7 @@ class Vault_test : public beast::unit_test::suite
|
||||
struct CaseArgs
|
||||
{
|
||||
bool enableClawback = true;
|
||||
bool requireAuth = true;
|
||||
};
|
||||
|
||||
auto testCase = [this](
|
||||
@@ -1356,16 +1379,20 @@ class Vault_test : public beast::unit_test::suite
|
||||
Vault vault{env};
|
||||
|
||||
MPTTester mptt{env, issuer, mptInitNoFund};
|
||||
auto const none = LedgerSpecificFlags(0);
|
||||
mptt.create(
|
||||
{.flags = tfMPTCanTransfer | tfMPTCanLock |
|
||||
(args.enableClawback ? lsfMPTCanClawback
|
||||
: LedgerSpecificFlags(0)) |
|
||||
tfMPTRequireAuth});
|
||||
(args.enableClawback ? tfMPTCanClawback : none) |
|
||||
(args.requireAuth ? tfMPTRequireAuth : none)});
|
||||
PrettyAsset asset = mptt.issuanceID();
|
||||
mptt.authorize({.account = owner});
|
||||
mptt.authorize({.account = issuer, .holder = owner});
|
||||
mptt.authorize({.account = depositor});
|
||||
mptt.authorize({.account = issuer, .holder = depositor});
|
||||
if (args.requireAuth)
|
||||
{
|
||||
mptt.authorize({.account = issuer, .holder = owner});
|
||||
mptt.authorize({.account = issuer, .holder = depositor});
|
||||
}
|
||||
|
||||
env(pay(issuer, depositor, asset(1000)));
|
||||
env.close();
|
||||
|
||||
@@ -1514,6 +1541,100 @@ class Vault_test : public beast::unit_test::suite
|
||||
}
|
||||
});
|
||||
|
||||
testCase(
|
||||
[this](
|
||||
Env& env,
|
||||
Account const& issuer,
|
||||
Account const& owner,
|
||||
Account const& depositor,
|
||||
PrettyAsset const& asset,
|
||||
Vault& vault,
|
||||
MPTTester& mptt) {
|
||||
testcase(
|
||||
"MPT 3rd party without MPToken cannot be withdrawal "
|
||||
"destination");
|
||||
|
||||
auto [tx, keylet] =
|
||||
vault.create({.owner = owner, .asset = asset});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
tx = vault.deposit(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(100)});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
{
|
||||
// Set destination to 3rd party without MPToken
|
||||
Account charlie{"charlie"};
|
||||
env.fund(XRP(1000), charlie);
|
||||
env.close();
|
||||
|
||||
tx = vault.withdraw(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(100)});
|
||||
tx[sfDestination] = charlie.human();
|
||||
env(tx, ter(tecNO_AUTH));
|
||||
}
|
||||
},
|
||||
{.requireAuth = false});
|
||||
|
||||
testCase(
|
||||
[this](
|
||||
Env& env,
|
||||
Account const& issuer,
|
||||
Account const& owner,
|
||||
Account const& depositor,
|
||||
PrettyAsset const& asset,
|
||||
Vault& vault,
|
||||
MPTTester& mptt) {
|
||||
testcase("MPT depositor without MPToken cannot withdraw");
|
||||
|
||||
auto [tx, keylet] =
|
||||
vault.create({.owner = owner, .asset = asset});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
tx = vault.deposit(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(1000)});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
{
|
||||
// Remove depositor's MPToken and withdraw will fail
|
||||
mptt.authorize(
|
||||
{.account = depositor, .flags = tfMPTUnauthorize});
|
||||
env.close();
|
||||
auto const mptoken =
|
||||
env.le(keylet::mptoken(mptt.issuanceID(), depositor));
|
||||
BEAST_EXPECT(mptoken == nullptr);
|
||||
|
||||
tx = vault.withdraw(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(100)});
|
||||
env(tx, ter(tecNO_AUTH));
|
||||
}
|
||||
|
||||
{
|
||||
// Restore depositor's MPToken and withdraw will succeed
|
||||
mptt.authorize({.account = depositor});
|
||||
env.close();
|
||||
|
||||
tx = vault.withdraw(
|
||||
{.depositor = depositor,
|
||||
.id = keylet.key,
|
||||
.amount = asset(100)});
|
||||
env(tx);
|
||||
}
|
||||
},
|
||||
{.requireAuth = false});
|
||||
|
||||
testCase([this](
|
||||
Env& env,
|
||||
Account const& issuer,
|
||||
@@ -1803,6 +1924,7 @@ class Vault_test : public beast::unit_test::suite
|
||||
|
||||
PrettyAsset const asset = issuer["IOU"];
|
||||
env.trust(asset(1000), owner);
|
||||
env.trust(asset(1000), charlie);
|
||||
env(pay(issuer, owner, asset(200)));
|
||||
env(rate(issuer, 1.25));
|
||||
env.close();
|
||||
@@ -2118,6 +2240,79 @@ class Vault_test : public beast::unit_test::suite
|
||||
env.close();
|
||||
});
|
||||
|
||||
testCase([&, this](
|
||||
Env& env,
|
||||
Account const& owner,
|
||||
Account const& issuer,
|
||||
Account const& charlie,
|
||||
auto,
|
||||
Vault& vault,
|
||||
PrettyAsset const& asset,
|
||||
auto&&...) {
|
||||
testcase("IOU no trust line to 3rd party");
|
||||
|
||||
auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
env(vault.deposit(
|
||||
{.depositor = owner, .id = keylet.key, .amount = asset(100)}));
|
||||
env.close();
|
||||
|
||||
Account const erin{"erin"};
|
||||
env.fund(XRP(1000), erin);
|
||||
env.close();
|
||||
|
||||
// Withdraw to 3rd party without trust line
|
||||
auto const tx1 = [&](ripple::Keylet keylet) {
|
||||
auto tx = vault.withdraw(
|
||||
{.depositor = owner,
|
||||
.id = keylet.key,
|
||||
.amount = asset(10)});
|
||||
tx[sfDestination] = erin.human();
|
||||
return tx;
|
||||
}(keylet);
|
||||
env(tx1, ter{tecNO_LINE});
|
||||
});
|
||||
|
||||
testCase([&, this](
|
||||
Env& env,
|
||||
Account const& owner,
|
||||
Account const& issuer,
|
||||
Account const& charlie,
|
||||
auto,
|
||||
Vault& vault,
|
||||
PrettyAsset const& asset,
|
||||
auto&&...) {
|
||||
testcase("IOU no trust line to depositor");
|
||||
|
||||
auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
|
||||
env(tx);
|
||||
env.close();
|
||||
|
||||
// reset limit, so deposit of all funds will delete the trust line
|
||||
env.trust(asset(0), owner);
|
||||
env.close();
|
||||
|
||||
env(vault.deposit(
|
||||
{.depositor = owner, .id = keylet.key, .amount = asset(200)}));
|
||||
env.close();
|
||||
|
||||
auto trustline =
|
||||
env.le(keylet::line(owner, asset.raw().get<Issue>()));
|
||||
BEAST_EXPECT(trustline == nullptr);
|
||||
|
||||
// Withdraw without trust line, will succeed
|
||||
auto const tx1 = [&](ripple::Keylet keylet) {
|
||||
auto tx = vault.withdraw(
|
||||
{.depositor = owner,
|
||||
.id = keylet.key,
|
||||
.amount = asset(10)});
|
||||
return tx;
|
||||
}(keylet);
|
||||
env(tx1);
|
||||
});
|
||||
|
||||
testCase([&, this](
|
||||
Env& env,
|
||||
Account const& owner,
|
||||
|
||||
Reference in New Issue
Block a user