From 2df730438db77df3c9d40a0e50ce78f62f532568 Mon Sep 17 00:00:00 2001 From: Bart Date: Tue, 7 Oct 2025 16:28:19 -0400 Subject: [PATCH 01/18] Set version to 3.0.0-b1 (#5859) --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index b54c74c80d..0cee2995db 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -36,7 +36,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "2.6.1" +char const* const versionString = "3.0.0-b1" // clang-format on #if defined(DEBUG) || defined(SANITIZER) From 176fd2b6e4486198b463d9f9b87375d069b9b7cc Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 8 Oct 2025 04:25:51 -0400 Subject: [PATCH 02/18] chore: exclude all `UNREACHABLE` blocks from codecov (#5846) --- include/xrpl/basics/IntrusivePointer.ipp | 2 ++ include/xrpl/beast/net/IPAddress.h | 4 ++++ include/xrpl/ledger/ApplyView.h | 2 ++ include/xrpl/protocol/detail/b58_utils.h | 2 ++ include/xrpl/resource/detail/Logic.h | 2 ++ src/libxrpl/basics/Log.cpp | 8 +++++++ src/libxrpl/basics/contract.cpp | 2 ++ src/libxrpl/json/Object.cpp | 4 ++-- src/libxrpl/json/json_value.cpp | 24 +++++++++++++++++++ src/libxrpl/ledger/ApplyStateTable.cpp | 2 ++ src/libxrpl/ledger/ApplyView.cpp | 2 ++ src/libxrpl/ledger/BookDirs.cpp | 4 ++++ src/libxrpl/ledger/View.cpp | 12 ++++++---- src/libxrpl/protocol/STBase.cpp | 2 ++ src/libxrpl/protocol/TxMeta.cpp | 2 ++ src/xrpld/app/ledger/Ledger.cpp | 4 ++++ src/xrpld/app/ledger/detail/InboundLedger.cpp | 4 ++++ src/xrpld/app/ledger/detail/LedgerMaster.cpp | 2 ++ src/xrpld/app/main/Application.cpp | 8 +++++++ src/xrpld/app/misc/NetworkOPs.cpp | 10 ++++++++ src/xrpld/app/misc/detail/ValidatorList.cpp | 14 ++++++----- src/xrpld/app/paths/Pathfinder.cpp | 2 ++ src/xrpld/app/paths/detail/BookStep.cpp | 6 +++++ src/xrpld/app/paths/detail/DirectStep.cpp | 2 ++ src/xrpld/app/paths/detail/FlowDebugInfo.h | 2 ++ src/xrpld/app/paths/detail/PaySteps.cpp | 6 +++++ src/xrpld/app/paths/detail/StrandFlow.h | 8 +++++++ src/xrpld/app/rdb/RelationalDatabase.h | 4 +++- src/xrpld/app/rdb/backend/detail/Node.cpp | 6 +++++ src/xrpld/app/tx/detail/Change.cpp | 2 ++ src/xrpld/app/tx/detail/DeleteAccount.cpp | 2 ++ src/xrpld/app/tx/detail/Offer.h | 2 ++ src/xrpld/app/tx/detail/OfferStream.cpp | 2 ++ src/xrpld/app/tx/detail/SetSignerList.cpp | 2 ++ src/xrpld/app/tx/detail/Transactor.cpp | 2 ++ src/xrpld/app/tx/detail/XChainBridge.cpp | 2 ++ src/xrpld/app/tx/detail/applySteps.cpp | 8 +++++++ src/xrpld/nodestore/backend/NuDBFactory.cpp | 2 ++ .../nodestore/backend/RocksDBFactory.cpp | 2 ++ src/xrpld/overlay/Compression.h | 4 ++++ src/xrpld/overlay/detail/PeerImp.cpp | 6 +++++ src/xrpld/peerfinder/detail/Counts.h | 2 ++ src/xrpld/peerfinder/detail/Logic.h | 2 ++ src/xrpld/perflog/detail/PerfLogImp.cpp | 18 ++++++++++++++ src/xrpld/rpc/detail/Handler.cpp | 2 ++ src/xrpld/rpc/detail/Status.cpp | 2 ++ src/xrpld/rpc/handlers/AccountChannels.cpp | 2 ++ src/xrpld/rpc/handlers/AccountLines.cpp | 2 ++ src/xrpld/rpc/handlers/AccountOffers.cpp | 2 ++ src/xrpld/rpc/handlers/AccountTx.cpp | 4 ++++ src/xrpld/rpc/handlers/Fee1.cpp | 3 +++ src/xrpld/shamap/detail/SHAMap.cpp | 4 ++++ src/xrpld/shamap/detail/SHAMapDelta.cpp | 6 +++++ 53 files changed, 224 insertions(+), 13 deletions(-) diff --git a/include/xrpl/basics/IntrusivePointer.ipp b/include/xrpl/basics/IntrusivePointer.ipp index 1ac3f2bab4..4d037bc329 100644 --- a/include/xrpl/basics/IntrusivePointer.ipp +++ b/include/xrpl/basics/IntrusivePointer.ipp @@ -654,12 +654,14 @@ SharedWeakUnion::convertToWeak() break; case destroy: // We just added a weak ref. How could we destroy? + // LCOV_EXCL_START UNREACHABLE( "ripple::SharedWeakUnion::convertToWeak : destroying freshly " "added ref"); delete p; unsafeSetRawPtr(nullptr); return true; // Should never happen + // LCOV_EXCL_STOP case partialDestroy: // This is a weird case. We just converted the last strong // pointer to a weak pointer. diff --git a/include/xrpl/beast/net/IPAddress.h b/include/xrpl/beast/net/IPAddress.h index fb5dac90ec..f3c7387bb8 100644 --- a/include/xrpl/beast/net/IPAddress.h +++ b/include/xrpl/beast/net/IPAddress.h @@ -94,7 +94,11 @@ hash_append(Hasher& h, beast::IP::Address const& addr) noexcept else if (addr.is_v6()) hash_append(h, addr.to_v6().to_bytes()); else + { + // LCOV_EXCL_START UNREACHABLE("beast::hash_append : invalid address type"); + // LCOV_EXCL_STOP + } } } // namespace beast diff --git a/include/xrpl/ledger/ApplyView.h b/include/xrpl/ledger/ApplyView.h index d8b9028d7c..f90033966a 100644 --- a/include/xrpl/ledger/ApplyView.h +++ b/include/xrpl/ledger/ApplyView.h @@ -284,12 +284,14 @@ public: { if (key.type != ltOFFER) { + // LCOV_EXCL_START UNREACHABLE( "ripple::ApplyView::dirAppend : only Offers are appended to " "book directories"); // Only Offers are appended to book directories. Call dirInsert() // instead return std::nullopt; + // LCOV_EXCL_STOP } return dirAdd(true, directory, key.key, describe); } diff --git a/include/xrpl/protocol/detail/b58_utils.h b/include/xrpl/protocol/detail/b58_utils.h index ecd301524f..3908822661 100644 --- a/include/xrpl/protocol/detail/b58_utils.h +++ b/include/xrpl/protocol/detail/b58_utils.h @@ -129,10 +129,12 @@ inplace_bigint_div_rem(std::span numerator, std::uint64_t divisor) { // should never happen, but if it does then it seems natural to define // the a null set of numbers to be zero, so the remainder is also zero. + // LCOV_EXCL_START UNREACHABLE( "ripple::b58_fast::detail::inplace_bigint_div_rem : empty " "numerator"); return 0; + // LCOV_EXCL_STOP } auto to_u128 = [](std::uint64_t high, diff --git a/include/xrpl/resource/detail/Logic.h b/include/xrpl/resource/detail/Logic.h index b07ee00e73..0fc5a9035a 100644 --- a/include/xrpl/resource/detail/Logic.h +++ b/include/xrpl/resource/detail/Logic.h @@ -436,10 +436,12 @@ public: admin_.erase(admin_.iterator_to(entry)); break; default: + // LCOV_EXCL_START UNREACHABLE( "ripple::Resource::Logic::release : invalid entry " "kind"); break; + // LCOV_EXCL_STOP } inactive_.push_back(entry); entry.whenExpires = m_clock.now() + secondsUntilExpiration; diff --git a/src/libxrpl/basics/Log.cpp b/src/libxrpl/basics/Log.cpp index 14873a3fd7..95419dda20 100644 --- a/src/libxrpl/basics/Log.cpp +++ b/src/libxrpl/basics/Log.cpp @@ -239,9 +239,11 @@ Logs::fromSeverity(beast::severities::Severity level) case kError: return lsERROR; + // LCOV_EXCL_START default: UNREACHABLE("ripple::Logs::fromSeverity : invalid severity"); [[fallthrough]]; + // LCOV_EXCL_STOP case kFatal: break; } @@ -265,9 +267,11 @@ Logs::toSeverity(LogSeverity level) return kWarning; case lsERROR: return kError; + // LCOV_EXCL_START default: UNREACHABLE("ripple::Logs::toSeverity : invalid severity"); [[fallthrough]]; + // LCOV_EXCL_STOP case lsFATAL: break; } @@ -292,9 +296,11 @@ Logs::toString(LogSeverity s) return "Error"; case lsFATAL: return "Fatal"; + // LCOV_EXCL_START default: UNREACHABLE("ripple::Logs::toString : invalid severity"); return "Unknown"; + // LCOV_EXCL_STOP } } @@ -356,9 +362,11 @@ Logs::format( case kError: output += "ERR "; break; + // LCOV_EXCL_START default: UNREACHABLE("ripple::Logs::format : invalid severity"); [[fallthrough]]; + // LCOV_EXCL_STOP case kFatal: output += "FTL "; break; diff --git a/src/libxrpl/basics/contract.cpp b/src/libxrpl/basics/contract.cpp index b5a7b3f368..ea75929be0 100644 --- a/src/libxrpl/basics/contract.cpp +++ b/src/libxrpl/basics/contract.cpp @@ -36,6 +36,7 @@ LogThrow(std::string const& title) [[noreturn]] void LogicError(std::string const& s) noexcept { + // LCOV_EXCL_START JLOG(debugLog().fatal()) << s; std::cerr << "Logic error: " << s << std::endl; // Use a non-standard contract naming here (without namespace) because @@ -45,6 +46,7 @@ LogicError(std::string const& s) noexcept // For the above reasons, we want this contract to stand out. UNREACHABLE("LogicError", {{"message", s}}); std::abort(); + // LCOV_EXCL_STOP } } // namespace ripple diff --git a/src/libxrpl/json/Object.cpp b/src/libxrpl/json/Object.cpp index 62f686e228..55e573c0db 100644 --- a/src/libxrpl/json/Object.cpp +++ b/src/libxrpl/json/Object.cpp @@ -174,7 +174,7 @@ Array::append(Json::Value const& v) return; } } - UNREACHABLE("Json::Array::append : invalid type"); + UNREACHABLE("Json::Array::append : invalid type"); // LCOV_EXCL_LINE } void @@ -209,7 +209,7 @@ Object::set(std::string const& k, Json::Value const& v) return; } } - UNREACHABLE("Json::Object::set : invalid type"); + UNREACHABLE("Json::Object::set : invalid type"); // LCOV_EXCL_LINE } //------------------------------------------------------------------------------ diff --git a/src/libxrpl/json/json_value.cpp b/src/libxrpl/json/json_value.cpp index 1df8f6cf31..7e4d8b6d81 100644 --- a/src/libxrpl/json/json_value.cpp +++ b/src/libxrpl/json/json_value.cpp @@ -213,8 +213,10 @@ Value::Value(ValueType type) : type_(type), allocated_(0) value_.bool_ = false; break; + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::Value(ValueType) : invalid type"); + // LCOV_EXCL_STOP } } @@ -290,8 +292,10 @@ Value::Value(Value const& other) : type_(other.type_) value_.map_ = new ObjectValues(*other.value_.map_); break; + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::Value(Value const&) : invalid type"); + // LCOV_EXCL_STOP } } @@ -318,8 +322,10 @@ Value::~Value() delete value_.map_; break; + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::~Value : invalid type"); + // LCOV_EXCL_STOP } } @@ -419,8 +425,10 @@ operator<(Value const& x, Value const& y) return *x.value_.map_ < *y.value_.map_; } + // LCOV_EXCL_START default: UNREACHABLE("Json::operator<(Value, Value) : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable @@ -465,8 +473,10 @@ operator==(Value const& x, Value const& y) return x.value_.map_->size() == y.value_.map_->size() && *x.value_.map_ == *y.value_.map_; + // LCOV_EXCL_START default: UNREACHABLE("Json::operator==(Value, Value) : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable @@ -506,8 +516,10 @@ Value::asString() const case objectValue: JSON_ASSERT_MESSAGE(false, "Type is not convertible to string"); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::asString : invalid type"); + // LCOV_EXCL_STOP } return ""; // unreachable @@ -548,8 +560,10 @@ Value::asInt() const case objectValue: JSON_ASSERT_MESSAGE(false, "Type is not convertible to int"); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::asInt : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable; @@ -590,8 +604,10 @@ Value::asUInt() const case objectValue: JSON_ASSERT_MESSAGE(false, "Type is not convertible to uint"); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::asUInt : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable; @@ -622,8 +638,10 @@ Value::asDouble() const case objectValue: JSON_ASSERT_MESSAGE(false, "Type is not convertible to double"); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::asDouble : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable; @@ -654,8 +672,10 @@ Value::asBool() const case objectValue: return value_.map_->size() != 0; + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::asBool : invalid type"); + // LCOV_EXCL_STOP } return false; // unreachable; @@ -710,8 +730,10 @@ Value::isConvertibleTo(ValueType other) const return other == objectValue || (other == nullValue && value_.map_->size() == 0); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::isConvertible : invalid type"); + // LCOV_EXCL_STOP } return false; // unreachable; @@ -744,8 +766,10 @@ Value::size() const case objectValue: return Int(value_.map_->size()); + // LCOV_EXCL_START default: UNREACHABLE("Json::Value::size : invalid type"); + // LCOV_EXCL_STOP } return 0; // unreachable; diff --git a/src/libxrpl/ledger/ApplyStateTable.cpp b/src/libxrpl/ledger/ApplyStateTable.cpp index 7b041939d4..aaad056c58 100644 --- a/src/libxrpl/ledger/ApplyStateTable.cpp +++ b/src/libxrpl/ledger/ApplyStateTable.cpp @@ -259,9 +259,11 @@ ApplyStateTable::apply( } else { + // LCOV_EXCL_START UNREACHABLE( "ripple::detail::ApplyStateTable::apply : unsupported " "operation type"); + // LCOV_EXCL_STOP } } diff --git a/src/libxrpl/ledger/ApplyView.cpp b/src/libxrpl/ledger/ApplyView.cpp index 8a0fd51bd8..b2a24f4582 100644 --- a/src/libxrpl/ledger/ApplyView.cpp +++ b/src/libxrpl/ledger/ApplyView.cpp @@ -133,8 +133,10 @@ ApplyView::emptyDirDelete(Keylet const& directory) if (directory.type != ltDIR_NODE || node->getFieldH256(sfRootIndex) != directory.key) { + // LCOV_EXCL_START UNREACHABLE("ripple::ApplyView::emptyDirDelete : invalid node type"); return false; + // LCOV_EXCL_STOP } // The directory still contains entries and so it cannot be removed diff --git a/src/libxrpl/ledger/BookDirs.cpp b/src/libxrpl/ledger/BookDirs.cpp index f777d23aca..61ec160b1b 100644 --- a/src/libxrpl/ledger/BookDirs.cpp +++ b/src/libxrpl/ledger/BookDirs.cpp @@ -36,7 +36,9 @@ BookDirs::BookDirs(ReadView const& view, Book const& book) { if (!cdirFirst(*view_, key_, sle_, entry_, index_)) { + // LCOV_EXCL_START UNREACHABLE("ripple::BookDirs::BookDirs : directory is empty"); + // LCOV_EXCL_STOP } } } @@ -110,9 +112,11 @@ BookDirs::const_iterator::operator++() } else if (!cdirFirst(*view_, cur_key_, sle_, entry_, index_)) { + // LCOV_EXCL_START UNREACHABLE( "ripple::BookDirs::const_iterator::operator++ : directory is " "empty"); + // LCOV_EXCL_STOP } } diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 10b7e81b4f..02de872ad3 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -324,10 +324,12 @@ isVaultPseudoAccountFrozen( auto const issuer = mptIssuance->getAccountID(sfIssuer); auto const mptIssuer = view.read(keylet::account(issuer)); if (mptIssuer == nullptr) - { // LCOV_EXCL_START + { + // LCOV_EXCL_START UNREACHABLE("ripple::isVaultPseudoAccountFrozen : null MPToken issuer"); return false; - } // LCOV_EXCL_STOP + // LCOV_EXCL_STOP + } if (!mptIssuer->isFieldPresent(sfVaultID)) return false; // not a Vault pseudo-account, common case @@ -338,7 +340,8 @@ isVaultPseudoAccountFrozen( { // LCOV_EXCL_START UNREACHABLE("ripple::isVaultPseudoAccountFrozen : null vault"); return false; - } // LCOV_EXCL_STOP + // LCOV_EXCL_STOP + } return isAnyFrozen(view, {issuer, account}, vault->at(sfAsset), depth + 1); } @@ -2676,7 +2679,8 @@ enforceMPTokenAuthorization( UNREACHABLE( "ripple::enforceMPTokenAuthorization : condition list is incomplete"); return tefINTERNAL; -} // LCOV_EXCL_STOP + // LCOV_EXCL_STOP +} TER canTransfer( diff --git a/src/libxrpl/protocol/STBase.cpp b/src/libxrpl/protocol/STBase.cpp index 417b7e2302..ecbe6833d0 100644 --- a/src/libxrpl/protocol/STBase.cpp +++ b/src/libxrpl/protocol/STBase.cpp @@ -112,7 +112,9 @@ void STBase::add(Serializer& s) const { // Should never be called + // LCOV_EXCL_START UNREACHABLE("ripple::STBase::add : not implemented"); + // LCOV_EXCL_STOP } bool diff --git a/src/libxrpl/protocol/TxMeta.cpp b/src/libxrpl/protocol/TxMeta.cpp index 2343a6a794..833a0677b9 100644 --- a/src/libxrpl/protocol/TxMeta.cpp +++ b/src/libxrpl/protocol/TxMeta.cpp @@ -238,9 +238,11 @@ TxMeta::getAffectedNode(uint256 const& node) if (n.getFieldH256(sfLedgerIndex) == node) return n; } + // LCOV_EXCL_START UNREACHABLE("ripple::TxMeta::getAffectedNode(uint256) : node not found"); Throw("Affected node not found"); return *(mNodes.begin()); // Silence compiler warning. + // LCOV_EXCL_STOP } STObject diff --git a/src/xrpld/app/ledger/Ledger.cpp b/src/xrpld/app/ledger/Ledger.cpp index 6de4f2cbde..64a6b16cbc 100644 --- a/src/xrpld/app/ledger/Ledger.cpp +++ b/src/xrpld/app/ledger/Ledger.cpp @@ -433,8 +433,10 @@ Ledger::read(Keylet const& k) const { if (k.key == beast::zero) { + // LCOV_EXCL_START UNREACHABLE("ripple::Ledger::read : zero key"); return nullptr; + // LCOV_EXCL_STOP } auto const& item = stateMap_.peekItem(k.key); if (!item) @@ -860,6 +862,7 @@ Ledger::assertSensible(beast::Journal ledgerJ) const return true; } + // LCOV_EXCL_START Json::Value j = getJson({*this, {}}); j[jss::accountTreeHash] = to_string(info_.accountHash); @@ -870,6 +873,7 @@ Ledger::assertSensible(beast::Journal ledgerJ) const UNREACHABLE("ripple::Ledger::assertSensible : ledger is not sensible"); return false; + // LCOV_EXCL_STOP } // update the skip list with the information from our previous ledger diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index eafa939506..47d546c3af 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -963,8 +963,10 @@ InboundLedger::takeAsRootNode(Slice const& data, SHAMapAddNode& san) if (!mHaveHeader) { + // LCOV_EXCL_START UNREACHABLE("ripple::InboundLedger::takeAsRootNode : no ledger header"); return false; + // LCOV_EXCL_STOP } AccountStateSF filter( @@ -988,8 +990,10 @@ InboundLedger::takeTxRootNode(Slice const& data, SHAMapAddNode& san) if (!mHaveHeader) { + // LCOV_EXCL_START UNREACHABLE("ripple::InboundLedger::takeTxRootNode : no ledger header"); return false; + // LCOV_EXCL_STOP } TransactionStateSF filter( diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 78f0375b16..f39e8af0b6 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -1273,11 +1273,13 @@ LedgerMaster::findNewLedgersToPublish( } else if (hash->isZero()) { + // LCOV_EXCL_START JLOG(m_journal.fatal()) << "Ledger: " << valSeq << " does not have hash for " << seq; UNREACHABLE( "ripple::LedgerMaster::findNewLedgersToPublish : ledger " "not found"); + // LCOV_EXCL_STOP } else { diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 05b8f5e5fa..616afc957d 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -1994,11 +1994,13 @@ ApplicationImp::loadOldLedger( if (!loadLedger) { + // LCOV_EXCL_START JLOG(m_journal.fatal()) << "Replay ledger missing/damaged"; UNREACHABLE( "ripple::ApplicationImp::loadOldLedger : replay ledger " "missing/damaged"); return false; + // LCOV_EXCL_STOP } } } @@ -2025,28 +2027,34 @@ ApplicationImp::loadOldLedger( if (loadLedger->info().accountHash.isZero()) { + // LCOV_EXCL_START JLOG(m_journal.fatal()) << "Ledger is empty."; UNREACHABLE( "ripple::ApplicationImp::loadOldLedger : ledger is empty"); return false; + // LCOV_EXCL_STOP } if (!loadLedger->walkLedger(journal("Ledger"), true)) { + // LCOV_EXCL_START JLOG(m_journal.fatal()) << "Ledger is missing nodes."; UNREACHABLE( "ripple::ApplicationImp::loadOldLedger : ledger is missing " "nodes"); return false; + // LCOV_EXCL_STOP } if (!loadLedger->assertSensible(journal("Ledger"))) { + // LCOV_EXCL_START JLOG(m_journal.fatal()) << "Ledger is not sensible."; UNREACHABLE( "ripple::ApplicationImp::loadOldLedger : ledger is not " "sensible"); return false; + // LCOV_EXCL_STOP } m_ledgerMaster->setLedgerRangePresent( diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index b9069442f8..e72b2732d0 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1798,11 +1798,13 @@ NetworkOPsImp::getOwnerInfo( case ltACCOUNT_ROOT: case ltDIR_NODE: + // LCOV_EXCL_START default: UNREACHABLE( "ripple::NetworkOPsImp::getOwnerInfo : invalid " "type"); break; + // LCOV_EXCL_STOP } } @@ -3831,12 +3833,14 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) accountId, minLedger, maxLedger, marker, 0, true}; return db->newestAccountTxPage(options); } + // LCOV_EXCL_START default: { UNREACHABLE( "ripple::NetworkOPsImp::addAccountHistoryJob::" "getMoreTxns : invalid database type"); return {}; } + // LCOV_EXCL_STOP } }; @@ -4030,10 +4034,12 @@ NetworkOPsImp::subAccountHistoryStart( } else { + // LCOV_EXCL_START UNREACHABLE( "ripple::NetworkOPsImp::subAccountHistoryStart : failed to " "access genesis account"); return; + // LCOV_EXCL_STOP } } subInfo.index_->historyLastLedgerSeq_ = ledger->seq(); @@ -4140,7 +4146,11 @@ NetworkOPsImp::subBook(InfoSub::ref isrListener, Book const& book) if (auto listeners = app_.getOrderBookDB().makeBookListeners(book)) listeners->addSubscriber(isrListener); else + { + // LCOV_EXCL_START UNREACHABLE("ripple::NetworkOPsImp::subBook : null book listeners"); + // LCOV_EXCL_STOP + } return true; } diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp index 2b45cec3be..92095b7211 100644 --- a/src/xrpld/app/misc/detail/ValidatorList.cpp +++ b/src/xrpld/app/misc/detail/ValidatorList.cpp @@ -1167,15 +1167,17 @@ ValidatorList::applyList( } if (!publicKeyType(*pubKeyOpt)) - { // LCOV_EXCL_START - // This is an impossible situation because we will never load an - // invalid public key type (see checks in `ValidatorList::load`) however - // we can only arrive here if the key used by the manifest matched one of - // the loaded keys + { + // This is an impossible situation because we will never load an + // invalid public key type (see checks in `ValidatorList::load`) however + // we can only arrive here if the key used by the manifest matched one + // of the loaded keys + // LCOV_EXCL_START UNREACHABLE( "ripple::ValidatorList::applyList : invalid public key type"); return PublisherListStats{result}; - } // LCOV_EXCL_STOP + // LCOV_EXCL_STOP + } PublicKey pubKey = *pubKeyOpt; if (result > ListDisposition::pending) diff --git a/src/xrpld/app/paths/Pathfinder.cpp b/src/xrpld/app/paths/Pathfinder.cpp index 41a3697888..4bc9304853 100644 --- a/src/xrpld/app/paths/Pathfinder.cpp +++ b/src/xrpld/app/paths/Pathfinder.cpp @@ -648,8 +648,10 @@ Pathfinder::getBestPaths( if (path.empty()) { + // LCOV_EXCL_START UNREACHABLE("ripple::Pathfinder::getBestPaths : path not found"); continue; + // LCOV_EXCL_STOP } bool startsWithIssuer = false; diff --git a/src/xrpld/app/paths/detail/BookStep.cpp b/src/xrpld/app/paths/detail/BookStep.cpp index 97cf87c046..54d0d8d0c9 100644 --- a/src/xrpld/app/paths/detail/BookStep.cpp +++ b/src/xrpld/app/paths/detail/BookStep.cpp @@ -1113,11 +1113,13 @@ BookStep::revImp( { case -1: { // something went very wrong + // LCOV_EXCL_START JLOG(j_.error()) << "BookStep remainingOut < 0 " << to_string(remainingOut); UNREACHABLE("ripple::BookStep::revImp : remaining less than zero"); cache_.emplace(beast::zero, beast::zero); return {beast::zero, beast::zero}; + // LCOV_EXCL_STOP } case 0: { // due to normalization, remainingOut can be zero without @@ -1283,12 +1285,14 @@ BookStep::fwdImp( switch (remainingIn.signum()) { case -1: { + // LCOV_EXCL_START // something went very wrong JLOG(j_.error()) << "BookStep remainingIn < 0 " << to_string(remainingIn); UNREACHABLE("ripple::BookStep::fwdImp : remaining less than zero"); cache_.emplace(beast::zero, beast::zero); return {beast::zero, beast::zero}; + // LCOV_EXCL_STOP } case 0: { // due to normalization, remainingIn can be zero without @@ -1421,8 +1425,10 @@ bookStepEqual(Step const& step, ripple::Book const& book) bool const outXRP = isXRP(book.out.currency); if (inXRP && outXRP) { + // LCOV_EXCL_START UNREACHABLE("ripple::test::bookStepEqual : no XRP to XRP book step"); return false; // no such thing as xrp/xrp book step + // LCOV_EXCL_STOP } if (inXRP && !outXRP) return equalHelper< diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp index 03d207e008..a0808985b5 100644 --- a/src/xrpld/app/paths/detail/DirectStep.cpp +++ b/src/xrpld/app/paths/detail/DirectStep.cpp @@ -931,10 +931,12 @@ DirectStepI::check(StrandContext const& ctx) const { if (!ctx.prevStep) { + // LCOV_EXCL_START UNREACHABLE( "ripple::DirectStepI::check : prev seen book without a " "prev step"); return temBAD_PATH_LOOP; + // LCOV_EXCL_STOP } // This is OK if the previous step is a book step that outputs this diff --git a/src/xrpld/app/paths/detail/FlowDebugInfo.h b/src/xrpld/app/paths/detail/FlowDebugInfo.h index eec1d7c5a6..290a201e68 100644 --- a/src/xrpld/app/paths/detail/FlowDebugInfo.h +++ b/src/xrpld/app/paths/detail/FlowDebugInfo.h @@ -126,10 +126,12 @@ struct FlowDebugInfo auto i = timePoints.find(tag); if (i == timePoints.end()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::path::detail::FlowDebugInfo::duration : timepoint not " "found"); return std::chrono::duration(0); + // LCOV_EXCL_STOP } auto const& t = i->second; return std::chrono::duration_cast>( diff --git a/src/xrpld/app/paths/detail/PaySteps.cpp b/src/xrpld/app/paths/detail/PaySteps.cpp index 6eb38eee83..f5621bcdf7 100644 --- a/src/xrpld/app/paths/detail/PaySteps.cpp +++ b/src/xrpld/app/paths/detail/PaySteps.cpp @@ -95,11 +95,13 @@ toStep( if (e1->isOffer() && e2->isAccount()) { + // LCOV_EXCL_START // should already be taken care of JLOG(j.error()) << "Found offer/account payment step. Aborting payment strand."; UNREACHABLE("ripple::toStep : offer/account payment payment strand"); return {temBAD_PATH, std::unique_ptr{}}; + // LCOV_EXCL_STOP } XRPL_ASSERT( @@ -392,8 +394,10 @@ toStrand( next->getCurrency() != curIssue.currency) { // Should never happen + // LCOV_EXCL_START UNREACHABLE("ripple::toStrand : offer currency mismatch"); return {temBAD_PATH, Strand{}}; + // LCOV_EXCL_STOP } auto s = toStep( @@ -457,9 +461,11 @@ toStrand( if (!checkStrand()) { + // LCOV_EXCL_START JLOG(j.warn()) << "Flow check strand failed"; UNREACHABLE("ripple::toStrand : invalid strand"); return {temBAD_PATH, Strand{}}; + // LCOV_EXCL_STOP } return {tesSUCCESS, std::move(result)}; diff --git a/src/xrpld/app/paths/detail/StrandFlow.h b/src/xrpld/app/paths/detail/StrandFlow.h index 47037c5f5e..7ccad9bf4f 100644 --- a/src/xrpld/app/paths/detail/StrandFlow.h +++ b/src/xrpld/app/paths/detail/StrandFlow.h @@ -167,6 +167,7 @@ flow( // Something is very wrong // throwing out the sandbox can only increase liquidity // yet the limiting is still limiting + // LCOV_EXCL_START JLOG(j.fatal()) << "Re-executed limiting step failed. r.first: " << to_string(get(r.first)) @@ -175,6 +176,7 @@ flow( "ripple::flow : first step re-executing the " "limiting step failed"); return Result{strand, std::move(ofrsToRm)}; + // LCOV_EXCL_STOP } } else if (!strand[i]->equalOut(r.second, stepOut)) @@ -202,6 +204,7 @@ flow( // Something is very wrong // throwing out the sandbox can only increase liquidity // yet the limiting is still limiting + // LCOV_EXCL_START #ifndef NDEBUG JLOG(j.fatal()) << "Re-executed limiting step failed. r.second: " @@ -213,6 +216,7 @@ flow( "ripple::flow : limiting step re-executing the " "limiting step failed"); return Result{strand, std::move(ofrsToRm)}; + // LCOV_EXCL_STOP } } @@ -238,6 +242,7 @@ flow( // The limits should already have been found, so executing a // strand forward from the limiting step should not find a // new limit + // LCOV_EXCL_START #ifndef NDEBUG JLOG(j.fatal()) << "Re-executed forward pass failed. r.first: " @@ -249,6 +254,7 @@ flow( "ripple::flow : non-limiting step re-executing the " "forward pass failed"); return Result{strand, std::move(ofrsToRm)}; + // LCOV_EXCL_STOP } stepIn = r.second; } @@ -499,8 +505,10 @@ public: { if (i >= cur_.size()) { + // LCOV_EXCL_START UNREACHABLE("ripple::ActiveStrands::get : input out of range"); return nullptr; + // LCOV_EXCL_STOP } return cur_[i]; } diff --git a/src/xrpld/app/rdb/RelationalDatabase.h b/src/xrpld/app/rdb/RelationalDatabase.h index 25b16f04a1..1a5b2ba830 100644 --- a/src/xrpld/app/rdb/RelationalDatabase.h +++ b/src/xrpld/app/rdb/RelationalDatabase.h @@ -235,12 +235,14 @@ rangeCheckedCast(C c) std::numeric_limits::is_signed && c < std::numeric_limits::lowest())) { - /* This should never happen */ + // This should never happen + // LCOV_EXCL_START UNREACHABLE("ripple::rangeCheckedCast : domain error"); JLOG(debugLog().error()) << "rangeCheckedCast domain error:" << " value = " << c << " min = " << std::numeric_limits::lowest() << " max: " << std::numeric_limits::max(); + // LCOV_EXCL_STOP } return static_cast(c); diff --git a/src/xrpld/app/rdb/backend/detail/Node.cpp b/src/xrpld/app/rdb/backend/detail/Node.cpp index 6a0544091b..88f11a272b 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.cpp +++ b/src/xrpld/app/rdb/backend/detail/Node.cpp @@ -58,9 +58,11 @@ to_string(TableType type) return "Transactions"; case TableType::AccountTransactions: return "AccountTransactions"; + // LCOV_EXCL_START default: UNREACHABLE("ripple::detail::to_string : invalid TableType"); return "Unknown"; + // LCOV_EXCL_STOP } } @@ -202,18 +204,22 @@ saveValidatedLedger( if (!ledger->info().accountHash.isNonZero()) { + // LCOV_EXCL_START JLOG(j.fatal()) << "AH is zero: " << getJson({*ledger, {}}); UNREACHABLE("ripple::detail::saveValidatedLedger : zero account hash"); + // LCOV_EXCL_STOP } if (ledger->info().accountHash != ledger->stateMap().getHash().as_uint256()) { + // LCOV_EXCL_START JLOG(j.fatal()) << "sAL: " << ledger->info().accountHash << " != " << ledger->stateMap().getHash(); JLOG(j.fatal()) << "saveAcceptedLedger: seq=" << seq << ", current=" << current; UNREACHABLE( "ripple::detail::saveValidatedLedger : mismatched account hash"); + // LCOV_EXCL_STOP } XRPL_ASSERT( diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index d6a31024f3..77d098ea0f 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -151,9 +151,11 @@ Change::doApply() return applyFee(); case ttUNL_MODIFY: return applyUNLModify(); + // LCOV_EXCL_START default: UNREACHABLE("ripple::Change::doApply : invalid transaction type"); return tefFAILURE; + // LCOV_EXCL_STOP } } diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 565d938c83..d52e84d755 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -399,12 +399,14 @@ DeleteAccount::doApply() return {result, SkipEntry::No}; } + // LCOV_EXCL_START UNREACHABLE( "ripple::DeleteAccount::doApply : undeletable item not found " "in preclaim"); JLOG(j_.error()) << "DeleteAccount undeletable item not " "found in preclaim."; return {tecHAS_OBLIGATIONS, SkipEntry::No}; + // LCOV_EXCL_STOP }, ctx_.journal); if (ter != tesSUCCESS) diff --git a/src/xrpld/app/tx/detail/Offer.h b/src/xrpld/app/tx/detail/Offer.h index c214bea23f..58bd65ce46 100644 --- a/src/xrpld/app/tx/detail/Offer.h +++ b/src/xrpld/app/tx/detail/Offer.h @@ -225,11 +225,13 @@ template void TOffer::setFieldAmounts() { + // LCOV_EXCL_START #ifdef _MSC_VER UNREACHABLE("ripple::TOffer::setFieldAmounts : must be specialized"); #else static_assert(sizeof(TOut) == -1, "Must be specialized"); #endif + // LCOV_EXCL_STOP } template diff --git a/src/xrpld/app/tx/detail/OfferStream.cpp b/src/xrpld/app/tx/detail/OfferStream.cpp index 8e1215f5c8..9d43e419d7 100644 --- a/src/xrpld/app/tx/detail/OfferStream.cpp +++ b/src/xrpld/app/tx/detail/OfferStream.cpp @@ -369,10 +369,12 @@ TOfferStreamBase::step() std::is_same_v)) return shouldRmSmallIncreasedQOffer(); } + // LCOV_EXCL_START UNREACHABLE( "rippls::TOfferStreamBase::step::rmSmallIncreasedQOffer : XRP " "vs XRP offer"); return false; + // LCOV_EXCL_STOP }(); if (rmSmallIncreasedQOffer) diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index b5d9d4d5b8..ec2f902009 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -134,8 +134,10 @@ SetSignerList::doApply() default: break; } + // LCOV_EXCL_START UNREACHABLE("ripple::SetSignerList::doApply : invalid operation"); return temMALFORMED; + // LCOV_EXCL_STOP } void diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index f6b8b3c9d2..920b1a58bc 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -1170,11 +1170,13 @@ Transactor::operator()() if (!s2.isEquivalent(ctx_.tx)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Transaction serdes mismatch"; JLOG(j_.info()) << to_string(ctx_.tx.getJson(JsonOptions::none)); JLOG(j_.fatal()) << s2.getJson(JsonOptions::none); UNREACHABLE( "ripple::Transactor::operator() : transaction serdes mismatch"); + // LCOV_EXCL_STOP } } #endif diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 2587845df5..5f5c081e2f 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -223,10 +223,12 @@ claimHelper( auto i = signersList.find(a.keyAccount); if (i == signersList.end()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::claimHelper : invalid inputs"); // should have already // been checked continue; + // LCOV_EXCL_STOP } weight += i->second; rewardAccounts.push_back(a.rewardAccount); diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index c2e4e13f08..1cad93eedc 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -129,10 +129,12 @@ invoke_preflight(PreflightContext const& ctx) catch (UnknownTxnType const& e) { // Should never happen + // LCOV_EXCL_START JLOG(ctx.j.fatal()) << "Unknown transaction type in preflight: " << e.txnType; UNREACHABLE("ripple::invoke_preflight : unknown transaction type"); return {temUNKNOWN, TxConsequences{temUNKNOWN}}; + // LCOV_EXCL_STOP } } @@ -183,10 +185,12 @@ invoke_preclaim(PreclaimContext const& ctx) catch (UnknownTxnType const& e) { // Should never happen + // LCOV_EXCL_START JLOG(ctx.j.fatal()) << "Unknown transaction type in preclaim: " << e.txnType; UNREACHABLE("ripple::invoke_preclaim : unknown transaction type"); return temUNKNOWN; + // LCOV_EXCL_STOP } } @@ -217,9 +221,11 @@ invoke_calculateBaseFee(ReadView const& view, STTx const& tx) } catch (UnknownTxnType const& e) { + // LCOV_EXCL_START UNREACHABLE( "ripple::invoke_calculateBaseFee : unknown transaction type"); return XRPAmount{0}; + // LCOV_EXCL_STOP } } @@ -277,10 +283,12 @@ invoke_apply(ApplyContext& ctx) catch (UnknownTxnType const& e) { // Should never happen + // LCOV_EXCL_START JLOG(ctx.journal.fatal()) << "Unknown transaction type in apply: " << e.txnType; UNREACHABLE("ripple::invoke_apply : unknown transaction type"); return {temUNKNOWN, false}; + // LCOV_EXCL_STOP } } diff --git a/src/xrpld/nodestore/backend/NuDBFactory.cpp b/src/xrpld/nodestore/backend/NuDBFactory.cpp index 2f4e9d502e..727dec6f3e 100644 --- a/src/xrpld/nodestore/backend/NuDBFactory.cpp +++ b/src/xrpld/nodestore/backend/NuDBFactory.cpp @@ -121,11 +121,13 @@ public: using namespace boost::filesystem; if (db_.is_open()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::NodeStore::NuDBBackend::open : database is already " "open"); JLOG(j_.error()) << "database is already open"; return; + // LCOV_EXCL_STOP } auto const folder = path(name_); auto const dp = (folder / "nudb.dat").string(); diff --git a/src/xrpld/nodestore/backend/RocksDBFactory.cpp b/src/xrpld/nodestore/backend/RocksDBFactory.cpp index 0e421cd6bd..57c136a10a 100644 --- a/src/xrpld/nodestore/backend/RocksDBFactory.cpp +++ b/src/xrpld/nodestore/backend/RocksDBFactory.cpp @@ -232,11 +232,13 @@ public: { if (m_db) { + // LCOV_EXCL_START UNREACHABLE( "ripple::NodeStore::RocksDBBackend::open : database is already " "open"); JLOG(m_journal.error()) << "database is already open"; return; + // LCOV_EXCL_STOP } rocksdb::DB* db = nullptr; m_options.create_if_missing = createIfMissing; diff --git a/src/xrpld/overlay/Compression.h b/src/xrpld/overlay/Compression.h index 3a278a3403..35af4bf925 100644 --- a/src/xrpld/overlay/Compression.h +++ b/src/xrpld/overlay/Compression.h @@ -60,12 +60,14 @@ decompress( in, inSize, decompressed, decompressedSize); else { + // LCOV_EXCL_START JLOG(debugLog().warn()) << "decompress: invalid compression algorithm " << static_cast(algorithm); UNREACHABLE( "ripple::compression::decompress : invalid compression " "algorithm"); + // LCOV_EXCL_STOP } } catch (...) @@ -98,11 +100,13 @@ compress( in, inSize, std::forward(bf)); else { + // LCOV_EXCL_START JLOG(debugLog().warn()) << "compress: invalid compression algorithm" << static_cast(algorithm); UNREACHABLE( "ripple::compression::compress : invalid compression " "algorithm"); + // LCOV_EXCL_STOP } } catch (...) diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 93371f42ab..47d01eb7c5 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -2234,10 +2234,12 @@ PeerImp::onValidatorListMessage( case ListDisposition::invalid: case ListDisposition::unsupported_version: break; + // LCOV_EXCL_START default: UNREACHABLE( "ripple::PeerImp::onValidatorListMessage : invalid best list " "disposition"); + // LCOV_EXCL_STOP } // Charge based on the worst result @@ -2278,10 +2280,12 @@ PeerImp::onValidatorListMessage( // If it happens frequently, that's probably bad. fee_.update(Resource::feeInvalidData, "version"); break; + // LCOV_EXCL_START default: UNREACHABLE( "ripple::PeerImp::onValidatorListMessage : invalid worst list " "disposition"); + // LCOV_EXCL_STOP } // Log based on all the results. @@ -2338,10 +2342,12 @@ PeerImp::onValidatorListMessage( << "Ignored " << count << "invalid " << messageType << "(s) from peer " << remote_address_; break; + // LCOV_EXCL_START default: UNREACHABLE( "ripple::PeerImp::onValidatorListMessage : invalid list " "disposition"); + // LCOV_EXCL_STOP } } } diff --git a/src/xrpld/peerfinder/detail/Counts.h b/src/xrpld/peerfinder/detail/Counts.h index 821431c5bb..a35473ddb5 100644 --- a/src/xrpld/peerfinder/detail/Counts.h +++ b/src/xrpld/peerfinder/detail/Counts.h @@ -295,10 +295,12 @@ private: m_closingCount += n; break; + // LCOV_EXCL_START default: UNREACHABLE( "ripple::PeerFinder::Counts::adjust : invalid input state"); break; + // LCOV_EXCL_STOP }; } diff --git a/src/xrpld/peerfinder/detail/Logic.h b/src/xrpld/peerfinder/detail/Logic.h index 4b92a1d143..74bec8431e 100644 --- a/src/xrpld/peerfinder/detail/Logic.h +++ b/src/xrpld/peerfinder/detail/Logic.h @@ -976,11 +976,13 @@ public: << slot->remote_endpoint(); break; + // LCOV_EXCL_START default: UNREACHABLE( "ripple::PeerFinder::Logic::on_closed : invalid slot " "state"); break; + // LCOV_EXCL_STOP } } diff --git a/src/xrpld/perflog/detail/PerfLogImp.cpp b/src/xrpld/perflog/detail/PerfLogImp.cpp index a34faf885c..583ec095e1 100644 --- a/src/xrpld/perflog/detail/PerfLogImp.cpp +++ b/src/xrpld/perflog/detail/PerfLogImp.cpp @@ -53,9 +53,11 @@ PerfLogImp::Counters::Counters( if (!inserted) { // Ensure that no other function populates this entry. + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::Counters::Counters : failed to " "insert label"); + // LCOV_EXCL_STOP } } } @@ -68,9 +70,11 @@ PerfLogImp::Counters::Counters( if (!inserted) { // Ensure that no other function populates this entry. + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::Counters::Counters : failed to " "insert job type"); + // LCOV_EXCL_STOP } } } @@ -329,8 +333,10 @@ PerfLogImp::rpcStart(std::string const& method, std::uint64_t const requestId) auto counter = counters_.rpc_.find(method); if (counter == counters_.rpc_.end()) { + // LCOV_EXCL_START UNREACHABLE("ripple::perf::PerfLogImp::rpcStart : valid method input"); return; + // LCOV_EXCL_STOP } { @@ -351,8 +357,10 @@ PerfLogImp::rpcEnd( auto counter = counters_.rpc_.find(method); if (counter == counters_.rpc_.end()) { + // LCOV_EXCL_START UNREACHABLE("ripple::perf::PerfLogImp::rpcEnd : valid method input"); return; + // LCOV_EXCL_STOP } steady_time_point startTime; { @@ -365,8 +373,10 @@ PerfLogImp::rpcEnd( } else { + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::rpcEnd : valid requestId input"); + // LCOV_EXCL_STOP } } std::lock_guard lock(counter->second.mutex); @@ -384,9 +394,11 @@ PerfLogImp::jobQueue(JobType const type) auto counter = counters_.jq_.find(type); if (counter == counters_.jq_.end()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::jobQueue : valid job type input"); return; + // LCOV_EXCL_STOP } std::lock_guard lock(counter->second.mutex); ++counter->second.value.queued; @@ -402,10 +414,13 @@ PerfLogImp::jobStart( auto counter = counters_.jq_.find(type); if (counter == counters_.jq_.end()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::jobStart : valid job type input"); return; + // LCOV_EXCL_STOP } + { std::lock_guard lock(counter->second.mutex); ++counter->second.value.started; @@ -422,10 +437,13 @@ PerfLogImp::jobFinish(JobType const type, microseconds dur, int instance) auto counter = counters_.jq_.find(type); if (counter == counters_.jq_.end()) { + // LCOV_EXCL_START UNREACHABLE( "ripple::perf::PerfLogImp::jobFinish : valid job type input"); return; + // LCOV_EXCL_STOP } + { std::lock_guard lock(counter->second.mutex); ++counter->second.value.finished; diff --git a/src/xrpld/rpc/detail/Handler.cpp b/src/xrpld/rpc/detail/Handler.cpp index 3b32524ee2..d15c5aaed0 100644 --- a/src/xrpld/rpc/detail/Handler.cpp +++ b/src/xrpld/rpc/detail/Handler.cpp @@ -39,8 +39,10 @@ byRef(Function const& f) result = f(context); if (result.type() != Json::objectValue) { + // LCOV_EXCL_START UNREACHABLE("ripple::RPC::byRef : result is object"); result = RPC::makeObjectValue(result); + // LCOV_EXCL_STOP } return Status(); diff --git a/src/xrpld/rpc/detail/Status.cpp b/src/xrpld/rpc/detail/Status.cpp index 738219b6b5..ce15003968 100644 --- a/src/xrpld/rpc/detail/Status.cpp +++ b/src/xrpld/rpc/detail/Status.cpp @@ -51,8 +51,10 @@ Status::codeString() const return sStr.str(); } + // LCOV_EXCL_START UNREACHABLE("ripple::RPC::codeString : invalid type"); return ""; + // LCOV_EXCL_STOP } void diff --git a/src/xrpld/rpc/handlers/AccountChannels.cpp b/src/xrpld/rpc/handlers/AccountChannels.cpp index 1b0046ab64..17e46f052f 100644 --- a/src/xrpld/rpc/handlers/AccountChannels.cpp +++ b/src/xrpld/rpc/handlers/AccountChannels.cpp @@ -169,8 +169,10 @@ doAccountChannels(RPC::JsonContext& context) std::shared_ptr const& sleCur) { if (!sleCur) { + // LCOV_EXCL_START UNREACHABLE("ripple::doAccountChannels : null SLE"); return false; + // LCOV_EXCL_STOP } if (++count == limit) diff --git a/src/xrpld/rpc/handlers/AccountLines.cpp b/src/xrpld/rpc/handlers/AccountLines.cpp index 893ca9a190..146a9527a9 100644 --- a/src/xrpld/rpc/handlers/AccountLines.cpp +++ b/src/xrpld/rpc/handlers/AccountLines.cpp @@ -193,8 +193,10 @@ doAccountLines(RPC::JsonContext& context) std::shared_ptr const& sleCur) { if (!sleCur) { + // LCOV_EXCL_START UNREACHABLE("ripple::doAccountLines : null SLE"); return false; + // LCOV_EXCL_STOP } if (++count == limit) diff --git a/src/xrpld/rpc/handlers/AccountOffers.cpp b/src/xrpld/rpc/handlers/AccountOffers.cpp index e65b39b35b..1f2b76efe4 100644 --- a/src/xrpld/rpc/handlers/AccountOffers.cpp +++ b/src/xrpld/rpc/handlers/AccountOffers.cpp @@ -145,8 +145,10 @@ doAccountOffers(RPC::JsonContext& context) std::shared_ptr const& sle) { if (!sle) { + // LCOV_EXCL_START UNREACHABLE("ripple::doAccountOffers : null SLE"); return false; + // LCOV_EXCL_STOP } if (++count == limit) diff --git a/src/xrpld/rpc/handlers/AccountTx.cpp b/src/xrpld/rpc/handlers/AccountTx.cpp index 6b1dccdba9..e053c2adc0 100644 --- a/src/xrpld/rpc/handlers/AccountTx.cpp +++ b/src/xrpld/rpc/handlers/AccountTx.cpp @@ -353,9 +353,13 @@ populateJsonResponse( jvObj[jss::meta], sttx, *txnMeta); } else + { + // LCOV_EXCL_START UNREACHABLE( "ripple::populateJsonResponse : missing " "transaction medatata"); + // LCOV_EXCL_STOP + } } } } diff --git a/src/xrpld/rpc/handlers/Fee1.cpp b/src/xrpld/rpc/handlers/Fee1.cpp index 6d15a4d95f..ecb4ad4b29 100644 --- a/src/xrpld/rpc/handlers/Fee1.cpp +++ b/src/xrpld/rpc/handlers/Fee1.cpp @@ -32,9 +32,12 @@ doFee(RPC::JsonContext& context) auto result = context.app.getTxQ().doRPC(context.app); if (result.type() == Json::objectValue) return result; + + // LCOV_EXCL_START UNREACHABLE("ripple::doFee : invalid result type"); RPC::inject_error(rpcINTERNAL, context.params); return context.params; + // LCOV_EXCL_STOP } } // namespace ripple diff --git a/src/xrpld/shamap/detail/SHAMap.cpp b/src/xrpld/shamap/detail/SHAMap.cpp index d2415a2ff2..026149be56 100644 --- a/src/xrpld/shamap/detail/SHAMap.cpp +++ b/src/xrpld/shamap/detail/SHAMap.cpp @@ -545,8 +545,10 @@ SHAMap::onlyBelow(SHAMapTreeNode* node) const if (!nextNode) { + // LCOV_EXCL_START UNREACHABLE("ripple::SHAMap::onlyBelow : no next node"); return no_item; + // LCOV_EXCL_STOP } node = nextNode; @@ -922,8 +924,10 @@ SHAMap::updateGiveItem( if (!node || (node->peekItem()->key() != tag)) { + // LCOV_EXCL_START UNREACHABLE("ripple::SHAMap::updateGiveItem : invalid node"); return false; + // LCOV_EXCL_STOP } if (node->getType() != type) diff --git a/src/xrpld/shamap/detail/SHAMapDelta.cpp b/src/xrpld/shamap/detail/SHAMapDelta.cpp index 2adce62efc..ebdaffad14 100644 --- a/src/xrpld/shamap/detail/SHAMapDelta.cpp +++ b/src/xrpld/shamap/detail/SHAMapDelta.cpp @@ -149,8 +149,10 @@ SHAMap::compare(SHAMap const& otherMap, Delta& differences, int maxCount) const if (!ourNode || !otherNode) { + // LCOV_EXCL_START UNREACHABLE("ripple::SHAMap::compare : missing a node"); Throw(type_, uint256()); + // LCOV_EXCL_STOP } if (ourNode->isLeaf() && otherNode->isLeaf()) @@ -230,7 +232,11 @@ SHAMap::compare(SHAMap const& otherMap, Delta& differences, int maxCount) const } } else + { + // LCOV_EXCL_START UNREACHABLE("ripple::SHAMap::compare : invalid node"); + // LCOV_EXCL_STOP + } } return true; From f61086b43c93322c73738a4f30efb54d986070d7 Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 8 Oct 2025 09:15:24 -0400 Subject: [PATCH 03/18] refactor: Update CI strategy matrix to use new RHEL 9 and RHEL 10 images (#5856) This change uses the new RHEL 9 and 10 images to build and test the binary, and adds support for having different Docker image SHAs per distro-compiler combination. Instead of supporting RHEL each minor version, we are simplifying our pipelines by only supporting RHEL major versions. Our CI Docker images have already been updated accordingly, and we recently added support for RHEL 10 as well. Up until now, the CI Docker images had all been rebuilt at the same time, but that is not necessarily true as the most recent push to the CI repo has shown where the RHEL images now have a different SHA than the Debian and Ubuntu ones. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- .github/scripts/strategy-matrix/generate.py | 8 +- .github/scripts/strategy-matrix/linux.json | 92 ++++++++++++-------- .github/scripts/strategy-matrix/macos.json | 3 +- .github/scripts/strategy-matrix/windows.json | 3 +- .github/workflows/reusable-build-test.yml | 2 +- .github/workflows/upload-conan-deps.yml | 5 +- 6 files changed, 66 insertions(+), 47 deletions(-) diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index fd05895b0e..025d553b5e 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -74,14 +74,14 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: continue # RHEL: - # - 9.4 using GCC 12: Debug and Unity on linux/amd64. - # - 9.6 using Clang: Release and no Unity on linux/amd64. + # - 9 using GCC 12: Debug and Unity on linux/amd64. + # - 10 using Clang: Release and no Unity on linux/amd64. if os['distro_name'] == 'rhel': skip = True - if os['distro_version'] == '9.4': + if os['distro_version'] == '9': if f'{os['compiler_name']}-{os['compiler_version']}' == 'gcc-12' and build_type == 'Debug' and '-Dunity=ON' in cmake_args and architecture['platform'] == 'linux/amd64': skip = False - elif os['distro_version'] == '9.6': + elif os['distro_version'] == '10': if f'{os['compiler_name']}-{os['compiler_version']}' == 'clang-any' and build_type == 'Release' and '-Dunity=OFF' in cmake_args and architecture['platform'] == 'linux/amd64': skip = False if skip: diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 44eaebd074..08313daf0a 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -14,139 +14,155 @@ "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", - "compiler_version": "12" + "compiler_version": "12", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", - "compiler_version": "13" + "compiler_version": "13", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", - "compiler_version": "14" + "compiler_version": "14", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", - "compiler_version": "15" + "compiler_version": "15", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", - "compiler_version": "16" + "compiler_version": "16", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", - "compiler_version": "17" + "compiler_version": "17", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", - "compiler_version": "18" + "compiler_version": "18", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", - "compiler_version": "19" + "compiler_version": "19", + "image_sha": "6f723eb" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", - "compiler_version": "20" + "compiler_version": "20", + "image_sha": "6f723eb" }, { "distro_name": "rhel", - "distro_version": "9.4", + "distro_version": "9", "compiler_name": "gcc", - "compiler_version": "12" + "compiler_version": "12", + "image_sha": "0ab1e4c" }, { "distro_name": "rhel", - "distro_version": "9.4", + "distro_version": "9", "compiler_name": "gcc", - "compiler_version": "13" + "compiler_version": "13", + "image_sha": "0ab1e4c" }, { "distro_name": "rhel", - "distro_version": "9.4", + "distro_version": "9", "compiler_name": "gcc", - "compiler_version": "14" + "compiler_version": "14", + "image_sha": "0ab1e4c" }, { "distro_name": "rhel", - "distro_version": "9.6", - "compiler_name": "gcc", - "compiler_version": "13" - }, - { - "distro_name": "rhel", - "distro_version": "9.6", - "compiler_name": "gcc", - "compiler_version": "14" - }, - { - "distro_name": "rhel", - "distro_version": "9.4", + "distro_version": "9", "compiler_name": "clang", - "compiler_version": "any" + "compiler_version": "any", + "image_sha": "0ab1e4c" }, { "distro_name": "rhel", - "distro_version": "9.6", + "distro_version": "10", + "compiler_name": "gcc", + "compiler_version": "14", + "image_sha": "0ab1e4c" + }, + { + "distro_name": "rhel", + "distro_version": "10", "compiler_name": "clang", - "compiler_version": "any" + "compiler_version": "any", + "image_sha": "0ab1e4c" }, { "distro_name": "ubuntu", "distro_version": "jammy", "compiler_name": "gcc", - "compiler_version": "12" + "compiler_version": "12", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", - "compiler_version": "13" + "compiler_version": "13", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", - "compiler_version": "14" + "compiler_version": "14", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", - "compiler_version": "16" + "compiler_version": "16", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", - "compiler_version": "17" + "compiler_version": "17", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", - "compiler_version": "18" + "compiler_version": "18", + "image_sha": "6f723eb" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", - "compiler_version": "19" + "compiler_version": "19", + "image_sha": "6f723eb" } ], "build_type": ["Debug", "Release"], diff --git a/.github/scripts/strategy-matrix/macos.json b/.github/scripts/strategy-matrix/macos.json index de37639ddd..14b6089620 100644 --- a/.github/scripts/strategy-matrix/macos.json +++ b/.github/scripts/strategy-matrix/macos.json @@ -10,7 +10,8 @@ "distro_name": "macos", "distro_version": "", "compiler_name": "", - "compiler_version": "" + "compiler_version": "", + "image_sha": "" } ], "build_type": ["Debug", "Release"], diff --git a/.github/scripts/strategy-matrix/windows.json b/.github/scripts/strategy-matrix/windows.json index 08b41e3f89..8637b31012 100644 --- a/.github/scripts/strategy-matrix/windows.json +++ b/.github/scripts/strategy-matrix/windows.json @@ -10,7 +10,8 @@ "distro_name": "windows", "distro_version": "", "compiler_name": "", - "compiler_version": "" + "compiler_version": "", + "image_sha": "" } ], "build_type": ["Debug", "Release"], diff --git a/.github/workflows/reusable-build-test.yml b/.github/workflows/reusable-build-test.yml index c274cf2b21..5bc9cf2557 100644 --- a/.github/workflows/reusable-build-test.yml +++ b/.github/workflows/reusable-build-test.yml @@ -52,7 +52,7 @@ jobs: cmake_args: ${{ matrix.cmake_args }} cmake_target: ${{ matrix.cmake_target }} runs_on: ${{ toJSON(matrix.architecture.runner) }} - image: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-5dd7158', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version) || '' }} + image: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-{4}', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version, matrix.os.image_sha) || '' }} config_name: ${{ matrix.config_name }} secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index cbae8a4c86..680602d978 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -40,11 +40,13 @@ concurrency: cancel-in-progress: true jobs: + # Generate the strategy matrix to be used by the following job. generate-matrix: uses: ./.github/workflows/reusable-strategy-matrix.yml with: strategy_matrix: ${{ github.event_name == 'pull_request' && 'minimal' || 'all' }} + # Build and upload the dependencies for each configuration. run-upload-conan-deps: needs: - generate-matrix @@ -53,8 +55,7 @@ jobs: matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }} max-parallel: 10 runs-on: ${{ matrix.architecture.runner }} - container: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-5dd7158', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version) || null }} - + container: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-{4}', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version, matrix.os.image_sha) || null }} steps: - name: Cleanup workspace if: ${{ runner.os == 'macOS' }} From 6b6b213cf543d98eb76af7ac4a6a1379ce26ca4e Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 8 Oct 2025 14:45:44 +0100 Subject: [PATCH 04/18] chore: Fix release build error (#5864) This change fixes a release build error with GCC 15.2. The `fields` variable is only used in `XRPL_ASSERT`, which evaluates to nothing in a Release build, leaving the variable unused. This change silences the build warning. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- src/libxrpl/ledger/View.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 02de872ad3..89d8137ac7 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -1134,7 +1134,7 @@ createPseudoAccount( uint256 const& pseudoOwnerKey, SField const& ownerField) { - auto const& fields = getPseudoAccountFields(); + [[maybe_unused]] auto const& fields = getPseudoAccountFields(); XRPL_ASSERT( std::count_if( fields.begin(), From 620fb26823777aedd62fb179f3d39078ac66c5cd Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 8 Oct 2025 23:36:09 +0900 Subject: [PATCH 05/18] test: Add more tests for Simulate RPC metadata (#5827) --- src/test/rpc/Simulate_test.cpp | 92 ++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 33 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 0a36a8a841..5e7ab5be4d 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -136,11 +136,12 @@ class Simulate_test : public beast::unit_test::suite jtx::Env& env, Json::Value const& tx, std::function const& validate, Json::Value const& expectedMetadataKey, - bool testSerialized = true) + Json::Value const& expectedMetadataValue) { env.close(); @@ -149,8 +150,13 @@ class Simulate_test : public beast::unit_test::suite validate( env.rpc("json", "simulate", to_string(params)), tx, - expectedMetadataKey); - validate(env.rpc("simulate", to_string(tx)), tx, expectedMetadataKey); + expectedMetadataKey, + expectedMetadataValue); + validate( + env.rpc("simulate", to_string(tx)), + tx, + expectedMetadataKey, + expectedMetadataValue); BEAST_EXPECTS( env.current()->txCount() == 0, @@ -1218,73 +1224,93 @@ class Simulate_test : public beast::unit_test::suite testcase("Successful transaction with additional metadata"); using namespace jtx; + using namespace std::chrono_literals; Env env{*this, envconfig([&](std::unique_ptr cfg) { cfg->NETWORK_ID = 1025; return cfg; })}; Account const alice("alice"); + Account const bob("bob"); - env.fund(XRP(10000), alice); + env.fund(XRP(10000), alice, bob); env.close(); + // deliver_amount is unavailable in the metadata before 2014-02-01 + // so proceed to 2014-02-01 + env.close(NetClock::time_point{446000000s}); { - auto validateOutput = [&](Json::Value const& resp, - Json::Value const& tx, - Json::Value const& expectedMetadataKey) { - auto result = resp[jss::result]; + auto validateOutput = + [&](Json::Value const& resp, + Json::Value const& tx, + Json::Value const& expectedMetadataKey, + Json::Value const& expectedMetadataValue) { + auto result = resp[jss::result]; - BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); - BEAST_EXPECT(result[jss::engine_result_code] == 0); - BEAST_EXPECT( - result[jss::engine_result_message] == - "The simulated transaction would have been applied."); - - if (BEAST_EXPECT( - result.isMember(jss::meta) || - result.isMember(jss::meta_blob))) - { - Json::Value const metadata = getJsonMetadata(result); - - BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); BEAST_EXPECT( - metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); - BEAST_EXPECT( - metadata.isMember(expectedMetadataKey.asString())); - } - }; + result[jss::engine_result_message] == + "The simulated transaction would have been applied."); + + if (BEAST_EXPECT( + result.isMember(jss::meta) || + result.isMember(jss::meta_blob))) + { + Json::Value const metadata = getJsonMetadata(result); + + BEAST_EXPECT( + metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == + "tesSUCCESS"); + BEAST_EXPECT( + metadata.isMember(expectedMetadataKey.asString())); + BEAST_EXPECT( + metadata[expectedMetadataKey.asString()] == + expectedMetadataValue); + } + }; { Json::Value tx; - tx[jss::Account] = env.master.human(); + tx[jss::Account] = alice.human(); tx[jss::TransactionType] = jss::Payment; - tx[sfDestination] = alice.human(); + tx[sfDestination] = bob.human(); tx[sfAmount] = "100"; // test delivered amount testTxJsonMetadataField( - env, tx, validateOutput, jss::delivered_amount); + env, tx, validateOutput, jss::delivered_amount, "100"); } { Json::Value tx; - tx[jss::Account] = env.master.human(); + tx[jss::Account] = alice.human(); tx[jss::TransactionType] = jss::NFTokenMint; tx[sfNFTokenTaxon] = 1; + Json::Value nftokenId = + to_string(token::getNextID(env, alice, 1)); // test nft synthetic testTxJsonMetadataField( - env, tx, validateOutput, jss::nftoken_id); + env, tx, validateOutput, jss::nftoken_id, nftokenId); } { Json::Value tx; - tx[jss::Account] = env.master.human(); + tx[jss::Account] = alice.human(); tx[jss::TransactionType] = jss::MPTokenIssuanceCreate; + Json::Value mptIssuanceId = + to_string(makeMptID(env.seq(alice), alice)); // test mpt issuance id testTxJsonMetadataField( - env, tx, validateOutput, jss::mpt_issuance_id); + env, + tx, + validateOutput, + jss::mpt_issuance_id, + mptIssuanceId); } } } From 5ecde3cf39dcb784250253e9d953e9a4d4d2bb34 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 8 Oct 2025 16:04:02 +0100 Subject: [PATCH 06/18] Add vault invariants (#5518) This change adds invariants for SingleAssetVault #5224 (XLS-065), which had been intentionally skipped earlier to keep the SAV PR size manageable. --- .../xrpl/protocol/detail/transactions.macro | 12 +- src/test/app/Invariants_test.cpp | 1706 ++++++++++++++++- src/test/app/Vault_test.cpp | 63 + src/xrpld/app/tx/detail/InvariantCheck.cpp | 950 +++++++++ src/xrpld/app/tx/detail/InvariantCheck.h | 73 +- src/xrpld/app/tx/detail/VaultDeposit.cpp | 5 +- src/xrpld/app/tx/detail/VaultSet.cpp | 3 + 7 files changed, 2801 insertions(+), 11 deletions(-) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 3ea4a3bbec..2565372ebc 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -851,7 +851,7 @@ TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, Delegation::delegatable, featureSingleAssetVault, - createPseudoAcct | createMPTIssuance, + createPseudoAcct | createMPTIssuance | mustModifyVault, ({ {sfAsset, soeREQUIRED, soeMPTSupported}, {sfAssetsMaximum, soeOPTIONAL}, @@ -869,7 +869,7 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, TRANSACTION(ttVAULT_SET, 66, VaultSet, Delegation::delegatable, featureSingleAssetVault, - noPriv, + mustModifyVault, ({ {sfVaultID, soeREQUIRED}, {sfAssetsMaximum, soeOPTIONAL}, @@ -884,7 +884,7 @@ TRANSACTION(ttVAULT_SET, 66, VaultSet, TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, Delegation::delegatable, featureSingleAssetVault, - mustDeleteAcct | destroyMPTIssuance, + mustDeleteAcct | destroyMPTIssuance | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, })) @@ -896,7 +896,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, Delegation::delegatable, featureSingleAssetVault, - mayAuthorizeMPT, + mayAuthorizeMPT | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, @@ -909,7 +909,7 @@ TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, Delegation::delegatable, featureSingleAssetVault, - mayDeleteMPT, + mayDeleteMPT | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, @@ -924,7 +924,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback, Delegation::delegatable, featureSingleAssetVault, - mayDeleteMPT, + mayDeleteMPT | mustModifyVault, ({ {sfVaultID, soeREQUIRED}, {sfHolder, soeREQUIRED}, diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index c91149b2f7..8781eee72b 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -24,9 +24,18 @@ #include #include +#include #include +#include +#include #include +#include +#include #include +#include +#include +#include +#include #include @@ -66,8 +75,9 @@ class Invariants_test : public beast::unit_test::suite * checker. * @preclose See "Preclose" above. Note that @preclose runs *before* * @precheck, but is the last parameter for historical reasons - * + * @setTxAccount optionally set to add sfAccount to tx (either A1 or A2) */ + enum class TxAccount : int { None = 0, A1, A2 }; void doInvariantCheck( std::vector const& expect_logs, @@ -76,7 +86,8 @@ class Invariants_test : public beast::unit_test::suite STTx tx = STTx{ttACCOUNT_SET, [](STObject&) {}}, std::initializer_list ters = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, - Preclose const& preclose = {}) + Preclose const& preclose = {}, + TxAccount setTxAccount = TxAccount::None) { using namespace test::jtx; FeatureBitset amendments = testable_amendments() | @@ -93,6 +104,9 @@ class Invariants_test : public beast::unit_test::suite OpenView ov{*env.current()}; test::StreamSink sink{beast::severities::kWarning}; beast::Journal jlog{sink}; + if (setTxAccount != TxAccount::None) + tx.setAccountID( + sfAccount, setTxAccount == TxAccount::A1 ? A1.id() : A2.id()); ApplyContext ac{ env.app(), ov, @@ -117,12 +131,13 @@ class Invariants_test : public beast::unit_test::suite BEAST_EXPECT( messages.starts_with("Invariant failed:") || messages.starts_with("Transaction caused an exception")); + // std::cerr << messages << '\n'; for (auto const& m : expect_logs) { if (messages.find(m) == std::string::npos) { // uncomment if you want to log the invariant failure - // message log << " --> " << m << std::endl; + // std::cerr << " --> " << m << std::endl; fail(); } } @@ -1726,6 +1741,1690 @@ class Invariants_test : public beast::unit_test::suite {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); } + void + testVault() + { + using namespace test::jtx; + + struct AccountAmount + { + AccountID account; + int amount; + }; + struct Adjustements + { + std::optional assetsTotal = {}; + std::optional assetsAvailable = {}; + std::optional lossUnrealized = {}; + std::optional assetsMaximum = {}; + std::optional sharesTotal = {}; + std::optional vaultAssets = {}; + std::optional accountAssets = {}; + std::optional accountShares = {}; + }; + auto constexpr adjust = [&](ApplyView& ac, + ripple::Keylet keylet, + Adjustements args) { + auto sleVault = ac.peek(keylet); + if (!sleVault) + return false; + + auto const mptIssuanceID = (*sleVault)[sfShareMPTID]; + auto sleShares = ac.peek(keylet::mptIssuance(mptIssuanceID)); + if (!sleShares) + return false; + + // These two fields are adjusted in absolute terms + if (args.lossUnrealized) + (*sleVault)[sfLossUnrealized] = *args.lossUnrealized; + if (args.assetsMaximum) + (*sleVault)[sfAssetsMaximum] = *args.assetsMaximum; + + // Remaining fields are adjusted in terms of difference + if (args.assetsTotal) + (*sleVault)[sfAssetsTotal] = + *(*sleVault)[sfAssetsTotal] + *args.assetsTotal; + if (args.assetsAvailable) + (*sleVault)[sfAssetsAvailable] = + *(*sleVault)[sfAssetsAvailable] + *args.assetsAvailable; + ac.update(sleVault); + + if (args.sharesTotal) + (*sleShares)[sfOutstandingAmount] = + *(*sleShares)[sfOutstandingAmount] + *args.sharesTotal; + ac.update(sleShares); + + auto const assets = *(*sleVault)[sfAsset]; + auto const pseudoId = *(*sleVault)[sfAccount]; + if (args.vaultAssets) + { + if (assets.native()) + { + auto slePseudoAccount = ac.peek(keylet::account(pseudoId)); + if (!slePseudoAccount) + return false; + (*slePseudoAccount)[sfBalance] = + *(*slePseudoAccount)[sfBalance] + *args.vaultAssets; + ac.update(slePseudoAccount); + } + else if (assets.holds()) + { + auto const mptId = assets.get().getMptID(); + auto sleMPToken = ac.peek(keylet::mptoken(mptId, pseudoId)); + if (!sleMPToken) + return false; + (*sleMPToken)[sfMPTAmount] = + *(*sleMPToken)[sfMPTAmount] + *args.vaultAssets; + ac.update(sleMPToken); + } + else + return false; // Not supporting testing with IOU + } + + if (args.accountAssets) + { + auto const& pair = *args.accountAssets; + if (assets.native()) + { + auto sleAccount = ac.peek(keylet::account(pair.account)); + if (!sleAccount) + return false; + (*sleAccount)[sfBalance] = + *(*sleAccount)[sfBalance] + pair.amount; + ac.update(sleAccount); + } + else if (assets.holds()) + { + auto const mptID = assets.get().getMptID(); + auto sleMPToken = + ac.peek(keylet::mptoken(mptID, pair.account)); + if (!sleMPToken) + return false; + (*sleMPToken)[sfMPTAmount] = + *(*sleMPToken)[sfMPTAmount] + pair.amount; + ac.update(sleMPToken); + } + else + return false; // Not supporting testing with IOU + } + + if (args.accountShares) + { + auto const& pair = *args.accountShares; + auto sleMPToken = + ac.peek(keylet::mptoken(mptIssuanceID, pair.account)); + if (!sleMPToken) + return false; + (*sleMPToken)[sfMPTAmount] = + *(*sleMPToken)[sfMPTAmount] + pair.amount; + ac.update(sleMPToken); + } + return true; + }; + + constexpr auto args = + [](AccountID id, int adjustement, auto fn) -> Adjustements { + Adjustements sample = { + .assetsTotal = adjustement, + .assetsAvailable = adjustement, + .lossUnrealized = 0, + .sharesTotal = adjustement, + .vaultAssets = adjustement, + .accountAssets = // + AccountAmount{id, -adjustement}, + .accountShares = // + AccountAmount{id, adjustement}}; + fn(sample); + return sample; + }; + + Account A3{"A3"}; + Account A4{"A4"}; + auto const precloseXrp = + [&](Account const& A1, Account const& A2, Env& env) -> bool { + env.fund(XRP(1000), A3, A4); + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + env(vault.deposit( + {.depositor = A1, .id = keylet.key, .amount = XRP(10)})); + env(vault.deposit( + {.depositor = A2, .id = keylet.key, .amount = XRP(10)})); + env(vault.deposit( + {.depositor = A3, .id = keylet.key, .amount = XRP(10)})); + return true; + }; + + testcase << "Vault general checks"; + doInvariantCheck( + {"vault deletion succeeded without deleting a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DELETE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault updated by a wrong transaction type"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().erase(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttPAYMENT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault updated by a wrong transaction type"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttPAYMENT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault updated by a wrong transaction type"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const sequence = ac.view().seq(); + auto const vaultKeylet = keylet::vault(A1.id(), sequence); + auto sleVault = std::make_shared(vaultKeylet); + auto const vaultPage = ac.view().dirInsert( + keylet::ownerDir(A1.id()), + sleVault->key(), + describeOwnerDir(A1.id())); + sleVault->setFieldU64(sfOwnerNode, *vaultPage); + ac.view().insert(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttPAYMENT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + + doInvariantCheck( + {"vault deleted by a wrong transaction type"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().erase(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation updated more than single vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().erase(sleVault); + } + { + auto const keylet = keylet::vault(A2.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().erase(sleVault); + } + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DELETE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + { + auto [tx, _] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + } + { + auto [tx, _] = + vault.create({.owner = A2, .asset = xrpIssue()}); + env(tx); + } + return true; + }); + + doInvariantCheck( + {"vault operation updated more than single vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const sequence = ac.view().seq(); + auto const insertVault = [&](Account const A) { + auto const vaultKeylet = keylet::vault(A.id(), sequence); + auto sleVault = std::make_shared(vaultKeylet); + auto const vaultPage = ac.view().dirInsert( + keylet::ownerDir(A.id()), + sleVault->key(), + describeOwnerDir(A.id())); + sleVault->setFieldU64(sfOwnerNode, *vaultPage); + ac.view().insert(sleVault); + }; + insertVault(A1); + insertVault(A2); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + + doInvariantCheck( + {"deleted vault must also delete shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().erase(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DELETE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"deleted vault must have no shares outstanding", + "deleted vault must have no assets outstanding", + "deleted vault must have no assets available"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + ac.view().erase(sleVault); + ac.view().erase(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DELETE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + env(vault.deposit( + {.depositor = A1, .id = keylet.key, .amount = XRP(10)})); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + // Note, such an "orphaned" update of MPT issuance attached to a + // vault is invalid; ttVAULT_SET must also update Vault object. + sleShares->setFieldH256(sfDomainID, uint256(13)); + ac.view().update(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + return true; + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CLAWBACK, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without modifying a vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DELETE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"updated vault must have shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfAssetsMaximum] = 200; + ac.view().update(sleVault); + + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + ac.view().erase(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, _] = vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault operation succeeded without updating shares", + "assets available must not be greater than assets outstanding"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfAssetsTotal] = 9; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + env(vault.deposit( + {.depositor = A1, .id = keylet.key, .amount = XRP(10)})); + return true; + }); + + doInvariantCheck( + {"set must not change assets outstanding", + "set must not change assets available", + "set must not change shares outstanding", + "set must not change vault balance", + "assets available must be positive", + "assets available must not be greater than assets outstanding", + "assets outstanding must be positive"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto slePseudoAccount = + ac.view().peek(keylet::account(*(*sleVault)[sfAccount])); + if (!slePseudoAccount) + return false; + (*slePseudoAccount)[sfBalance] = + *(*slePseudoAccount)[sfBalance] - 10; + ac.view().update(slePseudoAccount); + + // Move 10 drops to A4 to enforce total XRP balance + auto sleA4 = ac.view().peek(keylet::account(A4.id())); + if (!sleA4) + return false; + (*sleA4)[sfBalance] = *(*sleA4)[sfBalance] + 10; + ac.view().update(sleA4); + + return adjust( + ac.view(), + keylet, + args(A2.id(), 0, [&](Adjustements& sample) { + sample.assetsAvailable = (DROPS_PER_XRP * -100).value(); + sample.assetsTotal = (DROPS_PER_XRP * -200).value(); + sample.sharesTotal = -1; + })); + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"violation of vault immutable data"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + sleVault->setFieldIssue( + sfAsset, STIssue{sfAsset, MPTIssue(MPTID(42))}); + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"violation of vault immutable data"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + sleVault->setAccountID(sfAccount, A2.id()); + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"violation of vault immutable data"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfShareMPTID] = MPTID(42); + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"vault transaction must not change loss unrealized", + "set must not change assets outstanding"}, + [&](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(), 0, [&](Adjustements& sample) { + sample.lossUnrealized = 13; + sample.assetsTotal = 20; + })); + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"loss unrealized must not exceed the difference " + "between assets outstanding and available", + "vault transaction must not change loss unrealized"}, + [&](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(), 100, [&](Adjustements& sample) { + sample.lossUnrealized = 13; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { + tx.setFieldAmount(sfAmount, XRPAmount(200)); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"set assets outstanding must not exceed assets maximum"}, + [&](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(), 0, [&](Adjustements& sample) { + sample.assetsMaximum = 1; + })); + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"assets maximum must be positive"}, + [&](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(), 0, [&](Adjustements& sample) { + sample.assetsMaximum = -1; + })); + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"set must not change shares outstanding", + "updated zero sized vault must have no assets outstanding", + "updated zero sized vault must have no assets available"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + ac.view().update(sleVault); + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + (*sleShares)[sfOutstandingAmount] = 0; + ac.view().update(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"updated shares must not exceed maximum"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + (*sleShares)[sfMaximumAmount] = 10; + ac.view().update(sleShares); + + return adjust( + ac.view(), keylet, args(A2.id(), 10, [](Adjustements&) {})); + }, + XRPAmount{}, + STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"updated shares must not exceed maximum"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + adjust( + ac.view(), keylet, args(A2.id(), 10, [](Adjustements&) {})); + + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + (*sleShares)[sfOutstandingAmount] = maxMPTokenAmount + 1; + ac.view().update(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + testcase << "Vault create"; + doInvariantCheck( + { + "created vault must be empty", + "updated zero sized vault must have no assets outstanding", + "create operation must not have updated a vault", + }, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfAssetsTotal] = 9; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + { + "created vault must be empty", + "updated zero sized vault must have no assets available", + "assets available must not be greater than assets outstanding", + "create operation must not have updated a vault", + }, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfAssetsAvailable] = 9; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + { + "created vault must be empty", + "loss unrealized must not exceed the difference between assets " + "outstanding and available", + "vault transaction must not change loss unrealized", + "create operation must not have updated a vault", + }, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfLossUnrealized] = 1; + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + { + "created vault must be empty", + "create operation must not have updated a vault", + }, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + ac.view().update(sleVault); + (*sleShares)[sfOutstandingAmount] = 9; + ac.view().update(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + { + "assets maximum must be positive", + "create operation must not have updated a vault", + }, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + (*sleVault)[sfAssetsMaximum] = Number(-1); + ac.view().update(sleVault); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"create operation must not have updated a vault", + "shares issuer and vault pseudo-account must be the same", + "shares issuer must be a pseudo-account", + "shares issuer pseudo-account must point back to the vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + auto sleVault = ac.view().peek(keylet); + if (!sleVault) + return false; + auto sleShares = ac.view().peek( + keylet::mptIssuance((*sleVault)[sfShareMPTID])); + if (!sleShares) + return false; + ac.view().update(sleVault); + (*sleShares)[sfIssuer] = A1.id(); + ac.view().update(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + [&](Account const& A1, Account const& A2, Env& env) { + Vault vault{env}; + auto [tx, keylet] = + vault.create({.owner = A1, .asset = xrpIssue()}); + env(tx); + return true; + }); + + doInvariantCheck( + {"vault created by a wrong transaction type", + "account root created illegally"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + // The code below will create a valid vault with (almost) all + // the invariants holding. Except one: it is created by the + // wrong transaction type. + auto const sequence = ac.view().seq(); + auto const vaultKeylet = keylet::vault(A1.id(), sequence); + auto sleVault = std::make_shared(vaultKeylet); + auto const vaultPage = ac.view().dirInsert( + keylet::ownerDir(A1.id()), + sleVault->key(), + describeOwnerDir(A1.id())); + sleVault->setFieldU64(sfOwnerNode, *vaultPage); + + auto pseudoId = + pseudoAccountAddress(ac.view(), vaultKeylet.key); + // Create pseudo-account. + auto sleAccount = + std::make_shared(keylet::account(pseudoId)); + sleAccount->setAccountID(sfAccount, pseudoId); + sleAccount->setFieldAmount(sfBalance, STAmount{}); + std::uint32_t const seqno = // + ac.view().rules().enabled(featureSingleAssetVault) // + ? 0 // + : sequence; + sleAccount->setFieldU32(sfSequence, seqno); + sleAccount->setFieldU32( + sfFlags, + lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth); + sleAccount->setFieldH256(sfVaultID, vaultKeylet.key); + ac.view().insert(sleAccount); + + auto const sharesMptId = makeMptID(sequence, pseudoId); + auto const sharesKeylet = keylet::mptIssuance(sharesMptId); + auto sleShares = std::make_shared(sharesKeylet); + auto const sharesPage = ac.view().dirInsert( + keylet::ownerDir(pseudoId), + sharesKeylet, + describeOwnerDir(pseudoId)); + sleShares->setFieldU64(sfOwnerNode, *sharesPage); + + sleShares->at(sfFlags) = 0; + sleShares->at(sfIssuer) = pseudoId; + sleShares->at(sfOutstandingAmount) = 0; + sleShares->at(sfSequence) = sequence; + + sleVault->at(sfAccount) = pseudoId; + sleVault->at(sfFlags) = 0; + sleVault->at(sfSequence) = sequence; + sleVault->at(sfOwner) = A1.id(); + sleVault->at(sfAssetsTotal) = Number(0); + sleVault->at(sfAssetsAvailable) = Number(0); + sleVault->at(sfLossUnrealized) = Number(0); + sleVault->at(sfShareMPTID) = sharesMptId; + sleVault->at(sfWithdrawalPolicy) = + vaultStrategyFirstComeFirstServe; + + ac.view().insert(sleVault); + ac.view().insert(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_SET, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + doInvariantCheck( + {"shares issuer and vault pseudo-account must be the same", + "shares issuer pseudo-account must point back to the vault"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const sequence = ac.view().seq(); + auto const vaultKeylet = keylet::vault(A1.id(), sequence); + auto sleVault = std::make_shared(vaultKeylet); + auto const vaultPage = ac.view().dirInsert( + keylet::ownerDir(A1.id()), + sleVault->key(), + describeOwnerDir(A1.id())); + sleVault->setFieldU64(sfOwnerNode, *vaultPage); + + auto pseudoId = + pseudoAccountAddress(ac.view(), vaultKeylet.key); + // Create pseudo-account. + auto sleAccount = + std::make_shared(keylet::account(pseudoId)); + sleAccount->setAccountID(sfAccount, pseudoId); + sleAccount->setFieldAmount(sfBalance, STAmount{}); + std::uint32_t const seqno = // + ac.view().rules().enabled(featureSingleAssetVault) // + ? 0 // + : sequence; + sleAccount->setFieldU32(sfSequence, seqno); + sleAccount->setFieldU32( + sfFlags, + lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth); + // sleAccount->setFieldH256(sfVaultID, vaultKeylet.key); + // Setting wrong vault key + sleAccount->setFieldH256(sfVaultID, uint256(42)); + ac.view().insert(sleAccount); + + auto const sharesMptId = makeMptID(sequence, pseudoId); + auto const sharesKeylet = keylet::mptIssuance(sharesMptId); + auto sleShares = std::make_shared(sharesKeylet); + auto const sharesPage = ac.view().dirInsert( + keylet::ownerDir(pseudoId), + sharesKeylet, + describeOwnerDir(pseudoId)); + sleShares->setFieldU64(sfOwnerNode, *sharesPage); + + sleShares->at(sfFlags) = 0; + sleShares->at(sfIssuer) = pseudoId; + sleShares->at(sfOutstandingAmount) = 0; + sleShares->at(sfSequence) = sequence; + + // sleVault->at(sfAccount) = pseudoId; + // Setting wrong pseudo acocunt ID + sleVault->at(sfAccount) = A2.id(); + sleVault->at(sfFlags) = 0; + sleVault->at(sfSequence) = sequence; + sleVault->at(sfOwner) = A1.id(); + sleVault->at(sfAssetsTotal) = Number(0); + sleVault->at(sfAssetsAvailable) = Number(0); + sleVault->at(sfLossUnrealized) = Number(0); + sleVault->at(sfShareMPTID) = sharesMptId; + sleVault->at(sfWithdrawalPolicy) = + vaultStrategyFirstComeFirstServe; + + ac.view().insert(sleVault); + ac.view().insert(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + doInvariantCheck( + {"shares issuer and vault pseudo-account must be the same", + "shares issuer must exist"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const sequence = ac.view().seq(); + auto const vaultKeylet = keylet::vault(A1.id(), sequence); + auto sleVault = std::make_shared(vaultKeylet); + auto const vaultPage = ac.view().dirInsert( + keylet::ownerDir(A1.id()), + sleVault->key(), + describeOwnerDir(A1.id())); + sleVault->setFieldU64(sfOwnerNode, *vaultPage); + + auto const sharesMptId = makeMptID(sequence, A2.id()); + auto const sharesKeylet = keylet::mptIssuance(sharesMptId); + auto sleShares = std::make_shared(sharesKeylet); + auto const sharesPage = ac.view().dirInsert( + keylet::ownerDir(A2.id()), + sharesKeylet, + describeOwnerDir(A2.id())); + sleShares->setFieldU64(sfOwnerNode, *sharesPage); + + sleShares->at(sfFlags) = 0; + // Setting wrong pseudo acocunt ID + sleShares->at(sfIssuer) = AccountID(uint160(42)); + sleShares->at(sfOutstandingAmount) = 0; + sleShares->at(sfSequence) = sequence; + + sleVault->at(sfAccount) = A2.id(); + sleVault->at(sfFlags) = 0; + sleVault->at(sfSequence) = sequence; + sleVault->at(sfOwner) = A1.id(); + sleVault->at(sfAssetsTotal) = Number(0); + sleVault->at(sfAssetsAvailable) = Number(0); + sleVault->at(sfLossUnrealized) = Number(0); + sleVault->at(sfShareMPTID) = sharesMptId; + sleVault->at(sfWithdrawalPolicy) = + vaultStrategyFirstComeFirstServe; + + ac.view().insert(sleVault); + ac.view().insert(sleShares); + return true; + }, + XRPAmount{}, + STTx{ttVAULT_CREATE, [](STObject&) {}}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + + testcase << "Vault deposit"; + doInvariantCheck( + {"deposit must change vault balance"}, + [&](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(), 0, [&](Adjustements& sample) {})); + }, + XRPAmount{}, + STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"deposit assets outstanding must not exceed assets maximum"}, + [&](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(), 200, [&](Adjustements& sample) { + sample.assetsMaximum = 1; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { + tx.setFieldAmount(sfAmount, XRPAmount(200)); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + // This really convoluted unit tests makes the zero balance on the + // depositor, by sending them the same amount as the transaction fee. + // The operation makes no sense, but the defensive check in + // ValidVault::finalize is otherwise impossible to trigger. + doInvariantCheck( + {"deposit must increase vault balance", + "deposit must change depositor balance"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + + // Move 10 drops to A4 to enforce total XRP balance + auto sleA4 = ac.view().peek(keylet::account(A4.id())); + if (!sleA4) + return false; + (*sleA4)[sfBalance] = *(*sleA4)[sfBalance] + 10; + ac.view().update(sleA4); + + return adjust( + ac.view(), + keylet, + args(A3.id(), -10, [&](Adjustements& sample) { + sample.accountAssets->amount = -100; + })); + }, + XRPAmount{100}, + STTx{ + ttVAULT_DEPOSIT, + [&](STObject& tx) { + tx[sfFee] = XRPAmount(100); + tx[sfAccount] = A3.id(); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"deposit must increase vault balance", + "deposit must decrease depositor balance", + "deposit must change vault and depositor balance by equal amount", + "deposit and assets outstanding must add up", + "deposit and assets available must add up"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + + // Move 10 drops from A2 to A3 to enforce total XRP balance + auto sleA3 = ac.view().peek(keylet::account(A3.id())); + if (!sleA3) + return false; + (*sleA3)[sfBalance] = *(*sleA3)[sfBalance] + 10; + ac.view().update(sleA3); + + return adjust( + ac.view(), + keylet, + args(A2.id(), 10, [&](Adjustements& sample) { + sample.vaultAssets = -20; + sample.accountAssets->amount = 10; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"deposit must change depositor balance"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + + // Move 10 drops from A3 to vault to enforce total XRP balance + auto sleA3 = ac.view().peek(keylet::account(A3.id())); + if (!sleA3) + return false; + (*sleA3)[sfBalance] = *(*sleA3)[sfBalance] - 10; + ac.view().update(sleA3); + + return adjust( + ac.view(), + keylet, + args(A2.id(), 10, [&](Adjustements& sample) { + sample.accountAssets->amount = 0; + })); + }, + 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) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + return adjust( + ac.view(), + keylet, + args(A2.id(), 10, [&](Adjustements& sample) { + sample.accountShares->amount = 0; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"deposit must 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, [&](Adjustements& sample) { + sample.sharesTotal = 0; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"deposit must increase depositor shares", + "deposit must change depositor and vault shares by equal amount", + "deposit must not change vault balance by more than deposited " + "amount"}, + [&](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, [&](Adjustements& sample) { + sample.accountShares->amount = -5; + sample.sharesTotal = -10; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { tx[sfAmount] = XRPAmount(5); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"deposit and assets outstanding must add up", + "deposit and assets available must add up"}, + [&](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, [&](Adjustements& sample) { + sample.assetsTotal = 7; + sample.assetsAvailable = 7; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_DEPOSIT, + [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + testcase << "Vault withdrawal"; + doInvariantCheck( + {"withdrawal must change vault balance"}, + [&](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(), 0, [&](Adjustements& sample) {})); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + // Almost identical to the really convoluted test for deposit, where the + // depositor spends only the transaction fee. In case of withdrawal, + // this test is almost the same as normal withdrawal where the + // sfDestination would have been A4, but has been omitted. + doInvariantCheck( + {"withdrawal must change one destination balance"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + + // Move 10 drops to A4 to enforce total XRP balance + auto sleA4 = ac.view().peek(keylet::account(A4.id())); + if (!sleA4) + return false; + (*sleA4)[sfBalance] = *(*sleA4)[sfBalance] + 10; + ac.view().update(sleA4); + + return adjust( + ac.view(), + keylet, + args(A3.id(), -10, [&](Adjustements& sample) { + sample.accountAssets->amount = -100; + })); + }, + XRPAmount{100}, + STTx{ + ttVAULT_WITHDRAW, + [&](STObject& tx) { + tx[sfFee] = XRPAmount(100); + tx[sfAccount] = A3.id(); + // This commented out line causes the invariant violation. + // tx[sfDestination] = A4.id(); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + doInvariantCheck( + {"withdrawal must change vault and destination balance by " + "equal amount", + "withdrawal must decrease vault balance", + "withdrawal must increase destination balance", + "withdrawal and assets outstanding must add up", + "withdrawal and assets available must add up"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + + // Move 10 drops from A2 to A3 to enforce total XRP balance + auto sleA3 = ac.view().peek(keylet::account(A3.id())); + if (!sleA3) + return false; + (*sleA3)[sfBalance] = *(*sleA3)[sfBalance] + 10; + ac.view().update(sleA3); + + return adjust( + ac.view(), + keylet, + args(A2.id(), -10, [&](Adjustements& sample) { + sample.vaultAssets = 10; + sample.accountAssets->amount = -20; + })); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"withdrawal must change one destination balance"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq()); + if (!adjust( + ac.view(), + keylet, + args(A2.id(), -10, [&](Adjustements& sample) { + *sample.vaultAssets -= 5; + }))) + return false; + auto sleA3 = ac.view().peek(keylet::account(A3.id())); + if (!sleA3) + return false; + (*sleA3)[sfBalance] = *(*sleA3)[sfBalance] + 5; + ac.view().update(sleA3); + return true; + }, + XRPAmount{}, + STTx{ + ttVAULT_WITHDRAW, + [&](STObject& tx) { tx.setAccountID(sfDestination, A3.id()); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"withdrawal 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, [&](Adjustements& sample) { + sample.accountShares->amount = 0; + })); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"withdrawal must 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, [&](Adjustements& sample) { + sample.sharesTotal = 0; + })); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"withdrawal must decrease depositor shares", + "withdrawal must change depositor and vault shares by equal " + "amount"}, + [&](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, [&](Adjustements& sample) { + sample.accountShares->amount = 5; + sample.sharesTotal = 10; + })); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + doInvariantCheck( + {"withdrawal and assets outstanding must add up", + "withdrawal and assets available must add up"}, + [&](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, [&](Adjustements& sample) { + sample.assetsTotal = -15; + sample.assetsAvailable = -15; + })); + }, + XRPAmount{}, + STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp, + TxAccount::A2); + + auto const precloseMpt = + [&](Account const& A1, Account const& A2, Env& env) -> bool { + env.fund(XRP(1000), A3, A4); + + // Create MPT asset + { + Json::Value jv; + jv[sfAccount] = A3.human(); + jv[sfTransactionType] = jss::MPTokenIssuanceCreate; + jv[sfFlags] = tfMPTCanTransfer; + env(jv); + env.close(); + } + + auto const mptID = makeMptID(env.seq(A3) - 1, A3); + Asset asset = MPTIssue(mptID); + // Authorize A1 A2 A4 + { + Json::Value jv; + jv[sfAccount] = A1.human(); + jv[sfTransactionType] = jss::MPTokenAuthorize; + jv[sfMPTokenIssuanceID] = to_string(mptID); + env(jv); + jv[sfAccount] = A2.human(); + env(jv); + jv[sfAccount] = A4.human(); + env(jv); + + env.close(); + } + // Send tokens to A1 A2 A4 + { + env(pay(A3, A1, asset(1000))); + env(pay(A3, A2, asset(1000))); + env(pay(A3, A4, asset(1000))); + env.close(); + } + + Vault vault{env}; + auto [tx, keylet] = vault.create({.owner = A1, .asset = asset}); + env(tx); + env(vault.deposit( + {.depositor = A1, .id = keylet.key, .amount = asset(10)})); + env(vault.deposit( + {.depositor = A2, .id = keylet.key, .amount = asset(10)})); + env(vault.deposit( + {.depositor = A4, .id = keylet.key, .amount = asset(10)})); + return true; + }; + + doInvariantCheck( + {"withdrawal must decrease depositor shares", + "withdrawal must change depositor and vault shares by equal " + "amount"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A2.id(), -10, [&](Adjustements& sample) { + sample.accountShares->amount = 5; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_WITHDRAW, + [&](STObject& tx) { tx[sfAccount] = A3.id(); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt, + TxAccount::A2); + + testcase << "Vault clawback"; + doInvariantCheck( + {"clawback must change vault balance"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A2.id(), -1, [&](Adjustements& sample) { + sample.vaultAssets = 0; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_CLAWBACK, + [&](STObject& tx) { tx[sfAccount] = A3.id(); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt); + + // Not the same as below check: attempt to clawback XRP + doInvariantCheck( + {"clawback may only be performed by the asset issuer"}, + [&](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(), 0, [&](Adjustements& sample) {})); + }, + XRPAmount{}, + STTx{ttVAULT_CLAWBACK, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseXrp); + + // Not the same as above check: attempt to clawback MPT by bad account + doInvariantCheck( + {"clawback may only be performed by the asset issuer"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A2.id(), 0, [&](Adjustements& sample) {})); + }, + XRPAmount{}, + STTx{ + ttVAULT_CLAWBACK, + [&](STObject& tx) { tx[sfAccount] = A4.id(); }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt); + + doInvariantCheck( + {"clawback must decrease vault balance", + "clawback must decrease holder shares", + "clawback must change vault shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A4.id(), 10, [&](Adjustements& sample) { + sample.sharesTotal = 0; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_CLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = A3.id(); + tx[sfHolder] = A4.id(); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt); + + doInvariantCheck( + {"clawback must change holder shares"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A4.id(), -10, [&](Adjustements& sample) { + sample.accountShares->amount = 0; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_CLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = A3.id(); + tx[sfHolder] = A4.id(); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt); + + doInvariantCheck( + {"clawback must change holder and vault shares by equal amount", + "clawback and assets outstanding must add up", + "clawback and assets available must add up"}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + auto const keylet = keylet::vault(A1.id(), ac.view().seq() - 2); + return adjust( + ac.view(), + keylet, + args(A4.id(), -10, [&](Adjustements& sample) { + sample.accountShares->amount = -8; + sample.assetsTotal = -7; + sample.assetsAvailable = -7; + })); + }, + XRPAmount{}, + STTx{ + ttVAULT_CLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = A3.id(); + tx[sfHolder] = A4.id(); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseMpt); + } + public: void run() override @@ -1746,6 +3445,7 @@ public: testPermissionedDomainInvariants(); testValidPseudoAccounts(); testPermissionedDEX(); + testVault(); } }; diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 159bfd0796..4a731ddd57 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -177,6 +177,14 @@ class Vault_test : public beast::unit_test::suite env.close(); } + { + testcase(prefix + " set maximum is idempotent, set it again"); + auto tx = vault.set({.owner = owner, .id = keylet.key}); + tx[sfAssetsMaximum] = asset(150).number(); + env(tx); + env.close(); + } + { testcase(prefix + " set data"); auto tx = vault.set({.owner = owner, .id = keylet.key}); @@ -218,6 +226,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(1000)}); env(tx, ter(tecINSUFFICIENT_FUNDS)); + env.close(); } { @@ -385,6 +394,27 @@ class Vault_test : public beast::unit_test::suite env.balance(depositor, shares) == share(50 * scale)); } + if (!asset.raw().native()) + { + testcase(prefix + " issuer deposits"); + auto tx = vault.deposit( + {.depositor = issuer, + .id = keylet.key, + .amount = asset(10)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(issuer, shares) == share(10 * scale)); + + testcase(prefix + " issuer withdraws"); + tx = vault.withdraw( + {.depositor = issuer, + .id = keylet.key, + .amount = share(10 * scale)}); + env(tx); + env.close(); + BEAST_EXPECT(env.balance(issuer, shares) == share(0 * scale)); + } + { testcase(prefix + " withdraw remaining assets"); auto tx = vault.withdraw( @@ -454,6 +484,8 @@ class Vault_test : public beast::unit_test::suite .amount = asset(10)}); tx[sfDestination] = erin.human(); env(tx); + env.close(); + // Erin returns assets to issuer env(pay(erin, issuer, asset(10))); env.close(); @@ -479,12 +511,14 @@ class Vault_test : public beast::unit_test::suite testcase(prefix + " fail to delete because wrong owner"); auto tx = vault.del({.owner = issuer, .id = keylet.key}); env(tx, ter(tecNO_PERMISSION)); + env.close(); } { testcase(prefix + " delete empty vault"); auto tx = vault.del({.owner = owner, .id = keylet.key}); env(tx); + env.close(); BEAST_EXPECT(!env.le(keylet)); } }; @@ -1328,6 +1362,26 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; { + { + testcase("IOU fail because MPT is disabled"); + Env env{ + *this, + (testable_amendments() - featureMPTokensV1) | + featureSingleAssetVault}; + Account issuer{"issuer"}; + Account owner{"owner"}; + env.fund(XRP(1000), issuer, owner); + env.close(); + + Vault vault{env}; + Asset asset = issuer["IOU"].asset(); + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + + env(tx, ter(temDISABLED)); + env.close(); + } + { testcase("IOU fail create frozen"); Env env{*this, testable_amendments() | featureSingleAssetVault}; @@ -2878,6 +2932,12 @@ class Vault_test : public beast::unit_test::suite tx[sfDomainID] = to_string(domainId); env(tx); env.close(); + + // Should be idempotent + tx = vault.set({.owner = owner, .id = keylet.key}); + tx[sfDomainID] = to_string(domainId); + env(tx); + env.close(); } } @@ -3033,6 +3093,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = asset(50)}); env(tx); + env.close(); tx = vault.clawback( {.issuer = issuer, @@ -3047,6 +3108,7 @@ class Vault_test : public beast::unit_test::suite .holder = owner, .amount = asset(0)}); env(tx); + env.close(); tx = vault.del({ .owner = owner, @@ -3093,6 +3155,7 @@ class Vault_test : public beast::unit_test::suite auto tx = vault.deposit( {.depositor = owner, .id = keylet.key, .amount = asset(50)}); env(tx); + env.close(); } { diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 87ca9ea6c1..2cfcb6d258 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -24,17 +24,26 @@ #include #include +#include #include #include #include #include +#include +#include +#include +#include #include #include #include +#include #include #include #include +#include +#include + namespace ripple { /* @@ -78,6 +87,8 @@ enum Privilege { // object (except by issuer) mayDeleteMPT = 0x0400, // The transaction MAY delete an MPT object. May not create. + mustModifyVault = + 0x0800, // The transaction must modify, delete or create, a vault }; constexpr Privilege operator|(Privilege lhs, Privilege rhs) @@ -2170,4 +2181,943 @@ ValidAMM::finalize( return true; } +//------------------------------------------------------------------------------ + +ValidVault::Vault +ValidVault::Vault::make(SLE const& from) +{ + XRPL_ASSERT( + from.getType() == ltVAULT, + "ValidVault::Vault::make : from Vault object"); + + ValidVault::Vault self; + self.key = from.key(); + self.asset = from.at(sfAsset); + self.pseudoId = from.getAccountID(sfAccount); + self.shareMPTID = from.getFieldH192(sfShareMPTID); + self.assetsTotal = from.at(sfAssetsTotal); + self.assetsAvailable = from.at(sfAssetsAvailable); + self.assetsMaximum = from.at(sfAssetsMaximum); + self.lossUnrealized = from.at(sfLossUnrealized); + return self; +} + +ValidVault::Shares +ValidVault::Shares::make(SLE const& from) +{ + XRPL_ASSERT( + from.getType() == ltMPTOKEN_ISSUANCE, + "ValidVault::Shares::make : from MPTokenIssuance object"); + + 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); + return self; +} + +void +ValidVault::visitEntry( + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& 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 + // `isDelete` indicates whether an object is being deleted or modified. + XRPL_ASSERT( + after != nullptr && (before != nullptr || !isDelete), + "ripple::ValidVault::visitEntry : some object is available"); + + // `Number balance` 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 balance{}; + + // By default do not add anything to deltas + std::int8_t sign = 0; + if (before) + { + switch (before->getType()) + { + case ltVAULT: + beforeVault_.push_back(Vault::make(*before)); + break; + case ltMPTOKEN_ISSUANCE: + // 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)); + balance = static_cast( + before->getFieldU64(sfOutstandingAmount)); + sign = 1; + break; + case ltMPTOKEN: + balance = + static_cast(before->getFieldU64(sfMPTAmount)); + sign = -1; + break; + case ltACCOUNT_ROOT: + case ltRIPPLE_STATE: + balance = before->getFieldAmount(sfBalance); + sign = -1; + break; + default:; + } + } + + if (!isDelete && after) + { + switch (after->getType()) + { + case ltVAULT: + afterVault_.push_back(Vault::make(*after)); + break; + case ltMPTOKEN_ISSUANCE: + // 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)); + balance -= Number(static_cast( + after->getFieldU64(sfOutstandingAmount))); + sign = 1; + break; + case ltMPTOKEN: + balance -= Number( + static_cast(after->getFieldU64(sfMPTAmount))); + sign = -1; + break; + case ltACCOUNT_ROOT: + case ltRIPPLE_STATE: + balance -= Number(after->getFieldAmount(sfBalance)); + sign = -1; + break; + default:; + } + } + + uint256 const key = (before ? before->key() : after->key()); + if (sign && balance != zero) + deltas_[key] = balance * sign; +} + +bool +ValidVault::finalize( + STTx const& tx, + TER const ret, + XRPAmount const fee, + ReadView const& view, + beast::Journal const& j) +{ + bool const enforce = view.rules().enabled(featureSingleAssetVault); + + if (!isTesSuccess(ret)) + return true; // Do not perform checks + + if (afterVault_.empty() && beforeVault_.empty()) + { + if (hasPrivilege(tx, mustModifyVault)) + { + JLOG(j.fatal()) << // + "Invariant failed: vault operation succeeded without modifying " + "a vault"; + XRPL_ASSERT( + enforce, "ripple::ValidVault::finalize : vault noop invariant"); + return !enforce; + } + + return true; // Not a vault operation + } + else if (!hasPrivilege(tx, mustModifyVault)) // TODO: mayModifyVault + { + JLOG(j.fatal()) << // + "Invariant failed: vault updated by a wrong transaction type"; + XRPL_ASSERT( + enforce, + "ripple::ValidVault::finalize : illegal vault transaction " + "invariant"); + return !enforce; // Also not a vault operation + } + + if (beforeVault_.size() > 1 || afterVault_.size() > 1) + { + JLOG(j.fatal()) << // + "Invariant failed: vault operation updated more than single vault"; + XRPL_ASSERT( + enforce, "ripple::ValidVault::finalize : single vault invariant"); + return !enforce; // That's all we can do here + } + + auto const txnType = tx.getTxnType(); + + // We do special handling for ttVAULT_DELETE first, because it's the only + // vault-modifying transaction without an "after" state of the vault + if (afterVault_.empty()) + { + if (txnType != ttVAULT_DELETE) + { + JLOG(j.fatal()) << // + "Invariant failed: vault deleted by a wrong transaction type"; + XRPL_ASSERT( + enforce, + "ripple::ValidVault::finalize : illegal vault deletion " + "invariant"); + return !enforce; // That's all we can do here + } + + // Note, if afterVault_ is empty then we know that beforeVault_ is not + // empty, as enforced at the top of this function + auto const& beforeVault = beforeVault_[0]; + + // At this moment we only know a vault is being deleted and there + // might be some MPTokenIssuance objects which are deleted in the + // same transaction. Find the one matching this vault. + auto const deletedShares = [&]() -> std::optional { + for (auto const& e : beforeMPTs_) + { + if (e.share.getMptID() == beforeVault.shareMPTID) + return std::move(e); + } + return std::nullopt; + }(); + + if (!deletedShares) + { + JLOG(j.fatal()) << "Invariant failed: deleted vault must also " + "delete shares"; + XRPL_ASSERT( + enforce, + "ripple::ValidVault::finalize : shares deletion invariant"); + return !enforce; // That's all we can do here + } + + bool result = true; + if (deletedShares->sharesTotal != 0) + { + JLOG(j.fatal()) << "Invariant failed: deleted vault must have no " + "shares outstanding"; + result = false; + } + if (beforeVault.assetsTotal != zero) + { + JLOG(j.fatal()) << "Invariant failed: deleted vault must have no " + "assets outstanding"; + result = false; + } + if (beforeVault.assetsAvailable != zero) + { + JLOG(j.fatal()) << "Invariant failed: deleted vault must have no " + "assets available"; + result = false; + } + + return result; + } + else if (txnType == ttVAULT_DELETE) + { + JLOG(j.fatal()) << "Invariant failed: vault deletion succeeded without " + "deleting a vault"; + XRPL_ASSERT( + enforce, "ripple::ValidVault::finalize : vault deletion invariant"); + return !enforce; // That's all we can do here + } + + // Note, `afterVault_.empty()` is handled above + auto const& afterVault = afterVault_[0]; + XRPL_ASSERT( + beforeVault_.empty() || beforeVault_[0].key == afterVault.key, + "ripple::ValidVault::finalize : single vault operation"); + + auto const updatedShares = [&]() -> std::optional { + // At this moment we only know that a vault is being updated and there + // might be some MPTokenIssuance objects which are also updated in the + // same transaction. Find the one matching the shares to this vault. + // Note, we expect updatedMPTs collection to be extremely small. For + // such collections linear search is faster than lookup. + for (auto const& e : afterMPTs_) + { + if (e.share.getMptID() == afterVault.shareMPTID) + return e; + } + + auto const sleShares = + view.read(keylet::mptIssuance(afterVault.shareMPTID)); + + return sleShares ? std::optional(Shares::make(*sleShares)) + : std::nullopt; + }(); + + bool result = true; + + // Universal transaction checks + if (!beforeVault_.empty()) + { + auto const& beforeVault = beforeVault_[0]; + if (afterVault.asset != beforeVault.asset || + afterVault.pseudoId != beforeVault.pseudoId || + afterVault.shareMPTID != beforeVault.shareMPTID) + { + JLOG(j.fatal()) + << "Invariant failed: violation of vault immutable data"; + result = false; + } + } + + if (!updatedShares) + { + JLOG(j.fatal()) << "Invariant failed: updated vault must have shares"; + XRPL_ASSERT( + enforce, + "ripple::ValidVault::finalize : vault has shares invariant"); + return !enforce; // That's all we can do here + } + + if (updatedShares->sharesTotal == 0) + { + if (afterVault.assetsTotal != zero) + { + JLOG(j.fatal()) << "Invariant failed: updated zero sized " + "vault must have no assets outstanding"; + result = false; + } + if (afterVault.assetsAvailable != zero) + { + JLOG(j.fatal()) << "Invariant failed: updated zero sized " + "vault must have no assets available"; + result = false; + } + } + else if (updatedShares->sharesTotal > updatedShares->sharesMaximum) + { + JLOG(j.fatal()) // + << "Invariant failed: updated shares must not exceed maximum " + << updatedShares->sharesMaximum; + result = false; + } + + if (afterVault.assetsAvailable < zero) + { + JLOG(j.fatal()) + << "Invariant failed: assets available must be positive"; + result = false; + } + + if (afterVault.assetsAvailable > afterVault.assetsTotal) + { + JLOG(j.fatal()) << "Invariant failed: assets available must " + "not be greater than assets outstanding"; + result = false; + } + else if ( + afterVault.lossUnrealized > + afterVault.assetsTotal - afterVault.assetsAvailable) + { + JLOG(j.fatal()) // + << "Invariant failed: loss unrealized must not exceed " + "the difference between assets outstanding and available"; + result = false; + } + + if (afterVault.assetsTotal < zero) + { + JLOG(j.fatal()) + << "Invariant failed: assets outstanding must be positive"; + result = false; + } + + if (afterVault.assetsMaximum < zero) + { + JLOG(j.fatal()) << "Invariant failed: assets maximum must be positive"; + result = false; + } + + // Thanks to this check we can simply do `assert(!beforeVault_.empty()` when + // enforcing invariants on transaction types other than ttVAULT_CREATE + if (beforeVault_.empty() && txnType != ttVAULT_CREATE) + { + JLOG(j.fatal()) << // + "Invariant failed: vault created by a wrong transaction type"; + XRPL_ASSERT( + enforce, "ripple::ValidVault::finalize : vault creation invariant"); + return !enforce; // That's all we can do here + } + + if (!beforeVault_.empty() && + afterVault.lossUnrealized != beforeVault_[0].lossUnrealized) + { + JLOG(j.fatal()) << // + "Invariant failed: vault transaction must not change loss " + "unrealized"; + result = false; + } + + auto const beforeShares = [&]() -> std::optional { + if (beforeVault_.empty()) + return std::nullopt; + auto const& beforeVault = beforeVault_[0]; + + for (auto const& e : beforeMPTs_) + { + if (e.share.getMptID() == beforeVault.shareMPTID) + return std::move(e); + } + return std::nullopt; + }(); + + if (!beforeShares && + (tx.getTxnType() == ttVAULT_DEPOSIT || // + tx.getTxnType() == ttVAULT_WITHDRAW || // + tx.getTxnType() == ttVAULT_CLAWBACK)) + { + JLOG(j.fatal()) << "Invariant failed: vault operation succeeded " + "without updating shares"; + XRPL_ASSERT( + enforce, "ripple::ValidVault::finalize : shares noop invariant"); + return !enforce; // That's all we can do here + } + + 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 it->second * sign; + }; + + 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 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; + }; + + // Technically this does not need to be a lambda, but it's more + // 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; + + 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.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) + { + 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(), + "ripple::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(), + "ripple::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 = + [&]() -> std::optional { + if (auto ret = deltaAssets(tx[sfAccount]); ret) + { + // Compensate for transaction fee deduced from + // sfAccount + if (vaultAsset.native()) + *ret += fee.drops(); + if (*ret != zero) + return ret; + } + return std::nullopt; + }(); + + 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; + } + + auto const accountDeltaShares = deltaShares(tx[sfAccount]); + if (!accountDeltaShares) + { + JLOG(j.fatal()) << // + "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"; + result = false; + } + + auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); + if (!vaultDeltaShares) + { + JLOG(j.fatal()) << // + "Invariant failed: deposit must change vault shares"; + return false; // That's all we can do + } + + if (*vaultDeltaShares * -1 != *accountDeltaShares) + { + JLOG(j.fatal()) << // + "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(), + "ripple::ValidVault::finalize : withdrawal updated a " + "vault"); + auto const& beforeVault = beforeVault_[0]; + + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + + 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 = + [&]() -> std::optional { + if (auto ret = deltaAssets(tx[sfAccount]); ret) + { + // Compensate for transaction fee deduced from + // sfAccount + if (vaultAsset.native()) + *ret += fee.drops(); + if (*ret != zero) + return ret; + } + return std::nullopt; + }(); + + 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) + { + 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) + { + 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(), + "ripple::ValidVault::finalize : clawback updated a vault"); + auto const& beforeVault = beforeVault_[0]; + + if (vaultAsset.native() || + vaultAsset.getIssuer() != tx[sfAccount]) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback may only be performed by " + "the asset issuer"; + return false; // That's all we can do + } + + auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId); + + if (!vaultDeltaAssets) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must change vault balance"; + return false; // That's all we can do + } + + if (*vaultDeltaAssets >= zero) + { + JLOG(j.fatal()) << // + "Invariant failed: clawback must decrease vault " + "balance"; + result = false; + } + + 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) + { + 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; + } + + 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; + } + + return result; + } + + default: + // LCOV_EXCL_START + UNREACHABLE( + "ripple::ValidVault::finalize : unknown transaction type"); + return false; + // LCOV_EXCL_STOP + } + }(); + + if (!result) + { + // The comment at the top of this file starting with "assert(enforce)" + // explains this assert. + XRPL_ASSERT(enforce, "ripple::ValidVault::finalize : vault invariants"); + return !enforce; + } + + return true; +} + } // namespace ripple diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 5444f2f3a9..97d51f0fab 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -20,8 +20,10 @@ #ifndef RIPPLE_APP_TX_INVARIANTCHECK_H_INCLUDED #define RIPPLE_APP_TX_INVARIANTCHECK_H_INCLUDED +#include #include #include +#include #include #include #include @@ -732,6 +734,74 @@ private: beast::Journal const&) const; }; +/** + * @brief Invariants: Vault object and MPTokenIssuance for vault shares + * + * - vault deleted and vault created is empty + * - vault created must be linked to pseudo-account for shares and assets + * - vault must have MPTokenIssuance for shares + * - vault without shares outstanding must have no shares + * - loss unrealized does not exceed the difference between assets total and + * assets available + * - assets available do not exceed assets total + * - vault deposit increases assets and share issuance, and adds to: + * total assets, assets available, shares outstanding + * - vault withdrawal and clawback reduce assets and share issuance, and + * subtracts from: total assets, assets available, shares outstanding + * - vault set must not alter the vault assets or shares balance + * - no vault transaction can change loss unrealized (it's updated by loan + * transactions) + * + */ +class ValidVault +{ + Number static constexpr zero{}; + + struct Vault final + { + uint256 key = beast::zero; + Asset asset = {}; + AccountID pseudoId = {}; + uint192 shareMPTID = beast::zero; + Number assetsTotal = 0; + Number assetsAvailable = 0; + Number assetsMaximum = 0; + Number lossUnrealized = 0; + + Vault static make(SLE const&); + }; + + struct Shares final + { + MPTIssue share = {}; + std::uint64_t sharesTotal = 0; + std::uint64_t sharesMaximum = 0; + + Shares static make(SLE const&); + }; + + std::vector afterVault_ = {}; + std::vector afterMPTs_ = {}; + std::vector beforeVault_ = {}; + std::vector beforeMPTs_ = {}; + std::unordered_map deltas_ = {}; + +public: + void + visitEntry( + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize( + STTx const&, + TER const, + XRPAmount const, + ReadView const&, + beast::Journal const&); +}; + // additional invariant checks can be declared above and then added to this // tuple using InvariantChecks = std::tuple< @@ -754,7 +824,8 @@ using InvariantChecks = std::tuple< ValidPermissionedDomain, ValidPermissionedDEX, ValidAMM, - ValidPseudoAccounts>; + ValidPseudoAccounts, + ValidVault>; /** * @brief get a tuple of all invariant checks diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 75cf81b0b0..80382934ad 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -156,7 +156,10 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) !isTesSuccess(ter)) return ter; - if (accountHolds( + // Asset issuer does not have any balance, they can just create funds by + // depositing in the vault. + if ((vaultAsset.native() || vaultAsset.getIssuer() != account) && + accountHolds( ctx.view, account, vaultAsset, diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 6057e40cfa..170a850a36 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -183,6 +183,9 @@ VaultSet::doApply() view().update(sleIssuance); } + // Note, we must update Vault object even if only DomainID is being updated + // in Issuance object. Otherwise it's really difficult for Vault invariants + // to verify the operation. view().update(vault); return tesSUCCESS; From 46ba8a28fecd0665900e3a1caf276a426ad6640a Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 9 Oct 2025 13:27:26 -0400 Subject: [PATCH 07/18] refactor: Update Conan dependencies: OpenSSL (#5873) This change bumps OpenSSL from 1.1.1w to 3.6.0. --- .github/scripts/strategy-matrix/linux.json | 12 ++++++------ cmake/RippledCompiler.cmake | 17 ++++++++++------- conan.lock | 2 +- conanfile.py | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 08313daf0a..bae5c57087 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -78,42 +78,42 @@ "distro_version": "9", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "0ab1e4c" + "image_sha": "d133ce3" }, { "distro_name": "ubuntu", diff --git a/cmake/RippledCompiler.cmake b/cmake/RippledCompiler.cmake index bc3a62a48c..4d16222cbe 100644 --- a/cmake/RippledCompiler.cmake +++ b/cmake/RippledCompiler.cmake @@ -16,13 +16,16 @@ set(CMAKE_CXX_EXTENSIONS OFF) target_compile_definitions (common INTERFACE $<$:DEBUG _DEBUG> - $<$,$>>:NDEBUG>) - # ^^^^ NOTE: CMAKE release builds already have NDEBUG - # defined, so no need to add it explicitly except for - # this special case of (profile ON) and (assert OFF) - # -- presumably this is because we don't want profile - # builds asserting unless asserts were specifically - # requested + #[===[ + NOTE: CMAKE release builds already have NDEBUG defined, so no need to add it + explicitly except for the special case of (profile ON) and (assert OFF). + Presumably this is because we don't want profile builds asserting unless + asserts were specifically requested. + ]===] + $<$,$>>:NDEBUG> + # TODO: Remove once we have migrated functions from OpenSSL 1.x to 3.x. + OPENSSL_SUPPRESS_DEPRECATED +) if (MSVC) # remove existing exception flag since we set it to -EHa diff --git a/conan.lock b/conan.lock index ec790e16ce..9f52c606a7 100644 --- a/conan.lock +++ b/conan.lock @@ -9,7 +9,7 @@ "rocksdb/10.0.1#85537f46e538974d67da0c3977de48ac%1756234304.347", "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976", "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", - "openssl/1.1.1w#a8f0792d7c5121b954578a7149d23e03%1756223730.729", + "openssl/3.6.0#89e8af1d4a21afcac0557079d23d8890%1759746682.365", "nudb/2.0.9#c62cfd501e57055a7e0d8ee3d5e5427d%1756234237.107", "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504%1756234228.999", "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64", diff --git a/conanfile.py b/conanfile.py index 3146b887e0..4cd9bd3c5a 100644 --- a/conanfile.py +++ b/conanfile.py @@ -27,7 +27,7 @@ class Xrpl(ConanFile): 'grpc/1.50.1', 'libarchive/3.8.1', 'nudb/2.0.9', - 'openssl/1.1.1w', + 'openssl/3.6.0', 'soci/4.0.3', 'zlib/1.3.1', ] From 3c88786bb0d2c10a04ab991472224aa492d15aee Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 10 Oct 2025 10:18:24 -0400 Subject: [PATCH 08/18] refactor: Downgrades OpenSSL to 3.5.4 (#5878) This change downgrades OpenSSL 3.6.0 to 3.5.4. To avoid potential zero-day issues in a new major version of OpenSSL, 3.6.0, it is safer to stick with 3.5.4. While 3.6.0 has some nice new features, such as improved SHA512 hashing, it also introduces new features that could contain bugs. In contrast, 3.5.4 has seen quite a few bug fixes over 3.5.0 and has been used in the wild for a while now. --- conan.lock | 2 +- conanfile.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conan.lock b/conan.lock index 9f52c606a7..cb25777423 100644 --- a/conan.lock +++ b/conan.lock @@ -9,7 +9,7 @@ "rocksdb/10.0.1#85537f46e538974d67da0c3977de48ac%1756234304.347", "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976", "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", - "openssl/3.6.0#89e8af1d4a21afcac0557079d23d8890%1759746682.365", + "openssl/3.5.4#a1d5835cc6ed5c5b8f3cd5b9b5d24205%1759746684.671", "nudb/2.0.9#c62cfd501e57055a7e0d8ee3d5e5427d%1756234237.107", "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504%1756234228.999", "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64", diff --git a/conanfile.py b/conanfile.py index 4cd9bd3c5a..7f8ab24fbd 100644 --- a/conanfile.py +++ b/conanfile.py @@ -27,7 +27,7 @@ class Xrpl(ConanFile): 'grpc/1.50.1', 'libarchive/3.8.1', 'nudb/2.0.9', - 'openssl/3.6.0', + 'openssl/3.5.4', 'soci/4.0.3', 'zlib/1.3.1', ] From 8456b8275e9d0491be77e2c59c831bdabab27510 Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 10 Oct 2025 12:22:42 -0400 Subject: [PATCH 09/18] chore: Add wildcard to support triggering for release pipelines (#5879) This change adds a wildcard to the release branch in the CI pipeline spec. Namely, after adopting an improved release process, with release branches that now look like release-X.Y, the trigger pipeline was no longer running as it only searched for an exact match to release. --- .github/workflows/on-trigger.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index b06d475a4d..9d2ea81520 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -9,9 +9,9 @@ name: Trigger on: push: branches: - - develop - - release - - master + - "develop" + - "release*" + - "master" paths: # These paths are unique to `on-trigger.yml`. - ".github/workflows/reusable-check-missing-commits.yml" From 8637d606a4dca70fa2e3a4229aa8361e99febb6e Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 13 Oct 2025 10:56:18 -0400 Subject: [PATCH 10/18] chore: Exclude code/unreachable transaction code from Codecov (#5847) This change excludes from Codecov unreachable/difficult-to-test transaction code (such as `tecINTERNAL`) and old code (from amendments that have been enabled for a long time that are only around for ledger replay reasons). This removes about 200 lines of misses and increases the Codecov coverage by 0.3% (79.2% to 79.5%). --- src/libxrpl/ledger/CredentialHelpers.cpp | 12 ++-- src/libxrpl/ledger/View.cpp | 63 ++++++++++++------- src/xrpld/app/misc/NetworkOPs.cpp | 21 ++++++- src/xrpld/app/tx/detail/AMMBid.cpp | 2 + src/xrpld/app/tx/detail/AMMCreate.cpp | 4 +- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 4 +- src/xrpld/app/tx/detail/CancelCheck.cpp | 4 ++ src/xrpld/app/tx/detail/CancelOffer.cpp | 2 +- src/xrpld/app/tx/detail/CashCheck.cpp | 10 +++ src/xrpld/app/tx/detail/Clawback.cpp | 2 +- src/xrpld/app/tx/detail/CreateCheck.cpp | 6 +- src/xrpld/app/tx/detail/CreateOffer.cpp | 4 ++ src/xrpld/app/tx/detail/CreateTicket.cpp | 6 +- src/xrpld/app/tx/detail/Credentials.cpp | 12 ++-- src/xrpld/app/tx/detail/DID.cpp | 8 ++- src/xrpld/app/tx/detail/DeleteAccount.cpp | 6 +- src/xrpld/app/tx/detail/DepositPreauth.cpp | 14 +++-- src/xrpld/app/tx/detail/Escrow.cpp | 46 ++++++++++---- src/xrpld/app/tx/detail/LedgerStateFix.cpp | 4 +- src/xrpld/app/tx/detail/MPTokenAuthorize.cpp | 2 +- .../app/tx/detail/MPTokenIssuanceDestroy.cpp | 4 +- .../app/tx/detail/MPTokenIssuanceSet.cpp | 2 +- .../app/tx/detail/NFTokenAcceptOffer.cpp | 12 ++-- .../app/tx/detail/NFTokenCancelOffer.cpp | 2 + src/xrpld/app/tx/detail/NFTokenMint.cpp | 2 +- src/xrpld/app/tx/detail/NFTokenUtils.cpp | 4 +- src/xrpld/app/tx/detail/PayChan.cpp | 14 +++-- src/xrpld/app/tx/detail/Payment.cpp | 2 +- .../tx/detail/PermissionedDomainDelete.cpp | 8 ++- src/xrpld/app/tx/detail/SetAccount.cpp | 2 +- src/xrpld/app/tx/detail/SetRegularKey.cpp | 2 +- src/xrpld/app/tx/detail/SetSignerList.cpp | 8 ++- src/xrpld/app/tx/detail/SetTrust.cpp | 2 +- src/xrpld/app/tx/detail/Transactor.cpp | 8 +++ src/xrpld/app/tx/detail/XChainBridge.cpp | 55 +++++++++------- src/xrpld/rpc/detail/ServerHandler.cpp | 4 ++ src/xrpld/rpc/handlers/PayChanClaim.cpp | 2 + src/xrpld/rpc/handlers/Random.cpp | 2 +- 38 files changed, 246 insertions(+), 121 deletions(-) diff --git a/src/libxrpl/ledger/CredentialHelpers.cpp b/src/libxrpl/ledger/CredentialHelpers.cpp index 965d6f6911..4c625f5eee 100644 --- a/src/libxrpl/ledger/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/CredentialHelpers.cpp @@ -77,19 +77,23 @@ deleteSLE( AccountID const& account, SField const& node, bool isOwner) -> TER { auto const sleAccount = view.peek(keylet::account(account)); if (!sleAccount) - { // LCOV_EXCL_START + { + // LCOV_EXCL_START JLOG(j.fatal()) << "Internal error: can't retrieve Owner account."; return tecINTERNAL; - } // LCOV_EXCL_STOP + // LCOV_EXCL_STOP + } // Remove object from owner directory std::uint64_t const page = sleCredential->getFieldU64(node); if (!view.dirRemove( keylet::ownerDir(account), page, sleCredential->key(), false)) - { // LCOV_EXCL_START + { + // LCOV_EXCL_START JLOG(j.fatal()) << "Unable to delete Credential from owner."; return tefBAD_LEDGER; - } // LCOV_EXCL_STOP + // LCOV_EXCL_STOP + } if (isOwner) adjustOwnerCount(view, sleAccount, -1, j); diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 89d8137ac7..ace7a34f81 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -1292,7 +1292,7 @@ authorizeMPToken( { auto const sleAcct = view.peek(keylet::account(account)); if (!sleAcct) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE // If the account that submitted the tx is a holder // Note: `account_` is holder's account @@ -1357,17 +1357,17 @@ authorizeMPToken( auto const sleMptIssuance = view.read(keylet::mptIssuance(mptIssuanceID)); if (!sleMptIssuance) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE // If the account that submitted this tx is the issuer of the MPT // Note: `account_` is issuer's account // `holderID` is holder's account if (account != (*sleMptIssuance)[sfIssuer]) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const sleMpt = view.peek(keylet::mptoken(mptIssuanceID, *holderID)); if (!sleMpt) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE std::uint32_t const flagsIn = sleMpt->getFieldU32(sfFlags); std::uint32_t flagsOut = flagsIn; @@ -1424,7 +1424,7 @@ trustCreate( describeOwnerDir(uLowAccountID)); if (!lowNode) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE auto highNode = view.dirInsert( keylet::ownerDir(uHighAccountID), @@ -1432,14 +1432,14 @@ trustCreate( describeOwnerDir(uHighAccountID)); if (!highNode) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE bool const bSetDst = saLimit.getIssuer() == uDstAccountID; bool const bSetHigh = bSrcHigh ^ bSetDst; XRPL_ASSERT(sleAccount, "ripple::trustCreate : non-null SLE"); if (!sleAccount) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE XRPL_ASSERT( sleAccount->getAccountID(sfAccount) == @@ -1518,10 +1518,12 @@ removeEmptyHolding( { auto const sle = view.read(keylet::account(accountID)); if (!sle) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE + auto const balance = sle->getFieldAmount(sfBalance); if (balance.xrp() != 0) return tecHAS_OBLIGATIONS; + return tesSUCCESS; } @@ -1539,7 +1541,8 @@ removeEmptyHolding( auto sleLowAccount = view.peek(keylet::account(line->at(sfLowLimit)->getIssuer())); if (!sleLowAccount) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE + adjustOwnerCount(view, sleLowAccount, -1, journal); // It's not really necessary to clear the reserve flag, since the line // is about to be deleted, but this will make the metadata reflect an @@ -1553,7 +1556,8 @@ removeEmptyHolding( auto sleHighAccount = view.peek(keylet::account(line->at(sfHighLimit)->getIssuer())); if (!sleHighAccount) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE + adjustOwnerCount(view, sleHighAccount, -1, journal); // It's not really necessary to clear the reserve flag, since the line // is about to be deleted, but this will make the metadata reflect an @@ -1613,7 +1617,7 @@ trustDelete( sleRippleState->key(), false)) { - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE } JLOG(j.trace()) << "trustDelete: Deleting ripple line: high"; @@ -1624,7 +1628,7 @@ trustDelete( sleRippleState->key(), false)) { - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE } JLOG(j.trace()) << "trustDelete: Deleting ripple line: state"; @@ -1650,7 +1654,7 @@ offerDelete(ApplyView& view, std::shared_ptr const& sle, beast::Journal j) offerIndex, false)) { - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE } if (!view.dirRemove( @@ -1659,7 +1663,7 @@ offerDelete(ApplyView& view, std::shared_ptr const& sle, beast::Journal j) offerIndex, false)) { - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE } if (sle->isFieldPresent(sfAdditionalBooks)) @@ -1823,7 +1827,7 @@ rippleCreditIOU( auto const sleAccount = view.peek(keylet::account(uReceiverID)); if (!sleAccount) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE bool const noRipple = (sleAccount->getFlags() & lsfDefaultRipple) == 0; @@ -1913,14 +1917,16 @@ accountSendIOU( { if (saAmount < beast::zero || saAmount.holds()) { - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } } else { + // LCOV_EXCL_START XRPL_ASSERT( saAmount >= beast::zero && !saAmount.holds(), "ripple::accountSendIOU : minimum amount and not MPT"); + // LCOV_EXCL_STOP } /* If we aren't sending anything or if the sender is the same as the @@ -1977,8 +1983,10 @@ accountSendIOU( { // VFALCO Its laborious to have to mutate the // TER based on params everywhere + // LCOV_EXCL_START terResult = view.open() ? TER{telFAILED_PROCESSING} : TER{tecFAILED_PROCESSING}; + // LCOV_EXCL_STOP } else { @@ -2065,7 +2073,7 @@ rippleCreditMPT( view.update(sleIssuance); } else - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } else { @@ -2325,7 +2333,7 @@ issueIOU( auto const receiverAccount = view.peek(keylet::account(account)); if (!receiverAccount) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE bool noRipple = (receiverAccount->getFlags() & lsfDefaultRipple) == 0; @@ -2413,11 +2421,13 @@ redeemIOU( // In order to hold an IOU, a trust line *MUST* exist to track the // balance. If it doesn't, then something is very wrong. Don't try // to continue. + // LCOV_EXCL_START JLOG(j.fatal()) << "redeemIOU: " << to_string(account) << " attempts to redeem " << amount.getFullText() << " but no trust line exists!"; return tefINTERNAL; + // LCOV_EXCL_STOP } TER @@ -2437,7 +2447,7 @@ transferXRP( SLE::pointer const sender = view.peek(keylet::account(from)); SLE::pointer const receiver = view.peek(keylet::account(to)); if (!sender || !receiver) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE JLOG(j.trace()) << "transferXRP: " << to_string(from) << " -> " << to_string(to) << ") : " << amount.getFullText(); @@ -2447,8 +2457,10 @@ transferXRP( // VFALCO Its unfortunate we have to keep // mutating these TER everywhere // FIXME: this logic should be moved to callers maybe? + // LCOV_EXCL_START return view.open() ? TER{telFAILED_PROCESSING} : TER{tecFAILED_PROCESSING}; + // LCOV_EXCL_STOP } // Decrement XRP balance. @@ -2729,11 +2741,13 @@ cleanupOnAccountDelete( if (!sleItem) { // Directory node has an invalid index. Bail out. + // LCOV_EXCL_START JLOG(j.fatal()) << "DeleteAccount: Directory node in ledger " << view.seq() << " has index to object that is missing: " << to_string(dirEntry); return tefBAD_LEDGER; + // LCOV_EXCL_STOP } LedgerEntryType const nodeType{safe_cast( @@ -2766,9 +2780,11 @@ cleanupOnAccountDelete( "ripple::cleanupOnAccountDelete : minimum dir entries"); if (uDirEntry == 0) { + // LCOV_EXCL_START JLOG(j.error()) << "DeleteAccount iterator re-validation failed."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } if (skipEntry == SkipEntry::No) uDirEntry--; @@ -2788,7 +2804,7 @@ deleteAMMTrustLine( beast::Journal j) { if (!sleState || sleState->getType() != ltRIPPLE_STATE) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const& [low, high] = std::minmax( sleState->getFieldAmount(sfLowLimit).getIssuer(), @@ -2796,13 +2812,14 @@ deleteAMMTrustLine( auto sleLow = view.peek(keylet::account(low)); auto sleHigh = view.peek(keylet::account(high)); if (!sleLow || !sleHigh) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE + bool const ammLow = sleLow->isFieldPresent(sfAMMID); bool const ammHigh = sleHigh->isFieldPresent(sfAMMID); // can't both be AMM if (ammLow && ammHigh) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE // at least one must be if (!ammLow && !ammHigh) @@ -2822,7 +2839,7 @@ deleteAMMTrustLine( auto const uFlags = !ammLow ? lsfLowReserve : lsfHighReserve; if (!(sleState->getFlags() & uFlags)) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE adjustOwnerCount(view, !ammLow ? sleLow : sleHigh, -1, j); diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index e72b2732d0..3154426696 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -3728,6 +3728,9 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) if (databaseType == DatabaseType::None) { + // LCOV_EXCL_START + UNREACHABLE( + "ripple::NetworkOPsImp::addAccountHistoryJob : no database"); JLOG(m_journal.error()) << "AccountHistory job for account " << toBase58(subInfo.index_->accountId_) << " no database"; @@ -3737,6 +3740,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) unsubAccountHistory(sptr, subInfo.index_->accountId_, false); } return; + // LCOV_EXCL_STOP } app_.getJobQueue().addJob( @@ -3836,7 +3840,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) // LCOV_EXCL_START default: { UNREACHABLE( - "ripple::NetworkOPsImp::addAccountHistoryJob::" + "ripple::NetworkOPsImp::addAccountHistoryJob : " "getMoreTxns : invalid database type"); return {}; } @@ -3901,11 +3905,16 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) getMoreTxns(startLedgerSeq, lastLedgerSeq, marker); if (!dbResult) { + // LCOV_EXCL_START + UNREACHABLE( + "ripple::NetworkOPsImp::addAccountHistoryJob : " + "getMoreTxns failed"); JLOG(m_journal.debug()) << "AccountHistory job for account " << toBase58(accountId) << " getMoreTxns failed."; send(rpcError(rpcINTERNAL), true); return; + // LCOV_EXCL_STOP } auto const& txns = dbResult->first; @@ -3928,22 +3937,32 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) tx->getLedger()); if (!curTxLedger) { + // LCOV_EXCL_START + UNREACHABLE( + "ripple::NetworkOPsImp::addAccountHistoryJob : " + "getLedgerBySeq failed"); JLOG(m_journal.debug()) << "AccountHistory job for account " << toBase58(accountId) << " no ledger."; send(rpcError(rpcINTERNAL), true); return; + // LCOV_EXCL_STOP } std::shared_ptr stTxn = tx->getSTransaction(); if (!stTxn) { + // LCOV_EXCL_START + UNREACHABLE( + "NetworkOPsImp::addAccountHistoryJob : " + "getSTransaction failed"); JLOG(m_journal.debug()) << "AccountHistory job for account " << toBase58(accountId) << " getSTransaction failed."; send(rpcError(rpcINTERNAL), true); return; + // LCOV_EXCL_STOP } auto const mRef = std::ref(*meta); diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index 769668b07b..028c1aac8b 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -247,10 +247,12 @@ applyBid( if (saBurn >= lptAMMBalance) { // This error case should never occur. + // LCOV_EXCL_START JLOG(ctx_.journal.fatal()) << "AMM Bid: LP Token burn exceeds AMM balance " << burn << " " << lptAMMBalance; return tecINTERNAL; + // LCOV_EXCL_STOP } auto res = redeemIOU(sb, account_, saBurn, lpTokens.issue(), ctx_.journal); diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 63e20b42fb..a515783fb6 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -197,7 +197,7 @@ AMMCreate::preclaim(PreclaimContext const& ctx) return tesSUCCESS; if (auto const sle = ctx.view.read(keylet::account(issue.account)); !sle) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE else if (sle->getFlags() & lsfAllowTrustLineClawback) return tecNO_PERMISSION; return tesSUCCESS; @@ -291,7 +291,7 @@ applyCreate( if (SLE::pointer sleRippleState = sb.peek(keylet::line(accountId, amount.issue())); !sleRippleState) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE else { auto const flags = sleRippleState->getFlags(); diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index f5af9dfb9c..8b6b58013a 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -196,9 +196,11 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) if (amountBalance <= beast::zero || amount2Balance <= beast::zero || lptAMMBalance < beast::zero) { + // LCOV_EXCL_START JLOG(ctx.j.debug()) << "AMM Withdraw: reserves or tokens balance is zero."; - return tecINTERNAL; // LCOV_EXCL_LINE + return tecINTERNAL; + // LCOV_EXCL_STOP } auto const ammAccountID = ammSle->getAccountID(sfAccount); diff --git a/src/xrpld/app/tx/detail/CancelCheck.cpp b/src/xrpld/app/tx/detail/CancelCheck.cpp index f1a9b42a89..9dc792307c 100644 --- a/src/xrpld/app/tx/detail/CancelCheck.cpp +++ b/src/xrpld/app/tx/detail/CancelCheck.cpp @@ -93,8 +93,10 @@ CancelCheck::doApply() if (!view().dirRemove( keylet::ownerDir(dstId), page, sleCheck->key(), true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete check from destination."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } { @@ -102,8 +104,10 @@ CancelCheck::doApply() if (!view().dirRemove( keylet::ownerDir(srcId), page, sleCheck->key(), true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete check from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } diff --git a/src/xrpld/app/tx/detail/CancelOffer.cpp b/src/xrpld/app/tx/detail/CancelOffer.cpp index e7ec28ce17..ff5bfd085b 100644 --- a/src/xrpld/app/tx/detail/CancelOffer.cpp +++ b/src/xrpld/app/tx/detail/CancelOffer.cpp @@ -68,7 +68,7 @@ CancelOffer::doApply() auto const sle = view().read(keylet::account(account_)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if (auto sleOffer = view().peek(keylet::offer(account_, offerSequence))) { diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index f8ab6189a3..73dedba170 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -87,8 +87,10 @@ CashCheck::preclaim(PreclaimContext const& ctx) { // They wrote a check to themselves. This should be caught when // the check is created, but better late than never. + // LCOV_EXCL_START JLOG(ctx.j.error()) << "Malformed transaction: Cashing check to self."; return tecINTERNAL; + // LCOV_EXCL_STOP } { auto const sleSrc = ctx.view.read(keylet::account(srcId)); @@ -245,17 +247,21 @@ CashCheck::doApply() auto sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID])); if (!sleCheck) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Precheck did not verify check's existence."; return tecFAILED_PROCESSING; + // LCOV_EXCL_STOP } AccountID const srcId{sleCheck->getAccountID(sfAccount)}; if (!psb.exists(keylet::account(srcId)) || !psb.exists(keylet::account(account_))) { + // LCOV_EXCL_START JLOG(ctx_.journal.fatal()) << "Precheck did not verify source or destination's existence."; return tecFAILED_PROCESSING; + // LCOV_EXCL_STOP } // Preclaim already checked that source has at least the requested @@ -478,8 +484,10 @@ CashCheck::doApply() sleCheck->key(), true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete check from destination."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } // Remove check from check owner's directory. @@ -489,8 +497,10 @@ CashCheck::doApply() sleCheck->key(), true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete check from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } // If we succeeded, update the check owner's reserve. diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp index b346e4a1c1..1c279389ec 100644 --- a/src/xrpld/app/tx/detail/Clawback.cpp +++ b/src/xrpld/app/tx/detail/Clawback.cpp @@ -235,7 +235,7 @@ applyHelper(ApplyContext& ctx) // Replace the `issuer` field with issuer's account clawAmount.setIssuer(issuer); if (holder == issuer) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE // Get the spendable balance. Must use `accountHolds`. STAmount const spendableAmount = accountHolds( diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp index 57f3a92255..54bbdbdc25 100644 --- a/src/xrpld/app/tx/detail/CreateCheck.cpp +++ b/src/xrpld/app/tx/detail/CreateCheck.cpp @@ -159,7 +159,7 @@ CreateCheck::doApply() { auto const sle = view().peek(keylet::account(account_)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // A check counts against the reserve of the issuing account, but we // check the starting balance because we want to allow dipping into the @@ -209,7 +209,7 @@ CreateCheck::doApply() << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE sleCheck->setFieldU64(sfDestinationNode, *page); } @@ -225,7 +225,7 @@ CreateCheck::doApply() << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE sleCheck->setFieldU64(sfOwnerNode, *page); } diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 86750eb51d..a503f913fa 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -848,9 +848,11 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) if (!ownerNode) { + // LCOV_EXCL_START JLOG(j_.debug()) << "final result: failed to add offer to owner's directory"; return {tecDIR_FULL, true}; + // LCOV_EXCL_STOP } // Update owner count. @@ -894,8 +896,10 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) if (!bookNode) { + // LCOV_EXCL_START JLOG(j_.debug()) << "final result: failed to add offer to book"; return {tecDIR_FULL, true}; + // LCOV_EXCL_STOP } auto sleOffer = std::make_shared(offer_index); diff --git a/src/xrpld/app/tx/detail/CreateTicket.cpp b/src/xrpld/app/tx/detail/CreateTicket.cpp index d48da2d780..f0b62bc67f 100644 --- a/src/xrpld/app/tx/detail/CreateTicket.cpp +++ b/src/xrpld/app/tx/detail/CreateTicket.cpp @@ -76,7 +76,7 @@ CreateTicket::doApply() { SLE::pointer const sleAccountRoot = view().peek(keylet::account(account_)); if (!sleAccountRoot) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // Each ticket counts against the reserve of the issuing account, but we // check the starting balance because we want to allow dipping into the @@ -102,7 +102,7 @@ CreateTicket::doApply() // increment the account root Sequence. if (std::uint32_t const txSeq = ctx_.tx[sfSequence]; txSeq != 0 && txSeq != (firstTicketSeq - 1)) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE for (std::uint32_t i = 0; i < ticketCount; ++i) { @@ -123,7 +123,7 @@ CreateTicket::doApply() << ": " << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE sleTicket->setFieldU64(sfOwnerNode, *page); } diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index 4b77163c5d..b6471d6a20 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/src/xrpld/app/tx/detail/Credentials.cpp @@ -117,7 +117,7 @@ CredentialCreate::doApply() auto const sleCred = std::make_shared(credentialKey); if (!sleCred) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE auto const optExp = ctx_.tx[~sfExpiration]; if (optExp) @@ -137,7 +137,7 @@ CredentialCreate::doApply() auto const sleIssuer = view().peek(keylet::account(account_)); if (!sleIssuer) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE { STAmount const reserve{view().fees().accountReserve( @@ -162,7 +162,7 @@ CredentialCreate::doApply() << to_string(credentialKey.key) << ": " << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE sleCred->setFieldU64(sfIssuerNode, *page); adjustOwnerCount(view(), sleIssuer, 1, j_); @@ -182,7 +182,7 @@ CredentialCreate::doApply() << to_string(credentialKey.key) << ": " << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE sleCred->setFieldU64(sfSubjectNode, *page); view().update(view().peek(keylet::account(subject))); } @@ -258,7 +258,7 @@ CredentialDelete::doApply() auto const sleCred = view().peek(keylet::credential(subject, issuer, credType)); if (!sleCred) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if ((subject != account_) && (issuer != account_) && !checkExpired(sleCred, ctx_.view().info().parentCloseTime)) @@ -342,7 +342,7 @@ CredentialAccept::doApply() auto const sleIssuer = view().peek(keylet::account(issuer)); if (!sleSubject || !sleIssuer) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE { STAmount const reserve{view().fees().accountReserve( diff --git a/src/xrpld/app/tx/detail/DID.cpp b/src/xrpld/app/tx/detail/DID.cpp index b38b207d36..f466777920 100644 --- a/src/xrpld/app/tx/detail/DID.cpp +++ b/src/xrpld/app/tx/detail/DID.cpp @@ -76,7 +76,7 @@ addSLE( { auto const sleAccount = ctx.view().peek(keylet::account(owner)); if (!sleAccount) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // Check reserve availability for new object creation { @@ -96,7 +96,7 @@ addSLE( auto page = ctx.view().dirInsert( keylet::ownerDir(owner), sle->key(), describeOwnerDir(owner)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*sle)[sfOwnerNode] = *page; } adjustOwnerCount(ctx.view(), sleAccount, 1, ctx.journal); @@ -189,13 +189,15 @@ 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."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } auto const sleOwner = view.peek(keylet::account(owner)); if (!sleOwner) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE adjustOwnerCount(view, sleOwner, -1, j); view.update(sleOwner); diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index d52e84d755..805c023c04 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -141,7 +141,7 @@ removeNFTokenOfferFromLedger( beast::Journal) { if (!nft::deleteTokenOffer(view, sleDel)) - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE return tesSUCCESS; } @@ -336,11 +336,13 @@ DeleteAccount::preclaim(PreclaimContext const& ctx) if (!sleItem) { // Directory node has an invalid index. Bail out. + // LCOV_EXCL_START JLOG(ctx.j.fatal()) << "DeleteAccount: directory node in ledger " << ctx.view.seq() << " has index to object that is missing: " << to_string(dirEntry); return tefBAD_LEDGER; + // LCOV_EXCL_STOP } LedgerEntryType const nodeType{ @@ -373,7 +375,7 @@ DeleteAccount::doApply() dst, "ripple::DeleteAccount::doApply : non-null destination account"); if (!src || !dst) - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE if (ctx_.view().rules().enabled(featureDepositAuth) && ctx_.tx.isFieldPresent(sfCredentialIDs)) diff --git a/src/xrpld/app/tx/detail/DepositPreauth.cpp b/src/xrpld/app/tx/detail/DepositPreauth.cpp index 236b59a173..6f0e079290 100644 --- a/src/xrpld/app/tx/detail/DepositPreauth.cpp +++ b/src/xrpld/app/tx/detail/DepositPreauth.cpp @@ -138,7 +138,7 @@ DepositPreauth::preclaim(PreclaimContext const& ctx) return tecNO_ISSUER; auto [it, ins] = sorted.emplace(issuer, o[sfCredentialType]); if (!ins) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE } // Verify that the Preauth entry they asked to add is not already @@ -198,7 +198,7 @@ DepositPreauth::doApply() << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE slePreauth->setFieldU64(sfOwnerNode, *page); @@ -216,7 +216,7 @@ DepositPreauth::doApply() { auto const sleOwner = view().peek(keylet::account(account_)); if (!sleOwner) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // A preauth counts against the reserve of the issuing account, but we // check the starting balance because we want to allow dipping into the @@ -246,7 +246,7 @@ DepositPreauth::doApply() Keylet const preauthKey = keylet::depositPreauth(account_, sortedTX); auto slePreauth = std::make_shared(preauthKey); if (!slePreauth) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE slePreauth->setAccountID(sfAccount, account_); slePreauth->peekFieldArray(sfAuthorizeCredentials) = @@ -262,7 +262,7 @@ DepositPreauth::doApply() << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE slePreauth->setFieldU64(sfOwnerNode, *page); @@ -299,14 +299,16 @@ DepositPreauth::removeFromLedger( std::uint64_t const page{(*slePreauth)[sfOwnerNode]}; if (!view.dirRemove(keylet::ownerDir(account), page, preauthIndex, false)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Unable to delete DepositPreauth from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } // If we succeeded, update the DepositPreauth owner's reserve. auto const sleOwner = view.peek(keylet::account(account)); if (!sleOwner) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE adjustOwnerCount(view, sleOwner, -1, j); diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 969fd4dd4c..eb468626a4 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -414,10 +414,8 @@ escrowLockApplyHelper( beast::Journal journal) { // Defensive: Issuer cannot create an escrow - // LCOV_EXCL_START if (issuer == sender) - return tecINTERNAL; - // LCOV_EXCL_STOP + return tecINTERNAL; // LCOV_EXCL_LINE auto const ter = rippleCredit( view, @@ -441,10 +439,8 @@ escrowLockApplyHelper( beast::Journal journal) { // Defensive: Issuer cannot create an escrow - // LCOV_EXCL_START if (issuer == sender) - return tecINTERNAL; - // LCOV_EXCL_STOP + return tecINTERNAL; // LCOV_EXCL_LINE auto const ter = rippleLockEscrowMPT(view, sender, amount, journal); if (ter != tesSUCCESS) @@ -472,6 +468,9 @@ EscrowCreate::doApply() } else { + // This is old code that needs to stay to support replaying old ledgers, + // but does not need to be covered by new tests. + // LCOV_EXCL_START if (ctx_.tx[~sfCancelAfter]) { auto const cancelAfter = ctx_.tx[sfCancelAfter]; @@ -487,6 +486,7 @@ EscrowCreate::doApply() if (closeTime.time_since_epoch().count() >= finishAfter) return tecNO_PERMISSION; } + // LCOV_EXCL_STOP } auto const sle = ctx_.view().peek(keylet::account(account_)); @@ -514,12 +514,12 @@ EscrowCreate::doApply() auto const sled = ctx_.view().read(keylet::account(ctx_.tx[sfDestination])); if (!sled) - return tecNO_DST; + return tecNO_DST; // LCOV_EXCL_LINE if (((*sled)[sfFlags] & lsfRequireDestTag) && !ctx_.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; - // Obeying the lsfDissalowXRP flag was a bug. Piggyback on + // Obeying the lsfDisallowXRP flag was a bug. Piggyback on // featureDepositAuth to remove the bug. if (!ctx_.view().rules().enabled(featureDepositAuth) && ((*sled)[sfFlags] & lsfDisallowXRP)) @@ -601,7 +601,9 @@ EscrowCreate::doApply() }, amount.asset().value()); !isTesSuccess(ret)) + { return ret; // LCOV_EXCL_LINE + } } // increment owner count @@ -837,10 +839,8 @@ escrowUnlockApplyHelper( bool const receiverIssuer = issuer == receiver; bool const issuerHigh = issuer > receiver; - // LCOV_EXCL_START if (senderIssuer) - return tecINTERNAL; - // LCOV_EXCL_STOP + return tecINTERNAL; // LCOV_EXCL_LINE if (receiverIssuer) return tesSUCCESS; @@ -1057,6 +1057,9 @@ EscrowFinish::doApply() } else { + // This is old code that needs to stay to support replaying old ledgers, + // but does not need to be covered by new tests. + // LCOV_EXCL_START // Too soon? if ((*slep)[~sfFinishAfter] && ctx_.view().info().parentCloseTime.time_since_epoch().count() <= @@ -1068,6 +1071,7 @@ EscrowFinish::doApply() ctx_.view().info().parentCloseTime.time_since_epoch().count() <= (*slep)[sfCancelAfter]) return tecNO_PERMISSION; + // LCOV_EXCL_STOP } // Check cryptocondition fulfillment @@ -1082,6 +1086,7 @@ EscrowFinish::doApply() // simply re-run the check. if (cb && !any(flags & (SF_CF_INVALID | SF_CF_VALID))) { + // LCOV_EXCL_START auto const fb = ctx_.tx[~sfFulfillment]; if (!fb) @@ -1093,6 +1098,7 @@ EscrowFinish::doApply() flags = SF_CF_INVALID; ctx_.app.getHashRouter().setFlags(id, flags); + // LCOV_EXCL_STOP } // If the check failed, then simply return an error @@ -1139,8 +1145,10 @@ EscrowFinish::doApply() if (!ctx_.view().dirRemove( keylet::ownerDir(account), page, k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } @@ -1150,8 +1158,10 @@ EscrowFinish::doApply() if (!ctx_.view().dirRemove( keylet::ownerDir(destID), *optPage, k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from recipient."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } @@ -1193,8 +1203,10 @@ EscrowFinish::doApply() if (!ctx_.view().dirRemove( keylet::ownerDir(issuer), *optPage, k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from recipient."; return tefBAD_LEDGER; // LCOV_EXCL_LINE + // LCOV_EXCL_STOP } } } @@ -1338,11 +1350,15 @@ EscrowCancel::doApply() } else { + // This is old code that needs to stay to support replaying old ledgers, + // but does not need to be covered by new tests. + // LCOV_EXCL_START // Too soon? if (!(*slep)[~sfCancelAfter] || ctx_.view().info().parentCloseTime.time_since_epoch().count() <= (*slep)[sfCancelAfter]) return tecNO_PERMISSION; + // LCOV_EXCL_STOP } AccountID const account = (*slep)[sfAccount]; @@ -1353,8 +1369,10 @@ EscrowCancel::doApply() if (!ctx_.view().dirRemove( keylet::ownerDir(account), page, k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } @@ -1367,8 +1385,10 @@ EscrowCancel::doApply() k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from recipient."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } @@ -1409,8 +1429,10 @@ EscrowCancel::doApply() if (!ctx_.view().dirRemove( keylet::ownerDir(issuer), *optPage, k.key, true)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete Escrow from recipient."; - return tefBAD_LEDGER; // LCOV_EXCL_LINE + return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } } diff --git a/src/xrpld/app/tx/detail/LedgerStateFix.cpp b/src/xrpld/app/tx/detail/LedgerStateFix.cpp index 6059e15313..017f67f2ee 100644 --- a/src/xrpld/app/tx/detail/LedgerStateFix.cpp +++ b/src/xrpld/app/tx/detail/LedgerStateFix.cpp @@ -67,7 +67,7 @@ LedgerStateFix::preclaim(PreclaimContext const& ctx) } // preflight is supposed to verify that only valid FixTypes get to preclaim. - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } TER @@ -83,7 +83,7 @@ LedgerStateFix::doApply() } // preflight is supposed to verify that only valid FixTypes get to doApply. - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp index edeb12e5c0..b8728d23cf 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.cpp @@ -75,7 +75,7 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) auto const sleMptIssuance = ctx.view.read( keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); if (!sleMptIssuance) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE return tecHAS_OBLIGATIONS; } diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp index 4c502f1106..1dd3fbfe68 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceDestroy.cpp @@ -66,11 +66,11 @@ MPTokenIssuanceDestroy::doApply() auto const mpt = view().peek(keylet::mptIssuance(ctx_.tx[sfMPTokenIssuanceID])); if (account_ != mpt->getAccountID(sfIssuer)) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE if (!view().dirRemove( keylet::ownerDir(account_), (*mpt)[sfOwnerNode], mpt->key(), false)) - return tefBAD_LEDGER; + return tefBAD_LEDGER; // LCOV_EXCL_LINE view().erase(mpt); diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index 6fb87711c8..f2c9bd8a96 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -282,7 +282,7 @@ MPTokenIssuanceSet::doApply() sle = view().peek(keylet::mptIssuance(mptIssuanceID)); if (!sle) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE std::uint32_t const flagsIn = sle->getFieldU32(sfFlags); std::uint32_t flagsOut = flagsIn; diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp index 3b4a27ffd7..23874ee3e0 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp @@ -356,7 +356,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) auto const& offer = bo ? bo : so; if (!offer) // Purely defensive, should be caught in preflight. - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const& tokenID = offer->at(sfNFTokenID); auto const& amount = offer->at(sfAmount); @@ -428,7 +428,7 @@ NFTokenAcceptOffer::transferNFToken( auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID); if (!tokenAndPage) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE if (auto const ret = nft::removeToken( view(), seller, nftokenID, std::move(tokenAndPage->page)); @@ -437,7 +437,7 @@ NFTokenAcceptOffer::transferNFToken( auto const sleBuyer = view().read(keylet::account(buyer)); if (!sleBuyer) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE std::uint32_t const buyerOwnerCountBefore = sleBuyer->getFieldU32(sfOwnerCount); @@ -523,16 +523,20 @@ NFTokenAcceptOffer::doApply() if (bo && !nft::deleteTokenOffer(view(), bo)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete buy offer '" << to_string(bo->key()) << "': ignoring"; return tecINTERNAL; + // LCOV_EXCL_STOP } if (so && !nft::deleteTokenOffer(view(), so)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete sell offer '" << to_string(so->key()) << "': ignoring"; return tecINTERNAL; + // LCOV_EXCL_STOP } // Bridging two different offers @@ -603,7 +607,7 @@ NFTokenAcceptOffer::doApply() if (so) return acceptOffer(so); - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp index 86e804b1a5..2d3eb74775 100644 --- a/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenCancelOffer.cpp @@ -101,9 +101,11 @@ NFTokenCancelOffer::doApply() if (auto offer = view().peek(keylet::nftoffer(id)); offer && !nft::deleteTokenOffer(view(), offer)) { + // LCOV_EXCL_START JLOG(j_.fatal()) << "Unable to delete token offer " << id << " (ledger " << view().seq() << ")"; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index 8149d3b59d..dd82443fee 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -306,7 +306,7 @@ NFTokenMint::doApply() if (nfTokenTemplate == nullptr) // Should never happen. - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const nftokenID = createNFTokenID( extractNFTokenFlagsFromTxFlags(ctx_.tx.getFlags()), diff --git a/src/xrpld/app/tx/detail/NFTokenUtils.cpp b/src/xrpld/app/tx/detail/NFTokenUtils.cpp index ad3e6f4d35..f246e89e65 100644 --- a/src/xrpld/app/tx/detail/NFTokenUtils.cpp +++ b/src/xrpld/app/tx/detail/NFTokenUtils.cpp @@ -1046,7 +1046,7 @@ tokenOfferCreateApply( keylet::ownerDir(acctID), offerID, describeOwnerDir(acctID)); if (!ownerNode) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE bool const isSellOffer = txFlags & tfSellNFToken; @@ -1063,7 +1063,7 @@ tokenOfferCreateApply( }); if (!offerNode) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE std::uint32_t sleFlags = 0; diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index 32c0abeb93..b495e00b3f 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -126,9 +126,11 @@ closeChannel( auto const page = (*slep)[sfOwnerNode]; if (!view.dirRemove(keylet::ownerDir(src), page, key, true)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Could not remove paychan from src owner directory"; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } @@ -139,16 +141,18 @@ closeChannel( auto const dst = (*slep)[sfDestination]; if (!view.dirRemove(keylet::ownerDir(dst), *page, key, true)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Could not remove paychan from dst owner directory"; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } } // Transfer amount back to owner, decrement owner count auto const sle = view.peek(keylet::account(src)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE XRPL_ASSERT( (*slep)[sfAmount] >= (*slep)[sfBalance], @@ -257,7 +261,7 @@ PayChanCreate::doApply() auto const account = ctx_.tx[sfAccount]; auto const sle = ctx_.view().peek(keylet::account(account)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if (ctx_.view().rules().enabled(fixPayChanCancelAfter)) { @@ -301,7 +305,7 @@ PayChanCreate::doApply() payChanKeylet, describeOwnerDir(account)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*slep)[sfOwnerNode] = *page; } @@ -311,7 +315,7 @@ PayChanCreate::doApply() auto const page = ctx_.view().dirInsert( keylet::ownerDir(dst), payChanKeylet, describeOwnerDir(dst)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*slep)[sfDestinationNode] = *page; } @@ -389,7 +393,7 @@ PayChanFund::doApply() auto const sle = ctx_.view().peek(keylet::account(txAccount)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE { // Check reserve and funds availability diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 8bc0e891d0..81a083e336 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -632,7 +632,7 @@ Payment::doApply() auto const sleSrc = view().peek(keylet::account(account_)); if (!sleSrc) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // ownerCount is the number of entries in this ledger for this // account that require a reserve. diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp index 9fe48ba515..097a2d7bad 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -66,9 +66,11 @@ PermissionedDomainDelete::doApply() if (!view().dirRemove(keylet::ownerDir(account_), page, slePd->key(), true)) { - JLOG(j_.fatal()) // LCOV_EXCL_LINE - << "Unable to delete permissioned domain directory entry."; // LCOV_EXCL_LINE - return tefBAD_LEDGER; // LCOV_EXCL_LINE + // LCOV_EXCL_START + JLOG(j_.fatal()) + << "Unable to delete permissioned domain directory entry."; + return tefBAD_LEDGER; + // LCOV_EXCL_STOP } auto const ownerSle = view().peek(keylet::account(account_)); diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index c2129ba1e1..7c60ec646a 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -311,7 +311,7 @@ SetAccount::doApply() { auto const sle = view().peek(keylet::account(account_)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE std::uint32_t const uFlagsIn = sle->getFieldU32(sfFlags); std::uint32_t uFlagsOut = uFlagsIn; diff --git a/src/xrpld/app/tx/detail/SetRegularKey.cpp b/src/xrpld/app/tx/detail/SetRegularKey.cpp index 4e063e7d1f..93d5899861 100644 --- a/src/xrpld/app/tx/detail/SetRegularKey.cpp +++ b/src/xrpld/app/tx/detail/SetRegularKey.cpp @@ -66,7 +66,7 @@ SetRegularKey::doApply() { auto const sle = view().peek(keylet::account(account_)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if (!minimumFee(ctx_.app, ctx_.baseFee, view().fees(), view().flags())) sle->setFlag(lsfPasswordSpent); diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index ec2f902009..010b6baa96 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -226,8 +226,10 @@ removeSignersFromLedger( if (!view.dirRemove(ownerDirKeylet, hint, signerListKeylet.key, false)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Unable to delete SignerList from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } adjustOwnerCount( @@ -349,7 +351,7 @@ SetSignerList::replaceSignerList() auto const sle = view().peek(accountKeylet); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // Compute new reserve. Verify the account has funds to meet the reserve. std::uint32_t const oldOwnerCount{(*sle)[sfOwnerCount]}; @@ -387,7 +389,7 @@ SetSignerList::replaceSignerList() << ": " << (page ? "success" : "failure"); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE signerList->setFieldU64(sfOwnerNode, *page); @@ -405,7 +407,7 @@ SetSignerList::destroySignerList() // is enabled or there is a regular key. SLE::pointer ledgerEntry = view().peek(accountKeylet); if (!ledgerEntry) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if ((ledgerEntry->isFlag(lsfDisableMaster)) && (!ledgerEntry->isFieldPresent(sfRegularKey))) diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 21d4534f93..d881425960 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -377,7 +377,7 @@ SetTrust::doApply() auto const sle = view().peek(keylet::account(account_)); if (!sle) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE std::uint32_t const uOwnerCount = sle->getFieldU32(sfOwnerCount); diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 920b1a58bc..2f62a142c0 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -571,15 +571,19 @@ Transactor::ticketDelete( SLE::pointer const sleTicket = view.peek(keylet::ticket(ticketIndex)); if (!sleTicket) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Ticket disappeared from ledger."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } std::uint64_t const page{(*sleTicket)[sfOwnerNode]}; if (!view.dirRemove(keylet::ownerDir(account), page, ticketIndex, true)) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Unable to delete Ticket from owner."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } // Update the account root's TicketCount. If the ticket count drops to @@ -587,8 +591,10 @@ Transactor::ticketDelete( auto sleAccount = view.peek(keylet::account(account)); if (!sleAccount) { + // LCOV_EXCL_START JLOG(j.fatal()) << "Could not find Ticket owner account root."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } if (auto ticketCount = (*sleAccount)[~sfTicketCount]) @@ -600,8 +606,10 @@ Transactor::ticketDelete( } else { + // LCOV_EXCL_START JLOG(j.fatal()) << "TicketCount field missing from account root."; return tefBAD_LEDGER; + // LCOV_EXCL_STOP } // Update the Ticket owner's reserve. diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 5f5c081e2f..d7731d8d98 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -444,7 +444,7 @@ transferHelper( auto const sleSrc = psb.peek(keylet::account(src)); XRPL_ASSERT(sleSrc, "ripple::transferHelper : non-null source account"); if (!sleSrc) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE { auto const ownerCount = sleSrc->getFieldU32(sfOwnerCount); @@ -714,7 +714,7 @@ finalizeClaimHelper( } if (distributed > rewardPool) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE return tesSUCCESS; }(); @@ -1152,7 +1152,7 @@ applyCreateAccountAttestations( // subsequent claim ids auto const sleBridge = psb.peek(bridgeK); if (!sleBridge) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE (*sleBridge)[sfXChainAccountClaimCount] = attBegin->createCount; psb.update(sleBridge); } @@ -1172,12 +1172,12 @@ applyCreateAccountAttestations( claimIDKeylet, describeOwnerDir(doorAccount)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*createdSleClaimID)[sfOwnerNode] = *page; auto const sleDoor = psb.peek(doorK); if (!sleDoor) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE // Reserve was already checked adjustOwnerCount(psb, sleDoor, 1, j); @@ -1242,8 +1242,9 @@ TER attestationPreclaim(PreclaimContext const& ctx) { auto const att = toClaim(ctx.tx); + // checked in preflight if (!att) - return tecINTERNAL; // checked in preflight + return tecINTERNAL; // LCOV_EXCL_LINE STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge]; auto const sleBridge = readBridge(ctx.view, bridgeSpec); @@ -1274,7 +1275,7 @@ attestationDoApply(ApplyContext& ctx) auto const att = toClaim(ctx.tx); if (!att) // Should already be checked in preflight - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge]; @@ -1505,7 +1506,7 @@ XChainCreateBridge::doApply() auto const sleAcct = ctx_.view().peek(keylet::account(account)); if (!sleAcct) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE STXChainBridge::ChainType const chainType = STXChainBridge::srcChain(account == bridgeSpec.lockingChainDoor()); @@ -1527,7 +1528,7 @@ XChainCreateBridge::doApply() auto const page = ctx_.view().dirInsert( keylet::ownerDir(account), bridgeKeylet, describeOwnerDir(account)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*sleBridge)[sfOwnerNode] = *page; } @@ -1620,7 +1621,7 @@ BridgeModify::doApply() auto const sleAcct = ctx_.view().peek(keylet::account(account)); if (!sleAcct) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE STXChainBridge::ChainType const chainType = STXChainBridge::srcChain(account == bridgeSpec.lockingChainDoor()); @@ -1628,7 +1629,7 @@ BridgeModify::doApply() auto const sleBridge = ctx_.view().peek(keylet::bridge(bridgeSpec, chainType)); if (!sleBridge) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE if (reward) (*sleBridge)[sfSignatureReward] = *reward; @@ -1691,7 +1692,7 @@ XChainClaim::preclaim(PreclaimContext const& ctx) else if (thisDoor == bridgeSpec.issuingChainDoor()) isLockingChain = false; else - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } { @@ -1715,7 +1716,7 @@ XChainClaim::preclaim(PreclaimContext const& ctx) // Should have been caught when creating the bridge // Detect here so `otherChainAmount` doesn't switch from IOU -> XRP // and the numeric issues that need to be addressed with that. - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } auto const otherChainAmount = [&]() -> STAmount { @@ -1921,7 +1922,7 @@ XChainCommit::preclaim(PreclaimContext const& ctx) else if (thisDoor == bridgeSpec.issuingChainDoor()) isLockingChain = false; else - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } if (isLockingChain) @@ -1948,11 +1949,11 @@ XChainCommit::doApply() auto const bridgeSpec = ctx_.tx[sfXChainBridge]; if (!psb.read(keylet::account(account))) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const sleBridge = readBridge(psb, bridgeSpec); if (!sleBridge) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const dst = (*sleBridge)[sfAccount]; @@ -2040,21 +2041,27 @@ XChainCreateClaimID::doApply() auto const sleAcct = ctx_.view().peek(keylet::account(account)); if (!sleAcct) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const sleBridge = peekBridge(ctx_.view(), bridgeSpec); if (!sleBridge) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE std::uint32_t const claimID = (*sleBridge)[sfXChainClaimID] + 1; if (claimID == 0) - return tecINTERNAL; // overflow + { + // overflow + return tecINTERNAL; // LCOV_EXCL_LINE + } (*sleBridge)[sfXChainClaimID] = claimID; Keylet const claimIDKeylet = keylet::xChainClaimID(bridgeSpec, claimID); if (ctx_.view().exists(claimIDKeylet)) - return tecINTERNAL; // already checked out!?! + { + // already checked out!?! + return tecINTERNAL; // LCOV_EXCL_LINE + } auto const sleClaimID = std::make_shared(claimIDKeylet); @@ -2073,7 +2080,7 @@ XChainCreateClaimID::doApply() claimIDKeylet, describeOwnerDir(account)); if (!page) - return tecDIR_FULL; + return tecDIR_FULL; // LCOV_EXCL_LINE (*sleClaimID)[sfOwnerNode] = *page; } @@ -2191,7 +2198,7 @@ XChainCreateAccountCommit::preclaim(PreclaimContext const& ctx) else if (thisDoor == bridgeSpec.issuingChainDoor()) srcChain = STXChainBridge::ChainType::issuing; else - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } STXChainBridge::ChainType const dstChain = STXChainBridge::otherChain(srcChain); @@ -2217,11 +2224,11 @@ XChainCreateAccountCommit::doApply() auto const sle = psb.peek(keylet::account(account)); if (!sle) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const sleBridge = peekBridge(psb, bridge); if (!sleBridge) - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE auto const dst = (*sleBridge)[sfAccount]; diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index f5f5e53238..66f4efffc6 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -507,10 +507,12 @@ ServerHandler::processSession( } catch (std::exception const& ex) { + // LCOV_EXCL_START jr[jss::result] = RPC::make_error(rpcINTERNAL); JLOG(m_journal.error()) << "Exception while processing WS: " << ex.what() << "\n" << "Input JSON: " << Json::Compact{Json::Value{jv}}; + // LCOV_EXCL_STOP } is->getConsumer().charge(loadType); @@ -904,10 +906,12 @@ ServerHandler::processRequest( } catch (std::exception const& ex) { + // LCOV_EXCL_START result = RPC::make_error(rpcINTERNAL); JLOG(m_journal.error()) << "Internal error : " << ex.what() << " when processing request: " << Json::Compact{Json::Value{params}}; + // LCOV_EXCL_STOP } auto end = std::chrono::system_clock::now(); diff --git a/src/xrpld/rpc/handlers/PayChanClaim.cpp b/src/xrpld/rpc/handlers/PayChanClaim.cpp index 6945d2a051..6238af3ae3 100644 --- a/src/xrpld/rpc/handlers/PayChanClaim.cpp +++ b/src/xrpld/rpc/handlers/PayChanClaim.cpp @@ -94,9 +94,11 @@ doChannelAuthorize(RPC::JsonContext& context) } catch (std::exception const& ex) { + // LCOV_EXCL_START result = RPC::make_error( rpcINTERNAL, "Exception occurred during signing: " + std::string(ex.what())); + // LCOV_EXCL_STOP } return result; } diff --git a/src/xrpld/rpc/handlers/Random.cpp b/src/xrpld/rpc/handlers/Random.cpp index cea83a616c..c2a984a2c4 100644 --- a/src/xrpld/rpc/handlers/Random.cpp +++ b/src/xrpld/rpc/handlers/Random.cpp @@ -51,7 +51,7 @@ doRandom(RPC::JsonContext& context) } catch (std::exception const&) { - return rpcError(rpcINTERNAL); + return rpcError(rpcINTERNAL); // LCOV_EXCL_LINE } } From 459d0da010626e5669d53966ff14cd1ae9e77957 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 14 Oct 2025 16:18:34 +0100 Subject: [PATCH 11/18] chore: Support CMake 4 without workarounds (#5866) --- .github/scripts/strategy-matrix/linux.json | 44 +++++++++++----------- BUILD.md | 22 +---------- conan/profiles/default | 3 -- 3 files changed, 23 insertions(+), 46 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index bae5c57087..b8da322118 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -15,154 +15,154 @@ "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "15", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "20", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "d133ce3" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "jammy", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "6f723eb" + "image_sha": "6948666" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "6f723eb" + "image_sha": "6948666" } ], "build_type": ["Debug", "Release"], diff --git a/BUILD.md b/BUILD.md index fd7a0b855d..ef6b5259df 100644 --- a/BUILD.md +++ b/BUILD.md @@ -39,17 +39,12 @@ found here](./docs/build/environment.md). - [Python 3.11](https://www.python.org/downloads/), or higher - [Conan 2.17](https://conan.io/downloads.html)[^1], or higher -- [CMake 3.22](https://cmake.org/download/)[^2], or higher +- [CMake 3.22](https://cmake.org/download/), or higher [^1]: It is possible to build with Conan 1.60+, but the instructions are significantly different, which is why we are not recommending it. -[^2]: - CMake 4 is not yet supported by all dependencies required by this project. - If you are affected by this issue, follow [conan workaround for cmake - 4](#workaround-for-cmake-4) - `rippled` is written in the C++20 dialect and includes the `` header. The [minimum compiler versions][2] required are: @@ -282,21 +277,6 @@ sed -i.bak -e 's|^arch=.*$|arch=x86_64|' $(conan config home)/profiles/default sed -i.bak -e 's|^compiler\.runtime=.*$|compiler.runtime=static|' $(conan config home)/profiles/default ``` -#### Workaround for CMake 4 - -If your system CMake is version 4 rather than 3, you may have to configure Conan -profile to use CMake version 3 for dependencies, by adding the following two -lines to your profile: - -```text -[tool_requires] -!cmake/*: cmake/[>=3 <4] -``` - -This will force Conan to download and use a locally cached CMake 3 version, and -is needed because some of the dependencies used by this project do not support -CMake 4. - #### Clang workaround for grpc If your compiler is clang, version 19 or later, or apple-clang, version 17 or diff --git a/conan/profiles/default b/conan/profiles/default index 3a7bcda1c6..03f19ca118 100644 --- a/conan/profiles/default +++ b/conan/profiles/default @@ -29,6 +29,3 @@ tools.build:cxxflags=['-Wno-missing-template-arg-list-after-template-kw'] {% if compiler == "gcc" and compiler_version < 13 %} tools.build:cxxflags=['-Wno-restrict'] {% endif %} - -[tool_requires] -!cmake/*: cmake/[>=3 <4] From 3509de9c5f6a62ebe02a916265a4238dd4f3bcd1 Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 15 Oct 2025 03:37:39 +0900 Subject: [PATCH 12/18] refactor: Add `paychan` namespace and update related tests (#5840) This change adds a paychan namespace to the TestHelpers and implementation files, improving organization and clarity. Additionally, it updates the AMM test to use the new `paychan::create` function for payment channel creation. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- src/test/app/AMM_test.cpp | 2 +- src/test/app/PayChan_test.cpp | 2 ++ src/test/jtx/TestHelpers.h | 3 +++ src/test/jtx/impl/TestHelpers.cpp | 4 ++++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 1fe37bb7c1..3e0d65894f 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -3666,7 +3666,7 @@ private: auto const settleDelay = 100s; NetClock::time_point const cancelAfter = env.current()->info().parentCloseTime + 200s; - env(create( + env(paychan::create( carol, ammAlice.ammAccount(), XRP(1'000), diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index fe9b70cf7f..595a12aed9 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -31,6 +31,8 @@ namespace ripple { namespace test { +using namespace jtx::paychan; + struct PayChan_test : public beast::unit_test::suite { FeatureBitset const disallowIncoming{featureDisallowIncoming}; diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index 7d14f23c92..a7bdbb9b9f 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -502,6 +502,7 @@ expectLedgerEntryRoot( /* Payment Channel */ /******************************************************************************/ +namespace paychan { Json::Value create( @@ -561,6 +562,8 @@ channelBalance(ReadView const& view, uint256 const& chan); bool channelExists(ReadView const& view, uint256 const& chan); +} // namespace paychan + /* Crossing Limits */ /******************************************************************************/ diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index 20f24f0d84..71f44c691e 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -237,6 +237,8 @@ expectLedgerEntryRoot( /* Payment Channel */ /******************************************************************************/ +namespace paychan { + Json::Value create( AccountID const& account, @@ -328,6 +330,8 @@ channelExists(ReadView const& view, uint256 const& chan) return bool(slep); } +} // namespace paychan + /* Crossing Limits */ /******************************************************************************/ From 34619f2504c92f2967c855fcc0ca3d80682dcd3a Mon Sep 17 00:00:00 2001 From: zingero Date: Tue, 14 Oct 2025 22:10:19 +0300 Subject: [PATCH 13/18] docs: Fix typo in JSON writer documentation (#5881) Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- include/xrpl/json/json_writer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/json/json_writer.h b/include/xrpl/json/json_writer.h index 7e21e766e3..ba3a51013f 100644 --- a/include/xrpl/json/json_writer.h +++ b/include/xrpl/json/json_writer.h @@ -46,7 +46,7 @@ public: * without formatting (not human friendly). * * The JSON document is written in a single line. It is not intended for 'human' - * consumption, but may be useful to support feature such as RPC where bandwith + * consumption, but may be useful to support feature such as RPC where bandwidth * is limited. \sa Reader, Value */ From 97bc94a7f6c6be467728d1da363eabdcbd2803f9 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Tue, 14 Oct 2025 15:02:38 -0700 Subject: [PATCH 14/18] feat: Install validator-keys (#5841) * feat: Install validator-keys * output validator-keys with everything else --- cmake/RippledInstall.cmake | 4 ++-- cmake/RippledValidatorKeys.cmake | 16 +++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/cmake/RippledInstall.cmake b/cmake/RippledInstall.cmake index 95c25a212f..50f09d5a4b 100644 --- a/cmake/RippledInstall.cmake +++ b/cmake/RippledInstall.cmake @@ -38,7 +38,7 @@ install(CODE " set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\") include(create_symbolic_link) create_symbolic_link(xrpl \ - \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/ripple) + \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/ripple) ") install (EXPORT RippleExports @@ -72,7 +72,7 @@ if (is_root_project AND TARGET rippled) set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\") include(create_symbolic_link) create_symbolic_link(rippled${suffix} \ - \${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/xrpld${suffix}) + \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/xrpld${suffix}) ") endif () diff --git a/cmake/RippledValidatorKeys.cmake b/cmake/RippledValidatorKeys.cmake index b6760ca496..fa520ce9c1 100644 --- a/cmake/RippledValidatorKeys.cmake +++ b/cmake/RippledValidatorKeys.cmake @@ -1,4 +1,4 @@ -option (validator_keys "Enables building of validator-keys-tool as a separate target (imported via FetchContent)" OFF) +option (validator_keys "Enables building of validator-keys tool as a separate target (imported via FetchContent)" OFF) if (validator_keys) git_branch (current_branch) @@ -6,17 +6,15 @@ if (validator_keys) if (NOT (current_branch STREQUAL "release")) set (current_branch "master") endif () - message (STATUS "tracking ValidatorKeys branch: ${current_branch}") + message (STATUS "Tracking ValidatorKeys branch: ${current_branch}") FetchContent_Declare ( - validator_keys_src + validator_keys GIT_REPOSITORY https://github.com/ripple/validator-keys-tool.git GIT_TAG "${current_branch}" ) - FetchContent_GetProperties (validator_keys_src) - if (NOT validator_keys_src_POPULATED) - message (STATUS "Pausing to download ValidatorKeys...") - FetchContent_Populate (validator_keys_src) - endif () - add_subdirectory (${validator_keys_src_SOURCE_DIR} ${CMAKE_BINARY_DIR}/validator-keys) + FetchContent_MakeAvailable(validator_keys) + set_target_properties(validator-keys PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}") + install(TARGETS validator-keys RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) + endif () From 3d44758e5af811fc7dbdb47e81e887eaca87e184 Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 15 Oct 2025 14:23:44 -0400 Subject: [PATCH 15/18] fix: Update tools image shas (#5896) This change updates the Docker image hashes of the tools-rippled images to fix a missing dependency. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- .github/workflows/pre-commit.yml | 2 +- .github/workflows/publish-docs.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 9b85a3bd11..da0fb02c19 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -12,4 +12,4 @@ jobs: uses: XRPLF/actions/.github/workflows/pre-commit.yml@af1b0f0d764cda2e5435f5ac97b240d4bd4d95d3 with: runs_on: ubuntu-latest - container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-d1496b8" }' + container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-a8c7be1" }' diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index efd89a5b22..84771ee4f7 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -27,7 +27,7 @@ env: jobs: publish: runs-on: ubuntu-latest - container: ghcr.io/xrplf/ci/tools-rippled-documentation:sha-d1496b8 + container: ghcr.io/xrplf/ci/tools-rippled-documentation:sha-a8c7be1 permissions: contents: write steps: From 519d1dbc34d21691bc48adb97c67e0d883e36dfb Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 16 Oct 2025 05:50:06 +0900 Subject: [PATCH 16/18] refactor: Replace fee().accountReserve(0) with fee().reserve (#5843) This PR changes fee().accountReserve(0) to fee().reserve, as the current network reserve amount should be used instead of the account reserve. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- src/test/app/AccountDelete_test.cpp | 2 +- src/test/app/Check_test.cpp | 4 ++-- src/test/app/Credentials_test.cpp | 2 +- src/test/app/DID_test.cpp | 2 +- src/test/app/EscrowToken_test.cpp | 4 ++-- src/test/app/MPToken_test.cpp | 2 +- src/test/app/NFToken_test.cpp | 13 ++++++------- src/test/app/PermissionedDomains_test.cpp | 2 +- src/test/app/SetTrust_test.cpp | 2 +- src/xrpld/app/misc/FeeVoteImpl.cpp | 6 +++--- src/xrpld/app/misc/NetworkOPs.cpp | 12 ++++-------- src/xrpld/app/misc/detail/TxQ.cpp | 2 +- src/xrpld/app/paths/PathRequest.cpp | 3 +-- src/xrpld/app/paths/Pathfinder.cpp | 2 +- src/xrpld/app/tx/detail/Payment.cpp | 4 ++-- src/xrpld/app/tx/detail/XChainBridge.cpp | 2 +- 16 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index f7c4ddc509..a8f946c454 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -747,7 +747,7 @@ public: // Note that the fee structure for unit tests does not match the fees // on the production network (October 2019). Unit tests have a base // reserve of 200 XRP. - env.fund(env.current()->fees().accountReserve(0), noripple(alice)); + env.fund(env.current()->fees().reserve, noripple(alice)); env.close(); // Burn a chunk of alice's funds so she only has 1 XRP remaining in diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index e724b83535..74c8e9df6d 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -619,7 +619,7 @@ class Check_test : public beast::unit_test::suite } { // Write a check that chews into alice's reserve. - STAmount const reserve{env.current()->fees().accountReserve(0)}; + STAmount const reserve{env.current()->fees().reserve}; STAmount const checkAmount{ startBalance - reserve - drops(baseFeeDrops)}; uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; @@ -657,7 +657,7 @@ class Check_test : public beast::unit_test::suite } { // Write a check that goes one drop past what alice can pay. - STAmount const reserve{env.current()->fees().accountReserve(0)}; + STAmount const reserve{env.current()->fees().reserve}; STAmount const checkAmount{ startBalance - reserve - drops(baseFeeDrops - 1)}; uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index 102d516b89..8f953bf35c 100644 --- a/src/test/app/Credentials_test.cpp +++ b/src/test/app/Credentials_test.cpp @@ -578,7 +578,7 @@ struct Credentials_test : public beast::unit_test::suite using namespace jtx; Env env{*this, features}; - auto const reserve = drops(env.current()->fees().accountReserve(0)); + auto const reserve = drops(env.current()->fees().reserve); env.fund(reserve, subject, issuer); env.close(); diff --git a/src/test/app/DID_test.cpp b/src/test/app/DID_test.cpp index 21fb6b584e..870a42fd99 100644 --- a/src/test/app/DID_test.cpp +++ b/src/test/app/DID_test.cpp @@ -64,7 +64,7 @@ struct DID_test : public beast::unit_test::suite // Fund alice enough to exist, but not enough to meet // the reserve for creating a DID. - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const baseFee = env.current()->fees().base; env.fund(acctReserve, alice); diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index 6caedb53f1..8535582af9 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -691,7 +691,7 @@ struct EscrowToken_test : public beast::unit_test::suite { Env env{*this, features}; auto const baseFee = env.current()->fees().base; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const alice = Account("alice"); auto const bob = Account("bob"); @@ -2691,7 +2691,7 @@ struct EscrowToken_test : public beast::unit_test::suite { Env env{*this, features}; auto const baseFee = env.current()->fees().base; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const alice = Account("alice"); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index e9740e67de..f776695337 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -447,7 +447,7 @@ class MPToken_test : public beast::unit_test::suite // Test mptoken reserve requirement - first two mpts free (doApply) { Env env{*this, features}; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; // 1 drop diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 1c4314643c..9edbe4652c 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -208,7 +208,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Fund alice and minter enough to exist, but not enough to meet // the reserve for creating their first NFT. - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const baseFee = env.current()->fees().base; @@ -6744,8 +6744,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { // check reserve - auto const acctReserve = - env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; env.fund(acctReserve + incReserve, bob); @@ -7134,7 +7133,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Account const bob{"bob"}; Env env{*this, features}; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const baseFee = env.current()->fees().base; @@ -7217,7 +7216,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Account const bob{"bob"}; Env env{*this, features}; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; env.fund(XRP(10000), alice); @@ -7314,7 +7313,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Account const bob{"bob"}; Env env{*this, features}; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const baseFee = env.current()->fees().base; @@ -7365,7 +7364,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Account const broker{"broker"}; Env env{*this, features}; - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; auto const baseFee = env.current()->fees().base; diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 31e34ccf17..b177261e37 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -528,7 +528,7 @@ class PermissionedDomains_test : public beast::unit_test::suite // Fund alice enough to exist, but not enough to meet // the reserve. - auto const acctReserve = env.current()->fees().accountReserve(0); + auto const acctReserve = env.current()->fees().reserve; auto const incReserve = env.current()->fees().increment; env.fund(acctReserve, alice); env.close(); diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index 18457b5faf..00ca0aa3a7 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -192,7 +192,7 @@ public: auto const& assistor = createOnHighAcct ? acctC : acctD; auto const txFee = env.current()->fees().base; - auto const baseReserve = env.current()->fees().accountReserve(0); + auto const baseReserve = env.current()->fees().reserve; auto const threelineReserve = env.current()->fees().accountReserve(3); env.fund(XRP(10000), gwA, gwB, assistor); diff --git a/src/xrpld/app/misc/FeeVoteImpl.cpp b/src/xrpld/app/misc/FeeVoteImpl.cpp index 85b5791d67..c688cd3b19 100644 --- a/src/xrpld/app/misc/FeeVoteImpl.cpp +++ b/src/xrpld/app/misc/FeeVoteImpl.cpp @@ -142,7 +142,7 @@ FeeVoteImpl::doValidation( }; vote(lastFees.base, target_.reference_fee, "base fee", sfBaseFeeDrops); vote( - lastFees.accountReserve(0), + lastFees.reserve, target_.account_reserve, "base reserve", sfReserveBaseDrops); @@ -178,7 +178,7 @@ FeeVoteImpl::doValidation( vote(lastFees.base, target_.reference_fee, to64, "base fee", sfBaseFee); vote( - lastFees.accountReserve(0), + lastFees.reserve, target_.account_reserve, to32, "base reserve", @@ -207,7 +207,7 @@ FeeVoteImpl::doVoting( lastClosedLedger->fees().base, target_.reference_fee); detail::VotableValue baseReserveVote( - lastClosedLedger->fees().accountReserve(0), target_.account_reserve); + lastClosedLedger->fees().reserve, target_.account_reserve); detail::VotableValue incReserveVote( lastClosedLedger->fees().increment, target_.owner_reserve); diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 3154426696..d649bb3a29 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -2936,8 +2936,7 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) if (!human) { l[jss::base_fee] = baseFee.jsonClipped(); - l[jss::reserve_base] = - lpClosed->fees().accountReserve(0).jsonClipped(); + l[jss::reserve_base] = lpClosed->fees().reserve.jsonClipped(); l[jss::reserve_inc] = lpClosed->fees().increment.jsonClipped(); l[jss::close_time] = Json::Value::UInt( lpClosed->info().closeTime.time_since_epoch().count()); @@ -2945,8 +2944,7 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) else { l[jss::base_fee_xrp] = baseFee.decimalXRP(); - l[jss::reserve_base_xrp] = - lpClosed->fees().accountReserve(0).decimalXRP(); + l[jss::reserve_base_xrp] = lpClosed->fees().reserve.decimalXRP(); l[jss::reserve_inc_xrp] = lpClosed->fees().increment.decimalXRP(); if (auto const closeOffset = app_.timeKeeper().closeOffset(); @@ -3136,8 +3134,7 @@ NetworkOPsImp::pubLedger(std::shared_ptr const& lpAccepted) if (!lpAccepted->rules().enabled(featureXRPFees)) jvObj[jss::fee_ref] = Config::FEE_UNITS_DEPRECATED; jvObj[jss::fee_base] = lpAccepted->fees().base.jsonClipped(); - jvObj[jss::reserve_base] = - lpAccepted->fees().accountReserve(0).jsonClipped(); + jvObj[jss::reserve_base] = lpAccepted->fees().reserve.jsonClipped(); jvObj[jss::reserve_inc] = lpAccepted->fees().increment.jsonClipped(); @@ -4215,8 +4212,7 @@ NetworkOPsImp::subLedger(InfoSub::ref isrListener, Json::Value& jvResult) if (!lpClosed->rules().enabled(featureXRPFees)) jvResult[jss::fee_ref] = Config::FEE_UNITS_DEPRECATED; jvResult[jss::fee_base] = lpClosed->fees().base.jsonClipped(); - jvResult[jss::reserve_base] = - lpClosed->fees().accountReserve(0).jsonClipped(); + jvResult[jss::reserve_base] = lpClosed->fees().reserve.jsonClipped(); jvResult[jss::reserve_inc] = lpClosed->fees().increment.jsonClipped(); jvResult[jss::network_id] = app_.config().NETWORK_ID; } diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 6924dae6c8..7c0a6f07e2 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -1113,7 +1113,7 @@ TxQ::apply( comparable scale to the base fee, ignore the reserve. Only check the account balance. */ - auto const reserve = view.fees().accountReserve(0); + auto const reserve = view.fees().reserve; auto const base = view.fees().base; if (totalFee >= balance || (reserve > 10 * base && totalFee >= reserve)) diff --git a/src/xrpld/app/paths/PathRequest.cpp b/src/xrpld/app/paths/PathRequest.cpp index 8a88e774d0..a9db61d5b3 100644 --- a/src/xrpld/app/paths/PathRequest.cpp +++ b/src/xrpld/app/paths/PathRequest.cpp @@ -206,8 +206,7 @@ PathRequest::isValid(std::shared_ptr const& crCache) return false; } - if (!convert_all_ && - saDstAmount < STAmount(lrLedger->fees().accountReserve(0))) + if (!convert_all_ && saDstAmount < STAmount(lrLedger->fees().reserve)) { // Payment must meet reserve. jvStatus = rpcError(rpcDST_AMT_MALFORMED); diff --git a/src/xrpld/app/paths/Pathfinder.cpp b/src/xrpld/app/paths/Pathfinder.cpp index 4bc9304853..7f9c2f8437 100644 --- a/src/xrpld/app/paths/Pathfinder.cpp +++ b/src/xrpld/app/paths/Pathfinder.cpp @@ -278,7 +278,7 @@ Pathfinder::findPaths( return false; } - auto const reserve = STAmount(mLedger->fees().accountReserve(0)); + auto const reserve = STAmount(mLedger->fees().reserve); if (mDstAmount < reserve) { JLOG(j_.debug()) diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 81a083e336..b9bdbd0a34 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -346,7 +346,7 @@ Payment::preclaim(PreclaimContext const& ctx) // transaction would succeed. return telNO_DST_PARTIAL; } - else if (dstAmount < STAmount(ctx.view.fees().accountReserve(0))) + else if (dstAmount < STAmount(ctx.view.fees().reserve)) { // accountReserve is the minimum amount that an account can have. // Reserve is not scaled by load. @@ -690,7 +690,7 @@ Payment::doApply() // to get the account un-wedged. // Get the base reserve. - XRPAmount const dstReserve{view().fees().accountReserve(0)}; + XRPAmount const dstReserve{view().fees().reserve}; if (dstAmount > dstReserve || sleDst->getFieldAmount(sfBalance) > dstReserve) diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index d7731d8d98..503a01c3c9 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -476,7 +476,7 @@ transferHelper( // Already checked, but OK to check again return tecNO_DST; } - if (amt < psb.fees().accountReserve(0)) + if (amt < psb.fees().reserve) { JLOG(j.trace()) << "Insufficient payment to create account."; return tecNO_DST_INSUF_XRP; From 108f90586c68a83680e2f2710767684e22f5d97d Mon Sep 17 00:00:00 2001 From: Jingchen Date: Wed, 15 Oct 2025 21:53:01 +0100 Subject: [PATCH 17/18] chore: Reduce build log verbosity on Windows (#5865) Windows is extremely chatty and generates tons of logs when building, making it practically impossible to use the build logs to debug issues. This change sets the verbosity to 'quiet' on Windows. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- .github/actions/build-deps/action.yml | 7 +++++++ .github/workflows/upload-conan-deps.yml | 3 +++ conan/global.conf | 5 +---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/actions/build-deps/action.yml b/.github/actions/build-deps/action.yml index a908c656e8..7b2a3c385a 100644 --- a/.github/actions/build-deps/action.yml +++ b/.github/actions/build-deps/action.yml @@ -4,6 +4,10 @@ description: "Install Conan dependencies, optionally forcing a rebuild of all de # Note that actions do not support 'type' and all inputs are strings, see # https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax#inputs. inputs: + verbosity: + description: "The build verbosity." + required: false + default: "verbose" build_dir: description: "The directory where to build." required: true @@ -34,4 +38,7 @@ runs: --options:host='&:tests=True' \ --options:host='&:xrpld=True' \ --settings:all build_type='${{ env.BUILD_TYPE }}' \ + --conf:all tools.build:verbosity='${{ inputs.verbosity }}' \ + --conf:all tools.compilation:verbosity='${{ inputs.verbosity }}' \ + --conf:all tools.build:jobs=$(nproc) \ .. diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 680602d978..aedd367f65 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -79,6 +79,9 @@ jobs: build_dir: .build build_type: ${{ matrix.build_type }} force_build: ${{ github.event_name == 'schedule' || github.event.inputs.force_source_build == 'true' }} + # The verbosity is set to "quiet" for Windows to avoid an excessive amount of logs, while it + # is set to "verbose" otherwise to provide more information during the build process. + verbosity: ${{ runner.os == 'Windows' && 'quiet' || 'verbose' }} - name: Log into Conan remote if: ${{ github.repository_owner == 'XRPLF' && github.event_name != 'pull_request' }} diff --git a/conan/global.conf b/conan/global.conf index ae03818232..41ac76da89 100644 --- a/conan/global.conf +++ b/conan/global.conf @@ -1,9 +1,6 @@ # Global configuration for Conan. This is used to set the number of parallel -# downloads, uploads, and build jobs. The verbosity is set to verbose to -# provide more information during the build process. +# downloads, uploads, and build jobs. core:non_interactive=True core.download:parallel={{ os.cpu_count() }} core.upload:parallel={{ os.cpu_count() }} tools.build:jobs={{ (os.cpu_count() * 4/5) | int }} -tools.build:verbosity=verbose -tools.compilation:verbosity=verbose From a422855ea7415c1e2e8d135812021823d8a641c1 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 15 Oct 2025 16:55:11 -0400 Subject: [PATCH 18/18] refactor: replace JSON LastLedgerSequence with last_ledger_seq (#5884) This change replaces instances of JSON LastLedgerSequence with last_ledger_seq, which makes the tests a bit simpler and easier to read. Co-authored-by: Bart Thomee <11445373+bthomee@users.noreply.github.com> --- src/test/app/Transaction_ordering_test.cpp | 16 +++------ src/test/app/TxQ_test.cpp | 42 +++++++++------------- src/test/rpc/LedgerRPC_test.cpp | 2 +- 3 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/test/app/Transaction_ordering_test.cpp b/src/test/app/Transaction_ordering_test.cpp index 472d4e2ab8..5ace79db59 100644 --- a/src/test/app/Transaction_ordering_test.cpp +++ b/src/test/app/Transaction_ordering_test.cpp @@ -37,10 +37,8 @@ struct Transaction_ordering_test : public beast::unit_test::suite auto const aliceSequence = env.seq(alice); auto const tx1 = env.jt(noop(alice), seq(aliceSequence)); - auto const tx2 = env.jt( - noop(alice), - seq(aliceSequence + 1), - json(R"({"LastLedgerSequence":7})")); + auto const tx2 = + env.jt(noop(alice), seq(aliceSequence + 1), last_ledger_seq(7)); env(tx1); env.close(); @@ -83,10 +81,8 @@ struct Transaction_ordering_test : public beast::unit_test::suite auto const aliceSequence = env.seq(alice); auto const tx1 = env.jt(noop(alice), seq(aliceSequence)); - auto const tx2 = env.jt( - noop(alice), - seq(aliceSequence + 1), - json(R"({"LastLedgerSequence":7})")); + auto const tx2 = + env.jt(noop(alice), seq(aliceSequence + 1), last_ledger_seq(7)); env(tx2, ter(terPRE_SEQ)); BEAST_EXPECT(env.seq(alice) == aliceSequence); @@ -131,9 +127,7 @@ struct Transaction_ordering_test : public beast::unit_test::suite for (auto i = 0; i < 5; ++i) { tx.emplace_back(env.jt( - noop(alice), - seq(aliceSequence + i), - json(R"({"LastLedgerSequence":7})"))); + noop(alice), seq(aliceSequence + i), last_ledger_seq(7))); } for (auto i = 1; i < 5; ++i) diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index d0965cc8ff..190acfeddf 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -747,11 +747,9 @@ public: BEAST_EXPECT(env.current()->info().seq == 6); // Fail to queue an item with a low LastLedgerSeq - env(noop(alice), - json(R"({"LastLedgerSequence":7})"), - ter(telCAN_NOT_QUEUE)); + env(noop(alice), last_ledger_seq(7), ter(telCAN_NOT_QUEUE)); // Queue an item with a sufficient LastLedgerSeq. - env(noop(alice), json(R"({"LastLedgerSequence":8})"), queued); + env(noop(alice), last_ledger_seq(8), queued); constexpr auto largeFeeMultiplier = 700; auto const largeFee = baseFee * largeFeeMultiplier; @@ -2705,21 +2703,15 @@ public: auto const aliceSeq = env.seq(alice); BEAST_EXPECT(env.current()->info().seq == 3); - env(noop(alice), - seq(aliceSeq), - json(R"({"LastLedgerSequence":5})"), - ter(terQUEUED)); - env(noop(alice), - seq(aliceSeq + 1), - json(R"({"LastLedgerSequence":5})"), - ter(terQUEUED)); + env(noop(alice), seq(aliceSeq), last_ledger_seq(5), ter(terQUEUED)); + env(noop(alice), seq(aliceSeq + 1), last_ledger_seq(5), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 2), - json(R"({"LastLedgerSequence":10})"), + last_ledger_seq(10), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 3), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); checkMetrics(*this, env, 4, std::nullopt, 2, 1); auto const bobSeq = env.seq(bob); @@ -2816,39 +2808,39 @@ public: ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 11), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 12), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 13), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 14), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 15), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 16), - json(R"({"LastLedgerSequence": 5})"), + last_ledger_seq(5), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 17), - json(R"({"LastLedgerSequence": 5})"), + last_ledger_seq(5), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 18), - json(R"({"LastLedgerSequence": 5})"), + last_ledger_seq(5), ter(terQUEUED)); env(noop(alice), seq(aliceSeq + 19), - json(R"({"LastLedgerSequence":11})"), + last_ledger_seq(11), ter(terQUEUED)); checkMetrics(*this, env, 10, std::nullopt, 2, 1); @@ -4575,7 +4567,7 @@ public: env(noop(alice), seq(seqAlice++), fee(--feeDrops), - json(R"({"LastLedgerSequence": 7})"), + last_ledger_seq(7), ter(terQUEUED)); env(noop(alice), seq(seqAlice++), fee(--feeDrops), ter(terQUEUED)); env(noop(alice), seq(seqAlice++), fee(--feeDrops), ter(terQUEUED)); @@ -4585,7 +4577,7 @@ public: // The drop penalty works a little differently with tickets. env(noop(bob), ticket::use(bobTicketSeq + 0), - json(R"({"LastLedgerSequence": 7})"), + last_ledger_seq(7), ter(terQUEUED)); env(noop(bob), ticket::use(bobTicketSeq + 1), diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 9309fbdd6c..3c8da6dc13 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -527,7 +527,7 @@ class LedgerRPC_test : public beast::unit_test::suite // Alice auto aliceSeq = env.seq(alice); env(pay(alice, "george", XRP(1000)), - json(R"({"LastLedgerSequence":7})"), + last_ledger_seq(7), ter(terQUEUED)); env(offer(alice, XRP(50000), alice["USD"](5000)), seq(aliceSeq + 1),