From a444d80b45d0d23f207eb4d47e59627d0442dcb6 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Fri, 24 Apr 2026 17:46:03 +0200 Subject: [PATCH] fix: Re-apply vault invariant rounding to per-transactor checks Restore the DeltaInfo + roundToAsset rounding logic in VaultDeposit, VaultWithdraw, and VaultClawback finalizeInvariants methods. These changes were lost when the previous parent-branch merge was committed without staging the transactor files. --- .../tx/transactors/vault/VaultClawback.cpp | 37 +++++++--- .../tx/transactors/vault/VaultDeposit.cpp | 67 +++++++++++++------ .../tx/transactors/vault/VaultWithdraw.cpp | 67 +++++++++++++------ 3 files changed, 122 insertions(+), 49 deletions(-) diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index a1c41e6a2b..31ec87408e 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -500,9 +500,10 @@ VaultClawback::finalizeInvariants( auto const& beforeVault = invariantData_.beforeVault()[0]; auto const& afterVault = invariantData_.afterVault()[0]; + auto const& vaultAsset = afterVault.asset; auto const beforeShares = invariantData_.resolveBeforeShares(beforeVault); - if (afterVault.asset.native() || afterVault.asset.getIssuer() != tx[sfAccount]) + 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 @@ -518,22 +519,35 @@ VaultClawback::finalizeInvariants( auto result = true; - auto const vaultDeltaAssets = invariantData_.deltaAssets(afterVault.asset, afterVault.pseudoId); - if (vaultDeltaAssets) + auto const maybeVaultDeltaAssets = invariantData_.deltaAssets(vaultAsset, afterVault.pseudoId); + if (maybeVaultDeltaAssets) { - if (*vaultDeltaAssets >= beast::zero) + auto const totalDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset); + auto const availableDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset); + auto const minScale = VaultInvariantData::computeCoarsestScale( + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + auto const vaultDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + + if (vaultDeltaAssets >= beast::zero) { JLOG(j.fatal()) << "Invariant failed: clawback must decrease vault balance"; 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 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: clawback and assets available must add up"; result = false; @@ -545,15 +559,16 @@ VaultClawback::finalizeInvariants( return false; } - auto const accountDeltaShares = + // We don't need to round shares, they are integral MPT. + auto const maybeAccountDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, tx[sfHolder]); - if (!accountDeltaShares) + if (!maybeAccountDeltaShares) { JLOG(j.fatal()) << "Invariant failed: clawback must change holder shares"; return false; } - if (*accountDeltaShares >= beast::zero) + if (maybeAccountDeltaShares->delta >= beast::zero) { JLOG(j.fatal()) << "Invariant failed: clawback must decrease holder shares"; result = false; @@ -561,13 +576,13 @@ VaultClawback::finalizeInvariants( auto const vaultDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == beast::zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == beast::zero) { JLOG(j.fatal()) << "Invariant failed: clawback must change vault shares"; return false; } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != maybeAccountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder and vault shares by equal amount"; diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index 9ce416360a..5fab8197a6 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -20,6 +21,7 @@ #include #include +#include #include #include @@ -310,23 +312,35 @@ VaultDeposit::finalizeInvariants( auto result = true; auto const& beforeVault = invariantData_.beforeVault()[0]; auto const& afterVault = invariantData_.afterVault()[0]; + auto const& vaultAsset = afterVault.asset; - auto const vaultDeltaAssets = invariantData_.deltaAssets(afterVault.asset, afterVault.pseudoId); + auto const maybeVaultDeltaAssets = invariantData_.deltaAssets(vaultAsset, afterVault.pseudoId); - if (!vaultDeltaAssets) + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit must change vault balance"; return false; } - if (*vaultDeltaAssets > tx[sfAmount]) + // Get the coarsest scale to round calculations to. + auto const totalDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset); + auto const availableDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset); + auto const minScale = VaultInvariantData::computeCoarsestScale( + {*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 balance by more than deposited amount"; result = false; } - if (*vaultDeltaAssets <= beast::zero) + if (vaultDeltaAssets <= beast::zero) { JLOG(j.fatal()) << "Invariant failed: deposit must increase vault balance"; result = false; @@ -335,28 +349,35 @@ VaultDeposit::finalizeInvariants( // Any payments (including deposits) made by the issuer // do not change their balance, but create funds instead. bool const issuerDeposit = [&]() -> bool { - if (afterVault.asset.native()) + if (vaultAsset.native()) return false; - return tx[sfAccount] == afterVault.asset.getIssuer(); + return tx[sfAccount] == vaultAsset.getIssuer(); }(); if (!issuerDeposit) { - auto const accountDeltaAssets = invariantData_.deltaAssetsTxAccount( - tx[sfAccount], tx[~sfDelegate], afterVault.asset, fee); - if (!accountDeltaAssets) + auto const maybeAccDeltaAssets = + invariantData_.deltaAssetsTxAccount(tx[sfAccount], tx[~sfDelegate], vaultAsset, fee); + if (!maybeAccDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit must change depositor balance"; return false; } + auto const localMinScale = + std::max(minScale, VaultInvariantData::computeCoarsestScale({*maybeAccDeltaAssets})); - if (*accountDeltaAssets >= beast::zero) + auto const accountDeltaAssets = + roundToAsset(vaultAsset, maybeAccDeltaAssets->delta, localMinScale); + auto const localVaultDeltaAssets = + roundToAsset(vaultAsset, vaultDeltaAssets, localMinScale); + + if (accountDeltaAssets >= beast::zero) { JLOG(j.fatal()) << "Invariant failed: deposit must decrease depositor balance"; result = false; } - if (*accountDeltaAssets * -1 != *vaultDeltaAssets) + if (localVaultDeltaAssets * -1 != accountDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault and depositor balance by equal amount"; @@ -371,41 +392,49 @@ VaultDeposit::finalizeInvariants( result = false; } - auto const accountDeltaShares = + // We don't need to round shares, they are integral MPT. + auto const maybeAccDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, tx[sfAccount]); - if (!accountDeltaShares) + if (!maybeAccDeltaShares) { JLOG(j.fatal()) << "Invariant failed: deposit must change depositor shares"; return false; } + auto const& accountDeltaShares = *maybeAccDeltaShares; - if (*accountDeltaShares <= beast::zero) + if (accountDeltaShares.delta <= beast::zero) { JLOG(j.fatal()) << "Invariant failed: deposit must increase depositor shares"; result = false; } - auto const vaultDeltaShares = + auto const maybeVaultDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == beast::zero) + if (!maybeVaultDeltaShares || maybeVaultDeltaShares->delta == beast::zero) { JLOG(j.fatal()) << "Invariant failed: deposit must change vault shares"; return false; } + auto const& vaultDeltaShares = *maybeVaultDeltaShares; - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor and vault shares by equal amount"; 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"; result = false; diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index fa0ce8ef59..fa1d27494c 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -21,6 +22,7 @@ #include #include +#include #include #include @@ -327,16 +329,28 @@ VaultWithdraw::finalizeInvariants( auto result = true; auto const& beforeVault = invariantData_.beforeVault()[0]; auto const& afterVault = invariantData_.afterVault()[0]; + auto const& vaultAsset = afterVault.asset; - auto const vaultDeltaAssets = invariantData_.deltaAssets(afterVault.asset, afterVault.pseudoId); + auto const maybeVaultDeltaAssets = invariantData_.deltaAssets(vaultAsset, afterVault.pseudoId); - if (!vaultDeltaAssets) + if (!maybeVaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault balance"; return false; } - if (*vaultDeltaAssets >= beast::zero) + // Get the most coarse scale to round calculations to. + auto const totalDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsTotal, afterVault.assetsTotal, vaultAsset); + auto const availableDelta = VaultInvariantData::DeltaInfo::makeDelta( + beforeVault.assetsAvailable, afterVault.assetsAvailable, vaultAsset); + auto const minScale = VaultInvariantData::computeCoarsestScale( + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + + auto const vaultPseudoDeltaAssets = + roundToAsset(vaultAsset, maybeVaultDeltaAssets->delta, minScale); + + if (vaultPseudoDeltaAssets >= beast::zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must decrease vault balance"; result = false; @@ -345,39 +359,49 @@ VaultWithdraw::finalizeInvariants( // Any payments (including withdrawal) going to the issuer // do not change their balance, but destroy funds instead. bool const issuerWithdrawal = [&]() -> bool { - if (afterVault.asset.native()) + if (vaultAsset.native()) return false; auto const destination = tx[~sfDestination].value_or(tx[sfAccount]); - return destination == afterVault.asset.getIssuer(); + return destination == vaultAsset.getIssuer(); }(); if (!issuerWithdrawal) { - auto const accountDeltaAssets = invariantData_.deltaAssetsTxAccount( - tx[sfAccount], tx[~sfDelegate], afterVault.asset, fee); - auto const otherAccountDelta = [&]() -> std::optional { + auto const maybeAccDelta = + invariantData_.deltaAssetsTxAccount(tx[sfAccount], tx[~sfDelegate], vaultAsset, fee); + auto const maybeOtherAccDelta = [&]() -> std::optional { if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) - return invariantData_.deltaAssets(afterVault.asset, *destination); + return invariantData_.deltaAssets(vaultAsset, *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 destination balance"; return false; } - auto const destinationDelta = accountDeltaAssets ? *accountDeltaAssets : *otherAccountDelta; + auto const destinationDelta = maybeAccDelta ? *maybeAccDelta : *maybeOtherAccDelta; - if (destinationDelta <= beast::zero) + // The scale of destinationDelta can be coarser than minScale, so we + // take that into account when rounding. + auto const localMinScale = + std::max(minScale, VaultInvariantData::computeCoarsestScale({destinationDelta})); + + auto const roundedDestinationDelta = + roundToAsset(vaultAsset, destinationDelta.delta, localMinScale); + + if (roundedDestinationDelta <= beast::zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must increase destination balance"; 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 and destination balance by equal " @@ -386,6 +410,7 @@ VaultWithdraw::finalizeInvariants( } } + // We don't need to round shares, they are integral MPT. auto const accountDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, tx[sfAccount]); if (!accountDeltaShares) @@ -394,7 +419,7 @@ VaultWithdraw::finalizeInvariants( return false; } - if (*accountDeltaShares >= beast::zero) + if (accountDeltaShares->delta >= beast::zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must decrease depositor shares"; result = false; @@ -402,27 +427,31 @@ VaultWithdraw::finalizeInvariants( auto const vaultDeltaShares = invariantData_.deltaShares(afterVault.pseudoId, afterVault.shareMPTID, afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == beast::zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == beast::zero) { JLOG(j.fatal()) << "Invariant failed: withdrawal must change vault shares"; return false; } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { 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) + auto const assetTotalDelta = + roundToAsset(vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale); + // Note, vaultBalance is negative (see check above). + 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"; result = false;