From 3422c11d021abe3928174907e8c57544d7817c4f Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Feb 2026 11:14:39 +0100 Subject: [PATCH 01/28] adds lending v1.1 fix amendment --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/LoanBroker_test.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d8498ffa2f..1922b2eb0c 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,7 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. - +XRPL_FIX (LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 139350a881..2be150c11c 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1701,10 +1701,21 @@ class LoanBroker_test : public beast::unit_test::suite testRIPD4274MPT(); } + void + testFixAmendmentEnabled() + { + using namespace jtx; + testcase("testFixAmendmentEnabled"); + Env env{*this}; + + BEAST_EXPECT(env.enabled(fixLendingProtocolv1_1)); + } + public: void run() override { + testFixAmendmentEnabled(); testLoanBrokerSetDebtMaximum(); testLoanBrokerCoverDepositNullVault(); From 5e51893e9ba4a7ceef7cfa4dfd7b084bf3c86bb2 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Feb 2026 11:31:58 +0100 Subject: [PATCH 02/28] fixes a typo --- src/test/app/LoanBroker_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 2be150c11c..0945b4a04a 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1708,7 +1708,7 @@ class LoanBroker_test : public beast::unit_test::suite testcase("testFixAmendmentEnabled"); Env env{*this}; - BEAST_EXPECT(env.enabled(fixLendingProtocolv1_1)); + BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1)); } public: From c5d7ebe93dc6cb092f991543d3bc87941b84d631 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Thu, 5 Feb 2026 10:23:55 +0100 Subject: [PATCH 03/28] restores missing linebreak --- include/xrpl/protocol/detail/features.macro | 1 + 1 file changed, 1 insertion(+) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 1922b2eb0c..0196dccf86 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,6 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. + XRPL_FIX (LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo) From 79be4717f573ca1e0513c8dabd5d06ba021ef14e Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 18 Feb 2026 17:59:08 +0100 Subject: [PATCH 04/28] adds vault donation feature --- include/xrpl/protocol/TxFlags.h | 2 + src/test/app/Invariants_test.cpp | 40 ++ src/test/app/Vault_test.cpp | 178 ++++- src/test/jtx/impl/vault.cpp | 3 + src/test/jtx/vault.h | 1 + src/xrpld/app/tx/detail/InvariantCheck.cpp | 724 +++++++++++---------- src/xrpld/app/tx/detail/VaultDeposit.cpp | 111 +++- src/xrpld/app/tx/detail/VaultDeposit.h | 3 + 8 files changed, 649 insertions(+), 413 deletions(-) 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); From ba530260067a6032f2b3f6683e4985ed30ebda7e Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:13:29 +0100 Subject: [PATCH 05/28] adds sfMemoData field to VaultDelete transaction (#6356) * adds sfMemoData field to VaultDelete transaction --- .../xrpl/protocol/detail/transactions.macro | 1 + .../tx/transactors/Vault/VaultDelete.cpp | 7 ++ src/test/app/Vault_test.cpp | 88 +++++++++++++++---- 3 files changed, 80 insertions(+), 16 deletions(-) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index b696a1d1c2..c0ac1ba526 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -868,6 +868,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, mustDeleteAcct | destroyMPTIssuance | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, + {sfMemoData, soeOPTIONAL}, })) /** This transaction trades assets for shares with a vault. */ diff --git a/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp b/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp index 0b3aef19a8..2562672041 100644 --- a/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp +++ b/src/libxrpl/tx/transactors/Vault/VaultDelete.cpp @@ -18,6 +18,13 @@ VaultDelete::preflight(PreflightContext const& ctx) return temMALFORMED; } + if (ctx.tx.isFieldPresent(sfMemoData) && !ctx.rules.enabled(fixLendingProtocolV1_1)) + return temDISABLED; + + // The sfMemoData field is an optional field used to record the deletion reason. + if (auto const data = ctx.tx[~sfMemoData]; data && !validDataLength(data, maxDataPayloadLength)) + return temMALFORMED; + return tesSUCCESS; } diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 93ac94d7ce..541d3975f2 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1064,14 +1064,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"}; @@ -1354,14 +1353,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"}; @@ -5357,6 +5355,63 @@ class Vault_test : public beast::unit_test::suite } } + void + testVaultDeleteData() + { + using namespace test::jtx; + + Env env{*this}; + + Account const owner{"owner"}; + env.fund(XRP(1'000'000), owner); + env.close(); + + Vault vault{env}; + + auto const keylet = keylet::vault(owner.id(), 1); + auto delTx = vault.del({.owner = owner, .id = keylet.key}); + + // Test VaultDelete with fixLendingProtocolV1_1 disabled + // Transaction fails if the data field is provided + { + testcase("VaultDelete data fixLendingProtocolV1_1 disabled"); + env.disableFeature(fixLendingProtocolV1_1); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); + env(delTx, ter(temDISABLED), THISLINE); + env.close(); + env.enableFeature(fixLendingProtocolV1_1); + } + + // Transaction fails if the data field is too large + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data too large"); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); + env(delTx, ter(temMALFORMED), THISLINE); + env.close(); + } + + // Transaction fails if the data field is set, but is empty + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data empty"); + delTx[sfMemoData] = strHex(std::string(0, 'A')); + env(delTx, ter(temMALFORMED), THISLINE); + env.close(); + } + + { + testcase("VaultDelete data fixLendingProtocolV1_1 enabled data valid"); + PrettyAsset const xrpAsset = xrpIssue(); + auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); + env(tx, ter(tesSUCCESS), THISLINE); + env.close(); + // Recreate the transaction as the vault keylet changed + auto delTx = vault.del({.owner = owner, .id = keylet.key}); + delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); + env(delTx, ter(tesSUCCESS), THISLINE); + env.close(); + } + } + public: void run() override @@ -5378,6 +5433,7 @@ public: testVaultClawbackBurnShares(); testVaultClawbackAssets(); testAssetsMaximum(); + testVaultDeleteData(); } }; From b32209752948d8794e17964c92af067116022e7e Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 3 Mar 2026 13:51:15 +0100 Subject: [PATCH 06/28] fixes formatting errors --- src/test/app/Vault_test.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index e11861a2d2..fef16cad71 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1064,14 +1064,16 @@ 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"}; Account depositor{"depositor"}; @@ -1353,13 +1355,14 @@ 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"}; From 07a6f77ed25019cd744336b99a8564518bee414a Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Mar 2026 12:08:27 +0100 Subject: [PATCH 07/28] adds missing inclde --- src/libxrpl/tx/invariants/VaultInvariant.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index cda48999ec..8680a8007b 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include From 662325ace0f51a6ebc20a054a4f8cfce1c4960e4 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:44:03 +0100 Subject: [PATCH 08/28] adds aditional unit tests --- src/libxrpl/tx/invariants/VaultInvariant.cpp | 2 +- src/libxrpl/tx/transactors/Vault/VaultDeposit.cpp | 9 ++++++--- src/test/app/Invariants_test.cpp | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 8680a8007b..641c3f6c3b 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -379,7 +379,7 @@ ValidVault::finalize( return std::nullopt; }(); - bool const isDonate = !view.rules().enabled(fixLendingProtocolV1_1) || tx.isFlag(tfVaultDonate); + 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) || // diff --git a/src/libxrpl/tx/transactors/Vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/Vault/VaultDeposit.cpp index 79cf84b713..65e85d6e37 100644 --- a/src/libxrpl/tx/transactors/Vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/Vault/VaultDeposit.cpp @@ -166,6 +166,9 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } + auto const isDonate = + ctx_.view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); + auto const& vaultAccount = vault->at(sfAccount); // Note, vault owner is always authorized if (vault->isFlag(lsfVaultPrivate) && account_ != vault->at(sfOwner)) @@ -206,7 +209,7 @@ VaultDeposit::doApply() } } STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited; - if (view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate)) + if (isDonate) { XRPL_ASSERT( account_ == vault->at(sfOwner), "xrpl::VaultDeposit::doApply : account is owner"); @@ -286,10 +289,10 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } - if (view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate)) + if (isDonate) { XRPL_ASSERT( - sharesCreated == beast::zero, "xrpl::VaultDeposit::doApply: donation issued shares"); + sharesCreated == beast::zero, "xrpl::VaultDeposit::doApply : donation issued shares"); } else { diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index f8f62e10e8..b2a67cf895 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3366,6 +3366,21 @@ class Invariants_test : public beast::unit_test::suite precloseXrp, TxAccount::A2); + doInvariantCheck( + Env{*this, testable_amendments() - fixLendingProtocolV1_1}, + {"deposit must 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.reset(); + })); + }, + XRPAmount{}, + STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + doInvariantCheck( {"deposit must change depositor shares"}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { From 4067e5025f3d6f5da060110c8eb5bf2474f4c1eb Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 5 Mar 2026 10:38:42 +0100 Subject: [PATCH 09/28] Add rounding to Vault invariants (#6217) Co-authored-by: Ed Hennis --- include/xrpl/protocol/STAmount.h | 19 +- include/xrpl/tx/invariants/VaultInvariant.h | 15 +- .../tx/transactors/Lending/LendingHelpers.h | 2 +- src/libxrpl/tx/invariants/VaultInvariant.cpp | 276 +++++++++++++----- .../Lending/LoanBrokerCoverWithdraw.cpp | 2 +- src/test/app/Invariants_test.cpp | 128 ++++++++ src/test/app/LendingHelpers_test.cpp | 1 - src/test/app/Loan_test.cpp | 137 ++++++++- 8 files changed, 506 insertions(+), 74 deletions(-) diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index dadeec096f..88a4642159 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -42,8 +42,8 @@ private: public: using value_type = STAmount; - static int const cMinOffset = -96; - static int const cMaxOffset = 80; + static int constexpr cMinOffset = -96; + static int constexpr cMaxOffset = 80; // Maximum native value supported by the code constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull; @@ -739,6 +739,21 @@ canAdd(STAmount const& amt1, STAmount const& amt2); bool canSubtract(STAmount const& amt1, STAmount const& amt2); +/** Get the scale of a Number for a given asset. + * + * "scale" is similar to "exponent", but from the perspective of STAmount, which has different rules + * and mantissa ranges for determining the exponent than Number. + * + * @param number The Number to get the scale of. + * @param asset The asset to use for determining the scale. + * @return The scale of this Number for the given asset. + */ +inline int +scale(Number const& number, Asset const& asset) +{ + return STAmount{asset, number}.exponent(); +} + } // namespace xrpl //------------------------------------------------------------------------------ diff --git a/include/xrpl/tx/invariants/VaultInvariant.h b/include/xrpl/tx/invariants/VaultInvariant.h index ded9e4618b..1e1ded6fa1 100644 --- a/include/xrpl/tx/invariants/VaultInvariant.h +++ b/include/xrpl/tx/invariants/VaultInvariant.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -60,11 +61,19 @@ class ValidVault Shares static make(SLE const&); }; +public: + struct DeltaInfo final + { + Number delta = numZero; + std::optional scale; + }; + +private: std::vector afterVault_ = {}; std::vector afterMPTs_ = {}; std::vector beforeVault_ = {}; std::vector beforeMPTs_ = {}; - std::unordered_map deltas_ = {}; + std::unordered_map deltas_ = {}; public: void @@ -72,6 +81,10 @@ public: bool finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); + + // Compute the coarsest scale required to represent all numbers + [[nodiscard]] static std::int32_t + computeMinScale(Asset const& asset, std::vector const& numbers); }; } // namespace xrpl diff --git a/include/xrpl/tx/transactors/Lending/LendingHelpers.h b/include/xrpl/tx/transactors/Lending/LendingHelpers.h index 4057c9c173..8dd6866ac3 100644 --- a/include/xrpl/tx/transactors/Lending/LendingHelpers.h +++ b/include/xrpl/tx/transactors/Lending/LendingHelpers.h @@ -171,7 +171,7 @@ getAssetsTotalScale(SLE::const_ref vaultSle) { if (!vaultSle) return Number::minExponent - 1; // LCOV_EXCL_LINE - return STAmount{vaultSle->at(sfAsset), vaultSle->at(sfAssetsTotal)}.exponent(); + return scale(vaultSle->at(sfAssetsTotal), vaultSle->at(sfAsset)); } TER diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index c3db3a563a..69d29448d3 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -60,10 +61,12 @@ ValidVault::visitEntry( "xrpl::ValidVault::visitEntry : some object is available"); // Number balanceDelta will capture the difference (delta) between "before" - // state (zero if created) and "after" state (zero if destroyed), so the - // invariants can validate that the change in account balances matches the - // change in vault balances, stored to deltas_ at the end of this function. - Number balanceDelta{}; + // state (zero if created) and "after" state (zero if destroyed), and + // preserves value scale (exponent) to round values to the same scale during + // validation. It is used to validate that the change in account + // balances matches the change in vault balances, stored to deltas_ at the + // end of this function. + DeltaInfo balanceDelta{numZero, std::nullopt}; std::int8_t sign = 0; if (before) @@ -77,18 +80,34 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. beforeMPTs_.push_back(Shares::make(*before)); - balanceDelta = static_cast(before->getFieldU64(sfOutstandingAmount)); + balanceDelta.delta = + static_cast(before->getFieldU64(sfOutstandingAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta = static_cast(before->getFieldU64(sfMPTAmount)); + balanceDelta.delta = static_cast(before->getFieldU64(sfMPTAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta = before->getFieldAmount(sfBalance); + balanceDelta.delta = before->getFieldAmount(sfBalance); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = before->getFieldAmount(sfBalance); + balanceDelta.delta = amount; + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -104,19 +123,36 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. afterMPTs_.push_back(Shares::make(*after)); - balanceDelta -= + balanceDelta.delta -= Number(static_cast(after->getFieldU64(sfOutstandingAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta -= Number(static_cast(after->getFieldU64(sfMPTAmount))); + balanceDelta.delta -= + Number(static_cast(after->getFieldU64(sfMPTAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta -= Number(after->getFieldAmount(sfBalance)); + balanceDelta.delta -= Number(after->getFieldAmount(sfBalance)); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = after->getFieldAmount(sfBalance); + balanceDelta.delta -= Number(amount); + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + if (amount.exponent() > balanceDelta.scale) + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -128,7 +164,11 @@ ValidVault::visitEntry( // transferred to the account. We intentionally do not compare balanceDelta // against zero, to avoid missing such updates. if (sign != 0) - deltas_[key] = balanceDelta * sign; + { + XRPL_ASSERT_PARTS(balanceDelta.scale, "xrpl::ValidVault::visitEntry", "scale initialized"); + balanceDelta.delta *= sign; + deltas_[key] = balanceDelta; + } } bool @@ -390,13 +430,13 @@ ValidVault::finalize( } auto const& vaultAsset = afterVault.asset; - auto const deltaAssets = [&](AccountID const& id) -> std::optional { + auto const deltaAssets = [&](AccountID const& id) -> std::optional { auto const get = // - [&](auto const& it, std::int8_t sign = 1) -> std::optional { + [&](auto const& it, std::int8_t sign = 1) -> std::optional { if (it == deltas_.end()) return std::nullopt; - return it->second * sign; + return DeltaInfo{it->second.delta * sign, it->second.scale}; }; return std::visit( @@ -415,7 +455,7 @@ ValidVault::finalize( }, vaultAsset.value()); }; - auto const deltaAssetsTxAccount = [&]() -> std::optional { + auto const deltaAssetsTxAccount = [&]() -> std::optional { auto ret = deltaAssets(tx[sfAccount]); // Nothing returned or not XRP transaction if (!ret.has_value() || !vaultAsset.native()) @@ -426,20 +466,20 @@ ValidVault::finalize( delegate.has_value() && *delegate != tx[sfAccount]) return ret; - *ret += fee.drops(); - if (*ret == zero) + ret->delta += fee.drops(); + if (ret->delta == zero) return std::nullopt; return ret; }; - auto const deltaShares = [&](AccountID const& id) -> std::optional { + auto const deltaShares = [&](AccountID const& id) -> std::optional { auto const it = [&]() { if (id == afterVault.pseudoId) return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key); return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); }(); - return it != deltas_.end() ? std::optional(it->second) : std::nullopt; + return it != deltas_.end() ? std::optional(it->second) : std::nullopt; }; auto const vaultHoldsNoAssets = [&](Vault const& vault) { @@ -566,16 +606,38 @@ ValidVault::finalize( !beforeVault_.empty(), "xrpl::ValidVault::finalize : deposit updated a vault"); auto const& beforeVault = beforeVault_[0]; - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - - if (!vaultDeltaAssets) + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault balance"; return false; // That's all we can do } - if (*vaultDeltaAssets > tx[sfAmount]) + // Get the coarsest scale to round calculations to + DeltaInfo totalDelta{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + DeltaInfo availableDelta{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, + { + *maybeVaultDeltaAssets, + totalDelta, + availableDelta, + }); + + auto const vaultDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + auto const txAmount = roundToAsset(vaultAsset, tx[sfAmount], minScale); + + if (vaultDeltaAssets > txAmount) { JLOG(j.fatal()) << // "Invariant failed: deposit must not change vault " @@ -583,7 +645,7 @@ ValidVault::finalize( result = false; } - if (*vaultDeltaAssets <= zero) + if (vaultDeltaAssets <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase vault balance"; @@ -600,16 +662,23 @@ ValidVault::finalize( if (!issuerDeposit) { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - if (!accountDeltaAssets) + auto const maybeAccDeltaAssets = deltaAssetsTxAccount(); + if (!maybeAccDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor " "balance"; return false; } + auto const localMinScale = + std::max(minScale, computeMinScale(vaultAsset, {*maybeAccDeltaAssets})); - if (*accountDeltaAssets >= zero) + auto const accountDeltaAssets = + roundToAsset(vaultAsset, maybeAccDeltaAssets->delta, localMinScale); + auto const localVaultDeltaAssets = + roundToAsset(vaultAsset, vaultDeltaAssets, localMinScale); + + if (accountDeltaAssets >= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must decrease depositor " @@ -617,7 +686,7 @@ ValidVault::finalize( result = false; } - if (*accountDeltaAssets * -1 != *vaultDeltaAssets) + if (localVaultDeltaAssets * -1 != accountDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault and " @@ -635,16 +704,17 @@ ValidVault::finalize( result = false; } - auto const accountDeltaShares = deltaShares(tx[sfAccount]); - if (!accountDeltaShares) + auto const maybeAccDeltaShares = deltaShares(tx[sfAccount]); + if (!maybeAccDeltaShares) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor " "shares"; return false; // That's all we can do } - - if (*accountDeltaShares <= zero) + // We don't need to round shares, they are integral MPT + auto const& accountDeltaShares = *maybeAccDeltaShares; + if (accountDeltaShares.delta <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase depositor " @@ -652,15 +722,17 @@ ValidVault::finalize( result = false; } - auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId); + if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + // We don't need to round shares, they are integral MPT + auto const& vaultDeltaShares = *maybeVaultDeltaShares; + if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor and " @@ -668,13 +740,18 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + auto const assetTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); + if (assetTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "outstanding must add up"; result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + + auto const assetAvailableDelta = roundToAsset( + vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "available must add up"; @@ -692,16 +769,33 @@ ValidVault::finalize( "vault"); auto const& beforeVault = beforeVault_[0]; - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (!vaultDeltaAssets) + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal must " "change vault balance"; return false; // That's all we can do } - if (*vaultDeltaAssets >= zero) + // Get the most coarse scale to round calculations to + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + + auto const vaultPseudoDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + + if (vaultPseudoDeltaAssets >= zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must " "decrease vault balance"; @@ -719,15 +813,15 @@ ValidVault::finalize( if (!issuerWithdrawal) { - auto const accountDeltaAssets = deltaAssetsTxAccount(); - auto const otherAccountDelta = [&]() -> std::optional { + auto const maybeAccDelta = deltaAssetsTxAccount(); + auto const maybeOtherAccDelta = [&]() -> 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()) + if (maybeAccDelta.has_value() == maybeOtherAccDelta.has_value()) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change one " @@ -736,9 +830,17 @@ ValidVault::finalize( } auto const destinationDelta = // - accountDeltaAssets ? *accountDeltaAssets : *otherAccountDelta; + maybeAccDelta ? *maybeAccDelta : *maybeOtherAccDelta; - if (destinationDelta <= zero) + // the scale of destinationDelta can be coarser than + // minScale, so we take that into account when rounding + auto const localMinScale = + std::max(minScale, computeMinScale(vaultAsset, {destinationDelta})); + + auto const roundedDestinationDelta = + roundToAsset(vaultAsset, destinationDelta.delta, localMinScale); + + if (roundedDestinationDelta <= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must increase " @@ -746,7 +848,9 @@ ValidVault::finalize( result = false; } - if (*vaultDeltaAssets * -1 != destinationDelta) + auto const localPseudoDeltaAssets = + roundToAsset(vaultAsset, vaultPseudoDeltaAssets, localMinScale); + if (localPseudoDeltaAssets * -1 != roundedDestinationDelta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault " @@ -755,6 +859,7 @@ ValidVault::finalize( } } + // We don't need to round shares, they are integral MPT auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (!accountDeltaShares) { @@ -764,7 +869,7 @@ ValidVault::finalize( return false; } - if (*accountDeltaShares >= zero) + if (accountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must decrease depositor " @@ -772,15 +877,16 @@ ValidVault::finalize( result = false; } + // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change depositor " @@ -788,15 +894,20 @@ ValidVault::finalize( result = false; } + auto const assetTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); // Note, vaultBalance is negative (see check above) - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + if (assetTotalDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets outstanding must add up"; result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != afterVault.assetsAvailable) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale); + + if (assetAvailableDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets available must add up"; @@ -827,10 +938,24 @@ ValidVault::finalize( } } - auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (vaultDeltaAssets) + auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); + if (maybeVaultDeltaAssets) { - if (*vaultDeltaAssets >= zero) + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + scale(afterVault.assetsTotal, vaultAsset), + scale(beforeVault.assetsTotal, vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + scale(afterVault.assetsAvailable, vaultAsset), + scale(beforeVault.assetsAvailable, vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + auto const vaultDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + if (vaultDeltaAssets >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease vault " @@ -838,7 +963,9 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsTotal + *vaultDeltaAssets != afterVault.assetsTotal) + auto const assetsTotalDelta = roundToAsset( + vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); + if (assetsTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: clawback and assets outstanding " @@ -846,8 +973,11 @@ ValidVault::finalize( result = false; } - if (beforeVault.assetsAvailable + *vaultDeltaAssets != - afterVault.assetsAvailable) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, + afterVault.assetsAvailable - beforeVault.assetsAvailable, + minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: clawback and assets available " @@ -862,15 +992,15 @@ ValidVault::finalize( return false; // That's all we can do } - auto const accountDeltaShares = deltaShares(tx[sfHolder]); - if (!accountDeltaShares) + // We don't need to round shares, they are integral MPT + auto const maybeAccountDeltaShares = deltaShares(tx[sfHolder]); + if (!maybeAccountDeltaShares) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder shares"; return false; // That's all we can do } - - if (*accountDeltaShares >= zero) + if (maybeAccountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease holder " @@ -878,15 +1008,16 @@ ValidVault::finalize( result = false; } + // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != maybeAccountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder and " @@ -923,4 +1054,19 @@ ValidVault::finalize( return true; } +[[nodiscard]] std::int32_t +ValidVault::computeMinScale(Asset const& asset, std::vector const& numbers) +{ + if (numbers.size() == 0) + return 0; + + auto const max = + std::max_element(numbers.begin(), numbers.end(), [](auto const& a, auto const& b) -> bool { + return a.scale < b.scale; + }); + XRPL_ASSERT_PARTS( + max->scale, "xrpl::ValidVault::computeMinScale", "scale set for destinationDelta"); + return max->scale.value_or(STAmount::cMaxOffset); +} + } // namespace xrpl diff --git a/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp index 43ff3659ef..075f615210 100644 --- a/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/Lending/LoanBrokerCoverWithdraw.cpp @@ -121,7 +121,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) return roundToAsset( vaultAsset, tenthBipsOfValue(currentDebtTotal, TenthBips32(sleBroker->at(sfCoverRateMinimum))), - currentDebtTotal.exponent()); + scale(currentDebtTotal, vaultAsset)); }(); if (coverAvail < amount) return tecINSUFFICIENT_FUNDS; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index a01026c8ef..5be316c73e 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -16,9 +16,14 @@ #include #include #include +#include #include +#include +#include +#include + namespace xrpl { namespace test { @@ -3793,6 +3798,128 @@ class Invariants_test : public beast::unit_test::suite precloseMpt); } + void + testVaultComputeMinScale() + { + using namespace jtx; + + Account const issuer{"issuer"}; + PrettyAsset const vaultAsset = issuer["IOU"]; + + struct TestCase + { + std::string name; + std::int32_t expectedMinScale; + std::vector values; + }; + + NumberMantissaScaleGuard g{MantissaRange::large}; + + auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {n, scale(n, vaultAsset.raw())}; + }; + + auto const testCases = std::vector{ + { + .name = "No values", + .expectedMinScale = 0, + .values = {}, + }, + { + .name = "Mixed integer and Number values", + .expectedMinScale = -15, + .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, + }, + { + .name = "Mixed scales", + .expectedMinScale = -17, + .values = + {makeDelta(Number{1, -2}), makeDelta(Number{5, -3}), makeDelta(Number{3, -2})}, + }, + { + .name = "Equal scales", + .expectedMinScale = -16, + .values = + {makeDelta(Number{1, -1}), makeDelta(Number{5, -1}), makeDelta(Number{1, -1})}, + }, + { + .name = "Mixed mantissa sizes", + .expectedMinScale = -12, + .values = + {makeDelta(Number{1}), + makeDelta(Number{1234, -3}), + makeDelta(Number{12345, -6}), + makeDelta(Number{123, 1})}, + }, + }; + + for (auto const& tc : testCases) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = ValidVault::computeMinScale(vaultAsset, tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + for (auto const& num : tc.values) + { + // None of these scales are far enough apart that rounding the + // values would lose information, so check that the rounded + // value matches the original. + auto const actualRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + actualRounded == num.delta, + "number " + to_string(num.delta) + " rounded to scale " + + std::to_string(actualScale) + " is " + to_string(actualRounded)); + } + } + + auto const testCases2 = std::vector{ + { + .name = "False equivalence", + .expectedMinScale = -15, + .values = + { + makeDelta(Number{1234567890123456789, -18}), + makeDelta(Number{12345, -4}), + makeDelta(Number{1}), + }, + }, + }; + + // Unlike the first set of test cases, the values in these test could + // look equivalent if using the wrong scale. + for (auto const& tc : testCases2) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = ValidVault::computeMinScale(vaultAsset, tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + std::optional first; + Number firstRounded; + for (auto const& num : tc.values) + { + if (!first) + { + first = num.delta; + firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); + continue; + } + auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + numRounded != firstRounded, + "at a scale of " + std::to_string(actualScale) + " " + to_string(num.delta) + + " == " + to_string(*first)); + } + } + } + public: void run() override @@ -3818,6 +3945,7 @@ public: testValidPseudoAccounts(); testValidLoanBroker(); testVault(); + testVaultComputeMinScale(); } }; diff --git a/src/test/app/LendingHelpers_test.cpp b/src/test/app/LendingHelpers_test.cpp index aae60a252a..c826b111e2 100644 --- a/src/test/app/LendingHelpers_test.cpp +++ b/src/test/app/LendingHelpers_test.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 607e84abeb..ba28f25e71 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -2644,7 +2644,7 @@ protected: env(manage(lender, loanKeylet.key, tfLoanDefault), ter(tecNO_PERMISSION)); }); -#if LOANTODO +#if LOAN_TODO // TODO /* @@ -5316,7 +5316,7 @@ protected: } } -#if LOANTODO +#if LOAN_TODO void testLoanPayLateFullPaymentBypassesPenalties() { @@ -6967,14 +6967,145 @@ protected: BEAST_EXPECT(afterSecondCoverAvailable == 0); } + // Tests that vault withdrawals work correctly when the vault has unrealized + // loss from an impaired loan, ensuring the invariant check properly + // accounts for the loss. + void + testWithdrawReflectsUnrealizedLoss() + { + using namespace jtx; + using namespace loan; + using namespace std::chrono_literals; + + testcase("Vault withdraw reflects sfLossUnrealized"); + + // Test constants + static constexpr std::int64_t INITIAL_FUNDING = 1'000'000; + static constexpr std::int64_t LENDER_INITIAL_IOU = 5'000'000; + static constexpr std::int64_t DEPOSITOR_INITIAL_IOU = 1'000'000; + static constexpr std::int64_t BORROWER_INITIAL_IOU = 100'000; + static constexpr std::int64_t DEPOSIT_AMOUNT = 5'000; + static constexpr std::int64_t PRINCIPAL_AMOUNT = 99; + static constexpr std::uint64_t EXPECTED_SHARES_PER_DEPOSITOR = 5'000'000'000; + static constexpr std::uint32_t PAYMENT_INTERVAL = 600; + static constexpr std::uint32_t PAYMENT_TOTAL = 2; + + Env env(*this, all); + + // Setup accounts + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const depositorA{"lpA"}; + Account const depositorB{"lpB"}; + Account const borrower{"borrowerA"}; + + env.fund(XRP(INITIAL_FUNDING), issuer, lender, depositorA, depositorB, borrower); + env.close(); + + // Setup trust lines + PrettyAsset const iouAsset = issuer[iouCurrency]; + env(trust(lender, iouAsset(10'000'000))); + env(trust(depositorA, iouAsset(10'000'000))); + env(trust(depositorB, iouAsset(10'000'000))); + env(trust(borrower, iouAsset(10'000'000))); + env.close(); + + // Fund accounts with IOUs + env(pay(issuer, lender, iouAsset(LENDER_INITIAL_IOU))); + env(pay(issuer, depositorA, iouAsset(DEPOSITOR_INITIAL_IOU))); + env(pay(issuer, depositorB, iouAsset(DEPOSITOR_INITIAL_IOU))); + env(pay(issuer, borrower, iouAsset(BORROWER_INITIAL_IOU))); + env.close(); + + // Create vault and broker, then add deposits from two depositors + auto const broker = createVaultAndBroker(env, iouAsset, lender); + Vault v{env}; + + env(v.deposit({ + .depositor = depositorA, + .id = broker.vaultKeylet().key, + .amount = iouAsset(DEPOSIT_AMOUNT), + }), + ter(tesSUCCESS)); + env(v.deposit({ + .depositor = depositorB, + .id = broker.vaultKeylet().key, + .amount = iouAsset(DEPOSIT_AMOUNT), + }), + ter(tesSUCCESS)); + env.close(); + + // Create a loan + auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID)); + if (!BEAST_EXPECT(sleBroker)) + return; + + auto const loanKeylet = keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence)); + + env(set(borrower, broker.brokerID, PRINCIPAL_AMOUNT), + sig(sfCounterpartySignature, lender), + paymentTotal(PAYMENT_TOTAL), + paymentInterval(PAYMENT_INTERVAL), + fee(env.current()->fees().base * 2), + ter(tesSUCCESS)); + env.close(); + + // Impair the loan to create unrealized loss + env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); + env.close(); + + // Verify unrealized loss is recorded in the vault + auto const vaultAfterImpair = env.le(broker.vaultKeylet()); + if (!BEAST_EXPECT(vaultAfterImpair)) + return; + + BEAST_EXPECT( + vaultAfterImpair->at(sfLossUnrealized) == broker.asset(PRINCIPAL_AMOUNT).value()); + + // Helper to get share balance for a depositor + auto const shareAsset = vaultAfterImpair->at(sfShareMPTID); + auto const getShareBalance = [&](Account const& depositor) -> std::uint64_t { + auto const token = env.le(keylet::mptoken(shareAsset, depositor.id())); + return token ? token->getFieldU64(sfMPTAmount) : 0; + }; + + // Verify both depositors have equal shares + auto const sharesLpA = getShareBalance(depositorA); + auto const sharesLpB = getShareBalance(depositorB); + BEAST_EXPECT(sharesLpA == EXPECTED_SHARES_PER_DEPOSITOR); + BEAST_EXPECT(sharesLpB == EXPECTED_SHARES_PER_DEPOSITOR); + BEAST_EXPECT(sharesLpA == sharesLpB); + + // Helper to attempt withdrawal + auto const attemptWithdrawShares = [&](Account const& depositor, + std::uint64_t shareAmount, + TER expected) { + STAmount const shareAmt{MPTIssue{shareAsset}, Number(shareAmount)}; + env(v.withdraw( + {.depositor = depositor, .id = broker.vaultKeylet().key, .amount = shareAmt}), + ter(expected)); + env.close(); + }; + + // Regression test: Both depositors should successfully withdraw despite + // unrealized loss. Previously failed with invariant violation: + // "withdrawal must change vault and destination balance by equal + // amount". This was caused by sharesToAssetsWithdraw rounding down, + // creating a mismatch where vaultDeltaAssets * -1 != destinationDelta + // when unrealized loss exists. + attemptWithdrawShares(depositorA, sharesLpA, tesSUCCESS); + attemptWithdrawShares(depositorB, sharesLpB, tesSUCCESS); + } + public: void run() override { -#if LOANTODO +#if LOAN_TODO testLoanPayLateFullPaymentBypassesPenalties(); testLoanCoverMinimumRoundingExploit(); #endif + testWithdrawReflectsUnrealizedLoss(); testInvalidLoanSet(); testCoverDepositWithdrawNonTransferableMPT(); From 72221500959c0c7ba8b1fe68b7d835ac7c2059f9 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Mon, 16 Mar 2026 09:26:57 +0100 Subject: [PATCH 10/28] refactor: Rename fixLendingProtocolV1_1 to featureLendingProtocolV1_1 (#6527) Use XRPL_FEATURE macro instead of XRPL_FIX since LendingProtocolV1_1 is a feature amendment, not a fix. Update all references in VaultDelete and related tests. --- include/xrpl/protocol/detail/features.macro | 2 +- src/libxrpl/tx/transactors/vault/VaultDelete.cpp | 2 +- src/test/app/LoanBroker_test.cpp | 8 ++++---- src/test/app/Vault_test.cpp | 14 +++++++------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 5c7bd1bd4f..b806dc2fd0 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -16,7 +16,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. -XRPL_FIX (LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) +XRPL_FEATURE(LendingProtocolV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::no, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp index d7cf9e8c60..88f91cd1bc 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp @@ -18,7 +18,7 @@ VaultDelete::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (ctx.tx.isFieldPresent(sfMemoData) && !ctx.rules.enabled(fixLendingProtocolV1_1)) + if (ctx.tx.isFieldPresent(sfMemoData) && !ctx.rules.enabled(featureLendingProtocolV1_1)) return temDISABLED; // The sfMemoData field is an optional field used to record the deletion reason. diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 4c108d9007..a2e7d4c84b 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1788,20 +1788,20 @@ class LoanBroker_test : public beast::unit_test::suite } void - testFixAmendmentEnabled() + testFeatureLendingProtocolV1_1enabled() { using namespace jtx; - testcase("testFixAmendmentEnabled"); + testcase("featureLendingProtocolV1_1 enabled"); Env env{*this}; - BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1)); + BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1)); } public: void run() override { - testFixAmendmentEnabled(); + testFeatureLendingProtocolV1_1enabled(); testLoanBrokerSetDebtMaximum(); testLoanBrokerCoverDepositNullVault(); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index fef16cad71..6e1ede00fb 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5374,20 +5374,20 @@ class Vault_test : public beast::unit_test::suite auto const keylet = keylet::vault(owner.id(), 1); auto delTx = vault.del({.owner = owner, .id = keylet.key}); - // Test VaultDelete with fixLendingProtocolV1_1 disabled + // Test VaultDelete with featureLendingProtocolV1_1 disabled // Transaction fails if the data field is provided { - testcase("VaultDelete data fixLendingProtocolV1_1 disabled"); - env.disableFeature(fixLendingProtocolV1_1); + testcase("VaultDelete data featureLendingProtocolV1_1 disabled"); + env.disableFeature(featureLendingProtocolV1_1); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); env(delTx, ter(temDISABLED), THISLINE); env.close(); - env.enableFeature(fixLendingProtocolV1_1); + env.enableFeature(featureLendingProtocolV1_1); } // Transaction fails if the data field is too large { - testcase("VaultDelete data fixLendingProtocolV1_1 enabled data too large"); + testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large"); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); env(delTx, ter(temMALFORMED), THISLINE); env.close(); @@ -5395,14 +5395,14 @@ class Vault_test : public beast::unit_test::suite // Transaction fails if the data field is set, but is empty { - testcase("VaultDelete data fixLendingProtocolV1_1 enabled data empty"); + testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty"); delTx[sfMemoData] = strHex(std::string(0, 'A')); env(delTx, ter(temMALFORMED), THISLINE); env.close(); } { - testcase("VaultDelete data fixLendingProtocolV1_1 enabled data valid"); + testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid"); PrettyAsset const xrpAsset = xrpIssue(); auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); env(tx, ter(tesSUCCESS), THISLINE); From d02f53498769bcbe91b8e8fe66873464214dcfb5 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Sat, 21 Mar 2026 16:17:18 +0100 Subject: [PATCH 11/28] refactor: Rename fixLendingProtocolV1_1 to featureLendingProtocolV1_1 and remove THISLINE --- .../transactions/VaultDelete.h | 37 ++++++++++++++ src/libxrpl/tx/invariants/VaultInvariant.cpp | 5 +- .../tx/transactors/vault/VaultDeposit.cpp | 11 +++-- src/test/app/Invariants_test.cpp | 2 +- src/test/app/Vault_test.cpp | 37 +++++++------- .../transactions/VaultDeleteTests.cpp | 49 +++++++++++++++++++ 6 files changed, 115 insertions(+), 26 deletions(-) diff --git a/include/xrpl/protocol_autogen/transactions/VaultDelete.h b/include/xrpl/protocol_autogen/transactions/VaultDelete.h index 89a4ef2a2f..bc89ce2bb7 100644 --- a/include/xrpl/protocol_autogen/transactions/VaultDelete.h +++ b/include/xrpl/protocol_autogen/transactions/VaultDelete.h @@ -57,6 +57,32 @@ public: { return this->tx_->at(sfVaultID); } + + /** + * @brief Get sfMemoData (soeOPTIONAL) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getMemoData() const + { + if (hasMemoData()) + { + return this->tx_->at(sfMemoData); + } + return std::nullopt; + } + + /** + * @brief Check if sfMemoData is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasMemoData() const + { + return this->tx_->isFieldPresent(sfMemoData); + } }; /** @@ -112,6 +138,17 @@ public: return *this; } + /** + * @brief Set sfMemoData (soeOPTIONAL) + * @return Reference to this builder for method chaining. + */ + VaultDeleteBuilder& + setMemoData(std::decay_t const& value) + { + object_[sfMemoData] = value; + return *this; + } + /** * @brief Build and return the VaultDelete wrapper. * @param publicKey The public key for signing. diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index ed22bbeead..a609d2782a 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -419,7 +419,8 @@ ValidVault::finalize( return std::nullopt; }(); - bool const isDonate = view.rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); + bool const isDonate = + view.rules().enabled(featureLendingProtocolV1_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) || // @@ -710,7 +711,7 @@ ValidVault::finalize( } // If assets are donated, check share invariants - if (view.rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) + if (view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) { auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (accountDeltaShares) diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index abb0e3bfc5..79a6e58093 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -17,7 +17,7 @@ namespace xrpl { std::uint32_t VaultDeposit::getFlagsMask(PreflightContext const& ctx) { - if (ctx.rules.enabled(fixLendingProtocolV1_1)) + if (ctx.rules.enabled(featureLendingProtocolV1_1)) return tfVaultDepositMask; return tfVaultDepositMask | tfVaultDonate; @@ -77,7 +77,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) + if (ctx.view.rules().enabled(featureLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) { if (account != vault->at(sfOwner)) { @@ -169,7 +169,7 @@ VaultDeposit::doApply() } auto const isDonate = - ctx_.view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); + ctx_.view().rules().enabled(featureLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); auto const& vaultAccount = vault->at(sfAccount); // Note, vault owner is always authorized @@ -233,8 +233,11 @@ VaultDeposit::doApply() auto const maybeAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); if (!maybeAssets) + { return tecINTERNAL; // LCOV_EXCL_LINE - else if (*maybeAssets > amount) + } + + if (*maybeAssets > amount) { // LCOV_EXCL_START JLOG(j_.error()) << "VaultDeposit: would take more than offered."; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 80ac8a7129..ef36bc6780 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3380,7 +3380,7 @@ class Invariants_test : public beast::unit_test::suite TxAccount::A2); doInvariantCheck( - Env{*this, testable_amendments() - fixLendingProtocolV1_1}, + Env{*this, testable_amendments() - featureLendingProtocolV1_1}, {"deposit must change depositor shares"}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { auto const keylet = keylet::vault(A1.id(), ac.view().seq()); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index a9cd920854..3c088993df 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5252,7 +5252,7 @@ class Vault_test : public beast::unit_test::suite testcase("VaultDelete data featureLendingProtocolV1_1 disabled"); env.disableFeature(featureLendingProtocolV1_1); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(temDISABLED), THISLINE); + env(delTx, ter(temDISABLED)); env.close(); env.enableFeature(featureLendingProtocolV1_1); } @@ -5261,7 +5261,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large"); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5269,7 +5269,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty"); delTx[sfMemoData] = strHex(std::string(0, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5277,12 +5277,12 @@ class Vault_test : public beast::unit_test::suite testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid"); PrettyAsset const xrpAsset = xrpIssue(); auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); - env(tx, ter(tesSUCCESS), THISLINE); + env(tx, ter(tesSUCCESS)); env.close(); // Recreate the transaction as the vault keylet changed auto delTx = vault.del({.owner = owner, .id = keylet.key}); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(tesSUCCESS), THISLINE); + env(delTx, ter(tesSUCCESS)); env.close(); } } @@ -5321,21 +5321,21 @@ class Vault_test : public beast::unit_test::suite auto const depositAmount = XRP(10); auto const [tx, keylet] = vault.create({.owner = owner, .asset = xrpIssue()}); - env(tx, ter(tesSUCCESS), THISLINE); + env(tx, ter(tesSUCCESS)); env.close(); - // With fixLendingProtocolV1_1 disabled, donations fail + // With featureLendingProtocolV1_1 disabled, donations fail { - testcase(prefix + " fails with fixLendingProtocolV1_1 disabled"); - env.disableFeature(fixLendingProtocolV1_1); + testcase(prefix + " fails with featureLendingProtocolV1_1 disabled"); + env.disableFeature(featureLendingProtocolV1_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(tx, ter{temINVALID_FLAG}); + env.enableFeature(featureLendingProtocolV1_1); env.close(); } @@ -5348,7 +5348,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -5358,8 +5358,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = depositAmount, }), - ter{tesSUCCESS}, - THISLINE); + ter{tesSUCCESS}); env.close(); // Donation is not allowed by a non-owner @@ -5371,7 +5370,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -5383,7 +5382,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, }); tx[sfAssetsMaximum] = XRP(30).number(); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); tx = vault.deposit({ .depositor = owner, @@ -5392,7 +5391,7 @@ class Vault_test : public beast::unit_test::suite .flags = tfVaultDonate, }); - env(tx, ter{tecLIMIT_EXCEEDED}, THISLINE); + env(tx, ter{tecLIMIT_EXCEEDED}); env.close(); } @@ -5407,7 +5406,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); env.close(); auto const shareBalanceAfterDeposit = vaultShareBalance(keylet); @@ -5426,7 +5425,7 @@ class Vault_test : public beast::unit_test::suite Asset shareAsset(sleVault->at(sfShareMPTID)); tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, .amount = shareAsset(shareBalance)}); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); auto const shareBalanceAfterWithdraw = vaultShareBalance(keylet); auto const [assetsAvailableAfterWithdraw, assetsTotalAfterWithdraw] = diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp index 24c89d249a..2b4ec486a6 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp @@ -30,6 +30,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) // Transaction-specific field values auto const vaultIDValue = canonical_UINT256(); + auto const memoDataValue = canonical_VL(); VaultDeleteBuilder builder{ accountValue, @@ -39,6 +40,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) }; // Set optional fields + builder.setMemoData(memoDataValue); auto tx = builder.build(publicKey, secretKey); @@ -62,6 +64,14 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) } // Verify optional fields + { + auto const& expected = memoDataValue; + auto const actualOpt = tx.getMemoData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; + expectEqualField(expected, *actualOpt, "sfMemoData"); + EXPECT_TRUE(tx.hasMemoData()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -79,6 +89,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) // Transaction-specific field values auto const vaultIDValue = canonical_UINT256(); + auto const memoDataValue = canonical_VL(); // Build an initial transaction VaultDeleteBuilder initialBuilder{ @@ -88,6 +99,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) feeValue }; + initialBuilder.setMemoData(memoDataValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -112,6 +124,13 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) } // Verify optional fields + { + auto const& expected = memoDataValue; + auto const actualOpt = rebuiltTx.getMemoData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; + expectEqualField(expected, *actualOpt, "sfMemoData"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -142,5 +161,35 @@ TEST(TransactionsVaultDeleteTests, BuilderThrowsOnWrongTxType) EXPECT_THROW(VaultDeleteBuilder{wrongTx.getSTTx()}, std::runtime_error); } +// 5) Build with only required fields and verify optional fields return nullopt. +TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) +{ + // Generate a deterministic keypair for signing + auto const [publicKey, secretKey] = + generateKeyPair(KeyType::secp256k1, generateSeed("testVaultDeleteNullopt")); + + // Common transaction fields + auto const accountValue = calcAccountID(publicKey); + std::uint32_t const sequenceValue = 3; + auto const feeValue = canonical_AMOUNT(); + + // Transaction-specific required field values + auto const vaultIDValue = canonical_UINT256(); + + VaultDeleteBuilder builder{ + accountValue, + vaultIDValue, + sequenceValue, + feeValue + }; + + // Do NOT set optional fields + + auto tx = builder.build(publicKey, secretKey); + + // Verify optional fields are not present + EXPECT_FALSE(tx.hasMemoData()); + EXPECT_FALSE(tx.getMemoData().has_value()); +} } From 43547fcacc00ddb62d122e98e7b5114f464ee4ff Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Sat, 21 Mar 2026 17:45:14 +0100 Subject: [PATCH 12/28] fix: address PR review comments --- include/xrpl/ledger/View.h | 3 + include/xrpl/protocol/TxFlags.h | 6 +- src/libxrpl/ledger/View.cpp | 6 ++ src/libxrpl/tx/invariants/VaultInvariant.cpp | 5 +- .../tx/transactors/vault/VaultDeposit.cpp | 5 +- src/test/app/Vault_test.cpp | 75 +++++++++++++++++++ 6 files changed, 92 insertions(+), 8 deletions(-) diff --git a/include/xrpl/ledger/View.h b/include/xrpl/ledger/View.h index cd23cf4978..5940785ad9 100644 --- a/include/xrpl/ledger/View.h +++ b/include/xrpl/ledger/View.h @@ -1103,6 +1103,9 @@ sharesToAssetsWithdraw( std::shared_ptr const& issuance, STAmount const& shares); +[[nodiscard]] bool +isVaultDonate(Rules const& rules, STTx const& tx); + /** Has the specified time passed? @param now the current time diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index e55a3fdc15..088a0d150c 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -186,6 +186,10 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal; TF_FLAG(tfVaultShareNonTransferable, 0x00020000), \ MASK_ADJ(0)) \ \ + TRANSACTION(VaultDeposit, \ + TF_FLAG(tfVaultDonate, 0x00010000), \ + MASK_ADJ(0)) \ + \ TRANSACTION(Batch, \ TF_FLAG(tfAllOrNothing, 0x00010000) \ TF_FLAG(tfOnlyOne, 0x00020000) \ @@ -214,8 +218,6 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal; TF_FLAG(tfLoanUnimpair, 0x00040000), \ MASK_ADJ(0)) -constexpr std::uint32_t const tfVaultDonate = 0x00010000; -constexpr std::uint32_t const tfVaultDepositMask = ~(tfUniversal | tfVaultDonate); // clang-format on // Create all the flag values. diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index d5c94a9981..348d171d0a 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -3723,6 +3723,12 @@ rippleUnlockEscrowMPT( return tesSUCCESS; } +[[nodiscard]] bool +isVaultDonate(Rules const& rules, STTx const& tx) +{ + return rules.enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); +} + bool after(NetClock::time_point now, std::uint32_t mark) { diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index a609d2782a..051ea69c78 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -419,8 +419,7 @@ ValidVault::finalize( return std::nullopt; }(); - bool const isDonate = - view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); + bool const isDonate = isVaultDonate(view.rules(), tx); bool const shouldUpdateShares = // Vault Asset donation is the only operation that can succeed without updating shares ((tx.getTxnType() == ttVAULT_DEPOSIT && !isDonate) || // @@ -711,7 +710,7 @@ ValidVault::finalize( } // If assets are donated, check share invariants - if (view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) + if (isDonate) { auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (accountDeltaShares) diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index 79a6e58093..1cd3b2adc6 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -77,7 +77,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(featureLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) + if (isVaultDonate(ctx.view.rules(), ctx.tx)) { if (account != vault->at(sfOwner)) { @@ -168,8 +168,7 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } - auto const isDonate = - ctx_.view().rules().enabled(featureLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); + auto const isDonate = isVaultDonate(ctx_.view().rules(), ctx_.tx); auto const& vaultAccount = vault->at(sfAccount); // Note, vault owner is always authorized diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 3c088993df..075935bd91 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5434,6 +5434,81 @@ class Vault_test : public beast::unit_test::suite BEAST_EXPECT(assetsAvailableAfterWithdraw == 0); BEAST_EXPECT(assetsTotalAfterWithdraw == 0); } + + // Test donation with non-1:1 share ratio. + // A prior donation skews the ratio so that 1 share > 1 asset. + // The donated amount must land exactly, not rounded via shares. + { + testcase(prefix + " succeeds with non-1:1 share ratio"); + + // Create a fresh vault + auto const [createTx, vk] = vault.create({.owner = owner, .asset = xrpIssue()}); + env(createTx, ter{tesSUCCESS}); + env.close(); + + // Depositor puts in 10 XRP → gets 10 shares at 1:1 + env(vault.deposit({ + .depositor = depositor, + .id = vk.key, + .amount = XRP(10), + }), + ter{tesSUCCESS}); + env.close(); + + // Owner donates 7 XRP → ratio becomes 17 assets / 10 shares + env(vault.deposit({ + .depositor = owner, + .id = vk.key, + .amount = XRP(7), + .flags = tfVaultDonate, + }), + ter{tesSUCCESS}); + env.close(); + + auto const sharesAfterFirstDonate = vaultShareBalance(vk); + auto const [availAfterFirstDonate, totalAfterFirstDonate] = vaultAssetBalance(vk); + + // Shares unchanged (donation doesn't mint shares) + BEAST_EXPECT(sharesAfterFirstDonate == 10'000'000); + // Assets increased by exactly the donated amount + BEAST_EXPECT(availAfterFirstDonate == 17'000'000); + BEAST_EXPECT(totalAfterFirstDonate == 17'000'000); + + // Donate again at the skewed 17:10 ratio — 3 XRP + env(vault.deposit({ + .depositor = owner, + .id = vk.key, + .amount = XRP(3), + .flags = tfVaultDonate, + }), + ter{tesSUCCESS}); + env.close(); + + auto const sharesAfterSecondDonate = vaultShareBalance(vk); + auto const [availAfterSecondDonate, totalAfterSecondDonate] = vaultAssetBalance(vk); + + // Shares still unchanged + BEAST_EXPECT(sharesAfterSecondDonate == 10'000'000); + // Assets increased by exactly 3 XRP (20 total) + BEAST_EXPECT(availAfterSecondDonate == 20'000'000); + BEAST_EXPECT(totalAfterSecondDonate == 20'000'000); + + // Depositor withdraws all shares — should get all 20 XRP + auto const sleVault = env.le(vk); + if (!BEAST_EXPECT(sleVault)) + return; + Asset shareAsset(sleVault->at(sfShareMPTID)); + env(vault.withdraw( + {.depositor = depositor, + .id = vk.key, + .amount = shareAsset(sharesAfterSecondDonate)}), + ter{tesSUCCESS}); + env.close(); + + BEAST_EXPECT(vaultShareBalance(vk) == 0); + BEAST_EXPECT(vaultAssetBalance(vk).first == 0); + BEAST_EXPECT(vaultAssetBalance(vk).second == 0); + } } public: From b97461b8e671dc4ebf6b865604a525501b5af2c0 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 23 Mar 2026 13:43:21 +0100 Subject: [PATCH 13/28] fixes clang tidy --- src/libxrpl/tx/invariants/VaultInvariant.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 051ea69c78..24107e6ee7 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -1084,7 +1084,7 @@ ValidVault::finalize( [[nodiscard]] std::int32_t ValidVault::computeMinScale(Asset const& asset, std::vector const& numbers) { - if (numbers.size() == 0) + if (numbers.empty()) return 0; auto const max = From df8bfbe5af6fbfe615c164429b7fac474bffb544 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 24 Mar 2026 12:37:06 +0100 Subject: [PATCH 14/28] fix: errors introduced post-merge --- src/test/app/Vault_test.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 64bbd2f88a..5b70f8a32f 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5253,7 +5253,7 @@ class Vault_test : public beast::unit_test::suite testcase("VaultDelete data featureLendingProtocolV1_1 disabled"); env.disableFeature(featureLendingProtocolV1_1); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(temDISABLED), THISLINE); + env(delTx, ter(temDISABLED)); env.close(); env.enableFeature(featureLendingProtocolV1_1); } @@ -5262,7 +5262,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large"); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5270,7 +5270,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty"); delTx[sfMemoData] = strHex(std::string(0, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5278,12 +5278,12 @@ class Vault_test : public beast::unit_test::suite testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid"); PrettyAsset const xrpAsset = xrpIssue(); auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); - env(tx, ter(tesSUCCESS), THISLINE); + env(tx, ter(tesSUCCESS)); env.close(); // Recreate the transaction as the vault keylet changed auto delTx = vault.del({.owner = owner, .id = keylet.key}); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(tesSUCCESS), THISLINE); + env(delTx, ter(tesSUCCESS)); env.close(); } } From d94232007fec911e56be3e4afc9359c46c9160a9 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:34:54 +0100 Subject: [PATCH 15/28] fix: updates autogen files --- .../transactions/VaultDelete.h | 37 ++++++++++++++ .../transactions/VaultDeleteTests.cpp | 49 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/include/xrpl/protocol_autogen/transactions/VaultDelete.h b/include/xrpl/protocol_autogen/transactions/VaultDelete.h index 89a4ef2a2f..bc89ce2bb7 100644 --- a/include/xrpl/protocol_autogen/transactions/VaultDelete.h +++ b/include/xrpl/protocol_autogen/transactions/VaultDelete.h @@ -57,6 +57,32 @@ public: { return this->tx_->at(sfVaultID); } + + /** + * @brief Get sfMemoData (soeOPTIONAL) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getMemoData() const + { + if (hasMemoData()) + { + return this->tx_->at(sfMemoData); + } + return std::nullopt; + } + + /** + * @brief Check if sfMemoData is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasMemoData() const + { + return this->tx_->isFieldPresent(sfMemoData); + } }; /** @@ -112,6 +138,17 @@ public: return *this; } + /** + * @brief Set sfMemoData (soeOPTIONAL) + * @return Reference to this builder for method chaining. + */ + VaultDeleteBuilder& + setMemoData(std::decay_t const& value) + { + object_[sfMemoData] = value; + return *this; + } + /** * @brief Build and return the VaultDelete wrapper. * @param publicKey The public key for signing. diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp index 24c89d249a..2b4ec486a6 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp @@ -30,6 +30,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) // Transaction-specific field values auto const vaultIDValue = canonical_UINT256(); + auto const memoDataValue = canonical_VL(); VaultDeleteBuilder builder{ accountValue, @@ -39,6 +40,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) }; // Set optional fields + builder.setMemoData(memoDataValue); auto tx = builder.build(publicKey, secretKey); @@ -62,6 +64,14 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) } // Verify optional fields + { + auto const& expected = memoDataValue; + auto const actualOpt = tx.getMemoData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; + expectEqualField(expected, *actualOpt, "sfMemoData"); + EXPECT_TRUE(tx.hasMemoData()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -79,6 +89,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) // Transaction-specific field values auto const vaultIDValue = canonical_UINT256(); + auto const memoDataValue = canonical_VL(); // Build an initial transaction VaultDeleteBuilder initialBuilder{ @@ -88,6 +99,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) feeValue }; + initialBuilder.setMemoData(memoDataValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -112,6 +124,13 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) } // Verify optional fields + { + auto const& expected = memoDataValue; + auto const actualOpt = rebuiltTx.getMemoData(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; + expectEqualField(expected, *actualOpt, "sfMemoData"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -142,5 +161,35 @@ TEST(TransactionsVaultDeleteTests, BuilderThrowsOnWrongTxType) EXPECT_THROW(VaultDeleteBuilder{wrongTx.getSTTx()}, std::runtime_error); } +// 5) Build with only required fields and verify optional fields return nullopt. +TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) +{ + // Generate a deterministic keypair for signing + auto const [publicKey, secretKey] = + generateKeyPair(KeyType::secp256k1, generateSeed("testVaultDeleteNullopt")); + + // Common transaction fields + auto const accountValue = calcAccountID(publicKey); + std::uint32_t const sequenceValue = 3; + auto const feeValue = canonical_AMOUNT(); + + // Transaction-specific required field values + auto const vaultIDValue = canonical_UINT256(); + + VaultDeleteBuilder builder{ + accountValue, + vaultIDValue, + sequenceValue, + feeValue + }; + + // Do NOT set optional fields + + auto tx = builder.build(publicKey, secretKey); + + // Verify optional fields are not present + EXPECT_FALSE(tx.hasMemoData()); + EXPECT_FALSE(tx.getMemoData().has_value()); +} } From 2c0d1c915112cb23b6462bb2526df234d9374f25 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 31 Mar 2026 14:54:14 +0200 Subject: [PATCH 16/28] fix: fix build errors --- include/xrpl/ledger/helpers/VaultHelpers.h | 4 ++++ src/libxrpl/ledger/helpers/VaultHelpers.cpp | 7 +++++++ src/libxrpl/tx/invariants/VaultInvariant.cpp | 1 + 3 files changed, 12 insertions(+) diff --git a/include/xrpl/ledger/helpers/VaultHelpers.h b/include/xrpl/ledger/helpers/VaultHelpers.h index 8aef30aa27..176e5a4213 100644 --- a/include/xrpl/ledger/helpers/VaultHelpers.h +++ b/include/xrpl/ledger/helpers/VaultHelpers.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -78,4 +79,7 @@ sharesToAssetsWithdraw( std::shared_ptr const& issuance, STAmount const& shares); +[[nodiscard]] bool +isVaultDonate(Rules const& rules, STTx const& tx); + } // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/VaultHelpers.cpp b/src/libxrpl/ledger/helpers/VaultHelpers.cpp index 3ded720289..97b039f9e9 100644 --- a/src/libxrpl/ledger/helpers/VaultHelpers.cpp +++ b/src/libxrpl/ledger/helpers/VaultHelpers.cpp @@ -2,6 +2,7 @@ // #include #include +#include #include namespace xrpl { @@ -109,4 +110,10 @@ sharesToAssetsWithdraw( return assets; } +[[nodiscard]] bool +isVaultDonate(Rules const& rules, STTx const& tx) +{ + return rules.enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); +} + } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 3cca45e298..55a9b77244 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include From db997ecad9627e66621a79558df723d6aa44d3ab Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:19:05 +0200 Subject: [PATCH 17/28] chore: Reset VaultInvariant to develop state before merge VaultInvariant changes parked in /tmp/vault-invariant-changes.patch for later re-evaluation. --- include/xrpl/tx/invariants/VaultInvariant.h | 109 ++++- src/libxrpl/tx/invariants/VaultInvariant.cpp | 468 ++++++++++--------- 2 files changed, 334 insertions(+), 243 deletions(-) diff --git a/include/xrpl/tx/invariants/VaultInvariant.h b/include/xrpl/tx/invariants/VaultInvariant.h index 1e1ded6fa1..2a9ffc8282 100644 --- a/include/xrpl/tx/invariants/VaultInvariant.h +++ b/include/xrpl/tx/invariants/VaultInvariant.h @@ -4,9 +4,11 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -35,15 +37,15 @@ namespace xrpl { */ class ValidVault { - Number static constexpr zero{}; + static constexpr Number kZero{}; struct Vault final { - uint256 key = beast::zero; - Asset asset = {}; - AccountID pseudoId = {}; - AccountID owner = {}; - uint192 shareMPTID = beast::zero; + uint256 key = beast::kZero; + Asset asset; + AccountID pseudoId; + AccountID owner; + uint192 shareMPTID = beast::kZero; Number assetsTotal = 0; Number assetsAvailable = 0; Number assetsMaximum = 0; @@ -54,7 +56,7 @@ class ValidVault struct Shares final { - MPTIssue share = {}; + MPTIssue share; std::uint64_t sharesTotal = 0; std::uint64_t sharesMaximum = 0; @@ -64,27 +66,98 @@ class ValidVault public: struct DeltaInfo final { - Number delta = numZero; + Number delta = kNumZero; std::optional scale; + + // Compute the delta between two Numbers, taking the coarsest scale + [[nodiscard]] static DeltaInfo + makeDelta(Number const& before, Number const& after, Asset const& asset); }; private: - std::vector afterVault_ = {}; - std::vector afterMPTs_ = {}; - std::vector beforeVault_ = {}; - std::vector beforeMPTs_ = {}; - std::unordered_map deltas_ = {}; + std::vector afterVault_; + std::vector afterMPTs_; + std::vector beforeVault_; + std::vector beforeMPTs_; + std::unordered_map deltas_; + + /** + * @brief Compute the minimum STAmount scale for rounding invariant + * calculations. + * + * Post-amendment (@c fixCleanup3_2_0) this is simply the posterior + * @c assetsTotal scale. Pre-amendment it is the coarsest scale across + * @p vaultDelta and both asset-field deltas. + * + * @param vaultDelta Delta of the vault's asset balance for this transaction. + * @param rules Active ledger rules (used to check the amendment). + * @returns The minimum scale to apply when rounding vault-related amounts. + */ + [[nodiscard]] std::int32_t + computeVaultMinScale(DeltaInfo const& vaultDelta, Rules const& rules) const; + + /** + * @brief Return the vault-asset balance-change delta for an account. + * + * Looks up the ledger-entry delta recorded during @c visitEntry for the + * account entry (XRP), trust line (IOU), or MPToken (MPT) that corresponds + * to the vault asset held by @p id. + * + * @param id Account whose asset delta is requested. + * @returns The delta, or @c std::nullopt if the entry was not touched. + */ + [[nodiscard]] std::optional + deltaAssets(AccountID const& id) const; + + /** + * @brief Return the vault-asset delta for the transaction's sending + * account, adjusted for the fee. + * + * Calls @c deltaAssets for @c tx[sfAccount] and, for non-delegated XRP + * transactions, adds the consumed fee back so the invariant sees the net + * asset movement rather than the fee-reduced balance change. + * + * @param tx The transaction being applied. + * @param fee Fee charged by this transaction. + * @returns The fee-adjusted delta, or @c std::nullopt if the net delta is + * zero or the account entry was not touched. + */ + [[nodiscard]] std::optional + deltaAssetsTxAccount(STTx const& tx, XRPAmount fee) const; + + /** + * @brief Return the vault-share balance-change delta for an account. + * + * For the vault's pseudo-account the @c MPTokenIssuance outstanding-amount + * delta is returned; for all other accounts the @c MPToken delta is + * returned. + * + * @param id Account whose share delta is requested. + * @returns The delta, or @c std::nullopt if the entry was not touched. + */ + [[nodiscard]] std::optional + deltaShares(AccountID const& id) const; + + /** + * @brief Check whether a vault holds no assets. + * + * @param vault Snapshot of the vault to test. + * @returns @c true when both @c assetsAvailable and @c assetsTotal are + * zero. + */ + [[nodiscard]] static bool + isVaultEmpty(Vault const& vault); public: + // Compute the coarsest scale required to represent all numbers + [[nodiscard]] static std::int32_t + computeCoarsestScale(std::vector const& numbers); + void - visitEntry(bool, std::shared_ptr const&, std::shared_ptr const&); + visitEntry(bool, SLE::const_ref, SLE::const_ref); bool finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&); - - // Compute the coarsest scale required to represent all numbers - [[nodiscard]] static std::int32_t - computeMinScale(Asset const& asset, std::vector const& numbers); }; } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index e856b0fb5f..ba6cd2dbd0 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -1,18 +1,30 @@ -#include -// #include +#include +#include #include -#include +#include #include #include #include +#include #include #include #include #include -#include +#include +#include // IWYU pragma: keep +#include +#include #include +#include #include +#include + +#include +#include +#include +#include +#include namespace xrpl { @@ -44,15 +56,12 @@ ValidVault::Shares::make(SLE const& from) ValidVault::Shares self; self.share = MPTIssue(makeMptID(from.getFieldU32(sfSequence), from.getAccountID(sfIssuer))); self.sharesTotal = from.at(sfOutstandingAmount); - self.sharesMaximum = from[~sfMaximumAmount].value_or(maxMPTokenAmount); + self.sharesMaximum = from[~sfMaximumAmount].value_or(kMaxMpTokenAmount); return self; } void -ValidVault::visitEntry( - bool isDelete, - std::shared_ptr const& before, - std::shared_ptr const& after) +ValidVault::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { // If `before` is empty, this means an object is being created, in which // case `isDelete` must be false. Otherwise `before` and `after` are set and @@ -67,7 +76,7 @@ ValidVault::visitEntry( // validation. It is used to validate that the change in account // balances matches the change in vault balances, stored to deltas_ at the // end of this function. - DeltaInfo balanceDelta{numZero, std::nullopt}; + DeltaInfo balanceDelta{.delta = kNumZero, .scale = std::nullopt}; std::int8_t sign = 0; if (before) @@ -172,6 +181,101 @@ ValidVault::visitEntry( } } +std::optional +ValidVault::deltaAssets(AccountID const& id) const +{ + auto const& vaultAsset = afterVault_[0].asset; + auto const lookup = [&](uint256 const& key) -> std::optional { + auto const it = deltas_.find(key); + if (it == deltas_.end()) + return std::nullopt; + return it->second; + }; + + return std::visit( + [&](TIss const& issue) -> std::optional { + if constexpr (std::is_same_v) + { + if (isXRP(issue)) + return lookup(keylet::account(id).key); + auto result = lookup(keylet::line(id, issue).key); + // Trust-line balance is stored from the low-account's perspective; + // negate if id is the high account so the delta is in id's terms. + if (result && id > issue.getIssuer()) + result->delta = -result->delta; + return result; + } + else if constexpr (std::is_same_v) + { + return lookup(keylet::mptoken(issue.getMptID(), id).key); + } + }, + vaultAsset.value()); +} + +std::optional +ValidVault::deltaAssetsTxAccount(STTx const& tx, XRPAmount fee) const +{ + auto const& vaultAsset = afterVault_[0].asset; + auto ret = deltaAssets(tx[sfAccount]); + if (!ret.has_value() || !vaultAsset.native()) + return ret; + + if (auto const delegate = tx[~sfDelegate]; delegate.has_value() && *delegate != tx[sfAccount]) + return ret; + + ret->delta += fee.drops(); + if (ret->delta == kZero) + return std::nullopt; + + return ret; +} + +std::optional +ValidVault::deltaShares(AccountID const& id) const +{ + auto const& afterVault = afterVault_[0]; + auto const it = [&]() { + if (id == afterVault.pseudoId) + return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key); + return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); + }(); + + return it != deltas_.end() ? std::optional(it->second) : std::nullopt; +} + +bool +ValidVault::isVaultEmpty(Vault const& vault) +{ + return vault.assetsAvailable == 0 && vault.assetsTotal == 0; +} + +std::int32_t +ValidVault::computeVaultMinScale(DeltaInfo const& vaultDelta, Rules const& rules) const +{ + // Returns the posterior `assetsTotal` scale. + // + // 1. Because STAmounts are normalized, `assetsTotal` (being >= `assetsAvailable`) + // safely represents the coarsest exponent needed for both fields. + // + // 2. The scale may decrease (withdraw/clawback) or increase (deposit). In both cases + // we ensure the vault is in a legitimate state in the post-transaction scale. + auto const& afterVault = afterVault_[0]; + auto const& vaultAsset = afterVault.asset; + if (rules.enabled(fixCleanup3_2_0)) + { + NumberRoundModeGuard const roundGuard(Number::RoundingMode::ToNearest); + return scale(afterVault.assetsTotal, vaultAsset); + } + + auto const& beforeVault = beforeVault_[0]; + auto const totalDelta = + DeltaInfo::makeDelta(beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset); + auto const availableDelta = + DeltaInfo::makeDelta(beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset); + return computeCoarsestScale({vaultDelta, totalDelta, availableDelta}); +} + bool ValidVault::finalize( STTx const& tx, @@ -187,7 +291,7 @@ ValidVault::finalize( if (afterVault_.empty() && beforeVault_.empty()) { - if (hasPrivilege(tx, mustModifyVault)) + if (hasPrivilege(tx, MustModifyVault)) { JLOG(j.fatal()) << // "Invariant failed: vault operation succeeded without modifying " @@ -198,7 +302,7 @@ ValidVault::finalize( return true; // Not a vault operation } - if (!(hasPrivilege(tx, mustModifyVault) || hasPrivilege(tx, mayModifyVault))) + if (!(hasPrivilege(tx, MustModifyVault) || hasPrivilege(tx, MayModifyVault))) { JLOG(j.fatal()) << // "Invariant failed: vault updated by a wrong transaction type"; @@ -265,13 +369,13 @@ ValidVault::finalize( "shares outstanding"; result = false; } - if (beforeVault.assetsTotal != zero) + if (beforeVault.assetsTotal != kZero) { JLOG(j.fatal()) << "Invariant failed: deleted vault must have no " "assets outstanding"; result = false; } - if (beforeVault.assetsAvailable != zero) + if (beforeVault.assetsAvailable != kZero) { JLOG(j.fatal()) << "Invariant failed: deleted vault must have no " "assets available"; @@ -334,13 +438,13 @@ ValidVault::finalize( if (updatedShares->sharesTotal == 0) { - if (afterVault.assetsTotal != zero) + if (afterVault.assetsTotal != kZero) { JLOG(j.fatal()) << "Invariant failed: updated zero sized " "vault must have no assets outstanding"; result = false; } - if (afterVault.assetsAvailable != zero) + if (afterVault.assetsAvailable != kZero) { JLOG(j.fatal()) << "Invariant failed: updated zero sized " "vault must have no assets available"; @@ -355,7 +459,7 @@ ValidVault::finalize( result = false; } - if (afterVault.assetsAvailable < zero) + if (afterVault.assetsAvailable < kZero) { JLOG(j.fatal()) << "Invariant failed: assets available must be positive"; result = false; @@ -375,13 +479,13 @@ ValidVault::finalize( result = false; } - if (afterVault.assetsTotal < zero) + if (afterVault.assetsTotal < kZero) { JLOG(j.fatal()) << "Invariant failed: assets outstanding must be positive"; result = false; } - if (afterVault.assetsMaximum < zero) + if (afterVault.assetsMaximum < kZero) { JLOG(j.fatal()) << "Invariant failed: assets maximum must be positive"; result = false; @@ -431,61 +535,6 @@ ValidVault::finalize( } auto const& vaultAsset = afterVault.asset; - auto const deltaAssets = [&](AccountID const& id) -> std::optional { - auto const get = // - [&](auto const& it, std::int8_t sign = 1) -> std::optional { - if (it == deltas_.end()) - return std::nullopt; - - return DeltaInfo{it->second.delta * sign, it->second.scale}; - }; - - return std::visit( - [&](TIss const& issue) { - if constexpr (std::is_same_v) - { - if (isXRP(issue)) - return get(deltas_.find(keylet::account(id).key)); - return get( - deltas_.find(keylet::line(id, issue).key), id > issue.getIssuer() ? -1 : 1); - } - else if constexpr (std::is_same_v) - { - return get(deltas_.find(keylet::mptoken(issue.getMptID(), id).key)); - } - }, - vaultAsset.value()); - }; - auto const deltaAssetsTxAccount = [&]() -> std::optional { - auto ret = deltaAssets(tx[sfAccount]); - // Nothing returned or not XRP transaction - if (!ret.has_value() || !vaultAsset.native()) - return ret; - - // Delegated transaction; no need to compensate for fees - if (auto const delegate = tx[~sfDelegate]; - delegate.has_value() && *delegate != tx[sfAccount]) - return ret; - - ret->delta += fee.drops(); - if (ret->delta == zero) - return std::nullopt; - - return ret; - }; - auto const deltaShares = [&](AccountID const& id) -> std::optional { - auto const it = [&]() { - if (id == afterVault.pseudoId) - return deltas_.find(keylet::mptIssuance(afterVault.shareMPTID).key); - return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); - }(); - - return it != deltas_.end() ? std::optional(it->second) : std::nullopt; - }; - - auto const vaultHoldsNoAssets = [&](Vault const& vault) { - return vault.assetsAvailable == 0 && vault.assetsTotal == 0; - }; // Technically this does not need to be a lambda, but it's more // convenient thanks to early "return false"; the not-so-nice @@ -505,8 +554,8 @@ ValidVault::finalize( result = false; } - if (afterVault.assetsAvailable != zero || afterVault.assetsTotal != zero || - afterVault.lossUnrealized != zero || updatedShares->sharesTotal != 0) + if (afterVault.assetsAvailable != kZero || afterVault.assetsTotal != kZero || + afterVault.lossUnrealized != kZero || updatedShares->sharesTotal != 0) { JLOG(j.fatal()) // << "Invariant failed: created vault must be empty"; @@ -572,7 +621,7 @@ ValidVault::finalize( result = false; } - if (afterVault.assetsMaximum > zero && + if (afterVault.assetsMaximum > kZero && afterVault.assetsTotal > afterVault.assetsMaximum) { JLOG(j.fatal()) << // @@ -615,24 +664,8 @@ ValidVault::finalize( return false; // That's all we can do } - // Get the coarsest scale to round calculations to - DeltaInfo totalDelta{ - afterVault.assetsTotal - beforeVault.assetsTotal, - std::max( - scale(afterVault.assetsTotal, vaultAsset), - scale(beforeVault.assetsTotal, vaultAsset))}; - DeltaInfo availableDelta{ - afterVault.assetsAvailable - beforeVault.assetsAvailable, - std::max( - scale(afterVault.assetsAvailable, vaultAsset), - scale(beforeVault.assetsAvailable, vaultAsset))}; - auto const minScale = computeMinScale( - vaultAsset, - { - *maybeVaultDeltaAssets, - totalDelta, - availableDelta, - }); + // Get the posterior scale to round calculations to + auto const minScale = computeVaultMinScale(*maybeVaultDeltaAssets, view.rules()); auto const vaultDeltaAssets = roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); @@ -646,7 +679,7 @@ ValidVault::finalize( result = false; } - if (vaultDeltaAssets <= zero) + if (vaultDeltaAssets <= kZero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase vault balance"; @@ -663,81 +696,74 @@ ValidVault::finalize( if (!issuerDeposit) { - auto const maybeAccDeltaAssets = deltaAssetsTxAccount(); + auto const maybeAccDeltaAssets = deltaAssetsTxAccount(tx, fee); if (!maybeAccDeltaAssets) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor " - "balance"; + JLOG(j.fatal()) + << "Invariant failed: deposit must change depositor balance"; return false; } auto const localMinScale = - std::max(minScale, computeMinScale(vaultAsset, {*maybeAccDeltaAssets})); + std::max(minScale, computeCoarsestScale({*maybeAccDeltaAssets})); auto const accountDeltaAssets = roundToAsset(vaultAsset, maybeAccDeltaAssets->delta, localMinScale); auto const localVaultDeltaAssets = roundToAsset(vaultAsset, vaultDeltaAssets, localMinScale); - if (accountDeltaAssets >= zero) + // For IOUs, if the deposit amount is not-representable at depositor trustline + // scale deposit amount could round to zero, giving depositor shares for no + // assets. Unlike withdrawal, we do not allow that. + if (accountDeltaAssets >= kZero) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must decrease depositor " - "balance"; + JLOG(j.fatal()) + << "Invariant failed: deposit must decrease depositor balance"; result = false; } if (localVaultDeltaAssets * -1 != accountDeltaAssets) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change vault and " - "depositor balance by equal amount"; + JLOG(j.fatal()) << "Invariant failed: " << // + "deposit must change vault and depositor balance by equal amount"; result = false; } } - if (afterVault.assetsMaximum > zero && + if (afterVault.assetsMaximum > kZero && afterVault.assetsTotal > afterVault.assetsMaximum) { - JLOG(j.fatal()) << // - "Invariant failed: deposit assets outstanding must not " - "exceed assets maximum"; + JLOG(j.fatal()) << "Invariant failed: " << // + "deposit assets outstanding must not exceed assets maximum"; result = false; } auto const maybeAccDeltaShares = deltaShares(tx[sfAccount]); if (!maybeAccDeltaShares) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor " - "shares"; + JLOG(j.fatal()) << "Invariant failed: deposit must change depositor shares"; return false; // That's all we can do } - // We don't need to round shares, they are integral MPT + // We don't round shares, they are integral MPT auto const& accountDeltaShares = *maybeAccDeltaShares; - if (accountDeltaShares.delta <= zero) + if (accountDeltaShares.delta <= kZero) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must increase depositor " - "shares"; + JLOG(j.fatal()) << "Invariant failed: deposit must increase depositor shares"; result = false; } auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == zero) + if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == kZero) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change vault shares"; + JLOG(j.fatal()) << "Invariant failed: deposit must change vault shares"; return false; // That's all we can do } - // We don't need to round shares, they are integral MPT + // We don't round shares, they are integral MPT auto const& vaultDeltaShares = *maybeVaultDeltaShares; if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { - JLOG(j.fatal()) << // - "Invariant failed: deposit must change depositor and " - "vault shares by equal amount"; + JLOG(j.fatal()) << "Invariant failed: " << // + "deposit must change depositor and vault shares by equal amount"; result = false; } @@ -745,8 +771,8 @@ ValidVault::finalize( vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); if (assetTotalDelta != vaultDeltaAssets) { - JLOG(j.fatal()) << "Invariant failed: deposit and assets " - "outstanding must add up"; + JLOG(j.fatal()) + << "Invariant failed: deposit and assets outstanding must add up"; result = false; } @@ -754,8 +780,7 @@ ValidVault::finalize( vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale); if (assetAvailableDelta != vaultDeltaAssets) { - JLOG(j.fatal()) << "Invariant failed: deposit and assets " - "available must add up"; + JLOG(j.fatal()) << "Invariant failed: deposit and assets available must add up"; result = false; } @@ -766,40 +791,25 @@ ValidVault::finalize( XRPL_ASSERT( !beforeVault_.empty(), - "xrpl::ValidVault::finalize : withdrawal updated a " - "vault"); + "xrpl::ValidVault::finalize : withdrawal updated a vault"); auto const& beforeVault = beforeVault_[0]; auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); - if (!maybeVaultDeltaAssets) { - JLOG(j.fatal()) << "Invariant failed: withdrawal must " - "change vault balance"; + JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault balance"; return false; // That's all we can do } - // Get the most coarse scale to round calculations to - auto const totalDelta = DeltaInfo{ - afterVault.assetsTotal - beforeVault.assetsTotal, - std::max( - scale(afterVault.assetsTotal, vaultAsset), - scale(beforeVault.assetsTotal, vaultAsset))}; - auto const availableDelta = DeltaInfo{ - afterVault.assetsAvailable - beforeVault.assetsAvailable, - std::max( - scale(afterVault.assetsAvailable, vaultAsset), - scale(beforeVault.assetsAvailable, vaultAsset))}; - auto const minScale = computeMinScale( - vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + // Get the posterior scale to round calculations to + auto const minScale = computeVaultMinScale(*maybeVaultDeltaAssets, view.rules()); auto const vaultPseudoDeltaAssets = roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); - if (vaultPseudoDeltaAssets >= zero) + if (vaultPseudoDeltaAssets >= kZero) { - JLOG(j.fatal()) << "Invariant failed: withdrawal must " - "decrease vault balance"; + JLOG(j.fatal()) << "Invariant failed: withdrawal must decrease vault balance"; result = false; } @@ -814,7 +824,7 @@ ValidVault::finalize( if (!issuerWithdrawal) { - auto const maybeAccDelta = deltaAssetsTxAccount(); + auto const maybeAccDelta = deltaAssetsTxAccount(tx, fee); auto const maybeOtherAccDelta = [&]() -> std::optional { if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) @@ -825,8 +835,7 @@ ValidVault::finalize( if (maybeAccDelta.has_value() == maybeOtherAccDelta.has_value()) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change one " - "destination balance"; + "Invariant failed: withdrawal must change one destination balance"; return false; } @@ -835,63 +844,83 @@ ValidVault::finalize( // the scale of destinationDelta can be coarser than // minScale, so we take that into account when rounding - auto const localMinScale = - std::max(minScale, computeMinScale(vaultAsset, {destinationDelta})); + auto const destinationScale = computeCoarsestScale({destinationDelta}); + auto const localMinScale = std::max(minScale, destinationScale); auto const roundedDestinationDelta = roundToAsset(vaultAsset, destinationDelta.delta, localMinScale); - if (roundedDestinationDelta <= zero) + // Post-fixCleanup3_2_0: Tolerate zero-rounded destination deltas for IOUs only. + // If the receiver's trust line sits at a coarser scale, the inflow may + // safely round down to zero. + // + // XRP and MPT remain strict. Because they are integer-exact, a zero + // destination delta indicates a true accounting bug, not a rounding artifact. + bool const tolerateZeroDelta = + view.rules().enabled(fixCleanup3_2_0) && !vaultAsset.integral(); + auto const invalidBalanceChange = tolerateZeroDelta + ? roundedDestinationDelta < kZero + : roundedDestinationDelta <= kZero; + if (invalidBalanceChange) { JLOG(j.fatal()) << // - "Invariant failed: withdrawal must increase " - "destination balance"; + "Invariant failed: withdrawal must increase destination balance"; result = false; } auto const localPseudoDeltaAssets = roundToAsset(vaultAsset, vaultPseudoDeltaAssets, localMinScale); - if (localPseudoDeltaAssets * -1 != roundedDestinationDelta) + // For IOU assets near a precision boundary the destination's STAmount + // exponent can shift, making part of the sent value unrepresentable at the + // receiver's new scale — that portion is irreversibly absorbed by the IOU + // rail. Tolerate the mismatch only when the destroyed amount (vault outflow + // minus destination inflow, in Number space) is itself sub-ULP at the + // destination's scale. Floor rounding is used so that values exactly at the + // step boundary are not mistakenly dismissed. Any representable discrepancy + // indicates a real accounting bug and must be caught. + auto const destroyedIsSubUlp = tolerateZeroDelta && + roundToAsset( + vaultAsset, + maybeVaultDeltaAssets->delta * -1 - destinationDelta.delta, + destinationScale, + Number::RoundingMode::Downward) == kZero; + if (!destroyedIsSubUlp && + localPseudoDeltaAssets * -1 != roundedDestinationDelta) { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change vault " - "and destination balance by equal amount"; + JLOG(j.fatal()) << "Invariant failed: " << // + "withdrawal must change vault and destination balance by equal " + "amount"; result = false; } } - // We don't need to round shares, they are integral MPT + // We don't round shares, they are integral MPT auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (!accountDeltaShares) { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change depositor " - "shares"; + JLOG(j.fatal()) << "Invariant failed: withdrawal must change depositor shares"; return false; } - if (accountDeltaShares->delta >= zero) + if (accountDeltaShares->delta >= kZero) { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must decrease depositor " - "shares"; + JLOG(j.fatal()) + << "Invariant failed: withdrawal must decrease depositor shares"; result = false; } - // We don't need to round shares, they are integral MPT + // We don't round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || vaultDeltaShares->delta == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == kZero) { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change vault shares"; + JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault shares"; return false; // That's all we can do } if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { - JLOG(j.fatal()) << // - "Invariant failed: withdrawal must change depositor " - "and vault shares by equal amount"; + JLOG(j.fatal()) << "Invariant failed: " << // + "withdrawal must change depositor and vault shares by equal amount"; result = false; } @@ -900,8 +929,8 @@ ValidVault::finalize( // Note, vaultBalance is negative (see check above) if (assetTotalDelta != vaultPseudoDeltaAssets) { - JLOG(j.fatal()) << "Invariant failed: withdrawal and " - "assets outstanding must add up"; + JLOG(j.fatal()) + << "Invariant failed: withdrawal and assets outstanding must add up"; result = false; } @@ -910,8 +939,8 @@ ValidVault::finalize( if (assetAvailableDelta != vaultPseudoDeltaAssets) { - JLOG(j.fatal()) << "Invariant failed: withdrawal and " - "assets available must add up"; + JLOG(j.fatal()) + << "Invariant failed: withdrawal and assets available must add up"; result = false; } @@ -929,12 +958,11 @@ ValidVault::finalize( // 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])) + isVaultEmpty(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"; + 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 } } @@ -942,25 +970,13 @@ ValidVault::finalize( auto const maybeVaultDeltaAssets = deltaAssets(afterVault.pseudoId); if (maybeVaultDeltaAssets) { - auto const totalDelta = DeltaInfo{ - afterVault.assetsTotal - beforeVault.assetsTotal, - std::max( - scale(afterVault.assetsTotal, vaultAsset), - scale(beforeVault.assetsTotal, vaultAsset))}; - auto const availableDelta = DeltaInfo{ - afterVault.assetsAvailable - beforeVault.assetsAvailable, - std::max( - scale(afterVault.assetsAvailable, vaultAsset), - scale(beforeVault.assetsAvailable, vaultAsset))}; - auto const minScale = computeMinScale( - vaultAsset, {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + auto const minScale = + computeVaultMinScale(*maybeVaultDeltaAssets, view.rules()); auto const vaultDeltaAssets = roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); - if (vaultDeltaAssets >= zero) + if (vaultDeltaAssets >= kZero) { - JLOG(j.fatal()) << // - "Invariant failed: clawback must decrease vault " - "balance"; + JLOG(j.fatal()) << "Invariant failed: clawback must decrease vault balance"; result = false; } @@ -969,8 +985,7 @@ ValidVault::finalize( if (assetsTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // - "Invariant failed: clawback and assets outstanding " - "must add up"; + "Invariant failed: clawback and assets outstanding must add up"; result = false; } @@ -981,12 +996,11 @@ ValidVault::finalize( if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // - "Invariant failed: clawback and assets available " - "must add up"; + "Invariant failed: clawback and assets available must add up"; result = false; } } - else if (!vaultHoldsNoAssets(beforeVault)) + else if (!isVaultEmpty(beforeVault)) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault balance"; @@ -1001,17 +1015,16 @@ ValidVault::finalize( "Invariant failed: clawback must change holder shares"; return false; // That's all we can do } - if (maybeAccountDeltaShares->delta >= zero) + if (maybeAccountDeltaShares->delta >= kZero) { JLOG(j.fatal()) << // - "Invariant failed: clawback must decrease holder " - "shares"; + "Invariant failed: clawback must decrease holder shares"; result = false; } // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || vaultDeltaShares->delta == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == kZero) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault shares"; @@ -1020,9 +1033,8 @@ ValidVault::finalize( if (vaultDeltaShares->delta * -1 != maybeAccountDeltaShares->delta) { - JLOG(j.fatal()) << // - "Invariant failed: clawback must change holder and " - "vault shares by equal amount"; + JLOG(j.fatal()) << "Invariant failed: " << // + "clawback must change holder and vault shares by equal amount"; result = false; } @@ -1055,19 +1067,25 @@ ValidVault::finalize( return true; } -[[nodiscard]] std::int32_t -ValidVault::computeMinScale(Asset const& asset, std::vector const& numbers) +[[nodiscard]] ValidVault::DeltaInfo +ValidVault::DeltaInfo::makeDelta(Number const& before, Number const& after, Asset const& asset) { - if (numbers.size() == 0) + return { + .delta = after - before, + .scale = std::max(xrpl::scale(after, asset), xrpl::scale(before, asset))}; +} + +[[nodiscard]] std::int32_t +ValidVault::computeCoarsestScale(std::vector const& numbers) +{ + if (numbers.empty()) return 0; - auto const max = - std::max_element(numbers.begin(), numbers.end(), [](auto const& a, auto const& b) -> bool { - return a.scale < b.scale; - }); + auto const max = std::ranges::max_element( + numbers, [](auto const& a, auto const& b) -> bool { return a.scale < b.scale; }); XRPL_ASSERT_PARTS( - max->scale, "xrpl::ValidVault::computeMinScale", "scale set for destinationDelta"); - return max->scale.value_or(STAmount::cMaxOffset); + max->scale, "xrpl::ValidVault::computeCoarsestScale", "scale set for destinationDelta"); + return max->scale.value_or(STAmount::kMaxOffset); } } // namespace xrpl From da4513d096d8cdee46d9e7b615c27c265e9c2e1d Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:47:11 +0200 Subject: [PATCH 18/28] post-merge cleanup --- .../tx/transactors/vault/VaultDelete.cpp | 3 +- src/test/app/Loan_test.cpp | 131 ------------------ src/test/app/Vault_test.cpp | 16 +-- .../transactions/VaultDeleteTests.cpp | 45 ++---- 4 files changed, 22 insertions(+), 173 deletions(-) diff --git a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp index 1b163324d5..9fddbd2455 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp @@ -32,7 +32,8 @@ VaultDelete::preflight(PreflightContext const& ctx) return temDISABLED; // The sfMemoData field is an optional field used to record the deletion reason. - if (auto const data = ctx.tx[~sfMemoData]; data && !validDataLength(data, maxDataPayloadLength)) + if (auto const data = ctx.tx[~sfMemoData]; + data && !validDataLength(data, kMaxDataPayloadLength)) return temMALFORMED; return tesSUCCESS; diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index d12230961f..c3b5850231 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -7312,136 +7312,6 @@ protected: BEAST_EXPECT(afterSecondCoverAvailable == 0); } - // Tests that vault withdrawals work correctly when the vault has unrealized - // loss from an impaired loan, ensuring the invariant check properly - // accounts for the loss. - void - testWithdrawReflectsUnrealizedLoss() - { - using namespace jtx; - using namespace loan; - using namespace std::chrono_literals; - - testcase("Vault withdraw reflects sfLossUnrealized"); - - // Test constants - static constexpr std::int64_t INITIAL_FUNDING = 1'000'000; - static constexpr std::int64_t LENDER_INITIAL_IOU = 5'000'000; - static constexpr std::int64_t DEPOSITOR_INITIAL_IOU = 1'000'000; - static constexpr std::int64_t BORROWER_INITIAL_IOU = 100'000; - static constexpr std::int64_t DEPOSIT_AMOUNT = 5'000; - static constexpr std::int64_t PRINCIPAL_AMOUNT = 99; - static constexpr std::uint64_t EXPECTED_SHARES_PER_DEPOSITOR = 5'000'000'000; - static constexpr std::uint32_t PAYMENT_INTERVAL = 600; - static constexpr std::uint32_t PAYMENT_TOTAL = 2; - - Env env(*this, all); - - // Setup accounts - Account const issuer{"issuer"}; - Account const lender{"lender"}; - Account const depositorA{"lpA"}; - Account const depositorB{"lpB"}; - Account const borrower{"borrowerA"}; - - env.fund(XRP(INITIAL_FUNDING), issuer, lender, depositorA, depositorB, borrower); - env.close(); - - // Setup trust lines - PrettyAsset const iouAsset = issuer[iouCurrency]; - env(trust(lender, iouAsset(10'000'000))); - env(trust(depositorA, iouAsset(10'000'000))); - env(trust(depositorB, iouAsset(10'000'000))); - env(trust(borrower, iouAsset(10'000'000))); - env.close(); - - // Fund accounts with IOUs - env(pay(issuer, lender, iouAsset(LENDER_INITIAL_IOU))); - env(pay(issuer, depositorA, iouAsset(DEPOSITOR_INITIAL_IOU))); - env(pay(issuer, depositorB, iouAsset(DEPOSITOR_INITIAL_IOU))); - env(pay(issuer, borrower, iouAsset(BORROWER_INITIAL_IOU))); - env.close(); - - // Create vault and broker, then add deposits from two depositors - auto const broker = createVaultAndBroker(env, iouAsset, lender); - Vault v{env}; - - env(v.deposit({ - .depositor = depositorA, - .id = broker.vaultKeylet().key, - .amount = iouAsset(DEPOSIT_AMOUNT), - }), - ter(tesSUCCESS)); - env(v.deposit({ - .depositor = depositorB, - .id = broker.vaultKeylet().key, - .amount = iouAsset(DEPOSIT_AMOUNT), - }), - ter(tesSUCCESS)); - env.close(); - - // Create a loan - auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID)); - if (!BEAST_EXPECT(sleBroker)) - return; - - auto const loanKeylet = keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence)); - - env(set(borrower, broker.brokerID, PRINCIPAL_AMOUNT), - sig(sfCounterpartySignature, lender), - paymentTotal(PAYMENT_TOTAL), - paymentInterval(PAYMENT_INTERVAL), - fee(env.current()->fees().base * 2), - ter(tesSUCCESS)); - env.close(); - - // Impair the loan to create unrealized loss - env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS)); - env.close(); - - // Verify unrealized loss is recorded in the vault - auto const vaultAfterImpair = env.le(broker.vaultKeylet()); - if (!BEAST_EXPECT(vaultAfterImpair)) - return; - - BEAST_EXPECT( - vaultAfterImpair->at(sfLossUnrealized) == broker.asset(PRINCIPAL_AMOUNT).value()); - - // Helper to get share balance for a depositor - auto const shareAsset = vaultAfterImpair->at(sfShareMPTID); - auto const getShareBalance = [&](Account const& depositor) -> std::uint64_t { - auto const token = env.le(keylet::mptoken(shareAsset, depositor.id())); - return token ? token->getFieldU64(sfMPTAmount) : 0; - }; - - // Verify both depositors have equal shares - auto const sharesLpA = getShareBalance(depositorA); - auto const sharesLpB = getShareBalance(depositorB); - BEAST_EXPECT(sharesLpA == EXPECTED_SHARES_PER_DEPOSITOR); - BEAST_EXPECT(sharesLpB == EXPECTED_SHARES_PER_DEPOSITOR); - BEAST_EXPECT(sharesLpA == sharesLpB); - - // Helper to attempt withdrawal - auto const attemptWithdrawShares = [&](Account const& depositor, - std::uint64_t shareAmount, - TER expected) { - STAmount const shareAmt{MPTIssue{shareAsset}, Number(shareAmount)}; - env(v.withdraw( - {.depositor = depositor, .id = broker.vaultKeylet().key, .amount = shareAmt}), - ter(expected)); - env.close(); - }; - - // Regression test: Both depositors should successfully withdraw despite - // unrealized loss. Previously failed with invariant violation: - // "withdrawal must change vault and destination balance by equal - // amount". This was caused by sharesToAssetsWithdraw rounding down, - // creating a mismatch where vaultDeltaAssets * -1 != destinationDelta - // when unrealized loss exists. - attemptWithdrawShares(depositorA, sharesLpA, tesSUCCESS); - attemptWithdrawShares(depositorB, sharesLpB, tesSUCCESS); - } - void testYieldTheftRounding(std::uint32_t flags) { @@ -8916,7 +8786,6 @@ public: void run() override { - testWithdrawReflectsUnrealizedLoss(); runAmendmentIndependent(); for (auto const& features : amendmentCombinations({fixCleanup3_1_3, fixCleanup3_2_0, featureMPTokensV2})) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 5a9cabf666..1a63f2ec35 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -8048,8 +8048,8 @@ class Vault_test : public beast::unit_test::Suite { testcase("VaultDelete data featureLendingProtocolV1_1 disabled"); env.disableFeature(featureLendingProtocolV1_1); - delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(temDISABLED)); + delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength, 'A')); + env(delTx, Ter(temDISABLED)); env.close(); env.enableFeature(featureLendingProtocolV1_1); } @@ -8057,8 +8057,8 @@ class Vault_test : public beast::unit_test::Suite // Transaction fails if the data field is too large { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large"); - delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); - env(delTx, ter(temMALFORMED)); + delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength + 1, 'A')); + env(delTx, Ter(temMALFORMED)); env.close(); } @@ -8066,7 +8066,7 @@ class Vault_test : public beast::unit_test::Suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty"); delTx[sfMemoData] = strHex(std::string(0, 'A')); - env(delTx, ter(temMALFORMED)); + env(delTx, Ter(temMALFORMED)); env.close(); } @@ -8074,12 +8074,12 @@ class Vault_test : public beast::unit_test::Suite testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid"); PrettyAsset const xrpAsset = xrpIssue(); auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset}); - env(tx, ter(tesSUCCESS)); + env(tx, Ter(tesSUCCESS)); env.close(); // Recreate the transaction as the vault keylet changed auto delTx = vault.del({.owner = owner, .id = keylet.key}); - delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(tesSUCCESS)); + delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength, 'A')); + env(delTx, Ter(tesSUCCESS)); env.close(); } } diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp index 8d03aff9cb..48f7ff2a83 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp @@ -1,15 +1,13 @@ // Auto-generated unit tests for transaction VaultDelete - -#include - -#include - +#include #include #include -#include -#include #include +#include + +#include +#include #include @@ -32,12 +30,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) auto const vaultIDValue = canonical_UINT256(); auto const memoDataValue = canonical_VL(); - VaultDeleteBuilder builder{ - accountValue, - vaultIDValue, - sequenceValue, - feeValue - }; + VaultDeleteBuilder builder{accountValue, vaultIDValue, sequenceValue, feeValue}; // Set optional fields builder.setMemoData(memoDataValue); @@ -71,7 +64,6 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) expectEqualField(expected, *actualOpt, "sfMemoData"); EXPECT_TRUE(tx.hasMemoData()); } - } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -92,12 +84,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) auto const memoDataValue = canonical_VL(); // Build an initial transaction - VaultDeleteBuilder initialBuilder{ - accountValue, - vaultIDValue, - sequenceValue, - feeValue - }; + VaultDeleteBuilder initialBuilder{accountValue, vaultIDValue, sequenceValue, feeValue}; initialBuilder.setMemoData(memoDataValue); @@ -130,15 +117,13 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; expectEqualField(expected, *actualOpt, "sfMemoData"); } - } // 3) Verify wrapper throws when constructed from wrong transaction type. TEST(TransactionsVaultDeleteTests, WrapperThrowsOnWrongTxType) { // Build a valid transaction of a different type - auto const [pk, sk] = - generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); + auto const [pk, sk] = generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); auto const account = calcAccountID(pk); AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; @@ -151,8 +136,7 @@ TEST(TransactionsVaultDeleteTests, WrapperThrowsOnWrongTxType) TEST(TransactionsVaultDeleteTests, BuilderThrowsOnWrongTxType) { // Build a valid transaction of a different type - auto const [pk, sk] = - generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); + auto const [pk, sk] = generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); auto const account = calcAccountID(pk); AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; @@ -166,7 +150,7 @@ TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) { // Generate a deterministic keypair for signing auto const [publicKey, secretKey] = - generateKeyPair(KeyType::secp256k1, generateSeed("testVaultDeleteNullopt")); + generateKeyPair(KeyType::Secp256k1, generateSeed("testVaultDeleteNullopt")); // Common transaction fields auto const accountValue = calcAccountID(publicKey); @@ -176,12 +160,7 @@ TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) // Transaction-specific required field values auto const vaultIDValue = canonical_UINT256(); - VaultDeleteBuilder builder{ - accountValue, - vaultIDValue, - sequenceValue, - feeValue - }; + VaultDeleteBuilder builder{accountValue, vaultIDValue, sequenceValue, feeValue}; // Do NOT set optional fields @@ -192,4 +171,4 @@ TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getMemoData().has_value()); } -} +} // namespace xrpl::transactions From 24db40e56ca31b8c000128c23b9221ca8167528c Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:57:42 +0200 Subject: [PATCH 19/28] fix: remove unnecessary tests & clang-tidy --- include/xrpl/protocol/detail/features.macro | 2 +- src/libxrpl/tx/transactors/vault/VaultDelete.cpp | 2 ++ src/test/app/LoanBroker_test.cpp | 11 ----------- src/test/app/Vault_test.cpp | 2 +- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 88df5b5030..9468623344 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,10 +15,10 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. +XRPL_FEATURE(LendingProtocolV1_1, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_2_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(LendingProtocolV1_1, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionDelegationV1_1, Supported::No, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp index 9fddbd2455..bf34352a72 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDelete.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDelete.cpp @@ -7,8 +7,10 @@ #include #include #include +#include #include #include +#include #include #include #include // IWYU pragma: keep diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index a68469fdbe..0edb955b90 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -2031,16 +2031,6 @@ class LoanBroker_test : public beast::unit_test::Suite testRIPD4274MPT(); } - void - testFeatureLendingProtocolV1_1enabled() - { - using namespace jtx; - testcase("featureLendingProtocolV1_1 enabled"); - Env env{*this}; - - BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1)); - } - // Exercises canApplyToBrokerCover (fixCleanup3_2_0): a deposit, withdraw, // or clawback whose amount rounds to zero at sfCoverAvailable's precision // scale must be rejected with tecPRECISION_LOSS once the amendment is on, @@ -2254,7 +2244,6 @@ public: void run() override { - testFeatureLendingProtocolV1_1enabled(); testCoverPrecisionGuard(); testLoanBrokerSetDebtMaximum(); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 1a63f2ec35..e49b9c46c0 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -8038,7 +8038,7 @@ class Vault_test : public beast::unit_test::Suite env.fund(XRP(1'000'000), owner); env.close(); - Vault vault{env}; + Vault const vault{env}; auto const keylet = keylet::vault(owner.id(), 1); auto delTx = vault.del({.owner = owner, .id = keylet.key}); From beb8a1872d43144e2774caf278d9fd6848af33ff Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:07:19 +0200 Subject: [PATCH 20/28] fix: Regenerate protocol autogenerated files --- .../transactions/VaultDelete.h | 4 +- .../transactions/VaultDeleteTests.cpp | 43 ++++++++++++++----- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/include/xrpl/protocol_autogen/transactions/VaultDelete.h b/include/xrpl/protocol_autogen/transactions/VaultDelete.h index 3f7c2fa0a5..b4c08ae229 100644 --- a/include/xrpl/protocol_autogen/transactions/VaultDelete.h +++ b/include/xrpl/protocol_autogen/transactions/VaultDelete.h @@ -59,7 +59,7 @@ public: } /** - * @brief Get sfMemoData (soeOPTIONAL) + * @brief Get sfMemoData (SoeOptional) * @return The field value, or std::nullopt if not present. */ [[nodiscard]] @@ -139,7 +139,7 @@ public: } /** - * @brief Set sfMemoData (soeOPTIONAL) + * @brief Set sfMemoData (SoeOptional) * @return Reference to this builder for method chaining. */ VaultDeleteBuilder& diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp index 48f7ff2a83..8f0eb2e4cd 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp @@ -1,14 +1,16 @@ // Auto-generated unit tests for transaction VaultDelete -#include -#include -#include -#include -#include #include + #include +#include +#include +#include +#include +#include + #include namespace xrpl::transactions { @@ -30,7 +32,12 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) auto const vaultIDValue = canonical_UINT256(); auto const memoDataValue = canonical_VL(); - VaultDeleteBuilder builder{accountValue, vaultIDValue, sequenceValue, feeValue}; + VaultDeleteBuilder builder{ + accountValue, + vaultIDValue, + sequenceValue, + feeValue + }; // Set optional fields builder.setMemoData(memoDataValue); @@ -64,6 +71,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip) expectEqualField(expected, *actualOpt, "sfMemoData"); EXPECT_TRUE(tx.hasMemoData()); } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -84,7 +92,12 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) auto const memoDataValue = canonical_VL(); // Build an initial transaction - VaultDeleteBuilder initialBuilder{accountValue, vaultIDValue, sequenceValue, feeValue}; + VaultDeleteBuilder initialBuilder{ + accountValue, + vaultIDValue, + sequenceValue, + feeValue + }; initialBuilder.setMemoData(memoDataValue); @@ -117,13 +130,15 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip) ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present"; expectEqualField(expected, *actualOpt, "sfMemoData"); } + } // 3) Verify wrapper throws when constructed from wrong transaction type. TEST(TransactionsVaultDeleteTests, WrapperThrowsOnWrongTxType) { // Build a valid transaction of a different type - auto const [pk, sk] = generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); + auto const [pk, sk] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); auto const account = calcAccountID(pk); AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; @@ -136,7 +151,8 @@ TEST(TransactionsVaultDeleteTests, WrapperThrowsOnWrongTxType) TEST(TransactionsVaultDeleteTests, BuilderThrowsOnWrongTxType) { // Build a valid transaction of a different type - auto const [pk, sk] = generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); + auto const [pk, sk] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); auto const account = calcAccountID(pk); AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; @@ -160,7 +176,12 @@ TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) // Transaction-specific required field values auto const vaultIDValue = canonical_UINT256(); - VaultDeleteBuilder builder{accountValue, vaultIDValue, sequenceValue, feeValue}; + VaultDeleteBuilder builder{ + accountValue, + vaultIDValue, + sequenceValue, + feeValue + }; // Do NOT set optional fields @@ -171,4 +192,4 @@ TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getMemoData().has_value()); } -} // namespace xrpl::transactions +} From 605e6166d5a379fcf13feb9908ba7dff6f0fadf6 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:42:02 +0200 Subject: [PATCH 21/28] fix: Replace adjust/args with kAdjust/kArgs in Invariants_test vault tests The lending-fix-amendment renamed the adjust/args helper lambdas to kAdjust/kArgs. Three call sites in the vault donation invariant tests still referenced the old names, causing build failures. --- src/test/app/Invariants_test.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 66d6fdbbf9..ad783d47ab 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3695,13 +3695,13 @@ class Invariants_test : public beast::unit_test::Suite TxAccount::A2); doInvariantCheck( - Env{*this, testable_amendments() - featureLendingProtocolV1_1}, + Env{*this, testableAmendments() - featureLendingProtocolV1_1}, {"deposit must 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.reset(); - })); + return kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + sample.accountShares.reset(); + })); }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, @@ -3800,9 +3800,9 @@ class Invariants_test : public beast::unit_test::Suite {"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; - })); + return kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + sample.accountShares->amount = 10; + })); }, XRPAmount{}, STTx{ @@ -3819,10 +3819,10 @@ class Invariants_test : public beast::unit_test::Suite {"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; - })); + return kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + sample.sharesTotal = 10; + sample.accountShares = std::nullopt; + })); }, XRPAmount{}, STTx{ From ef92373c9f63e025ba3605cac6e17fde180171a4 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:56:21 +0200 Subject: [PATCH 22/28] fix: post merge issues --- src/libxrpl/ledger/helpers/VaultHelpers.cpp | 2 ++ src/libxrpl/tx/invariants/VaultInvariant.cpp | 1 - .../tx/transactors/vault/VaultDeposit.cpp | 2 ++ src/test/app/Invariants_test.cpp | 18 +++++----- src/test/app/Vault_test.cpp | 34 +++++++++---------- src/test/jtx/vault.h | 2 +- 6 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/libxrpl/ledger/helpers/VaultHelpers.cpp b/src/libxrpl/ledger/helpers/VaultHelpers.cpp index 9e317c1b14..8d83af0a73 100644 --- a/src/libxrpl/ledger/helpers/VaultHelpers.cpp +++ b/src/libxrpl/ledger/helpers/VaultHelpers.cpp @@ -4,12 +4,14 @@ #include #include #include +#include #include #include #include #include #include #include // IWYU pragma: keep +#include #include #include diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index 958e30190d..ec04c50205 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -18,7 +18,6 @@ #include // IWYU pragma: keep #include #include -#include #include #include #include diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index 49b4d5b1b2..d7a173dc57 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -20,9 +20,11 @@ #include #include #include +#include #include #include +#include #include namespace xrpl { diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index ad783d47ab..bd2328e980 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3697,9 +3697,9 @@ class Invariants_test : public beast::unit_test::Suite doInvariantCheck( Env{*this, testableAmendments() - featureLendingProtocolV1_1}, {"deposit must change depositor shares"}, - [&](Account const& A1, Account const& A2, ApplyContext& ac) { - auto const keylet = keylet::vault(A1.id(), ac.view().seq()); - return kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + [&](Account const& a1, Account const& a2, ApplyContext& ac) { + auto const keylet = keylet::vault(a1.id(), ac.view().seq()); + return kAdjust(ac.view(), keylet, kArgs(a2.id(), 10, [&](Adjustments& sample) { sample.accountShares.reset(); })); }, @@ -3798,9 +3798,9 @@ class Invariants_test : public beast::unit_test::Suite 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 kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + [&](Account const& a1, Account const& a2, ApplyContext& ac) { + auto const keylet = keylet::vault(a1.id(), ac.view().seq()); + return kAdjust(ac.view(), keylet, kArgs(a2.id(), 10, [&](Adjustments& sample) { sample.accountShares->amount = 10; })); }, @@ -3817,9 +3817,9 @@ class Invariants_test : public beast::unit_test::Suite 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 kAdjust(ac.view(), keylet, kArgs(A2.id(), 10, [&](Adjustments& sample) { + [&](Account const& a1, Account const& a2, ApplyContext& ac) { + auto const keylet = keylet::vault(a1.id(), ac.view().seq()); + return kAdjust(ac.view(), keylet, kArgs(a2.id(), 10, [&](Adjustments& sample) { sample.sharesTotal = 10; sample.accountShares = std::nullopt; })); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 9f16f51d34..5d348b868c 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -8091,7 +8091,7 @@ class Vault_test : public beast::unit_test::Suite std::string const prefix = "VaultDeposit donate"; Env env{*this}; - Vault vault{env}; + Vault const vault{env}; auto const vaultShareBalance = [&](Keylet const& vaultKeylet) { auto const sleVault = env.le(vaultKeylet); @@ -8118,7 +8118,7 @@ class Vault_test : public beast::unit_test::Suite auto const depositAmount = XRP(10); auto const [tx, keylet] = vault.create({.owner = owner, .asset = xrpIssue()}); - env(tx, ter(tesSUCCESS)); + env(tx, Ter(tesSUCCESS)); env.close(); // With featureLendingProtocolV1_1 disabled, donations fail @@ -8131,7 +8131,7 @@ class Vault_test : public beast::unit_test::Suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{temINVALID_FLAG}); + env(tx, Ter(temINVALID_FLAG)); env.enableFeature(featureLendingProtocolV1_1); env.close(); } @@ -8145,7 +8145,7 @@ class Vault_test : public beast::unit_test::Suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}); + env(tx, Ter(tecNO_PERMISSION)); env.close(); } @@ -8155,7 +8155,7 @@ class Vault_test : public beast::unit_test::Suite .id = keylet.key, .amount = depositAmount, }), - ter{tesSUCCESS}); + Ter(tesSUCCESS)); env.close(); // Donation is not allowed by a non-owner @@ -8167,7 +8167,7 @@ class Vault_test : public beast::unit_test::Suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}); + env(tx, Ter(tecNO_PERMISSION)); env.close(); } @@ -8179,7 +8179,7 @@ class Vault_test : public beast::unit_test::Suite .id = keylet.key, }); tx[sfAssetsMaximum] = XRP(30).number(); - env(tx, ter{tesSUCCESS}); + env(tx, Ter(tesSUCCESS)); tx = vault.deposit({ .depositor = owner, @@ -8188,7 +8188,7 @@ class Vault_test : public beast::unit_test::Suite .flags = tfVaultDonate, }); - env(tx, ter{tecLIMIT_EXCEEDED}); + env(tx, Ter(tecLIMIT_EXCEEDED)); env.close(); } @@ -8203,7 +8203,7 @@ class Vault_test : public beast::unit_test::Suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tesSUCCESS}); + env(tx, Ter(tesSUCCESS)); env.close(); auto const shareBalanceAfterDeposit = vaultShareBalance(keylet); @@ -8219,10 +8219,10 @@ class Vault_test : public beast::unit_test::Suite return; // The depositor can withdraw their assets and the donated amount - Asset shareAsset(sleVault->at(sfShareMPTID)); + Asset const shareAsset(sleVault->at(sfShareMPTID)); tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, .amount = shareAsset(shareBalance)}); - env(tx, ter{tesSUCCESS}); + env(tx, Ter(tesSUCCESS)); auto const shareBalanceAfterWithdraw = vaultShareBalance(keylet); auto const [assetsAvailableAfterWithdraw, assetsTotalAfterWithdraw] = @@ -8240,7 +8240,7 @@ class Vault_test : public beast::unit_test::Suite // Create a fresh vault auto const [createTx, vk] = vault.create({.owner = owner, .asset = xrpIssue()}); - env(createTx, ter{tesSUCCESS}); + env(createTx, Ter(tesSUCCESS)); env.close(); // Depositor puts in 10 XRP → gets 10 shares at 1:1 @@ -8249,7 +8249,7 @@ class Vault_test : public beast::unit_test::Suite .id = vk.key, .amount = XRP(10), }), - ter{tesSUCCESS}); + Ter(tesSUCCESS)); env.close(); // Owner donates 7 XRP → ratio becomes 17 assets / 10 shares @@ -8259,7 +8259,7 @@ class Vault_test : public beast::unit_test::Suite .amount = XRP(7), .flags = tfVaultDonate, }), - ter{tesSUCCESS}); + Ter(tesSUCCESS)); env.close(); auto const sharesAfterFirstDonate = vaultShareBalance(vk); @@ -8278,7 +8278,7 @@ class Vault_test : public beast::unit_test::Suite .amount = XRP(3), .flags = tfVaultDonate, }), - ter{tesSUCCESS}); + Ter(tesSUCCESS)); env.close(); auto const sharesAfterSecondDonate = vaultShareBalance(vk); @@ -8294,12 +8294,12 @@ class Vault_test : public beast::unit_test::Suite auto const sleVault = env.le(vk); if (!BEAST_EXPECT(sleVault)) return; - Asset shareAsset(sleVault->at(sfShareMPTID)); + Asset const shareAsset(sleVault->at(sfShareMPTID)); env(vault.withdraw( {.depositor = depositor, .id = vk.key, .amount = shareAsset(sharesAfterSecondDonate)}), - ter{tesSUCCESS}); + Ter(tesSUCCESS)); env.close(); BEAST_EXPECT(vaultShareBalance(vk) == 0); diff --git a/src/test/jtx/vault.h b/src/test/jtx/vault.h index a30e7ae1d3..1a15b853b5 100644 --- a/src/test/jtx/vault.h +++ b/src/test/jtx/vault.h @@ -54,7 +54,7 @@ struct Vault Account depositor; uint256 id; STAmount amount; - std::optional flags{}; + std::optional flags = std::nullopt; }; static json::Value From 664b9ba5fe1ee26d99531571f200f19c24610baa Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 8 Jun 2026 14:41:08 +0100 Subject: [PATCH 23/28] ci: Use new packaging images and don't cancel develop builds (#7417) Co-authored-by: Bart --- .github/scripts/strategy-matrix/linux.json | 6 ++--- .github/workflows/build-nix-images.yml | 3 ++- .github/workflows/build-packaging-images.yml | 3 ++- .github/workflows/pre-commit.yml | 2 +- .github/workflows/publish-docs.yml | 4 +-- .../workflows/reusable-build-test-config.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 6 ++--- .github/workflows/reusable-package.yml | 25 ------------------- .github/workflows/reusable-upload-recipe.yml | 2 +- .github/workflows/upload-conan-deps.yml | 2 +- src/tests/libxrpl/CMakeLists.txt | 2 +- 11 files changed, 17 insertions(+), 40 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 7da48a6a25..edacdbde4c 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-8abe82e", + "image_tag": "sha-63ffdc3", "configs": { "ubuntu": [ { @@ -67,7 +67,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], - "image": "debian:bookworm" + "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-63ffdc3" } ], @@ -76,7 +76,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], - "image": "registry.access.redhat.com/ubi9/ubi:latest" + "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-63ffdc3" } ] } diff --git a/.github/workflows/build-nix-images.yml b/.github/workflows/build-nix-images.yml index dc02f84e0f..4e38ca7c57 100644 --- a/.github/workflows/build-nix-images.yml +++ b/.github/workflows/build-nix-images.yml @@ -22,7 +22,8 @@ on: workflow_dispatch: concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + # Read `on-trigger.yml` for the rationale behind this concurrency group name. + group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }} cancel-in-progress: true defaults: diff --git a/.github/workflows/build-packaging-images.yml b/.github/workflows/build-packaging-images.yml index a11a16f298..c445dbf726 100644 --- a/.github/workflows/build-packaging-images.yml +++ b/.github/workflows/build-packaging-images.yml @@ -20,7 +20,8 @@ on: workflow_dispatch: concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + # Read `on-trigger.yml` for the rationale behind this concurrency group name. + group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }} cancel-in-progress: true defaults: diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index de6a4f40b4..aecf0c2a8b 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,7 +14,7 @@ on: jobs: # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks. run-hooks: - uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81 + uses: XRPLF/actions/.github/workflows/pre-commit.yml@312aaab296060ff89d7f798dcab59f019bea6e02 with: runs_on: ubuntu-latest container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }' diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 35f33b6446..bcf5968384 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,13 +41,13 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3 steps: - name: Checkout repository uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab + uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 with: enable_ccache: false diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index dc3336dd2a..d53cf97a39 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -113,7 +113,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab + uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 with: enable_ccache: ${{ inputs.ccache_enabled }} diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 31e06d05eb..9f10711b6f 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -29,14 +29,14 @@ jobs: if: ${{ inputs.check_only_changed }} permissions: contents: read - uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@224f3c48d3014d082a1129237b8291ff0b0a331f + uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@312aaab296060ff89d7f798dcab59f019bea6e02 run-clang-tidy: name: Run clang tidy needs: [determine-files] if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-63ffdc3" permissions: contents: read issues: write @@ -45,7 +45,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab + uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 with: enable_ccache: false diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 890277d184..0e3f657006 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -68,31 +68,6 @@ jobs: timeout-minutes: 30 steps: - # Packaging runs in a vanilla distro image, so the tooling has to come - # from the distro's archive: debhelper for deb, rpm-build (and the - # systemd / find-debuginfo macros it depends on) for rpm. Run this - # before actions/checkout so the latter can use git (real history) for - # build_pkg.sh's SOURCE_DATE_EPOCH; otherwise it falls back to a tarball - # download and the timestamp comes from wall-clock time. - - name: Install packaging tooling (deb) - if: ${{ matrix.distro == 'debian' }} - run: | - export DEBIAN_FRONTEND=noninteractive - apt-get update - apt-get install -y --no-install-recommends \ - ca-certificates \ - debhelper \ - git - - - name: Install packaging tooling (rpm) - if: ${{ matrix.distro == 'rhel' }} - run: | - dnf install -y --setopt=install_weak_deps=False \ - git \ - rpm-build \ - redhat-rpm-config \ - systemd-rpm-macros - - name: Checkout repository uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index 6e1ea943ca..1c90fb0e72 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,7 +40,7 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3 steps: - name: Checkout repository uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 6310c90899..1a52ceee63 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -67,7 +67,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab + uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 with: enable_ccache: false diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 60288e5f20..2dae6fccb9 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -53,4 +53,4 @@ foreach(module IN LISTS test_modules) ) endforeach() -gtest_discover_tests(xrpl_tests) +gtest_discover_tests(xrpl_tests DISCOVERY_TIMEOUT 60) From 9eaebf2eeab84c4a044ce93878ae7ae9213c0770 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 8 Jun 2026 18:10:05 +0100 Subject: [PATCH 24/28] ci: Use XRPLF/actions build-multiarch-image workflow (#7428) --- .github/workflows/build-nix-images.yml | 7 +- .github/workflows/build-packaging-images.yml | 7 +- .../workflows/reusable-build-docker-image.yml | 89 ------------------- .../reusable-build-merge-docker-images.yml | 89 ------------------- nix/docker/check-tools.sh | 1 + nix/packages.nix | 1 + 6 files changed, 6 insertions(+), 188 deletions(-) delete mode 100644 .github/workflows/reusable-build-docker-image.yml delete mode 100644 .github/workflows/reusable-build-merge-docker-images.yml diff --git a/.github/workflows/build-nix-images.yml b/.github/workflows/build-nix-images.yml index 4e38ca7c57..24f069902d 100644 --- a/.github/workflows/build-nix-images.yml +++ b/.github/workflows/build-nix-images.yml @@ -6,16 +6,12 @@ on: - develop paths: - ".github/workflows/build-nix-images.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - ".github/workflows/reusable-build-merge-docker-images.yml" - "flake.nix" - "flake.lock" - "nix/**" pull_request: paths: - ".github/workflows/build-nix-images.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - ".github/workflows/reusable-build-merge-docker-images.yml" - "flake.nix" - "flake.lock" - "nix/**" @@ -50,8 +46,9 @@ jobs: base_image: debian:bookworm - name: rhel base_image: registry.access.redhat.com/ubi9/ubi:latest - uses: ./.github/workflows/reusable-build-merge-docker-images.yml + uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@c1b480188519e0cad040e6aa70db1cbc5a797e07 with: image_name: ghcr.io/xrplf/xrpld/nix-${{ matrix.distro.name }} dockerfile: nix/docker/Dockerfile base_image: ${{ matrix.distro.base_image }} + push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} diff --git a/.github/workflows/build-packaging-images.yml b/.github/workflows/build-packaging-images.yml index c445dbf726..d6dabb0f95 100644 --- a/.github/workflows/build-packaging-images.yml +++ b/.github/workflows/build-packaging-images.yml @@ -6,15 +6,11 @@ on: - develop paths: - ".github/workflows/build-packaging-images.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - ".github/workflows/reusable-build-merge-docker-images.yml" - "package/Dockerfile" - "package/install-packaging-tools.sh" pull_request: paths: - ".github/workflows/build-packaging-images.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - ".github/workflows/reusable-build-merge-docker-images.yml" - "package/Dockerfile" - "package/install-packaging-tools.sh" workflow_dispatch: @@ -42,8 +38,9 @@ jobs: base_image: debian:bookworm - name: rhel base_image: registry.access.redhat.com/ubi9/ubi:latest - uses: ./.github/workflows/reusable-build-merge-docker-images.yml + uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@c1b480188519e0cad040e6aa70db1cbc5a797e07 with: image_name: ghcr.io/xrplf/xrpld/packaging-${{ matrix.distro.name }} dockerfile: package/Dockerfile base_image: ${{ matrix.distro.base_image }} + push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} diff --git a/.github/workflows/reusable-build-docker-image.yml b/.github/workflows/reusable-build-docker-image.yml deleted file mode 100644 index 253563c6a5..0000000000 --- a/.github/workflows/reusable-build-docker-image.yml +++ /dev/null @@ -1,89 +0,0 @@ -# Build a single-platform Docker image. On push, the image is pushed to -# GHCR with arch-suffixed tags (e.g. `:latest-amd64`, `:sha-abc-amd64`) -# so the calling workflow can stitch per-arch builds into a multi-arch -# manifest without needing to pass digests around. -name: Reusable build Docker image (single platform) - -on: - workflow_call: - inputs: - image_name: - description: "Full image name without tag (e.g. 'ghcr.io/xrplf/xrpld/nix-ubuntu')" - required: true - type: string - dockerfile: - description: "Path to the Dockerfile, relative to the repository root" - required: true - type: string - base_image: - description: "Value passed to the Dockerfile as the BASE_IMAGE build arg" - required: true - type: string - platform: - description: "Docker platform string, e.g. linux/amd64" - required: true - type: string - runner: - description: "GitHub Actions runner label to build on" - required: true - type: string - push: - description: "Whether to push the image to GHCR" - required: true - type: boolean - -defaults: - run: - shell: bash - -jobs: - build: - name: Build ${{ inputs.platform }} - runs-on: ${{ inputs.runner }} - permissions: - contents: read - packages: write - - steps: - - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - - - name: Determine arch - id: vars - env: - PLATFORM: ${{ inputs.platform }} - run: | - echo "arch=${PLATFORM##*/}" >>$GITHUB_OUTPUT - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 - - - name: Login to GitHub Container Registry - if: inputs.push - uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 - with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Docker metadata - id: meta - uses: docker/metadata-action@80c7e94dd9b9319bd5eb7a0e0fe9291e23a2a2e9 # v6.1.0 - with: - images: ${{ inputs.image_name }} - tags: | - type=sha,prefix=sha-,format=short - type=raw,value=latest - flavor: | - suffix=-${{ steps.vars.outputs.arch }},onlatest=true - - - name: Build and push - uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0 - with: - context: . - file: ${{ inputs.dockerfile }} - platforms: ${{ inputs.platform }} - push: ${{ inputs.push }} - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - build-args: BASE_IMAGE=${{ inputs.base_image }} diff --git a/.github/workflows/reusable-build-merge-docker-images.yml b/.github/workflows/reusable-build-merge-docker-images.yml deleted file mode 100644 index 98deb6ea3f..0000000000 --- a/.github/workflows/reusable-build-merge-docker-images.yml +++ /dev/null @@ -1,89 +0,0 @@ -name: Reusable build and merge Docker image (multi-arch) - -on: - workflow_call: - inputs: - image_name: - description: "Full image name without tag (e.g. 'ghcr.io/xrplf/xrpld/nix-ubuntu')" - required: true - type: string - dockerfile: - description: "Path to the Dockerfile, relative to the repository root" - required: true - type: string - base_image: - description: "Value passed to the Dockerfile as the BASE_IMAGE build arg" - required: true - type: string - -defaults: - run: - shell: bash - -jobs: - build: - name: Build ${{ inputs.image_name }} - permissions: - contents: read - packages: write - - strategy: - fail-fast: false - matrix: - target: - - platform: linux/amd64 - runner: ubuntu-latest - - platform: linux/arm64 - runner: ubuntu-24.04-arm - - uses: ./.github/workflows/reusable-build-docker-image.yml - with: - image_name: ${{ inputs.image_name }} - dockerfile: ${{ inputs.dockerfile }} - base_image: ${{ inputs.base_image }} - platform: ${{ matrix.target.platform }} - runner: ${{ matrix.target.runner }} - push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} - - merge: - name: Merge ${{ inputs.image_name }} - needs: build - runs-on: ubuntu-latest - permissions: - contents: read - packages: write - - steps: - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 - - - name: Docker metadata - id: meta - uses: docker/metadata-action@80c7e94dd9b9319bd5eb7a0e0fe9291e23a2a2e9 # v6.1.0 - with: - images: ${{ inputs.image_name }} - tags: | - type=sha,prefix=sha-,format=short - type=raw,value=latest - - - name: Login to GitHub Container Registry - uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 - with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Create multi-arch manifests - if: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} - run: | - for tag in $(jq -cr '.tags[]' <<<"$DOCKER_METADATA_OUTPUT_JSON"); do - docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64" - done - - - name: Inspect image - if: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} - env: - IMAGE_NAME: ${{ inputs.image_name }} - IMAGE_VERSION: ${{ steps.meta.outputs.version }} - run: | - docker buildx imagetools inspect "${IMAGE_NAME}:${IMAGE_VERSION}" diff --git a/nix/docker/check-tools.sh b/nix/docker/check-tools.sh index faa6520678..67bcdff8a9 100755 --- a/nix/docker/check-tools.sh +++ b/nix/docker/check-tools.sh @@ -15,6 +15,7 @@ gcc --version gcov --version gcovr --version git --version +git-cliff --version gpg --version less --version make --version diff --git a/nix/packages.nix b/nix/packages.nix index 6a83446d88..d40472634b 100644 --- a/nix/packages.nix +++ b/nix/packages.nix @@ -15,6 +15,7 @@ in doxygen gcovr git + git-cliff gnumake gnupg # needed for signing commits & codecov/codecov-action llvmPackages_22.clang-tools From 3dd44ae18a944fdd7f05a92a05a92305d70b56cf Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:04:54 +0200 Subject: [PATCH 25/28] address AI feedback --- src/test/app/Vault_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index e49b9c46c0..418d2f8533 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -8050,8 +8050,8 @@ class Vault_test : public beast::unit_test::Suite env.disableFeature(featureLendingProtocolV1_1); delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength, 'A')); env(delTx, Ter(temDISABLED)); - env.close(); env.enableFeature(featureLendingProtocolV1_1); + env.close(); } // Transaction fails if the data field is too large From 2fa8305e956f354bce00e9a71fb6ef03e801af8c Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:14:39 +0200 Subject: [PATCH 26/28] fix: restore changes made to .github --- .../scripts/levelization/results/ordering.txt | 24 +++++++- .github/scripts/strategy-matrix/generate.py | 59 ++++++++++++++++--- .github/scripts/strategy-matrix/linux.json | 11 ++-- .github/scripts/strategy-matrix/macos.json | 3 +- .github/scripts/strategy-matrix/windows.json | 8 ++- .github/workflows/build-nix-images.yml | 14 ++++- .github/workflows/build-packaging-images.yml | 6 +- .github/workflows/check-pr-description.yml | 2 +- .github/workflows/on-pr.yml | 2 +- .github/workflows/pre-commit.yml | 2 +- .github/workflows/publish-docs.yml | 4 +- .../workflows/reusable-build-test-config.yml | 46 +++++++++------ .../workflows/reusable-check-levelization.yml | 2 +- .github/workflows/reusable-check-rename.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 29 +++++---- .github/workflows/reusable-package.yml | 6 +- .../workflows/reusable-strategy-matrix.yml | 5 +- .github/workflows/reusable-upload-recipe.yml | 4 +- .github/workflows/upload-conan-deps.yml | 3 +- 19 files changed, 162 insertions(+), 70 deletions(-) diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index c2000d1768..547c1b3539 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -1,6 +1,8 @@ libxrpl.basics > xrpl.basics libxrpl.conditions > xrpl.basics libxrpl.conditions > xrpl.conditions +libxrpl.config > xrpl.basics +libxrpl.config > xrpl.config libxrpl.core > xrpl.basics libxrpl.core > xrpl.core libxrpl.core > xrpl.json @@ -12,11 +14,11 @@ libxrpl.ledger > xrpl.json libxrpl.ledger > xrpl.ledger libxrpl.ledger > xrpl.nodestore libxrpl.ledger > xrpl.protocol -libxrpl.ledger > xrpl.server libxrpl.ledger > xrpl.shamap libxrpl.net > xrpl.basics libxrpl.net > xrpl.net libxrpl.nodestore > xrpl.basics +libxrpl.nodestore > xrpl.config libxrpl.nodestore > xrpl.json libxrpl.nodestore > xrpl.nodestore libxrpl.nodestore > xrpl.protocol @@ -24,6 +26,7 @@ libxrpl.protocol > xrpl.basics libxrpl.protocol > xrpl.json libxrpl.protocol > xrpl.protocol libxrpl.rdb > xrpl.basics +libxrpl.rdb > xrpl.config libxrpl.rdb > xrpl.core libxrpl.rdb > xrpl.rdb libxrpl.resource > xrpl.basics @@ -31,6 +34,7 @@ libxrpl.resource > xrpl.json libxrpl.resource > xrpl.protocol libxrpl.resource > xrpl.resource libxrpl.server > xrpl.basics +libxrpl.server > xrpl.config libxrpl.server > xrpl.core libxrpl.server > xrpl.json libxrpl.server > xrpl.protocol @@ -52,6 +56,7 @@ libxrpl.tx > xrpl.tx test.app > test.jtx test.app > test.unit_test test.app > xrpl.basics +test.app > xrpl.config test.app > xrpl.core test.app > xrpld.app test.app > xrpld.consensus @@ -90,6 +95,7 @@ test.consensus > xrpl.tx test.core > test.jtx test.core > test.unit_test test.core > xrpl.basics +test.core > xrpl.config test.core > xrpl.core test.core > xrpld.core test.core > xrpl.json @@ -104,6 +110,7 @@ test.csf > xrpl.protocol test.json > test.jtx test.json > xrpl.json test.jtx > xrpl.basics +test.jtx > xrpl.config test.jtx > xrpl.core test.jtx > xrpld.app test.jtx > xrpld.core @@ -126,6 +133,7 @@ test.ledger > xrpl.protocol test.nodestore > test.jtx test.nodestore > test.unit_test test.nodestore > xrpl.basics +test.nodestore > xrpl.config test.nodestore > xrpld.core test.nodestore > xrpl.nodestore test.nodestore > xrpl.protocol @@ -133,6 +141,7 @@ test.nodestore > xrpl.rdb test.overlay > test.jtx test.overlay > test.unit_test test.overlay > xrpl.basics +test.overlay > xrpl.config test.overlay > xrpld.app test.overlay > xrpld.core test.overlay > xrpld.overlay @@ -159,6 +168,7 @@ test.resource > xrpl.basics test.resource > xrpl.resource test.rpc > test.jtx test.rpc > xrpl.basics +test.rpc > xrpl.config test.rpc > xrpl.core test.rpc > xrpld.app test.rpc > xrpld.core @@ -173,6 +183,7 @@ test.rpc > xrpl.tx test.server > test.jtx test.server > test.unit_test test.server > xrpl.basics +test.server > xrpl.config test.server > xrpld.app test.server > xrpld.core test.server > xrpl.json @@ -180,6 +191,7 @@ test.server > xrpl.protocol test.server > xrpl.server test.shamap > test.unit_test test.shamap > xrpl.basics +test.shamap > xrpl.config test.shamap > xrpl.nodestore test.shamap > xrpl.protocol test.shamap > xrpl.shamap @@ -188,6 +200,7 @@ test.toplevel > xrpl.json test.unit_test > xrpl.basics test.unit_test > xrpl.protocol tests.libxrpl > xrpl.basics +tests.libxrpl > xrpl.config tests.libxrpl > xrpl.core tests.libxrpl > xrpl.json tests.libxrpl > xrpl.ledger @@ -200,16 +213,17 @@ tests.libxrpl > xrpl.shamap tests.libxrpl > xrpl.tx xrpl.conditions > xrpl.basics xrpl.conditions > xrpl.protocol +xrpl.config > xrpl.basics xrpl.core > xrpl.basics xrpl.core > xrpl.json xrpl.core > xrpl.protocol xrpl.json > xrpl.basics xrpl.ledger > xrpl.basics xrpl.ledger > xrpl.protocol -xrpl.ledger > xrpl.server xrpl.ledger > xrpl.shamap xrpl.net > xrpl.basics xrpl.nodestore > xrpl.basics +xrpl.nodestore > xrpl.config xrpl.nodestore > xrpl.protocol xrpl.protocol > xrpl.basics xrpl.protocol > xrpl.json @@ -237,6 +251,7 @@ xrpl.tx > xrpl.ledger xrpl.tx > xrpl.protocol xrpld.app > test.unit_test xrpld.app > xrpl.basics +xrpld.app > xrpl.config xrpld.app > xrpl.core xrpld.app > xrpld.consensus xrpld.app > xrpld.core @@ -255,11 +270,13 @@ xrpld.consensus > xrpl.json xrpld.consensus > xrpl.ledger xrpld.consensus > xrpl.protocol xrpld.core > xrpl.basics +xrpld.core > xrpl.config xrpld.core > xrpl.core xrpld.core > xrpl.net xrpld.core > xrpl.protocol xrpld.core > xrpl.rdb xrpld.overlay > xrpl.basics +xrpld.overlay > xrpl.config xrpld.overlay > xrpl.core xrpld.overlay > xrpld.consensus xrpld.overlay > xrpld.core @@ -272,15 +289,18 @@ xrpld.overlay > xrpl.server xrpld.overlay > xrpl.shamap xrpld.overlay > xrpl.tx xrpld.peerfinder > xrpl.basics +xrpld.peerfinder > xrpl.config xrpld.peerfinder > xrpld.core xrpld.peerfinder > xrpl.protocol xrpld.peerfinder > xrpl.rdb xrpld.perflog > xrpl.basics +xrpld.perflog > xrpl.config xrpld.perflog > xrpl.core xrpld.perflog > xrpld.rpc xrpld.perflog > xrpl.json xrpld.perflog > xrpl.protocol xrpld.rpc > xrpl.basics +xrpld.rpc > xrpl.config xrpld.rpc > xrpl.core xrpld.rpc > xrpld.core xrpld.rpc > xrpl.json diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index aaf84a51d0..a269cb25d4 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -20,13 +20,24 @@ _SANITIZER_SUFFIX: dict[str, str] = { def get_cmake_args(build_type: str, extra_args: str) -> str: """Get the full list of CMake arguments for a config.""" args = _BASE_CMAKE_ARGS.copy() - if build_type == "Release": - args.append("-Dassert=ON") if extra_args: args.extend(extra_args.split()) return " ".join(args) +def runs_on_event(exclude_event_types: list[str], event: str | None) -> bool: + """Whether a config should run for the current event. + + 'exclude_event_types' is a list of GitHub event names (e.g. + ["pull_request"]) on which the config should NOT run; an empty list means + the config runs on every event. When no event is given (event is None), no + filtering is applied. + """ + if event is None: + return True + return event not in exclude_event_types + + # --------------------------------------------------------------------------- # Input types — shapes of the JSON config files # --------------------------------------------------------------------------- @@ -43,6 +54,9 @@ class LinuxConfig: suffix: str = "" extra_cmake_args: str = "" image: str = "" # only used by package_configs entries + # List of GitHub event names (e.g. "pull_request") on which this config + # should NOT run. Empty means it runs on every event. + exclude_event_types: list[str] = dataclasses.field(default_factory=list) @dataclasses.dataclass @@ -77,6 +91,9 @@ class PlatformConfig: build_type: list[str] build_only: bool = False # if true, skip tests (e.g. macos/Windows Debug) extra_cmake_args: str = "" + # List of GitHub event names (e.g. "pull_request") on which this config + # should NOT run. Empty means it runs on every event. + exclude_event_types: list[str] = dataclasses.field(default_factory=list) def __post_init__(self) -> None: if isinstance(self.build_type, str): @@ -151,16 +168,21 @@ _ARCHS: dict[str, Architecture] = { } -def expand_linux_matrix(linux: LinuxFile) -> list[MatrixEntry]: +def expand_linux_matrix( + linux: LinuxFile, event: str | None = None +) -> list[MatrixEntry]: """Expand a LinuxFile into a flat list of matrix entries. Each config entry is expanded over the cross-product of its - compiler, build_type, sanitizers, and architecture lists. + compiler, build_type, sanitizers, and architecture lists. Configs that + exclude the current event are skipped. """ entries: list[MatrixEntry] = [] for distro, configs in linux.configs.items(): for cfg in configs: + if not runs_on_event(cfg.exclude_event_types, event): + continue # An empty sanitizers list means "one entry with no sanitizer". effective_sanitizers = cfg.sanitizers or [""] effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch} @@ -218,13 +240,20 @@ def expand_linux_packaging(linux: LinuxFile) -> list[PackagingEntry]: return entries -def expand_platform_matrix(pf: PlatformFile) -> list[MatrixEntry]: - """Expand a PlatformFile (macOS or Windows) into matrix entries.""" +def expand_platform_matrix( + pf: PlatformFile, event: str | None = None +) -> list[MatrixEntry]: + """Expand a PlatformFile (macOS or Windows) into matrix entries. + + Configs that exclude the current event are skipped. + """ platform_name, arch = pf.platform.split("/") is_windows = platform_name == "windows" entries: list[MatrixEntry] = [] for cfg in pf.configs: + if not runs_on_event(cfg.exclude_event_types, event): + continue for build_type in cfg.build_type: entries.append( MatrixEntry( @@ -262,6 +291,14 @@ if __name__ == "__main__": help="Emit the Linux packaging matrix instead of the build/test matrix.", action="store_true", ) + parser.add_argument( + "-e", + "--event", + help="The GitHub event name that triggered the workflow (e.g. 'push', " + "'pull_request'). Configs are filtered by their 'event_type'. If " + "omitted, no filtering is applied.", + default=None, + ) args = parser.parse_args() matrix: list[MatrixEntry] | list[PackagingEntry] = [] @@ -270,12 +307,16 @@ if __name__ == "__main__": matrix = expand_linux_packaging(LinuxFile.load(THIS_DIR / "linux.json")) else: if args.config in ("linux", None): - matrix += expand_linux_matrix(LinuxFile.load(THIS_DIR / "linux.json")) + matrix += expand_linux_matrix( + LinuxFile.load(THIS_DIR / "linux.json"), args.event + ) if args.config in ("macos", None): - matrix += expand_platform_matrix(PlatformFile.load(THIS_DIR / "macos.json")) + matrix += expand_platform_matrix( + PlatformFile.load(THIS_DIR / "macos.json"), args.event + ) if args.config in ("windows", None): matrix += expand_platform_matrix( - PlatformFile.load(THIS_DIR / "windows.json") + PlatformFile.load(THIS_DIR / "windows.json"), args.event ) print(f"matrix={json.dumps({'include': [dataclasses.asdict(e) for e in matrix]})}") diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index edacdbde4c..a9b85b766a 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-63ffdc3", + "image_tag": "sha-fe4c8ae", "configs": { "ubuntu": [ { @@ -10,7 +10,7 @@ { "compiler": ["gcc", "clang"], - "build_type": ["Debug"], + "build_type": ["Debug", "Release"], "arch": ["amd64"], "sanitizers": ["address", "undefinedbehavior"] }, @@ -41,7 +41,8 @@ "build_type": ["Debug"], "arch": ["amd64"], "suffix": "unity", - "extra_cmake_args": "-Dunity=ON" + "extra_cmake_args": "-Dunity=ON", + "exclude_event_types": ["pull_request"] } ], @@ -67,7 +68,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], - "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-63ffdc3" + "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-577d745" } ], @@ -76,7 +77,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], - "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-63ffdc3" + "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-577d745" } ] } diff --git a/.github/scripts/strategy-matrix/macos.json b/.github/scripts/strategy-matrix/macos.json index 5b9e32f88e..66d7a55a43 100644 --- a/.github/scripts/strategy-matrix/macos.json +++ b/.github/scripts/strategy-matrix/macos.json @@ -9,7 +9,8 @@ { "build_type": "Debug", "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5", - "build_only": true + "build_only": true, + "exclude_event_types": ["pull_request"] } ] } diff --git a/.github/scripts/strategy-matrix/windows.json b/.github/scripts/strategy-matrix/windows.json index e4678b60db..370e9f5bc7 100644 --- a/.github/scripts/strategy-matrix/windows.json +++ b/.github/scripts/strategy-matrix/windows.json @@ -1,8 +1,12 @@ { "platform": "windows/amd64", - "runner": ["self-hosted", "Windows", "devbox"], + "runner": ["self-hosted", "Windows", "dev-box-windows-2026"], "configs": [ { "build_type": "Release" }, - { "build_type": "Debug", "build_only": true } + { + "build_type": "Debug", + "build_only": true, + "exclude_event_types": ["pull_request"] + } ] } diff --git a/.github/workflows/build-nix-images.yml b/.github/workflows/build-nix-images.yml index 24f069902d..54911ef6e0 100644 --- a/.github/workflows/build-nix-images.yml +++ b/.github/workflows/build-nix-images.yml @@ -9,12 +9,20 @@ on: - "flake.nix" - "flake.lock" - "nix/**" + - "!nix/docker/README.md" + - "!nix/devshell.nix" + - "bin/check-tools.sh" + - "bin/install-sanitizer-libs.sh" pull_request: paths: - ".github/workflows/build-nix-images.yml" - "flake.nix" - "flake.lock" - "nix/**" + - "!nix/docker/README.md" + - "!nix/devshell.nix" + - "bin/check-tools.sh" + - "bin/install-sanitizer-libs.sh" workflow_dispatch: concurrency: @@ -46,9 +54,9 @@ jobs: base_image: debian:bookworm - name: rhel base_image: registry.access.redhat.com/ubi9/ubi:latest - uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@c1b480188519e0cad040e6aa70db1cbc5a797e07 + uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@ee03d31bcc4501d7599dc1b1ecd7a34af582ad1c with: - image_name: ghcr.io/xrplf/xrpld/nix-${{ matrix.distro.name }} + image_name: xrpld/nix-${{ matrix.distro.name }} dockerfile: nix/docker/Dockerfile base_image: ${{ matrix.distro.base_image }} - push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} + push: ${{ github.event_name == 'push' }} diff --git a/.github/workflows/build-packaging-images.yml b/.github/workflows/build-packaging-images.yml index d6dabb0f95..3633847ef3 100644 --- a/.github/workflows/build-packaging-images.yml +++ b/.github/workflows/build-packaging-images.yml @@ -38,9 +38,9 @@ jobs: base_image: debian:bookworm - name: rhel base_image: registry.access.redhat.com/ubi9/ubi:latest - uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@c1b480188519e0cad040e6aa70db1cbc5a797e07 + uses: XRPLF/actions/.github/workflows/build-multiarch-image.yml@ee03d31bcc4501d7599dc1b1ecd7a34af582ad1c with: - image_name: ghcr.io/xrplf/xrpld/packaging-${{ matrix.distro.name }} + image_name: xrpld/packaging-${{ matrix.distro.name }} dockerfile: package/Dockerfile base_image: ${{ matrix.distro.base_image }} - push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} + push: ${{ github.event_name == 'push' }} diff --git a/.github/workflows/check-pr-description.yml b/.github/workflows/check-pr-description.yml index a60b83738a..744449f216 100644 --- a/.github/workflows/check-pr-description.yml +++ b/.github/workflows/check-pr-description.yml @@ -23,7 +23,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Write PR body to file env: diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 4b2edeb93d..0cc9b375a7 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Determine changed files # This step checks whether any files have changed that should # cause the next jobs to run. We do it this way rather than diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index aecf0c2a8b..0363534af5 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,7 +14,7 @@ on: jobs: # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks. run-hooks: - uses: XRPLF/actions/.github/workflows/pre-commit.yml@312aaab296060ff89d7f798dcab59f019bea6e02 + uses: XRPLF/actions/.github/workflows/pre-commit.yml@e06d4138c9ec8dceeb7c818645faa38087ea9e3d with: runs_on: ubuntu-latest container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }' diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index bcf5968384..cc7b6b6e7e 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,10 +41,10 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-fe4c8ae steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index d53cf97a39..fe441dba6e 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -82,7 +82,7 @@ jobs: name: ${{ inputs.config_name }} runs-on: ${{ fromJSON(inputs.runs_on) }} container: ${{ inputs.image != '' && inputs.image || null }} - timeout-minutes: ${{ inputs.sanitizers != '' && 360 || 60 }} + timeout-minutes: ${{ inputs.sanitizers != '' && 360 || 180 }} env: # Use a namespace to keep the objects separate for each configuration. CCACHE_NAMESPACE: ${{ inputs.config_name }} @@ -110,7 +110,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 @@ -121,6 +121,11 @@ jobs: if: ${{ inputs.ccache_enabled && runner.debug == '1' }} run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >>"${GITHUB_ENV}" + - name: Check tools + env: + CHECK_TOOLS_SKIP_CLONE: "1" + run: ./bin/check-tools.sh + - name: Print build environment uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 @@ -158,12 +163,33 @@ jobs: CMAKE_ARGS: ${{ inputs.cmake_args }} run: | cmake \ - -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \ + -G '${{ runner.os == 'Windows' && 'Visual Studio 18 2026' || 'Ninja' }}' \ -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ ${CMAKE_ARGS} \ .. + # Export the sanitizer options before any instrumented binary runs. The + # protocol code-gen and build steps below invoke instrumented dependency + # tools (protoc, grpc), so setting UBSAN_OPTIONS here lets the UBSan + # suppression list silence their diagnostics too, not just at test time. + # GITHUB_WORKSPACE (not the github.workspace context) is used so the path + # resolves correctly inside the container job. + - name: Set sanitizer options + if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }} + env: + CONFIG_NAME: ${{ inputs.config_name }} + run: | + SUPP="${GITHUB_WORKSPACE}/sanitizers/suppressions" + ASAN_OPTS="include=${SUPP}/runtime-asan-options.txt:suppressions=${SUPP}/asan.supp" + if [[ "${CONFIG_NAME}" == *gcc* ]]; then + ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0" + fi + echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV} + echo "TSAN_OPTIONS=include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp" >>${GITHUB_ENV} + echo "UBSAN_OPTIONS=include=${SUPP}/runtime-ubsan-options.txt:suppressions=${SUPP}/ubsan.supp" >>${GITHUB_ENV} + echo "LSAN_OPTIONS=include=${SUPP}/runtime-lsan-options.txt:suppressions=${SUPP}/lsan.supp" >>${GITHUB_ENV} + - name: Check protocol autogen files are up-to-date working-directory: ${{ env.BUILD_DIR }} env: @@ -279,20 +305,6 @@ jobs: run: | ./xrpld --version | grep libvoidstar - - name: Set sanitizer options - if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }} - env: - CONFIG_NAME: ${{ inputs.config_name }} - run: | - ASAN_OPTS="include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-asan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" - if [[ "${CONFIG_NAME}" == *gcc* ]]; then - ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0" - fi - echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV} - echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >>${GITHUB_ENV} - echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >>${GITHUB_ENV} - echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >>${GITHUB_ENV} - - name: Run the separate tests if: ${{ !inputs.build_only }} working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} diff --git a/.github/workflows/reusable-check-levelization.yml b/.github/workflows/reusable-check-levelization.yml index 813c0e1e36..88c95ac3ba 100644 --- a/.github/workflows/reusable-check-levelization.yml +++ b/.github/workflows/reusable-check-levelization.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Check levelization run: python .github/scripts/levelization/generate.py - name: Check for differences diff --git a/.github/workflows/reusable-check-rename.yml b/.github/workflows/reusable-check-rename.yml index 5002cc7f40..9a91e98ee3 100644 --- a/.github/workflows/reusable-check-rename.yml +++ b/.github/workflows/reusable-check-rename.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Check definitions run: .github/scripts/rename/definitions.sh . - name: Check copyright notices diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 9f10711b6f..e99ef574bf 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -20,29 +20,32 @@ env: BUILD_DIR: build BUILD_TYPE: Debug # Debug so that ASSERTS and such participate in clang-tidy check - OUTPUT_FILE: clang-tidy-output.txt - DIFF_FILE: clang-tidy-git-diff.txt - ISSUE_FILE: clang-tidy-issue.md + OUTPUT_FILE: /tmp/clang-tidy-output.txt + FILTERED_OUTPUT_FILE: /tmp/clang-tidy-filtered-output.txt + DIFF_FILE: /tmp/clang-tidy-git-diff.txt + ISSUE_FILE: /tmp/clang-tidy-issue.md + + COMPILER: clang jobs: determine-files: if: ${{ inputs.check_only_changed }} permissions: contents: read - uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@312aaab296060ff89d7f798dcab59f019bea6e02 + uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@c7045074aafe9fb92fa537aa4446f81fbfc17e8b run-clang-tidy: name: Run clang tidy needs: [determine-files] if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/xrpld/nix-debian:sha-63ffdc3" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-fe4c8ae" permissions: contents: read issues: write steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 @@ -59,7 +62,7 @@ jobs: - name: Set compiler environment uses: ./.github/actions/set-compiler-env with: - compiler: clang + compiler: ${{ env.COMPILER }} - name: Setup Conan uses: ./.github/actions/setup-conan @@ -150,21 +153,21 @@ jobs: run: | if [ -f "${OUTPUT_FILE}" ]; then # Extract lines containing 'error:', 'warning:', or 'note:' - grep -E '(error:|warning:|note:)' "${OUTPUT_FILE}" >filtered-output.txt || true + grep -E '(error:|warning:|note:)' "${OUTPUT_FILE}" >"${FILTERED_OUTPUT_FILE}" || true # If filtered output is empty, use original (might be a different error format) - if [ ! -s filtered-output.txt ]; then - cp "${OUTPUT_FILE}" filtered-output.txt + if [ ! -s "${FILTERED_OUTPUT_FILE}" ]; then + cp "${OUTPUT_FILE}" "${FILTERED_OUTPUT_FILE}" fi # Truncate if too large - head -c 60000 filtered-output.txt >>"${ISSUE_FILE}" - if [ "$(wc -c >"${ISSUE_FILE}" + if [ "$(wc -c <"${FILTERED_OUTPUT_FILE}")" -gt 60000 ]; then echo "" >>"${ISSUE_FILE}" echo "... (output truncated, see artifacts for full output)" >>"${ISSUE_FILE}" fi - rm filtered-output.txt + rm "${FILTERED_OUTPUT_FILE}" else echo "No output file found" >>"${ISSUE_FILE}" fi diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 0e3f657006..eed4bfc4a3 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -27,7 +27,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -45,7 +45,7 @@ jobs: version: ${{ steps.version.outputs.version }} steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: sparse-checkout: | .github/actions/generate-version @@ -69,7 +69,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Download pre-built binary uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index ea134b43b2..c1a1c1a78b 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -23,7 +23,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -35,4 +35,5 @@ jobs: id: generate env: GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }} - run: ./generate.py ${GENERATE_CONFIG} >>"${GITHUB_OUTPUT}" + GENERATE_EVENT: ${{ github.event_name }} + run: ./generate.py ${GENERATE_CONFIG} --event="${GENERATE_EVENT}" >>"${GITHUB_OUTPUT}" diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index 1c90fb0e72..a389e98771 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,10 +40,10 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-fe4c8ae steps: - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Generate build version number id: version diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 1a52ceee63..5d3712cf9e 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -30,6 +30,7 @@ on: - ".github/scripts/strategy-matrix/**" - conanfile.py - conan.lock + - conan/profiles/** env: CONAN_REMOTE_NAME: xrplf @@ -64,7 +65,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8 From 88cc3dd15ac1cf55ad7587aaee79af72fdd32596 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:19:59 +0200 Subject: [PATCH 27/28] fix: remove old files --- nix/docker/check-tools.sh | 36 ------------------------------------ 1 file changed, 36 deletions(-) delete mode 100755 nix/docker/check-tools.sh diff --git a/nix/docker/check-tools.sh b/nix/docker/check-tools.sh deleted file mode 100755 index 67bcdff8a9..0000000000 --- a/nix/docker/check-tools.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/bash -# Verify that every tool expected in the Nix CI env is present and runnable. -set -euo pipefail - -ccache --version -clang --version -clang++ --version -clang-format --version -cmake --version -conan --version -curl --version -doxygen --version -g++ --version -gcc --version -gcov --version -gcovr --version -git --version -git-cliff --version -gpg --version -less --version -make --version -mold --version -netstat --version -ninja --version -perl --version -pkg-config --version -pre-commit --version -python3 --version -run-clang-tidy --help -vim --version - -# A simple test to verify that git can clone a repository over HTTPS -# (i.e. the CA bundle is wired up). Clone to a temp dir and clean up. -tmp_clone="$(mktemp -d)" -git clone --depth 1 https://github.com/XRPLF/actions.git "${tmp_clone}/actions" -rm -rf "${tmp_clone}" From 5b77cad04c67721858dc77b758396380f2f3f0a5 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:33:28 +0200 Subject: [PATCH 28/28] fix: Use renamed keylet::mptokenIssuance in Vault_test after merge --- src/test/app/Vault_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index f229d58a68..b06c2495c6 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -8169,7 +8169,7 @@ class Vault_test : public beast::unit_test::Suite auto const sleVault = env.le(vaultKeylet); BEAST_EXPECT(sleVault != nullptr); - auto const sleIssuance = env.le(keylet::mptIssuance(sleVault->at(sfShareMPTID))); + auto const sleIssuance = env.le(keylet::mptokenIssuance(sleVault->at(sfShareMPTID))); BEAST_EXPECT(sleIssuance != nullptr); return sleIssuance->at(sfOutstandingAmount);