From ad581661e3268dfe41f37db76bbd6e77739e7dc7 Mon Sep 17 00:00:00 2001 From: John Freeman Date: Fri, 6 Dec 2024 15:40:41 -0600 Subject: [PATCH] wip clawback --- .../xrpl/protocol/detail/transactions.macro | 2 +- src/test/app/Vault_test.cpp | 82 +++++++++++++++++-- src/test/jtx/amount.h | 6 ++ src/test/jtx/impl/vault.cpp | 12 +++ src/test/jtx/vault.h | 11 +++ src/xrpld/app/tx/detail/InvariantCheck.cpp | 3 +- src/xrpld/app/tx/detail/VaultClawback.cpp | 62 +++++++++++++- src/xrpld/app/tx/detail/VaultDeposit.cpp | 33 ++++++-- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 28 +++---- src/xrpld/ledger/detail/View.cpp | 26 +++--- 10 files changed, 224 insertions(+), 41 deletions(-) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index aab559526b..14acd432f7 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -488,7 +488,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 65, VaultWithdraw, ({ TRANSACTION(ttVAULT_CLAWBACK, 66, VaultClawback, ({ {sfVaultID, soeREQUIRED}, {sfHolder, soeREQUIRED}, - {sfAmount, soeREQUIRED, soeMPTSupported}, + {sfAmount, soeDEFAULT, soeMPTSupported}, })) /** This system-generated transaction type is used to update the status of the various amendments. diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 12494939c8..b309a21a2f 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -61,7 +61,16 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.deposit( {.depositor = depositor, .id = keylet.key, - .amount = asset(100)}); + .amount = asset(50)}); + env(tx); + } + + { + testcase("deposit non-zero amount again"); + auto tx = vault.deposit( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(50)}); env(tx); } @@ -125,14 +134,38 @@ class Vault_test : public beast::unit_test::suite env(tx); } + if (!asset.raw().native()) + { + testcase("fail to clawback because wrong issuer"); + auto tx = vault.clawback( + {.issuer = owner, + .id = keylet.key, + .holder = depositor, + .amount = asset(50)}); + env(tx, ter(tecNO_PERMISSION)); + } + + { + testcase("clawback"); + auto code = + asset.raw().native() ? ter(tecNO_PERMISSION) : ter(tesSUCCESS); + auto tx = vault.clawback( + {.issuer = issuer, + .id = keylet.key, + .holder = depositor, + .amount = asset(50)}); + env(tx, code); + } + // TODO: redeem. { testcase("withdraw non-zero assets"); + auto number = asset.raw().native() ? 200 : 150; auto tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, - .amount = asset(200)}); + .amount = asset(number)}); env(tx); } @@ -284,17 +317,55 @@ class Vault_test : public beast::unit_test::suite auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); env(tx, ter(tecLOCKED)); } + } - AND_THEN("create"); + TEST_CASE(WithMPT) + { + using namespace test::jtx; + Env env{*this}; + Account issuer{"issuer"}; + Account owner{"owner"}; + Account depositor{"depositor"}; + env.fund(XRP(1000), issuer, owner, depositor); + env.close(); + auto vault = env.vault(); + + MPTTester mptt{env, issuer, {.fund = false}}; mptt.create({.flags = tfMPTCanTransfer | tfMPTCanLock}); - Asset asset = mptt.issuanceID(); + PrettyAsset asset = mptt.issuanceID(); + mptt.authorize({.account = depositor}); + env(pay(issuer, depositor, asset(1000))); + env.close(); SUBCASE("global lock") { - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); mptt.set({.account = issuer, .flags = tfMPTLock}); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); env(tx, ter(tecLOCKED)); } + + SUBCASE("deposit non-zero amount") + { + 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(); + // Check that the OutstandingAmount field of MPTIssuance + // accounts for the issued shares. + auto v = env.le(keylet); + BEAST_EXPECT(v); + MPTID share = (*v)[sfMPTokenIssuanceID]; + auto issuance = env.le(keylet::mptIssuance(share)); + BEAST_EXPECT(issuance); + Number outstandingShares = issuance->at(sfOutstandingAmount); + BEAST_EXPECT(outstandingShares > 0); + BEAST_EXPECT(outstandingShares == 100); + } } public: @@ -305,6 +376,7 @@ public: EXECUTE(CreateFailXRP); EXECUTE(CreateFailIOU); EXECUTE(CreateFailMPT); + EXECUTE(WithMPT); } }; diff --git a/src/test/jtx/amount.h b/src/test/jtx/amount.h index 1cec95730a..5d7cb7fac1 100644 --- a/src/test/jtx/amount.h +++ b/src/test/jtx/amount.h @@ -179,6 +179,12 @@ public: { } + Asset const& + raw() const + { + return asset_; + } + operator Asset const&() const { return asset_; diff --git a/src/test/jtx/impl/vault.cpp b/src/test/jtx/impl/vault.cpp index 31cbbc1f54..81325844a6 100644 --- a/src/test/jtx/impl/vault.cpp +++ b/src/test/jtx/impl/vault.cpp @@ -86,6 +86,18 @@ Vault::withdraw(WithdrawArgs const& args) return jv; } +Json::Value +Vault::clawback(ClawbackArgs const& args) +{ + Json::Value jv; + jv[jss::TransactionType] = jss::VaultClawback; + jv[jss::Account] = args.issuer.human(); + jv[jss::VaultID] = to_string(args.id); + jv[jss::Holder] = args.holder.human(); + jv[jss::Amount] = to_json(args.amount); + return jv; +} + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/vault.h b/src/test/jtx/vault.h index 5d26b84b92..d070e3451f 100644 --- a/src/test/jtx/vault.h +++ b/src/test/jtx/vault.h @@ -89,6 +89,17 @@ struct Vault Json::Value withdraw(WithdrawArgs const& args); + + struct ClawbackArgs + { + Account issuer; + uint256 id; + Account holder; + STAmount amount; + }; + + Json::Value + clawback(ClawbackArgs const& args); }; } // namespace jtx diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 299d0b4650..48e8e950b7 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1037,7 +1037,7 @@ ValidMPTIssuance::finalize( return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1; } - if (tx.getTxnType() == ttMPTOKEN_AUTHORIZE) + if (tx.getTxnType() == ttMPTOKEN_AUTHORIZE || tx.getTxnType() == ttVAULT_DEPOSIT) { bool const submittedByIssuer = tx.isFieldPresent(sfHolder); @@ -1064,6 +1064,7 @@ ValidMPTIssuance::finalize( } else if ( !submittedByIssuer && + (tx.getTxnType() != ttVAULT_DEPOSIT) && (mptokensCreated_ + mptokensDeleted_ != 1)) { // if the holder submitted this tx, then a mptoken must be diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 26a8870462..9ef7022b9a 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -19,6 +19,8 @@ #include +#include +#include #include #include #include @@ -52,10 +54,68 @@ VaultClawback::preclaim(PreclaimContext const& ctx) TER VaultClawback::doApply() { - auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID])); + auto const& tx = ctx_.tx; + auto const vault = view().peek(keylet::vault(tx[sfVaultID])); if (!vault) return tecOBJECT_NOT_FOUND; + Asset const asset = (*vault)[sfAsset]; + if (asset.native()) + // Cannot clawback XRP. + return tecNO_PERMISSION; + else if (asset.getIssuer() != account_) + // Only issuers can clawback. + return tecNO_PERMISSION; + + AccountID holder = tx[sfHolder]; + STAmount const amount = tx[sfAmount]; + + if (asset != amount.asset()) + return tecWRONG_ASSET; + + STAmount assets, shares; + if (amount == beast::zero) + { + Asset share = *(*vault)[sfMPTokenIssuanceID]; + shares = accountHolds( + view(), + holder, + share, + FreezeHandling::fhIGNORE_FREEZE, + AuthHandling::ahIGNORE_AUTH, + j_); + assets = sharesToAssetsWithdraw(view(), vault, shares); + } + else + { + assets = amount; + shares = assetsToSharesWithdraw(view(), vault, assets); + } + + // Clamp to maximum. + Number maxAssets = *vault->at(sfAssetAvailable); + if (assets > maxAssets) + { + assets = maxAssets; + shares = assetsToSharesWithdraw(view(), vault, assets); + } + + if (shares == beast::zero) + return tecINSUFFICIENT_FUNDS; + + vault->at(sfAssetTotal) -= assets; + vault->at(sfAssetAvailable) -= assets; + view().update(vault); + + // auto const& vaultAccount = vault->at(sfAccount); + // // Transfer shares from holder to vault. + // if (auto ter = accountSend(view(), holder, vaultAccount, shares, j_)) + // return ter; + + // // Transfer assets from vault to issuer. + // if (auto ter = accountSend(view(), vaultAccount, account_, assets, j_)) + // return ter; + return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 327c70adbe..810d97d3f6 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -60,9 +61,9 @@ VaultDeposit::doApply() // TODO: Check credentials. if (vault->getFlags() & lsfVaultPrivate) - return tecNO_AUTH; + return tecNO_PERMISSION; - auto assets = ctx_.tx[sfAmount]; + auto const assets = ctx_.tx[sfAmount]; Asset const& asset = vault->at(sfAsset); if (assets.asset() != asset) return tecWRONG_ASSET; @@ -78,11 +79,34 @@ VaultDeposit::doApply() return tecINSUFFICIENT_FUNDS; } + // Make sure the depositor can hold shares. + auto share = (*vault)[sfMPTokenIssuanceID]; + auto canHold = requireAuth(view(), MPTIssue(share), account_); + if (canHold == tecNO_LINE) + { + if (auto ter = MPTokenAuthorize::authorize( + view(), + j_, + {.priorBalance = mPriorBalance, + .mptIssuanceID = share, + .accountID = account_})) + return ter; + } + else if (canHold != tesSUCCESS) + { + return canHold; + } + + // Compute exchange before transferring any amounts. + auto const shares = assetsToSharesDeposit(view(), vault, assets); + XRPL_ASSERT(shares.asset() != assets.asset(), "do not mix up assets and shares"); + vault->at(sfAssetTotal) += assets; vault->at(sfAssetAvailable) += assets; + view().update(vault); // A deposit must not push the vault over its limit. - auto maximum = *vault->at(sfAssetMaximum); + auto const maximum = *vault->at(sfAssetMaximum); if (maximum != 0 && *vault->at(sfAssetTotal) > maximum) return tecLIMIT_EXCEEDED; @@ -92,12 +116,9 @@ VaultDeposit::doApply() return ter; // Transfer shares from vault to depositor. - auto shares = assetsToSharesDeposit(view(), vault, assets); if (auto ter = accountSend(view(), vaultAccount, account_, shares, j_)) return ter; - view().update(vault); - return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 72e303b5d3..49a36020ff 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -59,7 +59,7 @@ VaultWithdraw::doApply() // TODO: Check credentials. if (vault->getFlags() & lsfVaultPrivate) - return tecNO_AUTH; + return tecNO_PERMISSION; auto amount = ctx_.tx[sfAmount]; @@ -81,17 +81,18 @@ VaultWithdraw::doApply() return tecWRONG_ASSET; } - // The depositor must have enough shares. - if (accountHolds( - view(), - account_, - shares.asset(), - FreezeHandling::fhZERO_IF_FROZEN, - AuthHandling::ahZERO_IF_UNAUTHORIZED, - j_) < shares) - { - return tecINSUFFICIENT_FUNDS; - } + // // The depositor must have enough shares. + // // TODO: accountFunds throws here. Why? + // if (accountHolds( + // view(), + // account_, + // shares.asset(), + // FreezeHandling::fhZERO_IF_FROZEN, + // AuthHandling::ahZERO_IF_UNAUTHORIZED, + // j_) < shares) + // { + // return tecINSUFFICIENT_FUNDS; + // } // The vault must have enough assets on hand. // The vault may hold assets that it has already pledged. @@ -103,6 +104,7 @@ VaultWithdraw::doApply() vault->at(sfAssetTotal) -= assets; vault->at(sfAssetAvailable) -= assets; + view().update(vault); auto const& vaultAccount = vault->at(sfAccount); // Transfer shares from depositor to vault. @@ -113,8 +115,6 @@ VaultWithdraw::doApply() if (auto ter = accountSend(view(), vaultAccount, account_, assets, j_)) return ter; - view().update(vault); - return tesSUCCESS; } diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index df721208c1..abc4669c9c 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -2044,7 +2044,7 @@ requireAuth( // if account has no MPToken, fail if (!sleToken) - return tecNO_AUTH; + return tecNO_LINE; // mptoken must be authorized if issuance enabled requireAuth if (sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth && @@ -2245,12 +2245,12 @@ assetsToSharesDeposit( { assert(assets.asset() == vault->at(sfAsset)); Number assetTotal = *vault->at(sfAssetTotal); + STAmount shares{vault->at(sfMPTokenIssuanceID), static_cast(assets)}; if (assetTotal == 0) - return assets; + return shares; Number shareTotal = getShareTotal(view, vault); - auto shares = shareTotal * (assets / assetTotal); - STAmount amount{vault->at(sfMPTokenIssuanceID), shares}; - return amount; + shares = shareTotal * (assets / assetTotal); + return shares; } [[nodiscard]] STAmount @@ -2262,13 +2262,13 @@ assetsToSharesWithdraw( assert(assets.asset() == vault->at(sfAsset)); Number assetTotal = vault->at(sfAssetTotal); assetTotal -= vault->at(sfLossUnrealized); - STAmount amount{vault->at(sfMPTokenIssuanceID)}; + STAmount shares{vault->at(sfMPTokenIssuanceID)}; if (assetTotal == 0) - return amount; + return shares; Number shareTotal = getShareTotal(view, vault); - amount = shareTotal * (assets / assetTotal); + shares = shareTotal * (assets / assetTotal); // TODO: Limit by withdrawal policy? - return amount; + return shares; } [[nodiscard]] STAmount @@ -2280,13 +2280,13 @@ sharesToAssetsWithdraw( assert(shares.asset() == vault->at(sfMPTokenIssuanceID)); Number assetTotal = vault->at(sfAssetTotal); assetTotal -= vault->at(sfLossUnrealized); - STAmount amount{vault->at(sfAsset)}; + STAmount assets{vault->at(sfAsset)}; if (assetTotal == 0) - return amount; + return assets; Number shareTotal = getShareTotal(view, vault); - amount = assetTotal * (shares / shareTotal); + assets = assetTotal * (shares / shareTotal); // TODO: Limit by withdrawal policy? - return amount; + return assets; } } // namespace ripple