diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index 6c6ef5e369..4ab92c4547 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -291,6 +291,8 @@ constexpr std::uint32_t const tfLoanImpair = 0x00020000; constexpr std::uint32_t const tfLoanUnimpair = 0x00040000; constexpr std::uint32_t const tfLoanManageMask = ~(tfUniversal | tfLoanDefault | tfLoanImpair | tfLoanUnimpair); +constexpr std::uint32_t const tfVaultDonate = 0x00010000; +constexpr std::uint32_t const tfVaultDepositMask = ~(tfUniversal | tfVaultDonate); // clang-format on } // namespace xrpl diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 7558c951b0..e88dc0e94e 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -3375,6 +3376,45 @@ class Invariants_test : public beast::unit_test::suite precloseXrp, TxAccount::A2); + doInvariantCheck( + {"donation must not change depositor shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + return adjust(ac.view(), keylet, args(A2.id(), 10, [&](Adjustments& sample) { + sample.accountShares->amount = 10; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { + tx[sfAmount] = XRPAmount(10); + tx[sfFlags] = tfVaultDonate; + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"donation must not change vault shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + return adjust(ac.view(), keylet, args(A2.id(), 10, [&](Adjustments& sample) { + sample.sharesTotal = 10; + sample.accountShares = std::nullopt; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { + tx[sfAmount] = XRPAmount(10); + tx[sfFlags] = tfVaultDonate; + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + testcase << "Vault withdrawal"; doInvariantCheck( {"withdrawal must change vault balance"}, diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index cf86ab6033..74fa95aa48 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -969,14 +969,13 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this]( - std::function test) { + auto testCase = [this](std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account issuer{"issuer"}; Account owner{"owner"}; @@ -1256,14 +1255,13 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - auto testCase = [this]( - std::function test) { + auto testCase = [this](std::function test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account issuer{"issuer"}; Account owner{"owner"}; @@ -4992,6 +4990,153 @@ class Vault_test : public beast::unit_test::suite } } + void + testVaultDepositDonate() + { + using namespace test::jtx; + std::string const prefix = "VaultDeposit donate"; + + Env env{*this}; + Vault vault{env}; + + auto const vaultShareBalance = [&](Keylet const& vaultKeylet) { + auto const sleVault = env.le(vaultKeylet); + BEAST_EXPECT(sleVault != nullptr); + + auto const sleIssuance = env.le(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + BEAST_EXPECT(sleIssuance != nullptr); + + return sleIssuance->at(sfOutstandingAmount); + }; + + auto const vaultAssetBalance = [&](Keylet const& vaultKeylet) { + auto const sleVault = env.le(vaultKeylet); + BEAST_EXPECT(sleVault != nullptr); + + return std::make_pair(sleVault->at(sfAssetsAvailable), sleVault->at(sfAssetsTotal)); + }; + + Account const owner{"owner"}; + Account const depositor{"depositor"}; + env.fund(XRP(1'000'000), owner, depositor); + env.close(); + + auto const depositAmount = XRP(10); + + auto const [tx, keylet] = vault.create({.owner = owner, .asset = xrpIssue()}); + env(tx, ter(tesSUCCESS), THISLINE); + env.close(); + + // With fixLendingProtocolV1_1 disabled, donations fail + { + testcase(prefix + " fails with fixLendingProtocolV1_1 disabled"); + env.disableFeature(fixLendingProtocolV1_1); + auto const tx = vault.deposit({ + .depositor = owner, + .id = keylet.key, + .amount = depositAmount, + .flags = tfVaultDonate, + }); + env(tx, ter{temINVALID_FLAG}, THISLINE); + env.enableFeature(fixLendingProtocolV1_1); + env.close(); + } + + // Donation is not allowed to an empty vault + { + testcase(prefix + " fails to an empty vault"); + auto const tx = vault.deposit({ + .depositor = owner, + .id = keylet.key, + .amount = depositAmount, + .flags = tfVaultDonate, + }); + env(tx, ter{tecNO_PERMISSION}, THISLINE); + env.close(); + } + + // Further unit tests require assets in the Vault + env(vault.deposit({ + .depositor = depositor, + .id = keylet.key, + .amount = depositAmount, + }), + ter{tesSUCCESS}, + THISLINE); + env.close(); + + // Donation is not allowed by a non-owner + { + testcase(prefix + " fails by a non-owner"); + auto const tx = vault.deposit({ + .depositor = depositor, + .id = keylet.key, + .amount = depositAmount, + .flags = tfVaultDonate, + }); + env(tx, ter{tecNO_PERMISSION}, THISLINE); + env.close(); + } + + // Donation cannot exceed assets maximum + { + testcase(prefix + " cannot exceed assets maximum"); + auto tx = vault.set({ + .owner = owner, + .id = keylet.key, + }); + tx[sfAssetsMaximum] = XRP(30).number(); + env(tx, ter{tesSUCCESS}, THISLINE); + + tx = vault.deposit({ + .depositor = owner, + .id = keylet.key, + .amount = depositAmount + XRP(30), + .flags = tfVaultDonate, + }); + + env(tx, ter{tecLIMIT_EXCEEDED}, THISLINE); + env.close(); + } + + { + testcase(prefix + " succeeds"); + auto const shareBalance = vaultShareBalance(keylet); + auto const [assetsAvailable, assetsTotal] = vaultAssetBalance(keylet); + + auto tx = vault.deposit({ + .depositor = owner, + .id = keylet.key, + .amount = depositAmount, + .flags = tfVaultDonate, + }); + env(tx, ter{tesSUCCESS}, THISLINE); + env.close(); + + auto const shareBalanceAfterDeposit = vaultShareBalance(keylet); + auto const [assetsAvailableAfterDeposit, assetsTotalAfterDeposit] = vaultAssetBalance(keylet); + + BEAST_EXPECT(shareBalance == shareBalanceAfterDeposit); + BEAST_EXPECT(assetsAvailable + depositAmount.number() == assetsAvailableAfterDeposit); + BEAST_EXPECT(assetsTotal + depositAmount.number() == assetsTotalAfterDeposit); + + auto const sleVault = env.le(keylet); + if (!BEAST_EXPECT(sleVault)) + return; + + // The depositor can withdraw their assets and the donated amount + Asset shareAsset(sleVault->at(sfShareMPTID)); + tx = vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = shareAsset(shareBalance)}); + env(tx, ter{tesSUCCESS}, THISLINE); + + auto const shareBalanceAfterWithdraw = vaultShareBalance(keylet); + auto const [assetsAvailableAfterWithdraw, assetsTotalAfterWithdraw] = vaultAssetBalance(keylet); + BEAST_EXPECT(shareBalanceAfterWithdraw == 0); + BEAST_EXPECT(assetsAvailableAfterWithdraw == 0); + BEAST_EXPECT(assetsTotalAfterWithdraw == 0); + } + } + public: void run() override @@ -5013,6 +5158,7 @@ public: testVaultClawbackBurnShares(); testVaultClawbackAssets(); testAssetsMaximum(); + testVaultDepositDonate(); } }; diff --git a/src/test/jtx/impl/vault.cpp b/src/test/jtx/impl/vault.cpp index 90250aece0..f443fdbac8 100644 --- a/src/test/jtx/impl/vault.cpp +++ b/src/test/jtx/impl/vault.cpp @@ -52,6 +52,9 @@ Vault::deposit(DepositArgs const& args) jv[jss::Account] = args.depositor.human(); jv[sfVaultID] = to_string(args.id); jv[jss::Amount] = to_json(args.amount); + if (args.flags) + jv[jss::Flags] = *args.flags; + return jv; } diff --git a/src/test/jtx/vault.h b/src/test/jtx/vault.h index 65a5706354..f01323053e 100644 --- a/src/test/jtx/vault.h +++ b/src/test/jtx/vault.h @@ -55,6 +55,7 @@ struct Vault Account depositor; uint256 id; STAmount amount; + std::optional flags{}; }; Json::Value diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 24a37270ce..48e906cf6f 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -2775,10 +2776,14 @@ ValidVault::finalize(STTx const& tx, TER const ret, XRPAmount const fee, ReadVie return std::nullopt; }(); - if (!beforeShares && - (tx.getTxnType() == ttVAULT_DEPOSIT || // - tx.getTxnType() == ttVAULT_WITHDRAW || // - tx.getTxnType() == ttVAULT_CLAWBACK)) + bool const isDonate = !view.rules().enabled(fixLendingProtocolV1_1) || tx.isFlag(tfVaultDonate); + bool const shouldUpdateShares = + // Vault Asset donation is the only operation that can succeed without updating shares + ((tx.getTxnType() == ttVAULT_DEPOSIT && !isDonate) || // + tx.getTxnType() == ttVAULT_WITHDRAW || // + tx.getTxnType() == ttVAULT_CLAWBACK); + + if (!beforeShares && shouldUpdateShares) { JLOG(j.fatal()) << "Invariant failed: vault operation succeeded " "without updating shares"; @@ -2845,199 +2850,210 @@ ValidVault::finalize(STTx const& tx, TER const ret, XRPAmount const fee, ReadVie // convenient thanks to early "return false"; the not-so-nice // alternatives are several layers of nested if/else or more complex // (i.e. brittle) if statements. - result &= - [&]() { - switch (txnType) - { - case ttVAULT_CREATE: { - bool result = true; + result &= [&]() { + switch (txnType) + { + case ttVAULT_CREATE: { + bool result = true; - if (!beforeVault_.empty()) - { - JLOG(j.fatal()) // - << "Invariant failed: create operation must not have " - "updated a vault"; - result = false; - } + if (!beforeVault_.empty()) + { + JLOG(j.fatal()) // + << "Invariant failed: create operation must not have " + "updated a vault"; + result = false; + } - if (afterVault.assetsAvailable != zero || afterVault.assetsTotal != zero || - afterVault.lossUnrealized != zero || updatedShares->sharesTotal != 0) - { - JLOG(j.fatal()) // - << "Invariant failed: created vault must be empty"; - result = false; - } + if (afterVault.assetsAvailable != zero || afterVault.assetsTotal != zero || + afterVault.lossUnrealized != zero || updatedShares->sharesTotal != 0) + { + JLOG(j.fatal()) // + << "Invariant failed: created vault must be empty"; + result = false; + } - if (afterVault.pseudoId != updatedShares->share.getIssuer()) - { - JLOG(j.fatal()) // - << "Invariant failed: shares issuer and vault " - "pseudo-account must be the same"; - result = false; - } + if (afterVault.pseudoId != updatedShares->share.getIssuer()) + { + JLOG(j.fatal()) // + << "Invariant failed: shares issuer and vault " + "pseudo-account must be the same"; + result = false; + } - auto const sleSharesIssuer = view.read(keylet::account(updatedShares->share.getIssuer())); - if (!sleSharesIssuer) + auto const sleSharesIssuer = view.read(keylet::account(updatedShares->share.getIssuer())); + if (!sleSharesIssuer) + { + JLOG(j.fatal()) // + << "Invariant failed: shares issuer must exist"; + return false; + } + + if (!isPseudoAccount(sleSharesIssuer)) + { + JLOG(j.fatal()) // + << "Invariant failed: shares issuer must be a pseudo-account"; + result = false; + } + + if (auto const vaultId = (*sleSharesIssuer)[~sfVaultID]; !vaultId || *vaultId != afterVault.key) + { + JLOG(j.fatal()) // + << "Invariant failed: shares issuer pseudo-account must point back to the vault"; + result = false; + } + + return result; + } + case ttVAULT_SET: { + bool result = true; + + XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : set updated a vault"); + auto const& beforeVault = beforeVault_[0]; + + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (vaultDeltaAssets) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change vault balance"; + result = false; + } + + if (beforeVault.assetsTotal != afterVault.assetsTotal) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change assets outstanding"; + result = false; + } + + if (afterVault.assetsMaximum > zero && afterVault.assetsTotal > afterVault.assetsMaximum) + { + JLOG(j.fatal()) << // + "Invariant failed: set assets outstanding must not exceed assets maximum"; + result = false; + } + + if (beforeVault.assetsAvailable != afterVault.assetsAvailable) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change assets available"; + result = false; + } + + if (beforeShares && updatedShares && beforeShares->sharesTotal != updatedShares->sharesTotal) + { + JLOG(j.fatal()) << // + "Invariant failed: set must not change shares outstanding"; + result = false; + } + + return result; + } + case ttVAULT_DEPOSIT: { + bool result = true; + + XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : deposit updated a vault"); + auto const& beforeVault = beforeVault_[0]; + + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + + if (!vaultDeltaAssets) + { + JLOG(j.fatal()) << // + "Invariant failed: deposit must change vault balance"; + return false; // That's all we can do + } + + if (*vaultDeltaAssets > tx[sfAmount]) + { + JLOG(j.fatal()) << // + "Invariant failed: deposit must not change vault " + "balance by more than deposited amount"; + result = false; + } + + if (*vaultDeltaAssets <= zero) + { + JLOG(j.fatal()) << // + "Invariant failed: deposit must increase vault balance"; + result = false; + } + + // Any payments (including deposits) made by the issuer + // do not change their balance, but create funds instead. + bool const issuerDeposit = [&]() -> bool { + if (vaultAsset.native()) + return false; + return tx[sfAccount] == vaultAsset.getIssuer(); + }(); + + if (!issuerDeposit) + { + auto const accountDeltaAssets = deltaAssetsTxAccount(); + if (!accountDeltaAssets) { - JLOG(j.fatal()) // - << "Invariant failed: shares issuer must exist"; + JLOG(j.fatal()) << // + "Invariant failed: deposit must change depositor " + "balance"; return false; } - if (!isPseudoAccount(sleSharesIssuer)) + if (*accountDeltaAssets >= zero) { - JLOG(j.fatal()) // - << "Invariant failed: shares issuer must be a " - "pseudo-account"; + JLOG(j.fatal()) << // + "Invariant failed: deposit must decrease depositor " + "balance"; result = false; } - if (auto const vaultId = (*sleSharesIssuer)[~sfVaultID]; !vaultId || *vaultId != afterVault.key) + if (*accountDeltaAssets * -1 != *vaultDeltaAssets) { - JLOG(j.fatal()) // - << "Invariant failed: shares issuer pseudo-account " - "must point back to the vault"; + JLOG(j.fatal()) << // + "Invariant failed: deposit must change vault and " + "depositor balance by equal amount"; result = false; } - - return result; } - case ttVAULT_SET: { - bool result = true; - XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : set updated a vault"); - auto const& beforeVault = beforeVault_[0]; - - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (vaultDeltaAssets) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change vault balance"; - result = false; - } - - if (beforeVault.assetsTotal != afterVault.assetsTotal) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change assets " - "outstanding"; - result = false; - } - - if (afterVault.assetsMaximum > zero && afterVault.assetsTotal > afterVault.assetsMaximum) - { - JLOG(j.fatal()) << // - "Invariant failed: set assets outstanding must not " - "exceed assets maximum"; - result = false; - } - - if (beforeVault.assetsAvailable != afterVault.assetsAvailable) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change assets " - "available"; - result = false; - } - - if (beforeShares && updatedShares && beforeShares->sharesTotal != updatedShares->sharesTotal) - { - JLOG(j.fatal()) << // - "Invariant failed: set must not change shares " - "outstanding"; - result = false; - } - - return result; + if (afterVault.assetsMaximum > zero && afterVault.assetsTotal > afterVault.assetsMaximum) + { + JLOG(j.fatal()) << // + "Invariant failed: deposit assets outstanding must not exceed assets maximum"; + result = false; } - case ttVAULT_DEPOSIT: { - bool result = true; - XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : deposit updated a vault"); - auto const& beforeVault = beforeVault_[0]; - - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - - if (!vaultDeltaAssets) + // If assets are not donated, check share invariants + if (view.rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) + { + auto const accountDeltaShares = deltaShares(tx[sfAccount]); + if (accountDeltaShares) { JLOG(j.fatal()) << // - "Invariant failed: deposit must change vault balance"; + "Invariant failed: donation must not change depositor shares"; return false; // That's all we can do } - if (*vaultDeltaAssets > tx[sfAmount]) + auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); + if (vaultDeltaShares) { JLOG(j.fatal()) << // - "Invariant failed: deposit must not change vault " - "balance by more than deposited amount"; - result = false; + "Invariant failed: donation must not change vault shares"; + return false; // That's all we can do } - - if (*vaultDeltaAssets <= zero) - { - JLOG(j.fatal()) << // - "Invariant failed: deposit must increase vault balance"; - result = false; - } - - // Any payments (including deposits) made by the issuer - // do not change their balance, but create funds instead. - bool const issuerDeposit = [&]() -> bool { - if (vaultAsset.native()) - return false; - return tx[sfAccount] == vaultAsset.getIssuer(); - }(); - - if (!issuerDeposit) - { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - if (!accountDeltaAssets) - { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor " - "balance"; - return false; - } - - if (*accountDeltaAssets >= zero) - { - JLOG(j.fatal()) << // - "Invariant failed: deposit must decrease depositor " - "balance"; - result = false; - } - - if (*accountDeltaAssets * -1 != *vaultDeltaAssets) - { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change vault and " - "depositor balance by equal amount"; - result = false; - } - } - - if (afterVault.assetsMaximum > zero && afterVault.assetsTotal > afterVault.assetsMaximum) - { - JLOG(j.fatal()) << // - "Invariant failed: deposit assets outstanding must not " - "exceed assets maximum"; - result = false; - } - + } + else + { auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (!accountDeltaShares) { JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor " - "shares"; + "Invariant failed: deposit must change depositor shares"; return false; // That's all we can do } if (*accountDeltaShares <= zero) { JLOG(j.fatal()) << // - "Invariant failed: deposit must increase depositor " - "shares"; + "Invariant failed: deposit must increase depositor shares"; result = false; } @@ -3052,252 +3068,238 @@ ValidVault::finalize(STTx const& tx, TER const ret, XRPAmount const fee, ReadVie if (*vaultDeltaShares * -1 != *accountDeltaShares) { JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor and " - "vault shares by equal amount"; + "Invariant failed: deposit must change depositor and vault shares by equal amount"; result = false; } - - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) - { - JLOG(j.fatal()) << "Invariant failed: deposit and assets " - "outstanding must add up"; - result = false; - } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) - { - JLOG(j.fatal()) << "Invariant failed: deposit and assets " - "available must add up"; - result = false; - } - - return result; } - case ttVAULT_WITHDRAW: { - bool result = true; - XRPL_ASSERT( - !beforeVault_.empty(), - "xrpl::ValidVault::finalize : withdrawal updated a " - "vault"); - auto const& beforeVault = beforeVault_[0]; + if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + { + JLOG(j.fatal()) << "Invariant failed: deposit and assets outstanding must add up"; + result = false; + } + if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + { + JLOG(j.fatal()) << "Invariant failed: deposit and assets available must add up"; + result = false; + } - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + return result; + } + case ttVAULT_WITHDRAW: { + bool result = true; - if (!vaultDeltaAssets) - { - JLOG(j.fatal()) << "Invariant failed: withdrawal must " - "change vault balance"; - return false; // That's all we can do - } + XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : withdrawal updated a vault"); + auto const& beforeVault = beforeVault_[0]; - if (*vaultDeltaAssets >= zero) - { - JLOG(j.fatal()) << "Invariant failed: withdrawal must " - "decrease vault balance"; - result = false; - } + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - // Any payments (including withdrawal) going to the issuer - // do not change their balance, but destroy funds instead. - bool const issuerWithdrawal = [&]() -> bool { - if (vaultAsset.native()) - return false; - auto const destination = tx[~sfDestination].value_or(tx[sfAccount]); - return destination == vaultAsset.getIssuer(); + if (!vaultDeltaAssets) + { + JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault balance"; + return false; // That's all we can do + } + + if (*vaultDeltaAssets >= zero) + { + JLOG(j.fatal()) << "Invariant failed: withdrawal must decrease vault balance"; + result = false; + } + + // Any payments (including withdrawal) going to the issuer + // do not change their balance, but destroy funds instead. + bool const issuerWithdrawal = [&]() -> bool { + if (vaultAsset.native()) + return false; + auto const destination = tx[~sfDestination].value_or(tx[sfAccount]); + return destination == vaultAsset.getIssuer(); + }(); + + if (!issuerWithdrawal) + { + auto const accountDeltaAssets = deltaAssetsTxAccount(); + auto const otherAccountDelta = [&]() -> std::optional { + if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) + return deltaAssets(*destination); + return std::nullopt; }(); - if (!issuerWithdrawal) - { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - auto const otherAccountDelta = [&]() -> std::optional { - if (auto const destination = tx[~sfDestination]; - destination && *destination != tx[sfAccount]) - return deltaAssets(*destination); - return std::nullopt; - }(); - - if (accountDeltaAssets.has_value() == otherAccountDelta.has_value()) - { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change one " - "destination balance"; - return false; - } - - auto const destinationDelta = // - accountDeltaAssets ? *accountDeltaAssets : *otherAccountDelta; - - if (destinationDelta <= zero) - { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must increase " - "destination balance"; - result = false; - } - - if (*vaultDeltaAssets * -1 != destinationDelta) - { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change vault " - "and destination balance by equal amount"; - result = false; - } - } - - auto const accountDeltaShares = deltaShares(tx[sfAccount]); - if (!accountDeltaShares) + if (accountDeltaAssets.has_value() == otherAccountDelta.has_value()) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change depositor " - "shares"; + "Invariant failed: withdrawal must change one destination balance"; return false; } - if (*accountDeltaShares >= zero) + auto const destinationDelta = // + accountDeltaAssets ? *accountDeltaAssets : *otherAccountDelta; + + if (destinationDelta <= zero) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must decrease depositor " - "shares"; + "Invariant failed: withdrawal must increase destination balance"; result = false; } - auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (*vaultDeltaAssets * -1 != destinationDelta) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change vault shares"; + "Invariant failed: withdrawal must change vault and destination balance by equal amount"; + result = false; + } + } + + auto const accountDeltaShares = deltaShares(tx[sfAccount]); + if (!accountDeltaShares) + { + JLOG(j.fatal()) << // + "Invariant failed: withdrawal must change depositor " + "shares"; + return false; + } + + if (*accountDeltaShares >= zero) + { + JLOG(j.fatal()) << // + "Invariant failed: withdrawal must decrease depositor shares"; + result = false; + } + + auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); + if (!vaultDeltaShares || *vaultDeltaShares == zero) + { + JLOG(j.fatal()) << // + "Invariant failed: withdrawal must change vault shares"; + return false; // That's all we can do + } + + if (*vaultDeltaShares * -1 != *accountDeltaShares) + { + JLOG(j.fatal()) << // + "Invariant failed: withdrawal must change depositor and vault shares by equal amount"; + result = false; + } + + // Note, vaultBalance is negative (see check above) + if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + { + JLOG(j.fatal()) << "Invariant failed: withdrawal and assets outstanding must add up"; + result = false; + } + + if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + { + JLOG(j.fatal()) << "Invariant failed: withdrawal and " + "assets available must add up"; + result = false; + } + + return result; + } + case ttVAULT_CLAWBACK: { + bool result = true; + + XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : clawback updated a vault"); + auto const& beforeVault = beforeVault_[0]; + + if (vaultAsset.native() || vaultAsset.getIssuer() != tx[sfAccount]) + { + // The owner can use clawback to force-burn shares when the + // vault is empty but there are outstanding shares + if (!(beforeShares && beforeShares->sharesTotal > 0 && vaultHoldsNoAssets(beforeVault) && + beforeVault.owner == tx[sfAccount])) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback may only be performed " + "by the asset issuer, or by the vault owner of an " + "empty vault"; return false; // That's all we can do } + } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (vaultDeltaAssets) + { + if (*vaultDeltaAssets >= zero) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change depositor " - "and vault shares by equal amount"; + "Invariant failed: clawback must decrease vault " + "balance"; result = false; } - // Note, vaultBalance is negative (see check above) if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) { - JLOG(j.fatal()) << "Invariant failed: withdrawal and " - "assets outstanding must add up"; + JLOG(j.fatal()) << // + "Invariant failed: clawback and assets outstanding " + "must add up"; result = false; } if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) { - JLOG(j.fatal()) << "Invariant failed: withdrawal and " - "assets available must add up"; + JLOG(j.fatal()) << // + "Invariant failed: clawback and assets available " + "must add up"; result = false; } - - return result; } - case ttVAULT_CLAWBACK: { - bool result = true; - - XRPL_ASSERT(!beforeVault_.empty(), "xrpl::ValidVault::finalize : clawback updated a vault"); - auto const& beforeVault = beforeVault_[0]; - - if (vaultAsset.native() || vaultAsset.getIssuer() != tx[sfAccount]) - { - // The owner can use clawback to force-burn shares when the - // vault is empty but there are outstanding shares - if (!(beforeShares && beforeShares->sharesTotal > 0 && vaultHoldsNoAssets(beforeVault) && - beforeVault.owner == tx[sfAccount])) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback may only be performed " - "by the asset issuer, or by the vault owner of an " - "empty vault"; - return false; // That's all we can do - } - } - - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (vaultDeltaAssets) - { - if (*vaultDeltaAssets >= zero) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must decrease vault " - "balance"; - result = false; - } - - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback and assets outstanding " - "must add up"; - result = false; - } - - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback and assets available " - "must add up"; - result = false; - } - } - else if (!vaultHoldsNoAssets(beforeVault)) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must change vault balance"; - return false; // That's all we can do - } - - auto const accountDeltaShares = deltaShares(tx[sfHolder]); - if (!accountDeltaShares) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must change holder shares"; - return false; // That's all we can do - } - - if (*accountDeltaShares >= zero) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must decrease holder " - "shares"; - result = false; - } - - auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must change vault shares"; - return false; // That's all we can do - } - - if (*vaultDeltaShares * -1 != *accountDeltaShares) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback must change holder and " - "vault shares by equal amount"; - result = false; - } - - return result; + else if (!vaultHoldsNoAssets(beforeVault)) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must change vault balance"; + return false; // That's all we can do } - case ttLOAN_SET: - case ttLOAN_MANAGE: - case ttLOAN_PAY: { - // TBD - return true; + auto const accountDeltaShares = deltaShares(tx[sfHolder]); + if (!accountDeltaShares) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must change holder shares"; + return false; // That's all we can do } - default: - // LCOV_EXCL_START - UNREACHABLE("xrpl::ValidVault::finalize : unknown transaction type"); - return false; - // LCOV_EXCL_STOP + if (*accountDeltaShares >= zero) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must decrease holder " + "shares"; + result = false; + } + + auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); + if (!vaultDeltaShares || *vaultDeltaShares == zero) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must change vault shares"; + return false; // That's all we can do + } + + if (*vaultDeltaShares * -1 != *accountDeltaShares) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must change holder and " + "vault shares by equal amount"; + result = false; + } + + return result; } - }(); + + case ttLOAN_SET: + case ttLOAN_MANAGE: + case ttLOAN_PAY: { + // TBD + return true; + } + + default: + // LCOV_EXCL_START + UNREACHABLE("xrpl::ValidVault::finalize : unknown transaction type"); + return false; + // LCOV_EXCL_STOP + } + }(); if (!result) { diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index d5fc0e4ad6..3a5b869367 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -15,6 +15,15 @@ namespace xrpl { +std::uint32_t +VaultDeposit::getFlagsMask(PreflightContext const& ctx) +{ + if (ctx.rules.enabled(fixLendingProtocolV1_1)) + return tfVaultDepositMask; + + return tfVaultDepositMask | tfVaultDonate; +} + NotTEC VaultDeposit::preflight(PreflightContext const& ctx) { @@ -69,6 +78,22 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } + if (ctx.view.rules().enabled(fixLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) + { + if (account != vault->at(sfOwner)) + { + JLOG(ctx.j.debug()) << "VaultDeposit: only owner can donate to vault."; + return tecNO_PERMISSION; + } + + // Cannot donate to a vault with no shares + if (sleIssuance->at(sfOutstandingAmount) == 0) + { + JLOG(ctx.j.debug()) << "VaultDeposit: empty vault cannot receive donations."; + return tecNO_PERMISSION; + } + } + if (sleIssuance->isFlag(lsfMPTLocked)) { // LCOV_EXCL_START @@ -179,42 +204,49 @@ VaultDeposit::doApply() return err; } } - STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited; - try + if (view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate)) { - // Compute exchange before transferring any amounts. - { - auto const maybeShares = assetsToSharesDeposit(vault, sleIssuance, amount); - if (!maybeShares) - return tecINTERNAL; // LCOV_EXCL_LINE - sharesCreated = *maybeShares; - } - if (sharesCreated == beast::zero) - return tecPRECISION_LOSS; - - auto const maybeAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); - if (!maybeAssets) - return tecINTERNAL; // LCOV_EXCL_LINE - else if (*maybeAssets > amount) - { - // LCOV_EXCL_START - JLOG(j_.error()) << "VaultDeposit: would take more than offered."; - return tecINTERNAL; - // LCOV_EXCL_STOP - } - assetsDeposited = *maybeAssets; + XRPL_ASSERT(account_ == vault->at(sfOwner), "xrpl::VaultDeposit::doApply : account is owner"); + assetsDeposited = amount; } - catch (std::overflow_error const&) + else { - // It's easy to hit this exception from Number with large enough Scale - // so we avoid spamming the log and only use debug here. - JLOG(j_.debug()) // - << "VaultDeposit: overflow error with" - << " scale=" << (int)vault->at(sfScale).value() // - << ", assetsTotal=" << vault->at(sfAssetsTotal).value() - << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount) << ", amount=" << amount; - return tecPATH_DRY; + try + { + // Compute exchange before transferring any amounts. + { + auto const maybeShares = assetsToSharesDeposit(vault, sleIssuance, amount); + if (!maybeShares) + return tecINTERNAL; // LCOV_EXCL_LINE + sharesCreated = *maybeShares; + } + if (sharesCreated == beast::zero) + return tecPRECISION_LOSS; + + auto const maybeAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + else if (*maybeAssets > amount) + { + // LCOV_EXCL_START + JLOG(j_.error()) << "VaultDeposit: would take more than offered."; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + assetsDeposited = *maybeAssets; + } + catch (std::overflow_error const&) + { + // It's easy to hit this exception from Number with large enough Scale + // so we avoid spamming the log and only use debug here. + JLOG(j_.debug()) // + << "VaultDeposit: overflow error with" + << " scale=" << (int)vault->at(sfScale).value() // + << ", assetsTotal=" << vault->at(sfAssetsTotal).value() + << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount) << ", amount=" << amount; + return tecPATH_DRY; + } } XRPL_ASSERT( @@ -249,10 +281,17 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } - // Transfer shares from vault to depositor. - if (auto const ter = accountSend(view(), vaultAccount, account_, sharesCreated, j_, WaiveTransferFee::Yes); - !isTesSuccess(ter)) - return ter; + if (view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate)) + { + XRPL_ASSERT(sharesCreated == beast::zero, "xrpl::VaultDeposit::doApply: donation issued shares"); + } + else + { + // Transfer shares from vault to depositor. + if (auto const ter = accountSend(view(), vaultAccount, account_, sharesCreated, j_, WaiveTransferFee::Yes); + !isTesSuccess(ter)) + return ter; + } associateAsset(*vault, vaultAsset); diff --git a/src/xrpld/app/tx/detail/VaultDeposit.h b/src/xrpld/app/tx/detail/VaultDeposit.h index 6c63308407..373f7097a0 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.h +++ b/src/xrpld/app/tx/detail/VaultDeposit.h @@ -13,6 +13,9 @@ public: { } + static std::uint32_t + getFlagsMask(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx);