Define ASSERT consistently with other instrumentation macros

This also removes XRPL_UNREACHABLE, since the default JSON constructor
allows us to use UNREACHABLE macro as-is, without the extra parameter

CONTRIBUTING.md

@@ -349,14 +349,19 @@ We are using [Antithesis](https://antithesis.com/) for continuous fuzzing,
 and keep a copy of [Antithesis C++ SDK](https://github.com/antithesishq/antithesis-sdk-cpp/)
 in `external/antithesis-sdk`. One of the aims of fuzzing is to identify bugs
 by finding external conditions which cause contracts violations inside `rippled`.
-The contracts are expressed as `XRPL_ASSERT` or `XRPL_UNREACHABLE` (defined in 
+The contracts are expressed as `ASSERT` or `UNREACHABLE` (defined in
 `include/xrpl/beast/utility/instrumentation.h`), which are effectively (outside
 of Antithesis) wrappers for `assert(...)` with added name. The purpose of name
 is to provide contracts with stable identity which does not rely on line numbers.
 
 When `rippled` is built with the Antithesis instrumentation enabled
-(using `voidstar` CMake option), the `XRPL_...` contracts will report any
-violations on the Antithesis platform during fuzzing.
+(using `voidstar` CMake option) and ran on the Antithesis platform, the
+contracts become
+[test properties](https://antithesis.com/docs/using_antithesis/properties.html);
+otherwise they are just like a regular `assert`.
+To learn more about Antithesis, see
+[How Antithesis Works](https://antithesis.com/docs/introduction/how_antithesis_works.html)
+and [C++ SDK](https://antithesis.com/docs/using_antithesis/sdk/cpp/overview.html#)
 
 We continue to use the old style `assert` or `assert(false)` in certain
 locations, where the reporting of contract violations on the Antithesis
@@ -367,19 +372,28 @@ For this reason:
   * `constexpr` functions
   * unit tests i.e. files under `src/test`
   * unit tests-related modules (files under `beast/test` and `beast/unit_test`)
-* Outside of the listed locations, do not use `assert`; use `XRPL_ASSERT` instead,
+* Outside of the listed locations, do not use `assert`; use `ASSERT` instead,
   giving it unique name, with the short description of the contract.
 * Outside of the listed locations, do not user `assert(false)`; use
-  `XRPL_UNREACHABLE` instead, giving it unique name, with the description of the
+  `UNREACHABLE` instead, giving it unique name, with the description of the
   condition being violated
 * The contract name should start with a full name (including scope) of the
   function, optionally a named lambda, followed by a colon ` : ` and a brief
-  (typically at most five words) description. `XRPL_UNREACHABLE` contracts
+  (typically at most five words) description. `UNREACHABLE` contracts
   can use slightly longer descriptions. If there are multiple overloads of the
   function, use common sense to balance both brevity and unambiguity of the
   function name. NOTE: the purpose of name is to provide stable means of
   unique identification of every contract; for this reason try to avoid elements
-  which can change in some obvious refactors.
+  which can change in some obvious refactors or when reinforcing the condition.
+* Example good name for an
+  `UNREACHABLE` macro `"Json::operator==(Value, Value) : invalid type"`; example
+  good name for an `ASSERT` macro `"Json::Value::asCString : valid type"`.
+* Example **bad** name
+  `"RFC1751::insert(char* s, int x, int start, int length) : length is greater than or equal zero"`
+  (missing namespace, unnecessary full function signature, description too verbose).
+  Good name: `"ripple::RFC1751::insert : minimum length"`.
+* Do **not** rename a contract without a good reason (e.g. the name no longer
+  reflects the location or the condition being checked)
 * Do not use `std::unreachable`
 * Do not put contracts where they can be violated by an external condition
   (e.g. timing, data payload before mandatory validation etc.) as this creates

include/xrpl/basics/Buffer.h

@@ -112,10 +112,10 @@ class Buffer
     operator=(Slice s)
     {
         // Ensure the slice isn't a subset of the buffer.
-        XRPL_ASSERT(
-            "ripple::Buffer::operator=(Slice) : input not a subset",
+        ASSERT(
             s.size() == 0 || size_ == 0 || s.data() < p_.get() ||
-                s.data() >= p_.get() + size_);
+                s.data() >= p_.get() + size_,
+            "ripple::Buffer::operator=(Slice) : input not a subset");
 
         if (auto p = alloc(s.size()))
             std::memcpy(p, s.data(), s.size());

include/xrpl/basics/FeeUnits.h

@@ -417,13 +417,13 @@ mulDivU(Source1 value, Dest mul, Source2 div)
     {
         // split the asserts so if one hits, the user can tell which
         // without a debugger.
-        XRPL_ASSERT(
-            "ripple::feeunit::mulDivU : minimum value input",
-            value.value() >= 0);
-        XRPL_ASSERT(
-            "ripple::feeunit::mulDivU : minimum mul input", mul.value() >= 0);
-        XRPL_ASSERT(
-            "ripple::feeunit::mulDivU : minimum div input", div.value() >= 0);
+        ASSERT(
+            value.value() >= 0,
+            "ripple::feeunit::mulDivU : minimum value input");
+        ASSERT(
+            mul.value() >= 0, "ripple::feeunit::mulDivU : minimum mul input");
+        ASSERT(
+            div.value() >= 0, "ripple::feeunit::mulDivU : minimum div input");
         return std::nullopt;
     }
 

include/xrpl/basics/MathUtilities.h

@@ -43,7 +43,7 @@ namespace ripple {
 constexpr std::size_t
 calculatePercent(std::size_t count, std::size_t total)
 {
-    assert(total != 0);  // NOTE No XRPL_ASSERT here, because constexpr
+    assert(total != 0);  // NOTE No ASSERT here, because constexpr
     return ((std::min(count, total) * 100) + total - 1) / total;
 }
 

include/xrpl/basics/SlabAllocator.h

@@ -143,9 +143,9 @@ class SlabAllocator
         void
         deallocate(std::uint8_t* ptr) noexcept
         {
-            XRPL_ASSERT(
-                "ripple::SlabAllocator::SlabBlock::deallocate : own input",
-                own(ptr));
+            ASSERT(
+                own(ptr),
+                "ripple::SlabAllocator::SlabBlock::deallocate : own input");
 
             std::lock_guard l(m_);
 
@@ -188,9 +188,9 @@ class SlabAllocator
               boost::alignment::align_up(sizeof(Type) + extra, itemAlignment_))
         , slabSize_(alloc)
     {
-        XRPL_ASSERT(
-            "ripple::SlabAllocator::SlabAllocator : valid alignment",
-            (itemAlignment_ & (itemAlignment_ - 1)) == 0);
+        ASSERT(
+            (itemAlignment_ & (itemAlignment_ - 1)) == 0,
+            "ripple::SlabAllocator::SlabAllocator : valid alignment");
     }
 
     SlabAllocator(SlabAllocator const& other) = delete;
@@ -300,9 +300,10 @@ class SlabAllocator
     bool
     deallocate(std::uint8_t* ptr) noexcept
     {
-        XRPL_ASSERT(
-            "ripple::SlabAllocator::SlabAllocator::deallocate : non-null input",
-            ptr);
+        ASSERT(
+            ptr != nullptr,
+            "ripple::SlabAllocator::SlabAllocator::deallocate : non-null "
+            "input");
 
         for (auto slab = slabs_.load(); slab != nullptr; slab = slab->next_)
         {

include/xrpl/basics/Slice.h

@@ -103,9 +103,9 @@ class Slice
     std::uint8_t
     operator[](std::size_t i) const noexcept
     {
-        XRPL_ASSERT(
-            "ripple::Slice::operator[](std::size_t) const : input within range",
-            i < size_);
+        ASSERT(
+            i < size_,
+            "ripple::Slice::operator[](std::size_t) const : valid input");
         return data_[i];
     }
 

include/xrpl/basics/base_uint.h

@@ -290,9 +290,9 @@ class base_uint
             std::is_trivially_copyable<typename Container::value_type>::value>>
     explicit base_uint(Container const& c)
     {
-        XRPL_ASSERT(
-            "ripple::base_uint::base_uint(Container auto) : input size match",
-            c.size() * sizeof(typename Container::value_type) == size());
+        ASSERT(
+            c.size() * sizeof(typename Container::value_type) == size(),
+            "ripple::base_uint::base_uint(Container auto) : input size match");
         std::memcpy(data_.data(), c.data(), size());
     }
 
@@ -303,9 +303,9 @@ class base_uint
         base_uint&>
     operator=(Container const& c)
     {
-        XRPL_ASSERT(
-            "ripple::base_uint::operator=(Container auto) : input size match",
-            c.size() * sizeof(typename Container::value_type) == size());
+        ASSERT(
+            c.size() * sizeof(typename Container::value_type) == size(),
+            "ripple::base_uint::operator=(Container auto) : input size match");
         std::memcpy(data_.data(), c.data(), size());
         return *this;
     }

include/xrpl/basics/partitioned_unordered_map.h

@@ -246,10 +246,10 @@ class partitioned_unordered_map
             ? *partitions
             : std::thread::hardware_concurrency();
         map_.resize(partitions_);
-        XRPL_ASSERT(
+        ASSERT(
+            partitions_ != 0,
             "ripple::partitioned_unordered_map::partitioned_unordered_map : "
-            "nonzero partitions",
-            partitions_);
+            "nonzero partitions");
     }
 
     std::size_t

include/xrpl/basics/random.h

@@ -114,7 +114,7 @@ std::enable_if_t<
     Integral>
 rand_int(Engine& engine, Integral min, Integral max)
 {
-    XRPL_ASSERT("ripple::rand_int : max over min inputs", max > min);
+    ASSERT(max > min, "ripple::rand_int : max over min inputs");
 
     // This should have no state and constructing it should
     // be very cheap. If that turns out not to be the case

include/xrpl/basics/spinlock.h

@@ -117,9 +117,9 @@ class packed_spinlock
     packed_spinlock(std::atomic<T>& lock, int index)
         : bits_(lock), mask_(static_cast<T>(1) << index)
     {
-        XRPL_ASSERT(
-            "ripple::packed_spinlock::packed_spinlock : valid index and mask",
-            index >= 0 && (mask_ != 0));
+        ASSERT(
+            index >= 0 && (mask_ != 0),
+            "ripple::packed_spinlock::packed_spinlock : valid index and mask");
     }
 
     [[nodiscard]] bool

include/xrpl/beast/asio/io_latency_probe.h

@@ -174,10 +174,10 @@ class io_latency_probe
             , m_repeat(repeat)
             , m_probe(probe)
         {
-            XRPL_ASSERT(
+            ASSERT(
+                m_probe != nullptr,
                 "beast::io_latency_probe::sample_op::sample_op : non-null "
-                "probe input",
-                m_probe);
+                "probe input");
             m_probe->addref();
         }
 
@@ -187,10 +187,10 @@ class io_latency_probe
             , m_repeat(from.m_repeat)
             , m_probe(from.m_probe)
         {
-            XRPL_ASSERT(
+            ASSERT(
+                m_probe != nullptr,
                 "beast::io_latency_probe::sample_op::sample_op(sample_op&&) : "
-                "non-null probe input",
-                m_probe);
+                "non-null probe input");
             from.m_probe = nullptr;
         }
 

include/xrpl/beast/clock/manual_clock.h

@@ -61,9 +61,9 @@ class manual_clock : public abstract_clock<Clock>
     void
     set(time_point const& when)
     {
-        XRPL_ASSERT(
-            "beast::manual_clock::set(time_point) : forward input",
-            !Clock::is_steady || when >= now_);
+        ASSERT(
+            !Clock::is_steady || when >= now_,
+            "beast::manual_clock::set(time_point) : forward input");
         now_ = when;
     }
 
@@ -80,9 +80,9 @@ class manual_clock : public abstract_clock<Clock>
     void
     advance(std::chrono::duration<Rep, Period> const& elapsed)
     {
-        XRPL_ASSERT(
-            "beast::manual_clock::advance(duration) : forward input",
-            !Clock::is_steady || (now_ + elapsed) >= now_);
+        ASSERT(
+            !Clock::is_steady || (now_ + elapsed) >= now_,
+            "beast::manual_clock::advance(duration) : forward input");
         now_ += elapsed;
     }
 

include/xrpl/beast/container/detail/aged_unordered_container.h

@@ -1329,10 +1329,10 @@ class aged_unordered_container
     size_type
     bucket(Key const& k) const
     {
-        XRPL_ASSERT(
+        ASSERT(
+            bucket_count() != 0,
             "beast::detail::aged_unordered_container::bucket : nonzero bucket "
-            "count",
-            bucket_count() != 0);
+            "count");
         return m_cont.bucket(k, std::cref(m_config.hash_function()));
     }
 
@@ -1473,10 +1473,10 @@ class aged_unordered_container
     {
         if (would_exceed(additional))
             m_buck.resize(size() + additional, m_cont);
-        XRPL_ASSERT(
+        ASSERT(
+            load_factor() <= max_load_factor(),
             "beast::detail::aged_unordered_container::maybe_rehash : maximum "
-            "load factor",
-            load_factor() <= max_load_factor());
+            "load factor");
     }
 
     // map, set

include/xrpl/beast/core/LexicalCast.h

@@ -160,8 +160,9 @@ struct LexicalCast<Out, char const*>
     bool
     operator()(Out& out, char const* in) const
     {
-        XRPL_ASSERT(
-            "beast::detail::LexicalCast(char const*) : non-null input", in);
+        ASSERT(
+            in != nullptr,
+            "beast::detail::LexicalCast(char const*) : non-null input");
         return LexicalCast<Out, std::string_view>()(out, in);
     }
 };
@@ -176,7 +177,9 @@ struct LexicalCast<Out, char*>
     bool
     operator()(Out& out, char* in) const
     {
-        XRPL_ASSERT("beast::detail::LexicalCast(char*) : non-null input", in);
+        ASSERT(
+            in != nullptr,
+            "beast::detail::LexicalCast(char*) : non-null input");
         return LexicalCast<Out, std::string_view>()(out, in);
     }
 };

include/xrpl/beast/net/IPAddress.h

@@ -96,7 +96,7 @@ hash_append(Hasher& h, beast::IP::Address const& addr) noexcept
     else if (addr.is_v6())
         hash_append(h, addr.to_v6().to_bytes());
     else
-        XRPL_UNREACHABLE("beast::hash_append : invalid address type");
+        UNREACHABLE("beast::hash_append : invalid address type");
 }
 }  // namespace beast
 

include/xrpl/beast/utility/Journal.h

@@ -205,9 +205,9 @@ class Journal
         */
         Stream(Sink& sink, Severity level) : m_sink(sink), m_level(level)
         {
-            XRPL_ASSERT(
-                "beast::Journal::Stream::Stream : maximum level",
-                m_level < severities::kDisabled);
+            ASSERT(
+                m_level < severities::kDisabled,
+                "beast::Journal::Stream::Stream : maximum level");
         }
 
         /** Construct or copy another Stream. */

include/xrpl/beast/utility/instrumentation.h

@@ -28,19 +28,31 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #endif
 #include <antithesis_sdk.h>
 #else
-#define ALWAYS(cond, message, ...)
-#define ALWAYS_OR_UNREACHABLE(cond, message, ...) assert(message && (cond))
-#define SOMETIMES(cond, message, ...)
-#define REACHABLE(message, ...)
-#define UNREACHABLE(message, ...) assert(message && false)
+#define ALWAYS(cond, name, ...) assert((name) && (cond))
+#define ALWAYS_OR_UNREACHABLE(cond, name, ...) assert((name) && (cond))
+#define SOMETIMES(cond, name, ...)
+#define REACHABLE(name, ...)
+#define UNREACHABLE(name, ...) assert((name) && false)
 #endif
 
-#ifndef NDEBUG
-#define XRPL_ASSERT(M, ...) ALWAYS_OR_UNREACHABLE(((bool)(__VA_ARGS__)), M, {})
-#define XRPL_UNREACHABLE(M) UNREACHABLE(M, {})
-#else
-#define XRPL_ASSERT(...)
-#define XRPL_UNREACHABLE(M)
-#endif
+#define ASSERT ALWAYS_OR_UNREACHABLE
+
+// How to use the above macros:
+//
+// ALWAYS if it must be reached during fuzzing
+// ASSERT if you expect that it might, or might not, be reached during fuzzing
+// REACHABLE if it must be reached during fuzzing
+// SOMETIMES a hint for the fuzzer to try to make the condition true
+// UNREACHABLE if it must not be reached (in fuzzing or in normal use)
+//
+// NOTE: ASSERT has similar semantics as assert macro, with minor differences:
+// * ASSERT must have an unique name (naming convention in CONTRIBUTING.md)
+// * the condition (which comes first) must be *implicitly* convertible to bool
+// * during fuzzing, the program will continue execution past a failed ASSERT
+// We continue to use assert inside unit tests and in constexpr functions.
+//
+// NOTE: UNREACHABLE does *not* have the same semantics as std::unreachable.
+// The program will continue execution past an UNREACHABLE in a Release build
+// and during fuzzing (similar to ASSERT).
 
 #endif

include/xrpl/beast/utility/rngfill.h

@@ -42,8 +42,8 @@ rngfill(void* buffer, std::size_t bytes, Generator& g)
         bytes -= sizeof(v);
     }
 
-    XRPL_ASSERT(
-        "beast::rngfill(void*) : maximum bytes", bytes < sizeof(result_type));
+    ASSERT(
+        bytes < sizeof(result_type), "beast::rngfill(void*) : maximum bytes");
 
 #ifdef __GNUC__
     // gcc 11.1 (falsely) warns about an array-bounds overflow in release mode.