diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 90e1d9853c..40399539b8 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -102,16 +102,9 @@ jobs: echo 'CMake target: ${{ matrix.cmake_target }}' echo 'Config name: ${{ matrix.config_name }}' - - name: Clean workspace (MacOS) - if: ${{ inputs.os == 'macos' }} - run: | - WORKSPACE=${{ github.workspace }} - echo "Cleaning workspace '${WORKSPACE}'." - if [ -z "${WORKSPACE}" ] || [ "${WORKSPACE}" = "/" ]; then - echo "Invalid working directory '${WORKSPACE}'." - exit 1 - fi - find "${WORKSPACE}" -depth 1 | xargs rm -rfv + - name: Cleanup workspace + if: ${{ runner.os == 'macOS' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e - name: Checkout repository uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 9ee05bfb45..41c60d30a1 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -150,6 +150,24 @@ public: return (mantissa_ < 0) ? -1 : (mantissa_ ? 1 : 0); } + Number + truncate() const noexcept + { + if (exponent_ >= 0 || mantissa_ == 0) + return *this; + + Number ret = *this; + while (ret.exponent_ < 0 && ret.mantissa_ != 0) + { + ret.exponent_ += 1; + ret.mantissa_ /= rep(10); + } + // We are guaranteed that normalize() will never throw an exception + // because exponent is either negative or zero at this point. + ret.normalize(); + return ret; + } + friend constexpr bool operator>(Number const& x, Number const& y) noexcept { diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index bd39233cca..84b8c3889b 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -121,6 +121,13 @@ std::size_t constexpr maxDataPayloadLength = 256; /** Vault withdrawal policies */ std::uint8_t constexpr vaultStrategyFirstComeFirstServe = 1; +/** Default IOU scale factor for a Vault */ +std::uint8_t constexpr vaultDefaultIOUScale = 6; +/** Maximum scale factor for a Vault. The number is chosen to ensure that +1 IOU can be always converted to shares. +10^19 > maxMPTokenAmount (2^64-1) > 10^18 */ +std::uint8_t constexpr vaultMaximumIOUScale = 18; + /** Maximum recursion depth for vault shares being put as an asset inside * another vault; counted from 0 */ std::uint8_t constexpr maxAssetCheckDepth = 5; diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 967fb37b94..ac9ebc6069 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -499,6 +499,7 @@ LEDGER_ENTRY(ltVAULT, 0x0084, Vault, vault, ({ {sfLossUnrealized, soeREQUIRED}, {sfShareMPTID, soeREQUIRED}, {sfWithdrawalPolicy, soeREQUIRED}, + {sfScale, soeDEFAULT}, // no SharesTotal ever (use MPTIssuance.sfOutstandingAmount) // no PermissionedDomainID ever (use MPTIssuance.sfDomainID) })) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index e75ce67ab1..f9f74d2492 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -798,6 +798,7 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, {sfDomainID, soeOPTIONAL}, {sfWithdrawalPolicy, soeOPTIONAL}, {sfData, soeOPTIONAL}, + {sfScale, soeOPTIONAL}, })) /** This transaction updates a single asset vault. */ diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index bd4cc90af6..9e14d9e094 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -19,11 +19,14 @@ #include #include +#include +#include #include #include #include +#include #include #include #include @@ -73,12 +76,32 @@ class Vault_test : public beast::unit_test::suite env(tx); env.close(); BEAST_EXPECT(env.le(keylet)); + std::uint64_t const scale = asset.raw().holds() ? 1 : 1e6; - auto const share = [&env, keylet = keylet, this]() -> PrettyAsset { + auto const [share, vaultAccount] = + [&env, + keylet = keylet, + asset, + this]() -> std::tuple { auto const vault = env.le(keylet); BEAST_EXPECT(vault != nullptr); - return MPTIssue(vault->at(sfShareMPTID)); + if (asset.raw().holds() && !asset.raw().native()) + BEAST_EXPECT(vault->at(sfScale) == 6); + else + BEAST_EXPECT(vault->at(sfScale) == 0); + auto const shares = + env.le(keylet::mptIssuance(vault->at(sfShareMPTID))); + BEAST_EXPECT(shares != nullptr); + if (asset.raw().holds() && !asset.raw().native()) + BEAST_EXPECT(shares->at(sfAssetScale) == 6); + else + BEAST_EXPECT(shares->at(sfAssetScale) == 0); + return { + MPTIssue(vault->at(sfShareMPTID)), + Account("vault", vault->at(sfAccount))}; }(); + auto const shares = share.raw().get(); + env.memoize(vaultAccount); // Several 3rd party accounts which cannot receive funds Account alice{"alice"}; @@ -96,6 +119,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(10000)}); env(tx, ter(tecINSUFFICIENT_FUNDS)); + env.close(); } { @@ -105,6 +129,9 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(50)}); env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(50 * scale)); } { @@ -114,12 +141,16 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(50)}); env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(100 * scale)); } { testcase(prefix + " fail to delete non-empty vault"); auto tx = vault.del({.owner = owner, .id = keylet.key}); env(tx, ter(tecHAS_OBLIGATIONS)); + env.close(); } { @@ -127,6 +158,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = issuer, .id = keylet.key}); tx[sfAssetsMaximum] = asset(50).number(); env(tx, ter(tecNO_PERMISSION)); + env.close(); } { @@ -135,6 +167,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = owner, .id = keylet.key}); tx[sfAssetsMaximum] = asset(50).number(); env(tx, ter(tecLIMIT_EXCEEDED)); + env.close(); } { @@ -142,6 +175,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = owner, .id = keylet.key}); tx[sfAssetsMaximum] = asset(150).number(); env(tx); + env.close(); } { @@ -149,6 +183,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = owner, .id = keylet.key}); tx[sfData] = "0"; env(tx); + env.close(); } { @@ -156,6 +191,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = owner, .id = keylet.key}); tx[sfDomainID] = to_string(base_uint<256>(42ul)); env(tx, ter{tecNO_PERMISSION}); + env.close(); } { @@ -165,6 +201,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(100)}); env(tx, ter(tecLIMIT_EXCEEDED)); + env.close(); } { @@ -172,6 +209,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.set({.owner = owner, .id = keylet.key}); tx[sfAssetsMaximum] = asset(0).number(); env(tx); + env.close(); } { @@ -190,6 +228,9 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(100)}); env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(200 * scale)); } { @@ -202,6 +243,12 @@ class Vault_test : public beast::unit_test::suite .holder = depositor, .amount = asset(10)}); env(tx, code); + env.close(); + if (!asset.raw().native()) + { + BEAST_EXPECT( + env.balance(depositor, shares) == share(190 * scale)); + } } { @@ -211,6 +258,30 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.clawback( {.issuer = issuer, .id = keylet.key, .holder = depositor}); env(tx, code); + env.close(); + if (!asset.raw().native()) + { + BEAST_EXPECT(env.balance(depositor, shares) == share(0)); + + { + auto tx = vault.clawback( + {.issuer = issuer, + .id = keylet.key, + .holder = depositor, + .amount = asset(10)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + + { + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(10)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + } } if (!asset.raw().native()) @@ -221,6 +292,9 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(200)}); env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(200 * scale)); } { @@ -232,6 +306,7 @@ class Vault_test : public beast::unit_test::suite .amount = asset(100)}); tx[sfDestination] = alice.human(); env(tx, ter{tecNO_PERMISSION}); + env.close(); } { @@ -242,6 +317,7 @@ class Vault_test : public beast::unit_test::suite .amount = asset(1000)}); tx[sfDestination] = "0"; env(tx, ter(temMALFORMED)); + env.close(); } { @@ -254,6 +330,7 @@ class Vault_test : public beast::unit_test::suite .amount = asset(1000)}); tx[sfDestinationTag] = "0"; env(tx, ter(temMALFORMED)); + env.close(); } if (!asset.raw().native()) @@ -267,6 +344,77 @@ class Vault_test : public beast::unit_test::suite tx[sfDestination] = erin.human(); env(tx, ter{asset.raw().holds() ? tecNO_LINE : tecNO_AUTH}); + env.close(); + } + + { + testcase( + prefix + + " fail to withdraw to 3rd party lsfRequireDestTag"); + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(100)}); + tx[sfDestination] = dave.human(); + env(tx, ter{tecDST_TAG_NEEDED}); + env.close(); + } + + { + testcase(prefix + " withdraw to authorized 3rd party"); + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(100)}); + tx[sfDestination] = charlie.human(); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(100 * scale)); + } + + { + testcase(prefix + " withdraw to issuer"); + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(50)}); + tx[sfDestination] = issuer.human(); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(depositor, shares) == share(50 * scale)); + } + + { + testcase(prefix + " withdraw remaining assets"); + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(50)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(depositor, shares) == share(0)); + + if (!asset.raw().native()) + { + auto tx = vault.clawback( + {.issuer = issuer, + .id = keylet.key, + .holder = depositor, + .amount = asset(0)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + + { + auto tx = vault.withdraw( + {.depositor = depositor, + .id = keylet.key, + .amount = share(10)}); + env(tx, ter{tecINSUFFICIENT_FUNDS}); + env.close(); + } } if (!asset.raw().native() && asset.raw().holds()) @@ -280,10 +428,27 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.deposit( {.depositor = erin, .id = keylet.key, .amount = asset(10)}); env(tx); - env(pay(erin, depositor, share(10))); + env.close(); + { + auto tx = pay(erin, depositor, share(10 * scale)); + + // depositor no longer has MPToken for shares + env(tx, ter{tecNO_AUTH}); + env.close(); + + // depositor will gain MPToken for shares again + env(vault.deposit( + {.depositor = depositor, + .id = keylet.key, + .amount = asset(1)})); + env.close(); + + env(tx); + env.close(); + } testcase(prefix + " withdraw to authorized 3rd party"); - // Depositor withdraws shares, destined to Erin + // Depositor withdraws assets, destined to Erin tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, @@ -292,52 +457,23 @@ class Vault_test : public beast::unit_test::suite env(tx); // Erin returns assets to issuer env(pay(erin, issuer, asset(10))); + env.close(); testcase(prefix + " fail to pay to unauthorized 3rd party"); env(trust(erin, asset(0))); + env.close(); + // Erin has MPToken but is no longer authorized to hold assets env(pay(depositor, erin, share(1)), ter{tecNO_LINE}); - } + env.close(); - { - testcase( - prefix + - " fail to withdraw to 3rd party lsfRequireDestTag"); - auto tx = vault.withdraw( + // Depositor withdraws remaining single asset + tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, - .amount = asset(100)}); - tx[sfDestination] = dave.human(); - env(tx, ter{tecDST_TAG_NEEDED}); - } - - { - testcase(prefix + " withdraw to authorized 3rd party"); - auto tx = vault.withdraw( - {.depositor = depositor, - .id = keylet.key, - .amount = asset(100)}); - tx[sfDestination] = charlie.human(); - env(tx); - } - - { - testcase(prefix + " withdraw to issuer"); - auto tx = vault.withdraw( - {.depositor = depositor, - .id = keylet.key, - .amount = asset(50)}); - tx[sfDestination] = issuer.human(); - env(tx); - } - - { - testcase(prefix + " withdraw remaining assets"); - auto tx = vault.withdraw( - {.depositor = depositor, - .id = keylet.key, - .amount = asset(50)}); + .amount = asset(1)}); env(tx); + env.close(); } { @@ -740,6 +876,61 @@ class Vault_test : public beast::unit_test::suite } }); + testCase([&](Env& env, + Account const&, + Account const& owner, + Asset const& asset, + Vault& vault) { + testcase("create with Scale"); + + { + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 255; + env(tx, ter(temMALFORMED)); + } + + { + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 19; + env(tx, ter(temMALFORMED)); + } + + // accepted range from 0 to 18 + { + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 18; + env(tx); + env.close(); + auto const sleVault = env.le(keylet); + BEAST_EXPECT(sleVault); + BEAST_EXPECT((*sleVault)[sfScale] == 18); + } + + { + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 0; + env(tx); + env.close(); + auto const sleVault = env.le(keylet); + BEAST_EXPECT(sleVault); + BEAST_EXPECT((*sleVault)[sfScale] == 0); + } + + { + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + auto const sleVault = env.le(keylet); + BEAST_EXPECT(sleVault); + BEAST_EXPECT((*sleVault)[sfScale] == 6); + } + }); + testCase([&](Env& env, Account const&, Account const& owner, @@ -1105,6 +1296,32 @@ class Vault_test : public beast::unit_test::suite testcase("non-existing domain"); env(tx, ter{tecOBJECT_NOT_FOUND}); }); + + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + Asset const& asset, + Vault& vault) { + testcase("cannot set Scale=0"); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 0; + env(tx, ter{temMALFORMED}); + }); + + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + Asset const& asset, + Vault& vault) { + testcase("cannot set Scale=1"); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 1; + env(tx, ter{temMALFORMED}); + }); } void @@ -1221,21 +1438,65 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - Env env{*this, testable_amendments() | featureSingleAssetVault}; - Account issuer{"issuer"}; - Account owner{"owner"}; - Account depositor{"depositor"}; - env.fund(XRP(1000), issuer, owner, depositor); - env.close(); - Vault vault{env}; + auto testCase = [this](std::function test) { + Env env{*this, testable_amendments() | featureSingleAssetVault}; + Account issuer{"issuer"}; + Account owner{"owner"}; + Account depositor{"depositor"}; + env.fund(XRP(1000), issuer, owner, depositor); + env.close(); + Vault vault{env}; + MPTTester mptt{env, issuer, mptInitNoFund}; + // Locked because that is the default flag. + mptt.create(); + Asset asset = mptt.issuanceID(); - MPTTester mptt{env, issuer, mptInitNoFund}; + test(env, issuer, owner, depositor, asset, vault); + }; - // Locked because that is the default flag. - mptt.create(); - Asset asset = mptt.issuanceID(); - auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - env(tx, ter(tecNO_AUTH)); + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + Asset const& asset, + Vault& vault) { + testcase("MPT no authorization"); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx, ter(tecNO_AUTH)); + }); + + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + Asset const& asset, + Vault& vault) { + testcase("MPT cannot set Scale=0"); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 0; + env(tx, ter{temMALFORMED}); + }); + + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + Asset const& asset, + Vault& vault) { + testcase("MPT cannot set Scale=1"); + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 1; + env(tx, ter{temMALFORMED}); + }); } void @@ -1285,7 +1546,7 @@ class Vault_test : public beast::unit_test::suite jvVault[jss::result][jss::vault][sfAssetsTotal] == "100"); BEAST_EXPECT( jvVault[jss::result][jss::vault][jss::shares] - [sfOutstandingAmount] == "100"); + [sfOutstandingAmount] == "100000000"); // Vault pseudo-account return parseBase58( @@ -1324,7 +1585,7 @@ class Vault_test : public beast::unit_test::suite jvVault[jss::result][jss::vault][sfAssetsTotal] == "50"); BEAST_EXPECT( jvVault[jss::result][jss::vault][jss::shares] - [sfOutstandingAmount] == "50"); + [sfOutstandingAmount] == "50000000"); } { @@ -1508,6 +1769,10 @@ class Vault_test : public beast::unit_test::suite env(tx); env.close(); + // Clawback removed shares MPToken + auto const mptSle = env.le(keylet::mptoken(share, depositor.id())); + BEAST_EXPECT(mptSle == nullptr); + // Can delete empty vault, even if global lock tx = vault.del({.owner = owner, .id = keylet.key}); env(tx); @@ -1597,11 +1862,14 @@ class Vault_test : public beast::unit_test::suite vault.create({.owner = owner, .asset = asset}); env(tx); env.close(); + auto v = env.le(keylet); + BEAST_EXPECT(v); + MPTID share = (*v)[sfShareMPTID]; tx = vault.deposit( {.depositor = depositor, .id = keylet.key, - .amount = asset(1000)}); + .amount = asset(1000)}); // all assets held by depositor env(tx); env.close(); @@ -1629,8 +1897,14 @@ class Vault_test : public beast::unit_test::suite tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, - .amount = asset(100)}); + .amount = asset(1000)}); env(tx); + env.close(); + + // Withdraw removed shares MPToken + auto const mptSle = + env.le(keylet::mptoken(share, depositor.id())); + BEAST_EXPECT(mptSle == nullptr); } }, {.requireAuth = false}); @@ -1702,6 +1976,96 @@ class Vault_test : public beast::unit_test::suite env(vault.del({.owner = owner, .id = keylet.key})); }); + testCase([this]( + Env& env, + Account const& issuer, + Account const& owner, + Account const& depositor, + PrettyAsset const& asset, + Vault& vault, + MPTTester& mptt) { + testcase("MPT vault owner can receive shares unless unauthorized"); + + 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(); + + auto const issuanceId = [&env](ripple::Keylet keylet) -> MPTID { + auto const vault = env.le(keylet); + return vault->at(sfShareMPTID); + }(keylet); + PrettyAsset shares = MPTIssue(issuanceId); + + { + // owner has MPToken for shares they did not explicitly create + env(pay(depositor, owner, shares(1))); + env.close(); + + tx = vault.withdraw( + {.depositor = owner, + .id = keylet.key, + .amount = shares(1)}); + env(tx); + env.close(); + + // owner's MPToken for vault shares not destroyed by withdraw + env(pay(depositor, owner, shares(1))); + env.close(); + + tx = vault.clawback( + {.issuer = issuer, + .id = keylet.key, + .holder = owner, + .amount = asset(0)}); + env(tx); + env.close(); + + // owner's MPToken for vault shares not destroyed by clawback + env(pay(depositor, owner, shares(1))); + env.close(); + + // pay back, so we can destroy owner's MPToken now + env(pay(owner, depositor, shares(1))); + env.close(); + + { + // explicitly destroy vault owners MPToken with zero balance + Json::Value jv; + jv[sfAccount] = owner.human(); + jv[sfMPTokenIssuanceID] = to_string(issuanceId); + jv[sfFlags] = tfMPTUnauthorize; + jv[sfTransactionType] = jss::MPTokenAuthorize; + env(jv); + env.close(); + } + + // owner no longer has MPToken for vault shares + tx = pay(depositor, owner, shares(1)); + env(tx, ter{tecNO_AUTH}); + env.close(); + + // destroy all remaining shares, so we can delete vault + tx = vault.clawback( + {.issuer = issuer, + .id = keylet.key, + .holder = depositor, + .amount = asset(0)}); + env(tx); + env.close(); + + // will soft fail destroying MPToken for vault owner + env(vault.del({.owner = owner, .id = keylet.key})); + env.close(); + } + }); + testCase( [this]( Env& env, @@ -1771,6 +2135,8 @@ class Vault_test : public beast::unit_test::suite // Withdrawal to other (authorized) accounts works tx[sfDestination] = issuer.human(); env(tx); + env.close(); + tx[sfDestination] = owner.human(); env(tx); env.close(); @@ -1792,6 +2158,7 @@ class Vault_test : public beast::unit_test::suite .holder = depositor, .amount = asset(800)}); env(tx); + env.close(); env(vault.del({.owner = owner, .id = keylet.key})); }); @@ -1901,18 +2268,16 @@ class Vault_test : public beast::unit_test::suite using namespace test::jtx; auto testCase = - [&, this]( - std::function vaultAccount, - Vault& vault, - PrettyAsset const& asset, - std::function issuanceId, - std::function vaultBalance)> - test) { + [&, + this](std::function vaultAccount, + Vault& vault, + PrettyAsset const& asset, + std::function issuanceId)> test) { Env env{*this, testable_amendments() | featureSingleAssetVault}; Account const owner{"owner"}; Account const issuer{"issuer"}; @@ -1929,33 +2294,13 @@ class Vault_test : public beast::unit_test::suite env(rate(issuer, 1.25)); env.close(); - auto const [tx, keylet] = - vault.create({.owner = owner, .asset = asset}); - env(tx); - env.close(); - auto const vaultAccount = - [&env](ripple::Keylet keylet) -> AccountID { - return env.le(keylet)->at(sfAccount); + [&env](ripple::Keylet keylet) -> Account { + return Account("vault", env.le(keylet)->at(sfAccount)); }; auto const issuanceId = [&env](ripple::Keylet keylet) -> MPTID { return env.le(keylet)->at(sfShareMPTID); }; - auto const vaultBalance = // - [&env, &vaultAccount, issue = asset.raw().get()]( - ripple::Keylet keylet) -> PrettyAmount { - auto const account = vaultAccount(keylet); - auto const sle = env.le(keylet::line(account, issue)); - if (sle == nullptr) - return { - STAmount(issue, 0), - env.lookup(issue.account).name()}; - auto amount = sle->getFieldAmount(sfBalance); - amount.setIssuer(issue.account); - if (account > issue.account) - amount.negate(); - return {amount, env.lookup(issue.account).name()}; - }; test( env, @@ -1965,8 +2310,7 @@ class Vault_test : public beast::unit_test::suite vaultAccount, vault, asset, - issuanceId, - vaultBalance); + issuanceId); }; testCase([&, this]( @@ -2029,8 +2373,7 @@ class Vault_test : public beast::unit_test::suite auto vaultAccount, Vault& vault, PrettyAsset const& asset, - auto issuanceId, - auto) { + auto issuanceId) { testcase("IOU frozen trust line to vault account"); auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); @@ -2101,7 +2444,9 @@ class Vault_test : public beast::unit_test::suite env.close(); env(vault.withdraw( - {.depositor = owner, .id = keylet.key, .amount = share(50)})); + {.depositor = owner, + .id = keylet.key, + .amount = share(50'000'000)})); env(vault.del({.owner = owner, .id = keylet.key})); env.close(); @@ -2112,11 +2457,10 @@ class Vault_test : public beast::unit_test::suite Account const& owner, Account const& issuer, Account const& charlie, - auto, + auto vaultAccount, Vault& vault, PrettyAsset const& asset, - auto issuanceId, - auto vaultBalance) { + auto issuanceId) { testcase("IOU transfer fees not applied"); auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); @@ -2132,7 +2476,8 @@ class Vault_test : public beast::unit_test::suite // transfer fees ignored on deposit BEAST_EXPECT(env.balance(owner, issue) == asset(100)); - BEAST_EXPECT(vaultBalance(keylet) == asset(100)); + BEAST_EXPECT( + env.balance(vaultAccount(keylet), issue) == asset(100)); { auto tx = vault.clawback( @@ -2146,20 +2491,22 @@ class Vault_test : public beast::unit_test::suite // transfer fees ignored on clawback BEAST_EXPECT(env.balance(owner, issue) == asset(100)); - BEAST_EXPECT(vaultBalance(keylet) == asset(50)); + BEAST_EXPECT(env.balance(vaultAccount(keylet), issue) == asset(50)); env(vault.withdraw( - {.depositor = owner, .id = keylet.key, .amount = share(20)})); + {.depositor = owner, + .id = keylet.key, + .amount = share(20'000'000)})); // transfer fees ignored on withdraw BEAST_EXPECT(env.balance(owner, issue) == asset(120)); - BEAST_EXPECT(vaultBalance(keylet) == asset(30)); + BEAST_EXPECT(env.balance(vaultAccount(keylet), issue) == asset(30)); { auto tx = vault.withdraw( {.depositor = owner, .id = keylet.key, - .amount = share(30)}); + .amount = share(30'000'000)}); tx[sfDestination] = charlie.human(); env(tx); } @@ -2167,7 +2514,7 @@ class Vault_test : public beast::unit_test::suite // transfer fees ignored on withdraw to 3rd party BEAST_EXPECT(env.balance(owner, issue) == asset(120)); BEAST_EXPECT(env.balance(charlie, issue) == asset(30)); - BEAST_EXPECT(vaultBalance(keylet) == asset(0)); + BEAST_EXPECT(env.balance(vaultAccount(keylet), issue) == asset(0)); env(vault.del({.owner = owner, .id = keylet.key})); env.close(); @@ -2843,6 +3190,870 @@ class Vault_test : public beast::unit_test::suite env(tx, ter{terADDRESS_COLLISION}); } + void + testScaleIOU() + { + using namespace test::jtx; + + struct Data + { + Account const& owner; + Account const& issuer; + Account const& depositor; + Account const& vaultAccount; + MPTIssue shares; + PrettyAsset const& share; + Vault& vault; + ripple::Keylet keylet; + Issue assets; + PrettyAsset const& asset; + std::function)> peek; + }; + + auto testCase = [&, this]( + std::uint8_t scale, + std::function test) { + Env env{*this, testable_amendments() | featureSingleAssetVault}; + Account const owner{"owner"}; + Account const issuer{"issuer"}; + Account const depositor{"depositor"}; + Vault vault{env}; + env.fund(XRP(1000), issuer, owner, depositor); + env(fset(issuer, asfAllowTrustLineClawback)); + env.close(); + + PrettyAsset const asset = issuer["IOU"]; + env.trust(asset(1000), owner); + env.trust(asset(1000), depositor); + env(pay(issuer, owner, asset(200))); + env(pay(issuer, depositor, asset(200))); + env.close(); + + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = scale; + env(tx); + + auto const [vaultAccount, issuanceId] = + [&env](ripple::Keylet keylet) -> std::tuple { + auto const vault = env.le(keylet); + return { + Account("vault", vault->at(sfAccount)), + vault->at(sfShareMPTID)}; + }(keylet); + MPTIssue shares(issuanceId); + env.memoize(vaultAccount); + + auto const peek = + [=, &env, this](std::function fn) -> bool { + return env.app().openLedger().modify( + [&](OpenView& view, beast::Journal j) -> bool { + Sandbox sb(&view, tapNONE); + auto vault = sb.peek(keylet::vault(keylet.key)); + if (!BEAST_EXPECT(vault != nullptr)) + return false; + auto shares = sb.peek( + keylet::mptIssuance(vault->at(sfShareMPTID))); + if (!BEAST_EXPECT(shares != nullptr)) + return false; + if (fn(*vault, *shares)) + { + sb.update(vault); + sb.update(shares); + sb.apply(view); + return true; + } + return false; + }); + }; + + test( + env, + {.owner = owner, + .issuer = issuer, + .depositor = depositor, + .vaultAccount = vaultAccount, + .shares = shares, + .share = PrettyAsset(shares), + .vault = vault, + .keylet = keylet, + .assets = asset.raw().get(), + .asset = asset, + .peek = peek}); + }; + + testCase(18, [&, this](Env& env, Data d) { + testcase("Scale deposit overflow on first deposit"); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(10)}); + env(tx, ter{tecPATH_DRY}); + env.close(); + }); + + testCase(18, [&, this](Env& env, Data d) { + testcase("Scale deposit overflow on second deposit"); + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx); + env.close(); + } + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(10)}); + env(tx, ter{tecPATH_DRY}); + env.close(); + } + }); + + testCase(18, [&, this](Env& env, Data d) { + testcase("Scale deposit overflow on total shares"); + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx); + env.close(); + } + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx, ter{tecPATH_DRY}); + env.close(); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit exact"); + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(1)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(10)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - 1)); + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit insignificant amount"); + + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(9, -2))}); + env(tx, ter{tecPRECISION_LOSS}); + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit exact, using full precision"); + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(15, -1))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(15)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(15, -1))); + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit exact, truncating from .5"); + + auto const start = env.balance(d.depositor, d.assets).number(); + // Each of the cases below will transfer exactly 1.2 IOU to the + // vault and receive 12 shares in exchange + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(125, -2))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(12)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(12, -1))); + } + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(1201, -3))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(24)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(24, -1))); + } + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(1299, -3))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(36)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(36, -1))); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit exact, truncating from .01"); + + auto const start = env.balance(d.depositor, d.assets).number(); + // round to 12 + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(1201, -3))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(12)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(12, -1))); + + { + // round to 6 + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(69, -2))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(18)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(18, -1))); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + testcase("Scale deposit exact, truncating from .99"); + + auto const start = env.balance(d.depositor, d.assets).number(); + // round to 12 + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(1299, -3))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(12)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(12, -1))); + + { + // round to 6 + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(62, -2))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(18)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(18, -1))); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + // initial setup: deposit 100 IOU, receive 1000 shares + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(100, 0))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(1000)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(1000, 0))); + + { + testcase("Scale redeem exact"); + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 100 * 100 / 1000 = 100 * 0.1 = 10 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.share, Number(100, 0))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(10, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(90, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900, 0))); + } + + { + testcase("Scale redeem with rounding"); + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 90 * 25 / 900 = 90 * 0.02777... = 2.5 + + auto const start = env.balance(d.depositor, d.assets).number(); + d.peek([](SLE& vault, auto&) -> bool { + vault[sfAssetsAvailable] = Number(1); + return true; + }); + + // Note, this transaction fails first (because of above change + // in the open ledger) but then succeeds when the ledger is + // closed (because a modification like above is not persistent), + // which is why the checks below are expected to pass. + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.share, Number(25, 0))}); + env(tx, ter{tecINSUFFICIENT_FUNDS}); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900 - 25)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(25, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(900 - 25, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900 - 25, 0))); + } + + { + testcase("Scale redeem exact"); + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 87.5 * 21 / 875 = 87.5 * 0.024 = 2.1 + + auto const start = env.balance(d.depositor, d.assets).number(); + + tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.share, Number(21, 0))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(875 - 21)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(21, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(875 - 21, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(875 - 21, 0))); + } + + { + testcase("Scale redeem rest"); + auto const rest = env.balance(d.depositor, d.shares).number(); + + tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.share, rest)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares).number() == 0); + } + }); + + testCase(18, [&, this](Env& env, Data d) { + testcase("Scale withdraw overflow"); + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx); + env.close(); + } + + { + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx, ter{tecPATH_DRY}); + env.close(); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + // initial setup: deposit 100 IOU, receive 1000 shares + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(100, 0))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(1000)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(1000, 0))); + + { + testcase("Scale withdraw exact"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 1000 * 10 / 100 = 1000 * 0.1 = 100 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 100 * 100 / 1000 = 100 * 0.1 = 10 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(10, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(90, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900, 0))); + } + + { + testcase("Scale withdraw insignificant amount"); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(4, -2))}); + env(tx, ter{tecPRECISION_LOSS}); + } + + { + testcase("Scale withdraw with rounding assets"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 900 * 2.5 / 90 = 900 * 0.02777... = 25 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 90 * 25 / 900 = 90 * 0.02777... = 2.5 + + auto const start = env.balance(d.depositor, d.assets).number(); + d.peek([](SLE& vault, auto&) -> bool { + vault[sfAssetsAvailable] = Number(1); + return true; + }); + + // Note, this transaction fails first (because of above change + // in the open ledger) but then succeeds when the ledger is + // closed (because a modification like above is not persistent), + // which is why the checks below are expected to pass. + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(25, -1))}); + env(tx, ter{tecINSUFFICIENT_FUNDS}); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900 - 25)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(25, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(900 - 25, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900 - 25, 0))); + } + + { + testcase("Scale withdraw with rounding shares up"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 875 * 3.75 / 87.5 = 875 * 0.042857... = 37.5 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 87.5 * 38 / 875 = 87.5 * 0.043428... = 3.8 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(375, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(875 - 38)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(38, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(875 - 38, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(875 - 38, 0))); + } + + { + testcase("Scale withdraw with rounding shares down"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 837 * 3.72 / 83.7 = 837 * 0.04444... = 37.2 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 83.7 * 37 / 837 = 83.7 * 0.044205... = 3.7 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(372, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(837 - 37)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(37, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(837 - 37, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(837 - 37, 0))); + } + + { + testcase("Scale withdraw tiny amount"); + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(9, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(800 - 1)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start + Number(1, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(800 - 1, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(800 - 1, 0))); + } + + { + testcase("Scale withdraw rest"); + auto const rest = + env.balance(d.vaultAccount, d.assets).number(); + + tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, rest)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares).number() == 0); + } + }); + + testCase(18, [&, this](Env& env, Data d) { + testcase("Scale clawback overflow"); + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx); + env.close(); + } + + { + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx, ter{tecPATH_DRY}); + env.close(); + } + }); + + testCase(1, [&, this](Env& env, Data d) { + // initial setup: deposit 100 IOU, receive 1000 shares + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(100, 0))}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares) == d.share(1000)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start - Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(100, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(1000, 0))); + { + testcase("Scale clawback exact"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 1000 * 10 / 100 = 1000 * 0.1 = 100 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 100 * 100 / 1000 = 100 * 0.1 = 10 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start)); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(90, 0))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900, 0))); + } + + { + testcase("Scale clawback insignificant amount"); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(4, -2))}); + env(tx, ter{tecPRECISION_LOSS}); + } + + { + testcase("Scale clawback with rounding assets"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 900 * 2.5 / 90 = 900 * 0.02777... = 25 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 90 * 25 / 900 = 90 * 0.02777... = 2.5 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(25, -1))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(900 - 25)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start)); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(900 - 25, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(900 - 25, 0))); + } + + { + testcase("Scale clawback with rounding shares up"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 875 * 3.75 / 87.5 = 875 * 0.042857... = 37.5 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 87.5 * 38 / 875 = 87.5 * 0.043428... = 3.8 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(375, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(875 - 38)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start)); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(875 - 38, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(875 - 38, 0))); + } + + { + testcase("Scale clawback with rounding shares down"); + // assetsToSharesWithdraw: + // shares = sharesTotal * (assets / assetsTotal) + // shares = 837 * 3.72 / 83.7 = 837 * 0.04444... = 37.2 + // sharesToAssetsWithdraw: + // assets = assetsTotal * (shares / sharesTotal) + // assets = 83.7 * 37 / 837 = 83.7 * 0.044205... = 3.7 + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(372, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(837 - 37)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start)); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(837 - 37, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(837 - 37, 0))); + } + + { + testcase("Scale clawback tiny amount"); + + auto const start = env.balance(d.depositor, d.assets).number(); + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(9, -2))}); + env(tx); + env.close(); + BEAST_EXPECT( + env.balance(d.depositor, d.shares) == d.share(800 - 1)); + BEAST_EXPECT( + env.balance(d.depositor, d.assets) == + STAmount(d.asset, start)); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets) == + STAmount(d.asset, Number(800 - 1, -1))); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares) == + STAmount(d.share, Number(800 - 1, 0))); + } + + { + testcase("Scale clawback rest"); + auto const rest = + env.balance(d.vaultAccount, d.assets).number(); + d.peek([](SLE& vault, auto&) -> bool { + vault[sfAssetsAvailable] = Number(5); + return true; + }); + + // Note, this transaction yields two different results: + // * in the open ledger, with AssetsAvailable = 5 + // * when the ledger is closed with unmodified AssetsAvailable + // because a modification like above is not persistent. + tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, rest)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(d.depositor, d.shares).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.assets).number() == 0); + BEAST_EXPECT( + env.balance(d.vaultAccount, d.shares).number() == 0); + } + }); + } + void testRPC() { @@ -2943,7 +4154,8 @@ class Vault_test : public beast::unit_test::suite issuance, sfFlags, int(lsfMPTCanEscrow | lsfMPTCanTrade | lsfMPTCanTransfer))); - BEAST_EXPECT(checkString(issuance, sfOutstandingAmount, "50")); + BEAST_EXPECT( + checkString(issuance, sfOutstandingAmount, "50000000")); } }; @@ -3326,6 +4538,7 @@ public: testWithDomainCheckXRP(); testNonTransferableShares(); testFailedPseudoAccount(); + testScaleIOU(); testRPC(); } }; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 964cfe9614..f24c0b35e1 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -720,6 +720,30 @@ public: BEAST_EXPECT(res2 == STAmount{7518784}); } + void + test_truncate() + { + BEAST_EXPECT(Number(25, +1).truncate() == Number(250, 0)); + BEAST_EXPECT(Number(25, 0).truncate() == Number(25, 0)); + BEAST_EXPECT(Number(25, -1).truncate() == Number(2, 0)); + BEAST_EXPECT(Number(25, -2).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(99, -2).truncate() == Number(0, 0)); + + BEAST_EXPECT(Number(-25, +1).truncate() == Number(-250, 0)); + BEAST_EXPECT(Number(-25, 0).truncate() == Number(-25, 0)); + BEAST_EXPECT(Number(-25, -1).truncate() == Number(-2, 0)); + BEAST_EXPECT(Number(-25, -2).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(-99, -2).truncate() == Number(0, 0)); + + BEAST_EXPECT(Number(0, 0).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(0, 30000).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(0, -30000).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(100, -30000).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(100, -30000).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0)); + BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0)); + } + void run() override { @@ -740,6 +764,7 @@ public: test_stream(); test_inc_dec(); test_toSTAmount(); + test_truncate(); } }; diff --git a/src/test/jtx/Account.h b/src/test/jtx/Account.h index d91bb4a383..940960051a 100644 --- a/src/test/jtx/Account.h +++ b/src/test/jtx/Account.h @@ -74,6 +74,10 @@ public: /** @} */ + /** Create an Account from an account ID. Should only be used when the + * secret key is unavailable, such as for pseudo-accounts. */ + explicit Account(std::string name, AccountID const& id); + enum AcctStringType { base58Seed, other }; /** Create an account from a base58 seed string. Throws on invalid seed. */ Account(AcctStringType stringType, std::string base58SeedStr); diff --git a/src/test/jtx/impl/Account.cpp b/src/test/jtx/impl/Account.cpp index b61048e66f..fe901848f8 100644 --- a/src/test/jtx/impl/Account.cpp +++ b/src/test/jtx/impl/Account.cpp @@ -86,6 +86,14 @@ Account::Account(AcctStringType stringType, std::string base58SeedStr) { } +Account::Account(std::string name, AccountID const& id) + : Account(name, randomKeyPair(KeyType::secp256k1), privateCtorTag{}) +{ + // override the randomly generated values + id_ = id; + human_ = toBase58(id_); +} + IOU Account::operator[](std::string const& s) const { diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 83d5576836..201419e149 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1553,6 +1553,12 @@ ValidMPTIssuance::finalize( "not escrow finish tx"); return true; } + + if ((tx.getTxnType() == ttVAULT_CLAWBACK || + tx.getTxnType() == ttVAULT_WITHDRAW) && + mptokensDeleted_ == 1 && mptokensCreated_ == 0 && + mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0) + return true; } if (mptIssuancesCreated_ != 0) diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 7b4d15c592..4f824abba2 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -21,8 +21,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -148,7 +150,7 @@ VaultClawback::doApply() if (!vault) return tefINTERNAL; // LCOV_EXCL_LINE - auto const mptIssuanceID = (*vault)[sfShareMPTID]; + auto const mptIssuanceID = *((*vault)[sfShareMPTID]); auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleIssuance) { @@ -158,68 +160,169 @@ VaultClawback::doApply() // LCOV_EXCL_STOP } - Asset const asset = vault->at(sfAsset); + Asset const vaultAsset = vault->at(sfAsset); STAmount const amount = [&]() -> STAmount { auto const maybeAmount = tx[~sfAmount]; if (maybeAmount) return *maybeAmount; - return {sfAmount, asset, 0}; + return {sfAmount, vaultAsset, 0}; }(); XRPL_ASSERT( - amount.asset() == asset, + amount.asset() == vaultAsset, "ripple::VaultClawback::doApply : matching asset"); + auto assetsAvailable = vault->at(sfAssetsAvailable); + auto assetsTotal = vault->at(sfAssetsTotal); + [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized); + XRPL_ASSERT( + lossUnrealized <= (assetsTotal - assetsAvailable), + "ripple::VaultClawback::doApply : loss and assets do balance"); + AccountID holder = tx[sfHolder]; - STAmount assets, shares; - if (amount == beast::zero) + MPTIssue const share{mptIssuanceID}; + STAmount sharesDestroyed = {share}; + STAmount assetsRecovered; + try { - Asset share = *(*vault)[sfShareMPTID]; - shares = accountHolds( - view(), - holder, - share, - FreezeHandling::fhIGNORE_FREEZE, - AuthHandling::ahIGNORE_AUTH, - j_); - assets = sharesToAssetsWithdraw(vault, sleIssuance, shares); + if (amount == beast::zero) + { + sharesDestroyed = accountHolds( + view(), + holder, + share, + FreezeHandling::fhIGNORE_FREEZE, + AuthHandling::ahIGNORE_AUTH, + j_); + + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + assetsRecovered = *maybeAssets; + } + else + { + assetsRecovered = amount; + { + auto const maybeShares = + assetsToSharesWithdraw(vault, sleIssuance, assetsRecovered); + if (!maybeShares) + return tecINTERNAL; // LCOV_EXCL_LINE + sharesDestroyed = *maybeShares; + } + + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + assetsRecovered = *maybeAssets; + } + + // Clamp to maximum. + if (assetsRecovered > *assetsAvailable) + { + assetsRecovered = *assetsAvailable; + // Note, it is important to truncate the number of shares, otherwise + // the corresponding assets might breach the AssetsAvailable + { + auto const maybeShares = assetsToSharesWithdraw( + vault, sleIssuance, assetsRecovered, TruncateShares::yes); + if (!maybeShares) + return tecINTERNAL; // LCOV_EXCL_LINE + sharesDestroyed = *maybeShares; + } + + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + assetsRecovered = *maybeAssets; + if (assetsRecovered > *assetsAvailable) + { + // LCOV_EXCL_START + JLOG(j_.error()) + << "VaultClawback: invalid rounding of shares."; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + } } - else + catch (std::overflow_error const&) { - assets = amount; - shares = assetsToSharesWithdraw(vault, sleIssuance, assets); + // 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()) // + << "VaultClawback: overflow error with" + << " scale=" << (int)vault->at(sfScale).value() // + << ", assetsTotal=" << vault->at(sfAssetsTotal).value() + << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount) + << ", amount=" << amount.value(); + return tecPATH_DRY; } - // Clamp to maximum. - Number maxAssets = *vault->at(sfAssetsAvailable); - if (assets > maxAssets) - { - assets = maxAssets; - shares = assetsToSharesWithdraw(vault, sleIssuance, assets); - } + if (sharesDestroyed == beast::zero) + return tecPRECISION_LOSS; - if (shares == beast::zero) - return tecINSUFFICIENT_FUNDS; - - vault->at(sfAssetsTotal) -= assets; - vault->at(sfAssetsAvailable) -= assets; + assetsTotal -= assetsRecovered; + assetsAvailable -= assetsRecovered; view().update(vault); auto const& vaultAccount = vault->at(sfAccount); // Transfer shares from holder to vault. - if (auto ter = accountSend( - view(), holder, vaultAccount, shares, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + holder, + vaultAccount, + sharesDestroyed, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; + // Try to remove MPToken for shares, if the holder balance is zero. Vault + // pseudo-account will never set lsfMPTAuthorized, so we ignore flags. + // Keep MPToken if holder is the vault owner. + if (holder != vault->at(sfOwner)) + { + if (auto const ter = + removeEmptyHolding(view(), holder, sharesDestroyed.asset(), j_); + isTesSuccess(ter)) + { + JLOG(j_.debug()) // + << "VaultClawback: removed empty MPToken for vault shares" + << " MPTID=" << to_string(mptIssuanceID) // + << " account=" << toBase58(holder); + } + else if (ter != tecHAS_OBLIGATIONS) + { + // LCOV_EXCL_START + JLOG(j_.error()) // + << "VaultClawback: failed to remove MPToken for vault shares" + << " MPTID=" << to_string(mptIssuanceID) // + << " account=" << toBase58(holder) // + << " with result: " << transToken(ter); + return ter; + // LCOV_EXCL_STOP + } + // else quietly ignore, holder balance is not zero + } + // Transfer assets from vault to issuer. - if (auto ter = accountSend( - view(), vaultAccount, account_, assets, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + vaultAccount, + account_, + assetsRecovered, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; // Sanity check if (accountHolds( view(), vaultAccount, - assets.asset(), + assetsRecovered.asset(), FreezeHandling::fhIGNORE_FREEZE, AuthHandling::ahIGNORE_AUTH, j_) < beast::zero) diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 569c462e7a..8cc25e98df 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -25,8 +25,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -84,6 +86,16 @@ VaultCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } + if (auto const scale = ctx.tx[~sfScale]) + { + auto const vaultAsset = ctx.tx[sfAsset]; + if (vaultAsset.holds() || vaultAsset.native()) + return temMALFORMED; + + if (scale > vaultMaximumIOUScale) + return temMALFORMED; + } + return tesSUCCESS; } @@ -97,8 +109,8 @@ VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx) TER VaultCreate::preclaim(PreclaimContext const& ctx) { - auto vaultAsset = ctx.tx[sfAsset]; - auto account = ctx.tx[sfAccount]; + auto const vaultAsset = ctx.tx[sfAsset]; + auto const account = ctx.tx[sfAccount]; if (auto const ter = canAddHolding(ctx.view, vaultAsset)) return ter; @@ -124,7 +136,7 @@ VaultCreate::preclaim(PreclaimContext const& ctx) return tecOBJECT_NOT_FOUND; } - auto sequence = ctx.tx.getSeqValue(); + auto const sequence = ctx.tx.getSeqValue(); if (auto const accountId = pseudoAccountAddress( ctx.view, keylet::vault(account, sequence).key); accountId == beast::zero) @@ -141,8 +153,8 @@ VaultCreate::doApply() // we can consider downgrading them to `tef` or `tem`. auto const& tx = ctx_.tx; - auto sequence = tx.getSeqValue(); - auto owner = view().peek(keylet::account(account_)); + auto const sequence = tx.getSeqValue(); + auto const owner = view().peek(keylet::account(account_)); if (owner == nullptr) return tefINTERNAL; // LCOV_EXCL_LINE @@ -166,6 +178,10 @@ VaultCreate::doApply() !isTesSuccess(ter)) return ter; + std::uint8_t const scale = (asset.holds() || asset.native()) + ? 0 + : ctx_.tx[~sfScale].value_or(vaultDefaultIOUScale); + auto txFlags = tx.getFlags(); std::uint32_t mptFlags = 0; if ((txFlags & tfVaultShareNonTransferable) == 0) @@ -185,12 +201,13 @@ VaultCreate::doApply() .account = pseudoId->value(), .sequence = 1, .flags = mptFlags, + .assetScale = scale, .metadata = tx[~sfMPTokenMetadata], .domainId = tx[~sfDomainID], }); if (!maybeShare) return maybeShare.error(); // LCOV_EXCL_LINE - auto& share = *maybeShare; + auto const& mptIssuanceID = *maybeShare; vault->setFieldIssue(sfAsset, STIssue{sfAsset, asset}); vault->at(sfFlags) = txFlags & tfVaultPrivate; @@ -203,7 +220,7 @@ VaultCreate::doApply() // Leave default values for AssetTotal and AssetAvailable, both zero. if (auto value = tx[~sfAssetsMaximum]) vault->at(sfAssetsMaximum) = *value; - vault->at(sfShareMPTID) = share; + vault->at(sfShareMPTID) = mptIssuanceID; if (auto value = tx[~sfData]) vault->at(sfData) = *value; // Required field, default to vaultStrategyFirstComeFirstServe @@ -211,9 +228,31 @@ VaultCreate::doApply() vault->at(sfWithdrawalPolicy) = *value; else vault->at(sfWithdrawalPolicy) = vaultStrategyFirstComeFirstServe; - // No `LossUnrealized`. + if (scale) + vault->at(sfScale) = scale; view().insert(vault); + // Explicitly create MPToken for the vault owner + if (auto const err = authorizeMPToken( + view(), mPriorBalance, mptIssuanceID, account_, ctx_.journal); + !isTesSuccess(err)) + return err; + + // If the vault is private, set the authorized flag for the vault owner + if (txFlags & tfVaultPrivate) + { + if (auto const err = authorizeMPToken( + view(), + mPriorBalance, + mptIssuanceID, + pseudoId, + ctx_.journal, + {}, + account_); + !isTesSuccess(err)) + return err; + } + return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/VaultDelete.cpp b/src/xrpld/app/tx/detail/VaultDelete.cpp index 02b63c145d..ded23c4d28 100644 --- a/src/xrpld/app/tx/detail/VaultDelete.cpp +++ b/src/xrpld/app/tx/detail/VaultDelete.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -125,7 +126,8 @@ VaultDelete::doApply() // Destroy the share issuance. Do not use MPTokenIssuanceDestroy for this, // no special logic needed. First run few checks, duplicated from preclaim. - auto const mpt = view().peek(keylet::mptIssuance(vault->at(sfShareMPTID))); + auto const shareMPTID = *vault->at(sfShareMPTID); + auto const mpt = view().peek(keylet::mptIssuance(shareMPTID)); if (!mpt) { // LCOV_EXCL_START @@ -134,6 +136,24 @@ VaultDelete::doApply() // LCOV_EXCL_STOP } + // Try to remove MPToken for vault shares for the vault owner if it exists. + if (auto const mptoken = view().peek(keylet::mptoken(shareMPTID, account_))) + { + if (auto const ter = + removeEmptyHolding(view(), account_, MPTIssue(shareMPTID), j_); + !isTesSuccess(ter)) + { + // LCOV_EXCL_START + JLOG(j_.error()) // + << "VaultDelete: failed to remove vault owner's MPToken" + << " MPTID=" << to_string(shareMPTID) // + << " account=" << toBase58(account_) // + << " with result: " << transToken(ter); + return ter; + // LCOV_EXCL_STOP + } + } + if (!view().dirRemove( keylet::ownerDir(pseudoID), (*mpt)[sfOwnerNode], mpt->key(), false)) { diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index bd39731067..aaa1fba734 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -135,7 +136,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (isFrozen(ctx.view, account, vaultShare)) return tecLOCKED; - if (vault->isFlag(tfVaultPrivate) && account != vault->at(sfOwner)) + if (vault->isFlag(lsfVaultPrivate) && account != vault->at(sfOwner)) { auto const maybeDomainID = sleIssuance->at(~sfDomainID); // Since this is a private vault and the account is not its owner, we @@ -180,7 +181,7 @@ VaultDeposit::doApply() if (!vault) return tefINTERNAL; // LCOV_EXCL_LINE - auto const assets = ctx_.tx[sfAmount]; + auto const amount = ctx_.tx[sfAmount]; // Make sure the depositor can hold shares. auto const mptIssuanceID = (*vault)[sfShareMPTID]; auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); @@ -194,14 +195,14 @@ VaultDeposit::doApply() auto const& vaultAccount = vault->at(sfAccount); // Note, vault owner is always authorized - if ((vault->getFlags() & tfVaultPrivate) && account_ != vault->at(sfOwner)) + if (vault->isFlag(lsfVaultPrivate) && account_ != vault->at(sfOwner)) { if (auto const err = enforceMPTokenAuthorization( ctx_.view(), mptIssuanceID, account_, mPriorBalance, j_); !isTesSuccess(err)) return err; } - else + else // !vault->isFlag(lsfVaultPrivate) || account_ == vault->at(sfOwner) { // No authorization needed, but must ensure there is MPToken auto sleMpt = view().read(keylet::mptoken(mptIssuanceID, account_)); @@ -218,8 +219,12 @@ VaultDeposit::doApply() } // If the vault is private, set the authorized flag for the vault owner - if (vault->isFlag(tfVaultPrivate)) + if (vault->isFlag(lsfVaultPrivate)) { + // This follows from the reverse of the outer enclosing if condition + XRPL_ASSERT( + account_ == vault->at(sfOwner), + "ripple::VaultDeposit::doApply : account is owner"); if (auto const err = authorizeMPToken( view(), mPriorBalance, // priorBalance @@ -234,14 +239,52 @@ VaultDeposit::doApply() } } - // Compute exchange before transferring any amounts. - auto const shares = assetsToSharesDeposit(vault, sleIssuance, assets); + STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited; + 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( - shares.asset() != assets.asset(), + sharesCreated.asset() != assetsDeposited.asset(), "ripple::VaultDeposit::doApply : assets are not shares"); - vault->at(sfAssetsTotal) += assets; - vault->at(sfAssetsAvailable) += assets; + vault->at(sfAssetsTotal) += assetsDeposited; + vault->at(sfAssetsAvailable) += assetsDeposited; view().update(vault); // A deposit must not push the vault over its limit. @@ -250,15 +293,21 @@ VaultDeposit::doApply() return tecLIMIT_EXCEEDED; // Transfer assets from depositor to vault. - if (auto ter = accountSend( - view(), account_, vaultAccount, assets, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + account_, + vaultAccount, + assetsDeposited, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; // Sanity check if (accountHolds( view(), account_, - assets.asset(), + assetsDeposited.asset(), FreezeHandling::fhIGNORE_FREEZE, AuthHandling::ahIGNORE_AUTH, j_) < beast::zero) @@ -270,8 +319,14 @@ VaultDeposit::doApply() } // Transfer shares from vault to depositor. - if (auto ter = accountSend( - view(), vaultAccount, account_, shares, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + vaultAccount, + account_, + sharesCreated, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 30a99fe153..aff99b1e5f 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -105,7 +105,7 @@ VaultSet::preclaim(PreclaimContext const& ctx) if (auto const domain = ctx.tx[~sfDomainID]) { // We can only set domain if private flag was originally set - if ((vault->getFlags() & tfVaultPrivate) == 0) + if (!vault->isFlag(lsfVaultPrivate)) { JLOG(ctx.j.debug()) << "VaultSet: vault is not private"; return tecNO_PERMISSION; @@ -172,9 +172,9 @@ VaultSet::doApply() { if (*domainId != beast::zero) { - // In VaultSet::preclaim we enforce that tfVaultPrivate must have + // In VaultSet::preclaim we enforce that lsfVaultPrivate must have // been set in the vault. We currently do not support making such a - // vault public (i.e. removal of tfVaultPrivate flag). The + // vault public (i.e. removal of lsfVaultPrivate flag). The // sfDomainID flag must be set in the MPTokenIssuance object and can // be freely updated. sleIssuance->setFieldH256(sfDomainID, *domainId); diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index ae81037fc7..95db58060b 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -174,7 +174,7 @@ VaultWithdraw::doApply() if (!vault) return tefINTERNAL; // LCOV_EXCL_LINE - auto const mptIssuanceID = (*vault)[sfShareMPTID]; + auto const mptIssuanceID = *((*vault)[sfShareMPTID]); auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleIssuance) { @@ -189,24 +189,57 @@ VaultWithdraw::doApply() // to deposit into it, and this means you are also indefinitely authorized // to withdraw from it. - auto amount = ctx_.tx[sfAmount]; - auto const asset = vault->at(sfAsset); - auto const share = MPTIssue(mptIssuanceID); - STAmount shares, assets; - if (amount.asset() == asset) + auto const amount = ctx_.tx[sfAmount]; + Asset const vaultAsset = vault->at(sfAsset); + MPTIssue const share{mptIssuanceID}; + STAmount sharesRedeemed = {share}; + STAmount assetsWithdrawn; + try { - // Fixed assets, variable shares. - assets = amount; - shares = assetsToSharesWithdraw(vault, sleIssuance, assets); + if (amount.asset() == vaultAsset) + { + // Fixed assets, variable shares. + { + auto const maybeShares = + assetsToSharesWithdraw(vault, sleIssuance, amount); + if (!maybeShares) + return tecINTERNAL; // LCOV_EXCL_LINE + sharesRedeemed = *maybeShares; + } + + if (sharesRedeemed == beast::zero) + return tecPRECISION_LOSS; + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + assetsWithdrawn = *maybeAssets; + } + else if (amount.asset() == share) + { + // Fixed shares, variable assets. + sharesRedeemed = amount; + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed); + if (!maybeAssets) + return tecINTERNAL; // LCOV_EXCL_LINE + assetsWithdrawn = *maybeAssets; + } + else + return tefINTERNAL; // LCOV_EXCL_LINE } - else if (amount.asset() == share) + catch (std::overflow_error const&) { - // Fixed shares, variable assets. - shares = amount; - assets = sharesToAssetsWithdraw(vault, sleIssuance, shares); + // 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()) // + << "VaultWithdraw: overflow error with" + << " scale=" << (int)vault->at(sfScale).value() // + << ", assetsTotal=" << vault->at(sfAssetsTotal).value() + << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount) + << ", amount=" << amount.value(); + return tecPATH_DRY; } - else - return tefINTERNAL; // LCOV_EXCL_LINE if (accountHolds( view(), @@ -214,31 +247,72 @@ VaultWithdraw::doApply() share, FreezeHandling::fhZERO_IF_FROZEN, AuthHandling::ahIGNORE_AUTH, - j_) < shares) + j_) < sharesRedeemed) { JLOG(j_.debug()) << "VaultWithdraw: account doesn't hold enough shares"; return tecINSUFFICIENT_FUNDS; } - // The vault must have enough assets on hand. The vault may hold assets that - // it has already pledged. That is why we look at AssetAvailable instead of - // the pseudo-account balance. - if (*vault->at(sfAssetsAvailable) < assets) + auto assetsAvailable = vault->at(sfAssetsAvailable); + auto assetsTotal = vault->at(sfAssetsTotal); + [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized); + XRPL_ASSERT( + lossUnrealized <= (assetsTotal - assetsAvailable), + "ripple::VaultWithdraw::doApply : loss and assets do balance"); + + // The vault must have enough assets on hand. The vault may hold assets + // that it has already pledged. That is why we look at AssetAvailable + // instead of the pseudo-account balance. + if (*assetsAvailable < assetsWithdrawn) { JLOG(j_.debug()) << "VaultWithdraw: vault doesn't hold enough assets"; return tecINSUFFICIENT_FUNDS; } - vault->at(sfAssetsTotal) -= assets; - vault->at(sfAssetsAvailable) -= assets; + assetsTotal -= assetsWithdrawn; + assetsAvailable -= assetsWithdrawn; view().update(vault); auto const& vaultAccount = vault->at(sfAccount); // Transfer shares from depositor to vault. - if (auto ter = accountSend( - view(), account_, vaultAccount, shares, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + account_, + vaultAccount, + sharesRedeemed, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; + // Try to remove MPToken for shares, if the account balance is zero. Vault + // pseudo-account will never set lsfMPTAuthorized, so we ignore flags. + // Keep MPToken if holder is the vault owner. + if (account_ != vault->at(sfOwner)) + { + if (auto const ter = removeEmptyHolding( + view(), account_, sharesRedeemed.asset(), j_); + isTesSuccess(ter)) + { + JLOG(j_.debug()) // + << "VaultWithdraw: removed empty MPToken for vault shares" + << " MPTID=" << to_string(mptIssuanceID) // + << " account=" << toBase58(account_); + } + else if (ter != tecHAS_OBLIGATIONS) + { + // LCOV_EXCL_START + JLOG(j_.error()) // + << "VaultWithdraw: failed to remove MPToken for vault shares" + << " MPTID=" << to_string(mptIssuanceID) // + << " account=" << toBase58(account_) // + << " with result: " << transToken(ter); + return ter; + // LCOV_EXCL_STOP + } + // else quietly ignore, account balance is not zero + } + auto const dstAcct = [&]() -> AccountID { if (ctx_.tx.isFieldPresent(sfDestination)) return ctx_.tx.getAccountID(sfDestination); @@ -246,15 +320,21 @@ VaultWithdraw::doApply() }(); // Transfer assets from vault to depositor or destination account. - if (auto ter = accountSend( - view(), vaultAccount, dstAcct, assets, j_, WaiveTransferFee::Yes)) + if (auto const ter = accountSend( + view(), + vaultAccount, + dstAcct, + assetsWithdrawn, + j_, + WaiveTransferFee::Yes); + !isTesSuccess(ter)) return ter; // Sanity check if (accountHolds( view(), vaultAccount, - assets.asset(), + assetsWithdrawn.asset(), FreezeHandling::fhIGNORE_FREEZE, AuthHandling::ahIGNORE_AUTH, j_) < beast::zero) diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index 80dfeb3cd2..e5c50c80e0 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -928,28 +928,41 @@ deleteAMMTrustLine( std::optional const& ammAccountID, beast::Journal j); -// From the perspective of a vault, -// return the number of shares to give the depositor -// when they deposit a fixed amount of assets. -[[nodiscard]] STAmount +// From the perspective of a vault, return the number of shares to give the +// depositor when they deposit a fixed amount of assets. Since shares are MPT +// this number is integral and always truncated in this calculation. +[[nodiscard]] std::optional assetsToSharesDeposit( std::shared_ptr const& vault, std::shared_ptr const& issuance, STAmount const& assets); -// From the perspective of a vault, -// return the number of shares to demand from the depositor -// when they ask to withdraw a fixed amount of assets. -[[nodiscard]] STAmount +// From the perspective of a vault, return the number of assets to take from +// depositor when they receive a fixed amount of shares. Note, since shares are +// MPT, they are always an integral number. +[[nodiscard]] std::optional +sharesToAssetsDeposit( + std::shared_ptr const& vault, + std::shared_ptr const& issuance, + STAmount const& shares); + +enum class TruncateShares : bool { no = false, yes = true }; + +// From the perspective of a vault, return the number of shares to demand from +// the depositor when they ask to withdraw a fixed amount of assets. Since +// shares are MPT this number is integral, and it will be rounded to nearest +// unless explicitly requested to be truncated instead. +[[nodiscard]] std::optional assetsToSharesWithdraw( std::shared_ptr const& vault, std::shared_ptr const& issuance, - STAmount const& assets); + STAmount const& assets, + TruncateShares truncate = TruncateShares::no); -// From the perspective of a vault, -// return the number of assets to give the depositor -// when they redeem a fixed amount of shares. -[[nodiscard]] STAmount +// From the perspective of a vault, return the number of assets to give the +// depositor when they redeem a fixed amount of shares. Note, since shares are +// MPT, they are always an integral number. +[[nodiscard]] std::optional sharesToAssetsWithdraw( std::shared_ptr const& vault, std::shared_ptr const& issuance, diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index 3a46a4bd32..d329806177 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -2854,58 +2854,113 @@ rippleCredit( saAmount.asset().value()); } -[[nodiscard]] STAmount +[[nodiscard]] std::optional assetsToSharesDeposit( std::shared_ptr const& vault, std::shared_ptr const& issuance, STAmount const& assets) { + XRPL_ASSERT( + !assets.negative(), + "ripple::assetsToSharesDeposit : non-negative assets"); XRPL_ASSERT( assets.asset() == vault->at(sfAsset), "ripple::assetsToSharesDeposit : assets and vault match"); - Number assetTotal = vault->at(sfAssetsTotal); - STAmount shares{vault->at(sfShareMPTID), static_cast(assets)}; + if (assets.negative() || assets.asset() != vault->at(sfAsset)) + return std::nullopt; // LCOV_EXCL_LINE + + Number const assetTotal = vault->at(sfAssetsTotal); + STAmount shares{vault->at(sfShareMPTID)}; if (assetTotal == 0) - return shares; - Number shareTotal = issuance->at(sfOutstandingAmount); - shares = shareTotal * (assets / assetTotal); + return STAmount{ + shares.asset(), + Number(assets.mantissa(), assets.exponent() + vault->at(sfScale)) + .truncate()}; + + Number const shareTotal = issuance->at(sfOutstandingAmount); + shares = (shareTotal * (assets / assetTotal)).truncate(); return shares; } -[[nodiscard]] STAmount +[[nodiscard]] std::optional +sharesToAssetsDeposit( + std::shared_ptr const& vault, + std::shared_ptr const& issuance, + STAmount const& shares) +{ + XRPL_ASSERT( + !shares.negative(), + "ripple::sharesToAssetsDeposit : non-negative shares"); + XRPL_ASSERT( + shares.asset() == vault->at(sfShareMPTID), + "ripple::sharesToAssetsDeposit : shares and vault match"); + if (shares.negative() || shares.asset() != vault->at(sfShareMPTID)) + return std::nullopt; // LCOV_EXCL_LINE + + Number const assetTotal = vault->at(sfAssetsTotal); + STAmount assets{vault->at(sfAsset)}; + if (assetTotal == 0) + return STAmount{ + assets.asset(), + shares.mantissa(), + shares.exponent() - vault->at(sfScale), + false}; + + Number const shareTotal = issuance->at(sfOutstandingAmount); + assets = assetTotal * (shares / shareTotal); + return assets; +} + +[[nodiscard]] std::optional assetsToSharesWithdraw( std::shared_ptr const& vault, std::shared_ptr const& issuance, - STAmount const& assets) + STAmount const& assets, + TruncateShares truncate) { + XRPL_ASSERT( + !assets.negative(), + "ripple::assetsToSharesDeposit : non-negative assets"); XRPL_ASSERT( assets.asset() == vault->at(sfAsset), "ripple::assetsToSharesWithdraw : assets and vault match"); + if (assets.negative() || assets.asset() != vault->at(sfAsset)) + return std::nullopt; // LCOV_EXCL_LINE + Number assetTotal = vault->at(sfAssetsTotal); assetTotal -= vault->at(sfLossUnrealized); STAmount shares{vault->at(sfShareMPTID)}; if (assetTotal == 0) return shares; - Number shareTotal = issuance->at(sfOutstandingAmount); - shares = shareTotal * (assets / assetTotal); + Number const shareTotal = issuance->at(sfOutstandingAmount); + Number result = shareTotal * (assets / assetTotal); + if (truncate == TruncateShares::yes) + result = result.truncate(); + shares = result; return shares; } -[[nodiscard]] STAmount +[[nodiscard]] std::optional sharesToAssetsWithdraw( std::shared_ptr const& vault, std::shared_ptr const& issuance, STAmount const& shares) { + XRPL_ASSERT( + !shares.negative(), + "ripple::sharesToAssetsDeposit : non-negative shares"); XRPL_ASSERT( shares.asset() == vault->at(sfShareMPTID), "ripple::sharesToAssetsWithdraw : shares and vault match"); + if (shares.negative() || shares.asset() != vault->at(sfShareMPTID)) + return std::nullopt; // LCOV_EXCL_LINE + Number assetTotal = vault->at(sfAssetsTotal); assetTotal -= vault->at(sfLossUnrealized); STAmount assets{vault->at(sfAsset)}; if (assetTotal == 0) return assets; - Number shareTotal = issuance->at(sfOutstandingAmount); + Number const shareTotal = issuance->at(sfOutstandingAmount); assets = assetTotal * (shares / shareTotal); return assets; }