From eff344faf92ef14a221221168b9eda598dc83e4d Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 17 Mar 2026 11:22:49 +0000 Subject: [PATCH 1/4] chore: Move sanitizer runtime options out to files (#6371) This change moves the sanitizer runtime options out to dedicated files, such that they can be used in multiple places (CI, local runs) without any need to rewrite them. --- .github/workflows/reusable-build-test-config.yml | 8 ++++---- docs/build/sanitizers.md | 8 ++++---- sanitizers/suppressions/asan.supp | 9 --------- sanitizers/suppressions/runtime-asan-options.txt | 8 ++++++++ sanitizers/suppressions/runtime-lsan-options.txt | 1 + sanitizers/suppressions/runtime-tsan-options.txt | 3 +++ sanitizers/suppressions/runtime-ubsan-options.txt | 1 + sanitizers/suppressions/sanitizer-ignorelist.txt | 8 ++++++++ sanitizers/suppressions/ubsan.supp | 11 +++++++++++ 9 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 sanitizers/suppressions/runtime-asan-options.txt create mode 100644 sanitizers/suppressions/runtime-lsan-options.txt create mode 100644 sanitizers/suppressions/runtime-tsan-options.txt create mode 100644 sanitizers/suppressions/runtime-ubsan-options.txt diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 137ea5d16c..efec2afe0a 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -207,14 +207,14 @@ jobs: env: CONFIG_NAME: ${{ inputs.config_name }} run: | - ASAN_OPTS="halt_on_error=0:use_sigaltstack=0:print_stacktrace=1:detect_container_overflow=0:detect_stack_use_after_return=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" + 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=second_deadlock_stack=1:halt_on_error=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV} - echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV} - echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${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 }} diff --git a/docs/build/sanitizers.md b/docs/build/sanitizers.md index 3f9809ae98..ac3a0cc865 100644 --- a/docs/build/sanitizers.md +++ b/docs/build/sanitizers.md @@ -89,8 +89,8 @@ cmake --build . --parallel 4 **IMPORTANT**: ASAN with Boost produces many false positives. Use these options: ```bash -export ASAN_OPTIONS="print_stacktrace=1:detect_container_overflow=0:suppressions=path/to/asan.supp:halt_on_error=0:log_path=asan.log" -export LSAN_OPTIONS="suppressions=path/to/lsan.supp:halt_on_error=0:log_path=lsan.log" +export ASAN_OPTIONS="include=sanitizers/suppressions/runtime-asan-options.txt:suppressions=sanitizers/suppressions/asan.supp" +export LSAN_OPTIONS="include=sanitizers/suppressions/runtime-lsan-options.txt:suppressions=sanitizers/suppressions/lsan.supp" # Run tests ./xrpld --unittest --unittest-jobs=5 @@ -108,7 +108,7 @@ export LSAN_OPTIONS="suppressions=path/to/lsan.supp:halt_on_error=0:log_path=lsa ### ThreadSanitizer (TSan) ```bash -export TSAN_OPTIONS="suppressions=path/to/tsan.supp halt_on_error=0 log_path=tsan.log" +export TSAN_OPTIONS="include=sanitizers/suppressions/runtime-tsan-options.txt:suppressions=sanitizers/suppressions/tsan.supp" # Run tests ./xrpld --unittest --unittest-jobs=5 @@ -129,7 +129,7 @@ More details [here](https://github.com/google/sanitizers/wiki/AddressSanitizerLe ### UndefinedBehaviorSanitizer (UBSan) ```bash -export UBSAN_OPTIONS="suppressions=path/to/ubsan.supp:print_stacktrace=1:halt_on_error=0:log_path=ubsan.log" +export UBSAN_OPTIONS="include=sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=sanitizers/suppressions/ubsan.supp" # Run tests ./xrpld --unittest --unittest-jobs=5 diff --git a/sanitizers/suppressions/asan.supp b/sanitizers/suppressions/asan.supp index 6f2fc6b6d1..e02d580aa6 100644 --- a/sanitizers/suppressions/asan.supp +++ b/sanitizers/suppressions/asan.supp @@ -2,15 +2,6 @@ # # ASAN_OPTIONS="print_stacktrace=1:detect_container_overflow=0:suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0" -# Leaks in Doctest tests: xrpl.test.* -interceptor_name:src/libxrpl/net/HTTPClient.cpp -interceptor_name:src/libxrpl/net/RegisterSSLCerts.cpp -interceptor_name:src/tests/libxrpl/net/HTTPClient.cpp -interceptor_name:xrpl/net/AutoSocket.h -interceptor_name:xrpl/net/HTTPClient.h -interceptor_name:xrpl/net/HTTPClientSSLContext.h -interceptor_name:xrpl/net/RegisterSSLCerts.h - # Suppress false positive stack-buffer errors in thread stack allocation # Related to ASan's __asan_handle_no_return warnings (github.com/google/sanitizers/issues/189) # These occur during multi-threaded test initialization on macOS diff --git a/sanitizers/suppressions/runtime-asan-options.txt b/sanitizers/suppressions/runtime-asan-options.txt new file mode 100644 index 0000000000..fb2727f868 --- /dev/null +++ b/sanitizers/suppressions/runtime-asan-options.txt @@ -0,0 +1,8 @@ +detect_container_overflow=false +detect_stack_use_after_return=false +debug=true +halt_on_error=false +print_stats=true +print_cmdline=true +use_sigaltstack=0 +print_stacktrace=1 diff --git a/sanitizers/suppressions/runtime-lsan-options.txt b/sanitizers/suppressions/runtime-lsan-options.txt new file mode 100644 index 0000000000..fcfccf7bae --- /dev/null +++ b/sanitizers/suppressions/runtime-lsan-options.txt @@ -0,0 +1 @@ +halt_on_error=false diff --git a/sanitizers/suppressions/runtime-tsan-options.txt b/sanitizers/suppressions/runtime-tsan-options.txt new file mode 100644 index 0000000000..aafe997ea4 --- /dev/null +++ b/sanitizers/suppressions/runtime-tsan-options.txt @@ -0,0 +1,3 @@ +halt_on_error=false +verbosity=1 +second_deadlock_stack=1 diff --git a/sanitizers/suppressions/runtime-ubsan-options.txt b/sanitizers/suppressions/runtime-ubsan-options.txt new file mode 100644 index 0000000000..fcfccf7bae --- /dev/null +++ b/sanitizers/suppressions/runtime-ubsan-options.txt @@ -0,0 +1 @@ +halt_on_error=false diff --git a/sanitizers/suppressions/sanitizer-ignorelist.txt b/sanitizers/suppressions/sanitizer-ignorelist.txt index 5dbead48a2..dc9a31f8e4 100644 --- a/sanitizers/suppressions/sanitizer-ignorelist.txt +++ b/sanitizers/suppressions/sanitizer-ignorelist.txt @@ -27,3 +27,11 @@ src:core/JobQueue.cpp src:libxrpl/beast/utility/beast_Journal.cpp src:test/beast/beast_PropertyStream_test.cpp src:src/test/app/Invariants_test.cpp + +# ASan false positive: stack-use-after-scope in ErrorCodes.h inline functions. +# When Clang inlines the StaticString overloads (e.g. invalid_field_error(StaticString)), +# ASan scope-poisons the temporary std::string before the inlined callee finishes reading +# through the const ref. This corrupts the coroutine stack and crashes the Simulate test. +# See asan.supp comments for full explanation and planned fix. +[address] +src:*ErrorCodes.h diff --git a/sanitizers/suppressions/ubsan.supp b/sanitizers/suppressions/ubsan.supp index 1504ef685f..bd38e43c32 100644 --- a/sanitizers/suppressions/ubsan.supp +++ b/sanitizers/suppressions/ubsan.supp @@ -182,6 +182,17 @@ signed-integer-overflow:src/test/beast/LexicalCast_test.cpp # External library suppressions unsigned-integer-overflow:nudb/detail/xxhash.hpp +# Loan_test.cpp intentional underflow in test arithmetic +unsigned-integer-overflow:src/test/app/Loan_test.cpp +undefined:src/test/app/Loan_test.cpp + +# Source tree restructured paths (libxrpl/tx/transactors/) +# These duplicate the xrpld/app/tx/detail entries above for the new layout +unsigned-integer-overflow:src/libxrpl/tx/transactors/oracle/SetOracle.cpp +undefined:src/libxrpl/tx/transactors/oracle/SetOracle.cpp +unsigned-integer-overflow:src/libxrpl/tx/transactors/nft/NFTokenMint.cpp +undefined:src/libxrpl/tx/transactors/nft/NFTokenMint.cpp + # Protobuf intentional overflows in hash functions # Protobuf uses intentional unsigned overflow for hash computation (stringpiece.h:393) unsigned-integer-overflow:google/protobuf/stubs/stringpiece.h From 5ae97fa8ae955bf8067919bcbebe2a18b64e3ffd Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 17 Mar 2026 13:10:32 +0000 Subject: [PATCH 2/4] refactor: Add no-ASAN macro for Throw statements (#6373) Throwing exceptions from code sometime confuses ASAN, as it cannot keep track of stack frames. This change therefore adds a macro to skip instrumentation around the `Throw` function. --- cspell.config.yaml | 1 + include/xrpl/basics/contract.h | 17 +++++++++++++++-- include/xrpl/basics/sanitizers.h | 13 +++++++++++++ src/libxrpl/basics/Number.cpp | 4 ++-- 4 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 include/xrpl/basics/sanitizers.h diff --git a/cspell.config.yaml b/cspell.config.yaml index ec0a522663..eeae3d12ff 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -112,6 +112,7 @@ words: - gpgcheck - gpgkey - hotwallet + - hwaddress - hwrap - ifndef - inequation diff --git a/include/xrpl/basics/contract.h b/include/xrpl/basics/contract.h index aba021b726..f41c272b61 100644 --- a/include/xrpl/basics/contract.h +++ b/include/xrpl/basics/contract.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -23,16 +24,28 @@ LogThrow(std::string const& title); When called from within a catch block, it will pass control to the next matching exception handler, if any. Otherwise, std::terminate will be called. + + ASAN can't handle sudden jumps in control flow very well. This + function is marked as XRPL_NO_SANITIZE_ADDRESS to prevent it from + triggering false positives, since it throws. */ -[[noreturn]] inline void +[[noreturn]] XRPL_NO_SANITIZE_ADDRESS inline void Rethrow() { LogThrow("Re-throwing exception"); throw; } +/* + Logs and throws an exception of type E. + + ASAN can't handle sudden jumps in control flow very well. This + function is marked as XRPL_NO_SANITIZE_ADDRESS to prevent it from + triggering false positives, since it throws. +*/ + template -[[noreturn]] inline void +[[noreturn]] XRPL_NO_SANITIZE_ADDRESS inline void Throw(Args&&... args) { static_assert( diff --git a/include/xrpl/basics/sanitizers.h b/include/xrpl/basics/sanitizers.h new file mode 100644 index 0000000000..b954952848 --- /dev/null +++ b/include/xrpl/basics/sanitizers.h @@ -0,0 +1,13 @@ +#pragma once + +// Helper to disable ASan/HwASan for specific functions +/* + ASAN flags some false positives with sudden jumps in control flow, like + exceptions, or when encountering coroutine stack switches. This macro can be used to disable ASAN + intrumentation for specific functions. +*/ +#if defined(__GNUC__) || defined(__clang__) +#define XRPL_NO_SANITIZE_ADDRESS __attribute__((no_sanitize("address", "hwaddress"))) +#else +#define XRPL_NO_SANITIZE_ADDRESS +#endif diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 59a41157db..8bd048ae9e 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -258,7 +258,7 @@ Number::Guard::doRoundUp( } bringIntoRange(negative, mantissa, exponent, minMantissa); if (exponent > maxExponent) - throw std::overflow_error(location); + Throw(std::string(location)); } template @@ -298,7 +298,7 @@ Number::Guard::doRound(rep& drops, std::string location) // or "(maxRep + 1) / 10", neither of which will round up when // converting to rep, though the latter might overflow _before_ // rounding. - throw std::overflow_error(location); // LCOV_EXCL_LINE + Throw(std::string(location)); // LCOV_EXCL_LINE } ++drops; } From 252c6768dfd88c8f239ed828ae8f1f85132c976f Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 17 Mar 2026 10:12:16 -0400 Subject: [PATCH 3/4] refactor: Clean up `getFeePayer`, `mSourceBalance`, and `mPriorBalance` (#6478) This change: * Introduces a new helper function on `STTx`, `getFeePayer`. * Removes the usage of `mSourceBalance` and replaces it with SLE balance lookups. * Renames `mPriorBalance` to `preFeeBalance_` This simplifies some of the code in the transactors and makes it a lot more readable. --- include/xrpl/protocol/STTx.h | 3 ++ include/xrpl/tx/Transactor.h | 3 +- src/libxrpl/protocol/STTx.cpp | 14 ++++++ src/libxrpl/tx/Transactor.cpp | 44 ++++++------------- .../tx/transactors/account/DeleteAccount.cpp | 9 ++-- .../tx/transactors/account/SetSignerList.cpp | 2 +- .../tx/transactors/bridge/XChainBridge.cpp | 11 ++--- .../tx/transactors/check/CashCheck.cpp | 2 +- .../tx/transactors/check/CreateCheck.cpp | 2 +- .../credentials/CredentialAccept.cpp | 2 +- .../credentials/CredentialCreate.cpp | 2 +- .../tx/transactors/delegate/DelegateSet.cpp | 2 +- .../tx/transactors/dex/AMMClawback.cpp | 8 ++-- .../tx/transactors/dex/AMMWithdraw.cpp | 4 +- .../tx/transactors/dex/CreateOffer.cpp | 2 +- .../tx/transactors/escrow/EscrowCancel.cpp | 2 +- .../tx/transactors/escrow/EscrowCreate.cpp | 5 ++- .../tx/transactors/escrow/EscrowFinish.cpp | 2 +- .../lending/LoanBrokerCoverWithdraw.cpp | 2 +- .../tx/transactors/lending/LoanBrokerSet.cpp | 4 +- .../tx/transactors/lending/LoanSet.cpp | 2 +- .../tx/transactors/nft/NFTokenAcceptOffer.cpp | 2 +- .../tx/transactors/nft/NFTokenCreateOffer.cpp | 2 +- .../tx/transactors/nft/NFTokenMint.cpp | 4 +- .../tx/transactors/payment/DepositPreauth.cpp | 4 +- .../tx/transactors/payment/Payment.cpp | 8 ++-- .../tx/transactors/system/CreateTicket.cpp | 2 +- .../tx/transactors/token/MPTokenAuthorize.cpp | 2 +- .../token/MPTokenIssuanceCreate.cpp | 2 +- src/libxrpl/tx/transactors/token/SetTrust.cpp | 6 +-- .../tx/transactors/vault/VaultCreate.cpp | 8 ++-- .../tx/transactors/vault/VaultDeposit.cpp | 6 +-- .../tx/transactors/vault/VaultWithdraw.cpp | 2 +- 33 files changed, 88 insertions(+), 87 deletions(-) diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index f00f288008..f200c19eac 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -83,6 +83,9 @@ public: std::uint32_t getSeqValue() const; + AccountID + getFeePayer() const; + boost::container::flat_set getMentionedAccounts() const; diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index 0e88cf92e1..b415316358 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -114,8 +114,7 @@ protected: beast::Journal const j_; AccountID const account_; - XRPAmount mPriorBalance; // Balance before fees. - XRPAmount mSourceBalance; // Balance after fees. + XRPAmount preFeeBalance_; // Balance before fees. virtual ~Transactor() = default; Transactor(Transactor const&) = delete; diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 6f201ba066..edc668bbd5 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -211,6 +211,20 @@ STTx::getSeqValue() const return getSeqProxy().value(); } +AccountID +STTx::getFeePayer() const +{ + // If sfDelegate is present, the delegate account is the payer + // note: if a delegate is specified, its authorization to act on behalf of the account is + // enforced in `Transactor::checkPermission` + // cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`) + if (isFieldPresent(sfDelegate)) + return getAccountID(sfDelegate); + + // Default payer + return getAccountID(sfAccount); +} + void STTx::sign( PublicKey const& publicKey, diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 3ddac350e8..2ae4a8335c 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -352,8 +352,7 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee) if (feePaid == beast::zero) return tesSUCCESS; - auto const id = ctx.tx.isFieldPresent(sfDelegate) ? ctx.tx.getAccountID(sfDelegate) - : ctx.tx.getAccountID(sfAccount); + auto const id = ctx.tx.getFeePayer(); auto const sle = ctx.view.read(keylet::account(id)); if (!sle) return terNO_ACCOUNT; @@ -382,32 +381,18 @@ Transactor::payFee() { auto const feePaid = ctx_.tx[sfFee].xrp(); - if (ctx_.tx.isFieldPresent(sfDelegate)) - { - // Delegated transactions are paid by the delegated account. - auto const delegate = ctx_.tx.getAccountID(sfDelegate); - auto const delegatedSle = view().peek(keylet::account(delegate)); - if (!delegatedSle) - return tefINTERNAL; // LCOV_EXCL_LINE + auto const feePayer = ctx_.tx.getFeePayer(); + auto const sle = view().peek(keylet::account(feePayer)); + if (!sle) + return tefINTERNAL; // LCOV_EXCL_LINE - delegatedSle->setFieldAmount(sfBalance, delegatedSle->getFieldAmount(sfBalance) - feePaid); - view().update(delegatedSle); - } - else - { - auto const sle = view().peek(keylet::account(account_)); - if (!sle) - return tefINTERNAL; // LCOV_EXCL_LINE - - // Deduct the fee, so it's not available during the transaction. - // Will only write the account back if the transaction succeeds. - - mSourceBalance -= feePaid; - sle->setFieldAmount(sfBalance, mSourceBalance); - - // VFALCO Should we call view().rawDestroyXRP() here as well? - } + // Deduct the fee, so it's not available during the transaction. + // Will only write the account back if the transaction succeeds. + sle->setFieldAmount(sfBalance, sle->getFieldAmount(sfBalance) - feePaid); + if (feePayer != account_) + view().update(sle); // done in `apply()` for the account + // VFALCO Should we call view().rawDestroyXRP() here as well? return tesSUCCESS; } @@ -606,8 +591,7 @@ Transactor::apply() if (sle) { - mPriorBalance = STAmount{(*sle)[sfBalance]}.xrp(); - mSourceBalance = mPriorBalance; + preFeeBalance_ = STAmount{(*sle)[sfBalance]}.xrp(); TER result = consumeSeqProxy(sle); if (result != tesSUCCESS) @@ -1023,9 +1007,7 @@ Transactor::reset(XRPAmount fee) if (!txnAcct) return {tefINTERNAL, beast::zero}; - auto const payerSle = ctx_.tx.isFieldPresent(sfDelegate) - ? view().peek(keylet::account(ctx_.tx.getAccountID(sfDelegate))) - : txnAcct; + auto const payerSle = view().peek(keylet::account(ctx_.tx.getFeePayer())); if (!payerSle) return {tefINTERNAL, beast::zero}; // LCOV_EXCL_LINE diff --git a/src/libxrpl/tx/transactors/account/DeleteAccount.cpp b/src/libxrpl/tx/transactors/account/DeleteAccount.cpp index a3fb97fa0d..a4f1408298 100644 --- a/src/libxrpl/tx/transactors/account/DeleteAccount.cpp +++ b/src/libxrpl/tx/transactors/account/DeleteAccount.cpp @@ -371,9 +371,10 @@ DeleteAccount::doApply() return ter; // Transfer any XRP remaining after the fee is paid to the destination: - (*dst)[sfBalance] = (*dst)[sfBalance] + mSourceBalance; - (*src)[sfBalance] = (*src)[sfBalance] - mSourceBalance; - ctx_.deliver(mSourceBalance); + auto const remainingBalance = src->getFieldAmount(sfBalance).xrp(); + (*dst)[sfBalance] = (*dst)[sfBalance] + remainingBalance; + (*src)[sfBalance] = (*src)[sfBalance] - remainingBalance; + ctx_.deliver(remainingBalance); XRPL_ASSERT( (*src)[sfBalance] == XRPAmount(0), "xrpl::DeleteAccount::doApply : source balance is zero"); @@ -387,7 +388,7 @@ DeleteAccount::doApply() } // Re-arm the password change fee if we can and need to. - if (mSourceBalance > XRPAmount(0) && dst->isFlag(lsfPasswordSpent)) + if (remainingBalance > XRPAmount(0) && dst->isFlag(lsfPasswordSpent)) dst->clearFlag(lsfPasswordSpent); view().update(dst); diff --git a/src/libxrpl/tx/transactors/account/SetSignerList.cpp b/src/libxrpl/tx/transactors/account/SetSignerList.cpp index d5b41c6acf..864c46013f 100644 --- a/src/libxrpl/tx/transactors/account/SetSignerList.cpp +++ b/src/libxrpl/tx/transactors/account/SetSignerList.cpp @@ -306,7 +306,7 @@ SetSignerList::replaceSignerList() // We check the reserve against the starting balance because we want to // allow dipping into the reserve to pay fees. This behavior is consistent // with CreateTicket. - if (mPriorBalance < newReserve) + if (preFeeBalance_ < newReserve) return tecINSUFFICIENT_RESERVE; // Everything's ducky. Add the ltSIGNER_LIST to the ledger. diff --git a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp index 1454bb7c77..ba40d6da36 100644 --- a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp +++ b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp @@ -337,7 +337,7 @@ enum class DepositAuthPolicy { normal, dstCanBypass }; struct TransferHelperSubmittingAccountInfo { AccountID account; - STAmount preFeeBalance; + STAmount preFeeBalance_; STAmount postFeeBalance; }; @@ -423,7 +423,7 @@ transferHelper( if (!submittingAccountInfo || submittingAccountInfo->account != src || submittingAccountInfo->postFeeBalance != curBal) return curBal; - return submittingAccountInfo->preFeeBalance; + return submittingAccountInfo->preFeeBalance_; }(); if (availableBalance < amt + reserve) @@ -1852,7 +1852,8 @@ XChainCommit::doApply() auto const amount = ctx_.tx[sfAmount]; auto const bridgeSpec = ctx_.tx[sfXChainBridge]; - if (!psb.read(keylet::account(account))) + auto const sleAccount = psb.read(keylet::account(account)); + if (!sleAccount) return tecINTERNAL; // LCOV_EXCL_LINE auto const sleBridge = readBridge(psb, bridgeSpec); @@ -1863,7 +1864,7 @@ XChainCommit::doApply() // Support dipping into reserves to pay the fee TransferHelperSubmittingAccountInfo submittingAccountInfo{ - account_, mPriorBalance, mSourceBalance}; + account_, preFeeBalance_, (*sleAccount)[sfBalance]}; auto const thTer = transferHelper( psb, @@ -2132,7 +2133,7 @@ XChainCreateAccountCommit::doApply() // Support dipping into reserves to pay the fee TransferHelperSubmittingAccountInfo submittingAccountInfo{ - account_, mPriorBalance, mSourceBalance}; + account_, preFeeBalance_, (*sle)[sfBalance]}; STAmount const toTransfer = amount + reward; auto const thTer = transferHelper( psb, diff --git a/src/libxrpl/tx/transactors/check/CashCheck.cpp b/src/libxrpl/tx/transactors/check/CashCheck.cpp index 1628b284a9..3d15da374c 100644 --- a/src/libxrpl/tx/transactors/check/CashCheck.cpp +++ b/src/libxrpl/tx/transactors/check/CashCheck.cpp @@ -309,7 +309,7 @@ CashCheck::doApply() // Can the account cover the trust line's reserve? if (std::uint32_t const ownerCount = {sleDst->at(sfOwnerCount)}; - mPriorBalance < psb.fees().accountReserve(ownerCount + 1)) + preFeeBalance_ < psb.fees().accountReserve(ownerCount + 1)) { JLOG(j_.trace()) << "Trust line does not exist. " "Insufficent reserve to create line."; diff --git a/src/libxrpl/tx/transactors/check/CreateCheck.cpp b/src/libxrpl/tx/transactors/check/CreateCheck.cpp index 9e883e7fbb..1530b20f40 100644 --- a/src/libxrpl/tx/transactors/check/CreateCheck.cpp +++ b/src/libxrpl/tx/transactors/check/CreateCheck.cpp @@ -140,7 +140,7 @@ CreateCheck::doApply() { STAmount const reserve{view().fees().accountReserve(sle->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/credentials/CredentialAccept.cpp b/src/libxrpl/tx/transactors/credentials/CredentialAccept.cpp index c3315873e8..1c5608c09f 100644 --- a/src/libxrpl/tx/transactors/credentials/CredentialAccept.cpp +++ b/src/libxrpl/tx/transactors/credentials/CredentialAccept.cpp @@ -85,7 +85,7 @@ CredentialAccept::doApply() { STAmount const reserve{ view().fees().accountReserve(sleSubject->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/credentials/CredentialCreate.cpp b/src/libxrpl/tx/transactors/credentials/CredentialCreate.cpp index 40f8a0c925..5dafd146c7 100644 --- a/src/libxrpl/tx/transactors/credentials/CredentialCreate.cpp +++ b/src/libxrpl/tx/transactors/credentials/CredentialCreate.cpp @@ -117,7 +117,7 @@ CredentialCreate::doApply() { STAmount const reserve{ view().fees().accountReserve(sleIssuer->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp index afb84c1c1e..87e6c076b5 100644 --- a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp +++ b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp @@ -70,7 +70,7 @@ DelegateSet::doApply() STAmount const reserve{ ctx_.view().fees().accountReserve(sleOwner->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; auto const& permissions = ctx_.tx.getFieldArray(sfPermissions); diff --git a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp index 168583ff69..938432e248 100644 --- a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp @@ -172,7 +172,7 @@ AMMClawback::applyGuts(Sandbox& sb) 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, - mPriorBalance, + preFeeBalance_, ctx_.journal); else std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -251,7 +251,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, - mPriorBalance, + preFeeBalance_, ctx_.journal); auto const& rules = sb.rules(); @@ -282,7 +282,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::No, - mPriorBalance, + preFeeBalance_, ctx_.journal); } @@ -301,7 +301,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::No, - mPriorBalance, + preFeeBalance_, ctx_.journal); } diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index 782f79cf44..39156f31d4 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -414,7 +414,7 @@ AMMWithdraw::withdraw( tfee, FreezeHandling::fhZERO_IF_FROZEN, isWithdrawAll(ctx_.tx), - mPriorBalance, + preFeeBalance_, j_); return {ter, newLPTokenBalance}; } @@ -628,7 +628,7 @@ AMMWithdraw::equalWithdrawTokens( tfee, FreezeHandling::fhZERO_IF_FROZEN, isWithdrawAll(ctx_.tx), - mPriorBalance, + preFeeBalance_, ctx_.journal); return {ter, newLPTokenBalance}; } diff --git a/src/libxrpl/tx/transactors/dex/CreateOffer.cpp b/src/libxrpl/tx/transactors/dex/CreateOffer.cpp index 7440dd3bf8..c176203da7 100644 --- a/src/libxrpl/tx/transactors/dex/CreateOffer.cpp +++ b/src/libxrpl/tx/transactors/dex/CreateOffer.cpp @@ -727,7 +727,7 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) { XRPAmount reserve = sb.fees().accountReserve(sleCreator->getFieldU32(sfOwnerCount) + 1); - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) { // If we are here, the signing account had an insufficient reserve // *prior* to our processing. If something actually crossed, then diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp index c69e112f1a..28194e4133 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp @@ -164,7 +164,7 @@ EscrowCancel::doApply() ctx_.view(), parityRate, slep, - mPriorBalance, + preFeeBalance_, amount, issuer, account, // sender and receiver are the same diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp index d7cb588f8d..ef46328de1 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp @@ -394,13 +394,14 @@ EscrowCreate::doApply() auto const reserve = ctx_.view().fees().accountReserve((*sle)[sfOwnerCount] + 1); - if (mSourceBalance < reserve) + auto const balance = sle->getFieldAmount(sfBalance).xrp(); + if (balance < reserve) return tecINSUFFICIENT_RESERVE; // Check reserve and funds availability if (isXRP(amount)) { - if (mSourceBalance < reserve + STAmount(amount).xrp()) + if (balance < reserve + STAmount(amount).xrp()) return tecUNFUNDED; } diff --git a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp index 885afb1918..bab2016aae 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp @@ -332,7 +332,7 @@ EscrowFinish::doApply() ctx_.view(), lockedRate, sled, - mPriorBalance, + preFeeBalance_, amount, issuer, account, diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp index 9140a0dc23..57d9cbc173 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp @@ -167,7 +167,7 @@ LoanBrokerCoverWithdraw::doApply() associateAsset(*broker, vaultAsset); - return doWithdraw(view(), tx, account_, dstAcct, brokerPseudoID, mPriorBalance, amount, j_); + return doWithdraw(view(), tx, account_, dstAcct, brokerPseudoID, preFeeBalance_, amount, j_); } //------------------------------------------------------------------------------ diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index c825a7fafe..2632b55812 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -220,7 +220,7 @@ LoanBrokerSet::doApply() // one for the pseudo-account. adjustOwnerCount(view, owner, 2, j_); auto const ownerCount = owner->at(sfOwnerCount); - if (mPriorBalance < view.fees().accountReserve(ownerCount)) + if (preFeeBalance_ < view.fees().accountReserve(ownerCount)) return tecINSUFFICIENT_RESERVE; auto maybePseudo = createPseudoAccount(view, broker->key(), sfLoanBrokerID); @@ -229,7 +229,7 @@ LoanBrokerSet::doApply() auto& pseudo = *maybePseudo; auto pseudoId = pseudo->at(sfAccount); - if (auto ter = addEmptyHolding(view, pseudoId, mPriorBalance, sleVault->at(sfAsset), j_)) + if (auto ter = addEmptyHolding(view, pseudoId, preFeeBalance_, sleVault->at(sfAsset), j_)) return ter; // Initialize data fields: diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index f74522e737..848af47957 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -474,7 +474,7 @@ LoanSet::doApply() { auto const ownerCount = borrowerSle->at(sfOwnerCount); auto const balance = - account_ == borrower ? mPriorBalance : borrowerSle->at(sfBalance).value().xrp(); + account_ == borrower ? preFeeBalance_ : borrowerSle->at(sfBalance).value().xrp(); if (balance < view.fees().accountReserve(ownerCount)) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp index 32af49355a..4cdb7751c6 100644 --- a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp +++ b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp @@ -361,7 +361,7 @@ NFTokenAcceptOffer::transferNFToken( // NFTs free of reserve. if (view().rules().enabled(fixNFTokenReserve)) { - // To check if there is sufficient reserve, we cannot use mPriorBalance + // To check if there is sufficient reserve, we cannot use preFeeBalance_ // because NFT is sold for a price. So we must use the balance after // the deduction of the potential offer price. A small caveat here is // that the balance has already deducted the transaction fee, meaning diff --git a/src/libxrpl/tx/transactors/nft/NFTokenCreateOffer.cpp b/src/libxrpl/tx/transactors/nft/NFTokenCreateOffer.cpp index ca90bcafab..847d15a3b7 100644 --- a/src/libxrpl/tx/transactors/nft/NFTokenCreateOffer.cpp +++ b/src/libxrpl/tx/transactors/nft/NFTokenCreateOffer.cpp @@ -74,7 +74,7 @@ NFTokenCreateOffer::doApply() ctx_.tx[~sfExpiration], ctx_.tx.getSeqProxy(), ctx_.tx[sfNFTokenID], - mPriorBalance, + preFeeBalance_, j_, ctx_.tx.getFlags()); } diff --git a/src/libxrpl/tx/transactors/nft/NFTokenMint.cpp b/src/libxrpl/tx/transactors/nft/NFTokenMint.cpp index a643a6042c..35e2705034 100644 --- a/src/libxrpl/tx/transactors/nft/NFTokenMint.cpp +++ b/src/libxrpl/tx/transactors/nft/NFTokenMint.cpp @@ -293,7 +293,7 @@ NFTokenMint::doApply() ctx_.tx[~sfExpiration], ctx_.tx.getSeqProxy(), nftokenID, - mPriorBalance, + preFeeBalance_, j_); !isTesSuccess(ter)) return ter; @@ -308,7 +308,7 @@ NFTokenMint::doApply() ownerCountAfter > ownerCountBefore) { if (auto const reserve = view().fees().accountReserve(ownerCountAfter); - mPriorBalance < reserve) + preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp index cd86a6906c..ceb90411a4 100644 --- a/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp +++ b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp @@ -147,7 +147,7 @@ DepositPreauth::doApply() STAmount const reserve{ view().fees().accountReserve(sleOwner->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } @@ -194,7 +194,7 @@ DepositPreauth::doApply() STAmount const reserve{ view().fees().accountReserve(sleOwner->getFieldU32(sfOwnerCount) + 1)}; - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 1605c7d886..6844d90aed 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -544,16 +544,16 @@ Payment::doApply() // This is the total reserve in drops. auto const reserve = view().fees().accountReserve(ownerCount); - // mPriorBalance is the balance on the sending account BEFORE the + // preFeeBalance_ is the balance on the sending account BEFORE the // fees were charged. We want to make sure we have enough reserve // to send. Allow final spend to use reserve for fee. auto const mmm = std::max(reserve, ctx_.tx.getFieldAmount(sfFee).xrp()); - if (mPriorBalance < dstAmount.xrp() + mmm) + if (preFeeBalance_ < dstAmount.xrp() + mmm) { // Vote no. However the transaction might succeed, if applied in // a different order. - JLOG(j_.trace()) << "Delay transaction: Insufficient funds: " << to_string(mPriorBalance) + JLOG(j_.trace()) << "Delay transaction: Insufficient funds: " << to_string(preFeeBalance_) << " / " << to_string(dstAmount.xrp() + mmm) << " (" << to_string(reserve) << ")"; @@ -601,7 +601,7 @@ Payment::doApply() } // Do the arithmetic for the transfer and make the ledger change. - sleSrc->setFieldAmount(sfBalance, mSourceBalance - dstAmount); + sleSrc->setFieldAmount(sfBalance, sleSrc->getFieldAmount(sfBalance) - dstAmount); sleDst->setFieldAmount(sfBalance, sleDst->getFieldAmount(sfBalance) + dstAmount); // Re-arm the password change fee if we can and need to. diff --git a/src/libxrpl/tx/transactors/system/CreateTicket.cpp b/src/libxrpl/tx/transactors/system/CreateTicket.cpp index 7334d88be4..b16cdeb7a2 100644 --- a/src/libxrpl/tx/transactors/system/CreateTicket.cpp +++ b/src/libxrpl/tx/transactors/system/CreateTicket.cpp @@ -65,7 +65,7 @@ CreateTicket::doApply() XRPAmount const reserve = view().fees().accountReserve(sleAccountRoot->getFieldU32(sfOwnerCount) + ticketCount); - if (mPriorBalance < reserve) + if (preFeeBalance_ < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp b/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp index 01513e17e0..208936a5e6 100644 --- a/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp +++ b/src/libxrpl/tx/transactors/token/MPTokenAuthorize.cpp @@ -161,7 +161,7 @@ MPTokenAuthorize::doApply() auto const& tx = ctx_.tx; return authorizeMPToken( ctx_.view(), - mPriorBalance, + preFeeBalance_, tx[sfMPTokenIssuanceID], account_, ctx_.journal, diff --git a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp index cf435a30ca..f9b9f9c01e 100644 --- a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp +++ b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp @@ -138,7 +138,7 @@ MPTokenIssuanceCreate::doApply() view(), j_, { - .priorBalance = mPriorBalance, + .priorBalance = preFeeBalance_, .account = account_, .sequence = tx.getSeqValue(), .flags = tx.getFlags(), diff --git a/src/libxrpl/tx/transactors/token/SetTrust.cpp b/src/libxrpl/tx/transactors/token/SetTrust.cpp index b6ae288a8c..f636827745 100644 --- a/src/libxrpl/tx/transactors/token/SetTrust.cpp +++ b/src/libxrpl/tx/transactors/token/SetTrust.cpp @@ -570,7 +570,7 @@ SetTrust::doApply() terResult = trustDelete(view(), sleRippleState, uLowAccountID, uHighAccountID, viewJ); } // Reserve is not scaled by load. - else if (bReserveIncrease && mPriorBalance < reserveCreate) + else if (bReserveIncrease && preFeeBalance_ < reserveCreate) { JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to " "add trust line."; @@ -598,8 +598,8 @@ SetTrust::doApply() JLOG(j_.trace()) << "Redundant: Setting non-existent ripple line to defaults."; return tecNO_LINE_REDUNDANT; } - else if (mPriorBalance < reserveCreate) // Reserve is not scaled by - // load. + else if (preFeeBalance_ < reserveCreate) // Reserve is not scaled by + // load. { JLOG(j_.trace()) << "Delay transaction: Line does not exist. " "Insufficent reserve to create line."; diff --git a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp index b1f227ddf3..4e9faeb56c 100644 --- a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp @@ -137,7 +137,7 @@ VaultCreate::doApply() // We will create Vault and PseudoAccount, hence increase OwnerCount by 2 adjustOwnerCount(view(), owner, 2, j_); auto const ownerCount = owner->at(sfOwnerCount); - if (mPriorBalance < view().fees().accountReserve(ownerCount)) + if (preFeeBalance_ < view().fees().accountReserve(ownerCount)) return tecINSUFFICIENT_RESERVE; auto maybePseudo = createPseudoAccount(view(), vault->key(), sfVaultID); @@ -147,7 +147,7 @@ VaultCreate::doApply() auto pseudoId = pseudo->at(sfAccount); auto asset = tx[sfAsset]; - if (auto ter = addEmptyHolding(view(), pseudoId, mPriorBalance, asset, j_); !isTesSuccess(ter)) + if (auto ter = addEmptyHolding(view(), pseudoId, preFeeBalance_, asset, j_); !isTesSuccess(ter)) return ter; std::uint8_t const scale = (asset.holds() || asset.native()) @@ -206,7 +206,7 @@ VaultCreate::doApply() // Explicitly create MPToken for the vault owner if (auto const err = - authorizeMPToken(view(), mPriorBalance, mptIssuanceID, account_, ctx_.journal); + authorizeMPToken(view(), preFeeBalance_, mptIssuanceID, account_, ctx_.journal); !isTesSuccess(err)) return err; @@ -214,7 +214,7 @@ VaultCreate::doApply() if (txFlags & tfVaultPrivate) { if (auto const err = authorizeMPToken( - view(), mPriorBalance, mptIssuanceID, pseudoId, ctx_.journal, {}, account_); + view(), preFeeBalance_, mptIssuanceID, pseudoId, ctx_.journal, {}, account_); !isTesSuccess(err)) return err; } diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index 58912f5010..fe3d8d313a 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -146,7 +146,7 @@ VaultDeposit::doApply() if (vault->isFlag(lsfVaultPrivate) && account_ != vault->at(sfOwner)) { if (auto const err = enforceMPTokenAuthorization( - ctx_.view(), mptIssuanceID, account_, mPriorBalance, j_); + ctx_.view(), mptIssuanceID, account_, preFeeBalance_, j_); !isTesSuccess(err)) return err; } @@ -156,7 +156,7 @@ VaultDeposit::doApply() if (!view().exists(keylet::mptoken(mptIssuanceID, account_))) { if (auto const err = authorizeMPToken( - view(), mPriorBalance, mptIssuanceID->value(), account_, ctx_.journal); + view(), preFeeBalance_, mptIssuanceID->value(), account_, ctx_.journal); !isTesSuccess(err)) return err; } @@ -169,7 +169,7 @@ VaultDeposit::doApply() account_ == vault->at(sfOwner), "xrpl::VaultDeposit::doApply : account is owner"); if (auto const err = authorizeMPToken( view(), - mPriorBalance, // priorBalance + preFeeBalance_, // priorBalance mptIssuanceID->value(), // mptIssuanceID sleIssuance->at(sfIssuer), // account ctx_.journal, diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 9f808c2dfc..725c54a3fd 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -230,7 +230,7 @@ VaultWithdraw::doApply() associateAsset(*vault, vaultAsset); return doWithdraw( - view(), ctx_.tx, account_, dstAcct, vaultAccount, mPriorBalance, assetsWithdrawn, j_); + view(), ctx_.tx, account_, dstAcct, vaultAccount, preFeeBalance_, assetsWithdrawn, j_); } } // namespace xrpl From 78b2d70a1198ae6a366cea8979be82fbe977bb90 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 17 Mar 2026 10:44:07 -0400 Subject: [PATCH 4/4] refactor: Assorted small DID fixes (#6552) This change: * Makes `addSLE` in `DIDSet` a static function, instead of a free function. * Renames `Attestation` to `Data` everywhere (an artifact of a previous name for the field). * Actually runs a set of tests that were not included in the `run` function of `DID_test`. --- include/xrpl/protocol/Protocol.h | 2 +- src/libxrpl/tx/transactors/did/DIDDelete.cpp | 2 +- src/libxrpl/tx/transactors/did/DIDSet.cpp | 4 ++-- src/test/app/DID_test.cpp | 4 +++- src/test/jtx/did.h | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 7ddf43d175..0db7b217f0 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -209,7 +209,7 @@ std::size_t constexpr maxDIDDocumentLength = 256; std::size_t constexpr maxDIDURILength = 256; /** The maximum length of an Attestation inside a DID */ -std::size_t constexpr maxDIDAttestationLength = 256; +std::size_t constexpr maxDIDDataLength = 256; /** The maximum length of a domain */ std::size_t constexpr maxDomainLength = 256; diff --git a/src/libxrpl/tx/transactors/did/DIDDelete.cpp b/src/libxrpl/tx/transactors/did/DIDDelete.cpp index 02157d77d2..e09fdd3510 100644 --- a/src/libxrpl/tx/transactors/did/DIDDelete.cpp +++ b/src/libxrpl/tx/transactors/did/DIDDelete.cpp @@ -33,7 +33,7 @@ DIDDelete::deleteSLE( if (!view.dirRemove(keylet::ownerDir(owner), (*sle)[sfOwnerNode], sle->key(), true)) { // LCOV_EXCL_START - JLOG(j.fatal()) << "Unable to delete DID Token from owner."; + JLOG(j.fatal()) << "Unable to delete DID from owner."; return tefBAD_LEDGER; // LCOV_EXCL_STOP } diff --git a/src/libxrpl/tx/transactors/did/DIDSet.cpp b/src/libxrpl/tx/transactors/did/DIDSet.cpp index f094e96bc8..32edbce82e 100644 --- a/src/libxrpl/tx/transactors/did/DIDSet.cpp +++ b/src/libxrpl/tx/transactors/did/DIDSet.cpp @@ -41,13 +41,13 @@ DIDSet::preflight(PreflightContext const& ctx) }; if (isTooLong(sfURI, maxDIDURILength) || isTooLong(sfDIDDocument, maxDIDDocumentLength) || - isTooLong(sfData, maxDIDAttestationLength)) + isTooLong(sfData, maxDIDDataLength)) return temMALFORMED; return tesSUCCESS; } -TER +static TER addSLE(ApplyContext& ctx, std::shared_ptr const& sle, AccountID const& owner) { auto const sleAccount = ctx.view().peek(keylet::account(owner)); diff --git a/src/test/app/DID_test.cpp b/src/test/app/DID_test.cpp index f83571deab..20c367d64f 100644 --- a/src/test/app/DID_test.cpp +++ b/src/test/app/DID_test.cpp @@ -189,7 +189,7 @@ struct DID_test : public beast::unit_test::suite Account const edna{"edna"}; Account const francis{"francis"}; Account const george{"george"}; - env.fund(XRP(5000), alice, bob, charlie, dave, edna, francis); + env.fund(XRP(5000), alice, bob, charlie, dave, edna, francis, george); env.close(); BEAST_EXPECT(ownerCount(env, alice) == 0); BEAST_EXPECT(ownerCount(env, bob) == 0); @@ -355,12 +355,14 @@ struct DID_test : public beast::unit_test::suite testAccountReserve(all); testSetInvalid(all); testDeleteInvalid(all); + testSetValidInitial(all); testSetModify(all); testEnabled(all - emptyDID); testAccountReserve(all - emptyDID); testSetInvalid(all - emptyDID); testDeleteInvalid(all - emptyDID); + testSetValidInitial(all - emptyDID); testSetModify(all - emptyDID); } }; diff --git a/src/test/jtx/did.h b/src/test/jtx/did.h index 9cd98a1324..e6b06f8f7c 100644 --- a/src/test/jtx/did.h +++ b/src/test/jtx/did.h @@ -53,7 +53,7 @@ public: } }; -/** Sets the optional Attestation on a DIDSet. */ +/** Sets the optional Data on a DIDSet. */ class data { private: