util: Add util::NotNull<SmartPtrType> #34844

pull maflcko wants to merge 3 commits into bitcoin:master from maflcko:2603-not-null changing 6 files +473 −6
  1. maflcko commented at 7:42 PM on March 17, 2026: member

    The C++ standard library lacks a type to denote a smart pointer is not null. For raw pointer there is std::reference_wrapper, or a plain reference.

    Other third-party libraries provide such a type, such as gsl::strict_not_null.

    Fix all issues by adding util::NotNull<SmartPtrType>, an alias of gsl::strinct_not_null, which documents (and checks) that the inner pointer is never null.

    This type can be used when passing never-null smart pointers between functions. It removes the need to Assert() the pointer before dereference. For example, in a getter function:

    util::NotNull<std::unique_ptr<Stats>> GetStats()
{
    return util::NotNull{std::make_unique<Stats>()};
}

int main()
{
    auto stats{GetStats()};
    stats->foo; // This can never lead to a nullptr deref
    // Assert(stats)->foo; // This is redundant and won't compile
}

    Fixes https://github.com/bitcoin/bitcoin/issues/24423

  2. DrahtBot added the label Utils/log/libs on Mar 17, 2026
  3. DrahtBot commented at 7:43 PM on March 17, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34844.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK stickies-v, l0rinc

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. maflcko force-pushed on Mar 17, 2026
  5. DrahtBot added the label CI failed on Mar 17, 2026
  6. DrahtBot commented at 8:03 PM on March 17, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/23213194537/job/67467180922</sub> <sub>LLM reason (✨ experimental): Build failed during the cmake build, due to a compilation error in src/test/util_pointers_tests.cpp.</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  7. stickies-v commented at 8:08 PM on March 17, 2026: contributor

    Concept ACK

  8. maflcko force-pushed on Mar 17, 2026
  9. maflcko commented at 9:41 PM on March 17, 2026: member

    Looks like C++20 CTAD alias templates are not supported with clang-18, so I replaced this one-liner with a more verbose alternative in the last force push.

     template <class T> using NotNull = gsl_detail::strict_not_null<T>;
  10. DrahtBot removed the label CI failed on Mar 17, 2026
  11. maflcko force-pushed on Mar 24, 2026
  12. maflcko force-pushed on Mar 24, 2026
  13. DrahtBot added the label CI failed on Mar 24, 2026
  14. DrahtBot removed the label CI failed on Mar 24, 2026
  15. maflcko force-pushed on Apr 3, 2026
  16. DrahtBot added the label CI failed on Apr 3, 2026
  17. DrahtBot removed the label CI failed on Apr 3, 2026
  18. maflcko commented at 11:38 AM on April 17, 2026: member

    Probably not going to push here, but another place where this would be suitable is GetWalletForJSONRPCRequest

  19. maflcko force-pushed on Apr 21, 2026
  20. maflcko force-pushed on Apr 21, 2026
  21. DrahtBot added the label CI failed on Apr 21, 2026
  22. DrahtBot removed the label CI failed on Apr 21, 2026
  23. maflcko commented at 4:28 PM on April 21, 2026: member

    Removed the movable feature of NotNull for now, which can be added/reviewed later

  24. maflcko force-pushed on Apr 23, 2026
  25. util: Add util::NotNull<AnyPtrType>
    Based on gsl::strict_not_null, with minimal modifications.

It can be reviewed by calling

curl -fL 'https://raw.githubusercontent.com/microsoft/GSL/688ffcde9018910bef22dae9da9de974803dc982/include/gsl/pointers' -o ./src/util/pointers.h

and then confirming that the difference is documented and correct.
    fa1e0bb7d0
  26. refactor: Use util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context
    This allows to drop the runtime Assert every time the context is
accessed, because the type is known to be not null at compile-time.

Note there is still the runtime Assert inside the util::NotNull
constructor itself. However, this is called at most once, with the
possibility of the compiler being able to optimize it away.
    faef857079
  27. refactor: In CNode use util::NotNull<std::unique_ptr<Transport>> m_transport
    This documents (and checks) that the transport pointer is never null.
    77773e8c85
  28. maflcko force-pushed on May 22, 2026
  29. in src/test/util_pointers_tests.cpp:30 in fa1e0bb7d0 
      25 | +        ThrowingMoveNullPtr(ThrowingMoveNullPtr&& other) {};
  26 | +        bool operator==(std::nullptr_t) const { return true; }
  27 | +        int* p{};
  28 | +    };
  29 | +    static_assert(!std::is_nothrow_move_constructible_v<ThrowingMoveNullPtr>);
  30 | +    BOOST_CHECK_EXCEPTION(util::NotNull{ThrowingMoveNullPtr{}}, NonFatalCheckError, HasReason{"Internal bug detected: ptr_ != nullptr"});
    l0rinc commented at 10:40 AM on May 23, 2026:

    fa1e0bb util: Add util::NotNull<AnyPtrType>:

    nit: This failure reason is a bit hard to read: ptr_ != nullptr is the invariant that failed, so this test is actually checking the null case. In other words, the message prints the failed condition, not the observed value. Likely not something we can fix here...

  30. in src/dbwrapper.h:192 in faef857079 
     188 | @@ -188,7 +189,7 @@ class CDBWrapper
 189 |      friend const Obfuscation& dbwrapper_private::GetObfuscation(const CDBWrapper&);
 190 |  private:
 191 |      //! holds all leveldb-specific fields of this class
 192 | -    std::unique_ptr<LevelDBContext> m_db_context;
 193 | +    util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context;
    l0rinc commented at 11:10 AM on May 23, 2026:

    faef857 refactor: Use util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context:

    Could we avoid repeating the verbose template instantiation for all non-null smart pointers? I like the extra guarantee, but I don't like how verbose the results are. We could add NotNullUniquePtr and NotNullSharedPtr aliases and demo them at the mentioned call sites:

    diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 4c0b6e2629..46ce177c5c 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -189,7 +189,7 @@ class CDBWrapper
     friend const Obfuscation& dbwrapper_private::GetObfuscation(const CDBWrapper&);
 private:
     //! holds all leveldb-specific fields of this class
-    util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context;
+    util::NotNullUniquePtr<LevelDBContext> m_db_context;
 
     //! the name of this database
     std::string m_name;
diff --git a/src/net.cpp b/src/net.cpp
index f4c5dbd157..fab392814e 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -4004,12 +4004,12 @@ ServiceFlags CConnman::GetLocalServices() const
     return m_local_services;
 }
 
-static util::NotNull<std::unique_ptr<Transport>> MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept
+static util::NotNullUniquePtr<Transport> MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept
 {
     if (use_v2transport) {
-        return util::NotNull<std::unique_ptr<Transport>>{std::make_unique<V2Transport>(id, /*initiating=*/!inbound)};
+        return util::NotNullUniquePtr<Transport>{std::make_unique<V2Transport>(id, /*initiating=*/!inbound)};
     } else {
-        return util::NotNull<std::unique_ptr<Transport>>{std::make_unique<V1Transport>(id)};
+        return util::NotNullUniquePtr<Transport>{std::make_unique<V1Transport>(id)};
     }
 }
 
diff --git a/src/net.h b/src/net.h
index e6a680e048..0dbb692149 100644
--- a/src/net.h
+++ b/src/net.h
@@ -682,7 +682,7 @@ class CNode
 public:
     /** Transport serializer/deserializer. The receive side functions are only called under cs_vRecv, while
      * the sending side functions are only called under cs_vSend. */
-    const util::NotNull<std::unique_ptr<Transport>> m_transport;
+    const util::NotNullUniquePtr<Transport> m_transport;
 
     const NetPermissionFlags m_permission_flags;
 
diff --git a/src/test/util_pointers_tests.cpp b/src/test/util_pointers_tests.cpp
index 5d77e89e15..38c49a5cd6 100644
--- a/src/test/util_pointers_tests.cpp
+++ b/src/test/util_pointers_tests.cpp
@@ -7,9 +7,14 @@
 
 #include <boost/test/unit_test.hpp>
 
+#include <memory>
 #include <set>
+#include <type_traits>
 #include <unordered_set>
 
+static_assert(std::is_same_v<util::NotNullUniquePtr<int>, util::NotNull<std::unique_ptr<int>>>);
+static_assert(std::is_same_v<util::NotNullSharedPtr<int>, util::NotNull<std::shared_ptr<int>>>);
+
 BOOST_AUTO_TEST_SUITE(util_pointers_tests)
 
 BOOST_AUTO_TEST_CASE(check_nullptr)
diff --git a/src/util/pointers.h b/src/util/pointers.h
index 08af3c1b28..e757c6012b 100644
--- a/src/util/pointers.h
+++ b/src/util/pointers.h
@@ -16,6 +16,8 @@
 //    - strict_make_not_null, because it is not needed.
 // * Remove the not_null->strict_not_null converting constructors, because they
 //   are not needed.
+// * Add NotNullUniquePtr and NotNullSharedPtr aliases to keep smart-pointer
+//   call sites readable.
 //
 // All original code is covered by:
 
@@ -357,6 +359,12 @@ struct NotNull : public gsl_detail::strict_not_null<T> {
 template <typename T>
 NotNull(T) -> NotNull<T>;
 
+template <typename T, typename Deleter = std::default_delete<T>>
+using NotNullUniquePtr = NotNull<std::unique_ptr<T, Deleter>>;
+
+template <typename T>
+using NotNullSharedPtr = NotNull<std::shared_ptr<T>>;
+
 } // namespace util
 
 namespace std
  31. in src/util/pointers.h:115 in fa1e0bb7d0 
     110 | +    static_assert(details::is_comparable_to_nullptr<T>::value, "T cannot be compared to nullptr.");
 111 | +
 112 | +    using element_type = T;
 113 | +
 114 | +    template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
 115 | +    constexpr not_null(U&& u) noexcept(std::is_nothrow_move_constructible<T>::value) : ptr_(std::forward<U>(u))
    l0rinc commented at 11:17 AM on May 23, 2026:

    fa1e0bb util: Add util::NotNull<AnyPtrType>:

    The noexcept annotations are a bit confusing since that's exactly how we're testing the methods - check_nullptr works because it deliberately avoids the noexcept path. I understand that test_only_CheckFailuresAreExceptionsNotAborts isn't used in prod and we don't like modifying prod for tests, but we do have a failure case inside and I'm just not sure what happens if we are explicit about not throwing while deliberately relying on throwing behavior (currently this terminates before the test can observe NonFatalCheckError. Maybe that is acceptable for production, but it makes the main smart-pointer null checks harder to test).

    This way we can't test for example:

    BOOST_AUTO_TEST_CASE(check_null_smart_pointer)
{
    test_only_CheckFailuresAreExceptionsNotAborts mock_checks{};

    BOOST_CHECK_THROW(util::NotNull{std::unique_ptr<int>{}}, NonFatalCheckError);
    BOOST_CHECK_THROW(util::NotNull{std::shared_ptr<int>{}}, NonFatalCheckError);
}

    it just fails with

    unknown location:0: fatal error: in "util_pointers_tests/check_null_smart_pointer": signal: SIGABRT (application abort requested)

    But without the noexcept the above passes:

    diff --git a/src/util/pointers.h b/src/util/pointers.h
index f985c5331b..28dd458f58 100644
--- a/src/util/pointers.h
+++ b/src/util/pointers.h
@@ -116,19 +116,19 @@ public:
     using element_type = T;
 
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr not_null(U&& u) noexcept(std::is_nothrow_move_constructible<T>::value) : ptr_(std::forward<U>(u))
+    constexpr not_null(U&& u) : ptr_(std::forward<U>(u))
     {
         Assert(ptr_ != nullptr);
     }
 
     template <typename = std::enable_if_t<!std::is_same<std::nullptr_t, T>::value>>
-    constexpr not_null(T u) noexcept(std::is_nothrow_move_constructible<T>::value) : ptr_(std::move(u))
+    constexpr not_null(T u) : ptr_(std::move(u))
     {
         Assert(ptr_ != nullptr);
     }
 
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr not_null(const not_null<U>& other) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null(other.get())
+    constexpr not_null(const not_null<U>& other) : not_null(other.get())
     {}
 
     not_null(const not_null& other) = default;
@@ -169,7 +169,7 @@ void swap(not_null<T>& a, not_null<T>& b) noexcept
 }
 
 template <class T>
-auto make_not_null(T&& t) noexcept
+auto make_not_null(T&& t)
 {
     return not_null<std::remove_cv_t<std::remove_reference_t<T>>>{std::forward<T>(t)};
 }
@@ -284,15 +284,15 @@ class strict_not_null : public not_null<T>
 {
 public:
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr explicit strict_not_null(U&& u) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(std::forward<U>(u))
+    constexpr explicit strict_not_null(U&& u) : not_null<T>(std::forward<U>(u))
     {}
 
     template <typename = std::enable_if_t<!std::is_same<std::nullptr_t, T>::value>>
-    constexpr explicit strict_not_null(T u) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(std::move(u))
+    constexpr explicit strict_not_null(T u) : not_null<T>(std::move(u))
     {}
 
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr strict_not_null(const strict_not_null<U>& other) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(other)
+    constexpr strict_not_null(const strict_not_null<U>& other) : not_null<T>(other)
     {}
 
     // To avoid invalidating the "not null" invariant, the contained pointer is actually copied
@@ -326,7 +326,7 @@ template <class T>
 strict_not_null<T> operator+(std::ptrdiff_t, const strict_not_null<T>&) = delete;
 
 template <class T>
-auto make_strict_not_null(T&& t) noexcept
+auto make_strict_not_null(T&& t)
 {
     return strict_not_null<std::remove_cv_t<std::remove_reference_t<T>>>{std::forward<T>(t)};
 }
  32. in src/net.cpp:4012 in 77773e8c85 
    4010 |      if (use_v2transport) {
4011 | -        return std::make_unique<V2Transport>(id, /*initiating=*/!inbound);
4012 | +        return util::NotNull<std::unique_ptr<Transport>>{std::make_unique<V2Transport>(id, /*initiating=*/!inbound)};
4013 |      } else {
4014 | -        return std::make_unique<V1Transport>(id);
4015 | +        return util::NotNull<std::unique_ptr<Transport>>{std::make_unique<V1Transport>(id)};
    l0rinc commented at 11:48 AM on May 23, 2026:

    77773e8 refactor: In CNode use util::NotNull<std::unique_ptr<Transport>> m_transport:

    This extra verbosity is a bit of a turn-off. Can we add a forwarding constructor for class-type pointer wrappers (diff assumes the previous NotNullUniquePtr suggestion)?

    diff --git a/src/net.cpp b/src/net.cpp
index fab392814e..aaf946f4ab 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -4007,9 +4007,9 @@ ServiceFlags CConnman::GetLocalServices() const
 static util::NotNullUniquePtr<Transport> MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept
 {
     if (use_v2transport) {
-        return util::NotNullUniquePtr<Transport>{std::make_unique<V2Transport>(id, /*initiating=*/!inbound)};
+        return std::make_unique<V2Transport>(id, /*initiating=*/!inbound);
     } else {
-        return util::NotNullUniquePtr<Transport>{std::make_unique<V1Transport>(id)};
+        return std::make_unique<V1Transport>(id);
     }
 }
 
diff --git a/src/test/util_pointers_tests.cpp b/src/test/util_pointers_tests.cpp
index 38c49a5cd6..85807f8e73 100644
--- a/src/test/util_pointers_tests.cpp
+++ b/src/test/util_pointers_tests.cpp
@@ -63,6 +63,7 @@ BOOST_AUTO_TEST_CASE(check_swap)
 BOOST_AUTO_TEST_CASE(check_deref)
 {
     int v{2};
+    static_assert(!std::is_convertible_v<int*, util::NotNull<int*>>); // Keep raw-pointer NotNull construction explicit.
     util::NotNull p(&v);
     *p = 3;
     BOOST_CHECK_EQUAL(v, 3);
diff --git a/src/util/pointers.h b/src/util/pointers.h
index e757c6012b..73bb45e3e7 100644
--- a/src/util/pointers.h
+++ b/src/util/pointers.h
@@ -18,6 +18,7 @@
 //   are not needed.
 // * Add NotNullUniquePtr and NotNullSharedPtr aliases to keep smart-pointer
 //   call sites readable.
+// * Add a forwarding constructor for concise non-null smart-pointer returns.
 //
 // All original code is covered by:
 
@@ -353,8 +354,13 @@ struct hash<gsl_detail::strict_not_null<T>> : gsl_detail::not_null_hash<gsl_deta
 namespace util {
 
 template <class T>
-struct NotNull : public gsl_detail::strict_not_null<T> {
-    using gsl_detail::strict_not_null<T>::strict_not_null;
+struct NotNull : gsl_detail::strict_not_null<T> {
+    using Base = gsl_detail::strict_not_null<T>;
+    using Base::Base;
+
+    template <typename U>
+    requires (!std::is_pointer_v<T> && std::is_convertible_v<U, T>)
+    constexpr NotNull(U&& u) : Base{std::forward<U>(u)} {}
 };
 template <typename T>
 NotNull(T) -> NotNull<T>;

    Which obviously begs the question: do we ever want to convert back. Given your hint for converting GetWalletForJSONRPCRequest it may be necessary (unless we propagate the type further), but I agree with you that this could also be done in a followup.

    <details><summary>Migrate `GetWalletForJSONRPCRequest` to `util::NotNullSharedPtr<CWallet>`</summary>

    diff --git a/src/util/pointers.h b/src/util/pointers.h
index 8f740472a3..5826e21cdc 100644
--- a/src/util/pointers.h
+++ b/src/util/pointers.h
@@ -21,6 +21,8 @@
 // * Add a forwarding constructor for concise non-null smart-pointer returns.
 // * Delete util::NotNull moves so the wrapper does not advertise misleading
 //   move operations.
+// * Add compatible shared_ptr conversions so NotNullSharedPtr<T> can initialize
+//   std::shared_ptr<const T> callers without rebuilding the handle.
 //
 // All original code is covered by:
 
@@ -78,6 +80,12 @@ namespace details
                                             const T,
                                             const T&>;
 
+    template <typename T>
+    struct is_shared_ptr : std::false_type {};
+
+    template <typename T>
+    struct is_shared_ptr<std::shared_ptr<T>> : std::true_type {};
+
 } // namespace details
 
 //
@@ -380,6 +388,10 @@ struct NotNull : gsl_detail::strict_not_null<T> {
     // underlying smart pointer at transfer boundaries.
     NotNull(NotNull&&) = delete;
     NotNull& operator=(NotNull&&) = delete;
+
+    template <typename U>
+        requires (!std::is_same_v<U, T> && gsl_detail::details::is_shared_ptr<T>::value && gsl_detail::details::is_shared_ptr<U>::value && std::is_convertible_v<T, U>)
+    constexpr operator U() const { return this->get(); }
 };
 template <typename T>
 NotNull(T) -> NotNull<T>;
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index ed966d8944..9082407d6d 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -39,7 +39,6 @@ RPCMethod getnewaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -88,7 +87,6 @@ RPCMethod getrawchangeaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -132,7 +130,6 @@ RPCMethod setlabel()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -183,7 +180,6 @@ RPCMethod listaddressgroupings()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -232,7 +228,6 @@ RPCMethod keypoolrefill()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -423,7 +418,6 @@ RPCMethod getaddressinfo()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -536,7 +530,6 @@ RPCMethod getaddressesbylabel()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -600,7 +593,6 @@ RPCMethod listlabels()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -648,7 +640,6 @@ RPCMethod walletdisplayaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
-            if (!wallet) return UniValue::VNULL;
             CWallet* const pwallet = wallet.get();
 
             LOCK(pwallet->cs_wallet);
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 396be62825..a1cc43613c 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -50,7 +50,6 @@ RPCMethod importprunedfunds()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     CMutableTransaction tx;
     if (!DecodeHexTx(tx, request.params[0].get_str())) {
@@ -108,7 +107,6 @@ RPCMethod removeprunedfunds()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -377,7 +375,6 @@ RPCMethod importdescriptors()
         [](const RPCMethod& self, const JSONRPCRequest& main_request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(main_request);
-    if (!pwallet) return UniValue::VNULL;
     CWallet& wallet{*pwallet};
 
     // Make sure the results are valid at least up to the most recent block
@@ -515,7 +512,6 @@ RPCMethod listdescriptors()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> wallet = GetWalletForJSONRPCRequest(request);
-    if (!wallet) return UniValue::VNULL;
 
     const bool priv = !request.params[0].isNull() && request.params[0].get_bool();
     if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && priv) {
@@ -608,7 +604,6 @@ RPCMethod backupwallet()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index ab869b0d3f..8eadaf1ded 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -105,7 +105,6 @@ RPCMethod getreceivedbyaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -147,7 +146,6 @@ RPCMethod getreceivedbylabel()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -188,7 +186,6 @@ RPCMethod getbalance()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -258,7 +255,6 @@ RPCMethod lockunspent()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -376,7 +372,6 @@ RPCMethod listlockunspent()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     LOCK(pwallet->cs_wallet);
 
@@ -424,7 +419,6 @@ RPCMethod getbalances()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> rpc_wallet = GetWalletForJSONRPCRequest(request);
-    if (!rpc_wallet) return UniValue::VNULL;
     const CWallet& wallet = *rpc_wallet;
 
     // Make sure the results are valid at least up to the most recent block
@@ -520,7 +514,6 @@ RPCMethod listunspent()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     int nMinDepth = 1;
     if (!request.params[0].isNull()) {
diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp
index 68a80eb80e..ee2327ab98 100644
--- a/src/wallet/rpc/encrypt.cpp
+++ b/src/wallet/rpc/encrypt.cpp
@@ -35,7 +35,6 @@ RPCMethod walletpassphrase()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
-    if (!wallet) return UniValue::VNULL;
     CWallet* const pwallet = wallet.get();
 
     int64_t nSleepTime;
@@ -132,7 +131,6 @@ RPCMethod walletpassphrasechange()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     if (!pwallet->HasEncryptionKeys()) {
         throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called.");
@@ -197,7 +195,6 @@ RPCMethod walletlock()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     if (!pwallet->HasEncryptionKeys()) {
         throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called.");
@@ -250,7 +247,6 @@ RPCMethod encryptwallet()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
         throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: wallet does not contain private keys, nothing to encrypt.");
diff --git a/src/wallet/rpc/signmessage.cpp b/src/wallet/rpc/signmessage.cpp
index bd49f3e393..9073bf5762 100644
--- a/src/wallet/rpc/signmessage.cpp
+++ b/src/wallet/rpc/signmessage.cpp
@@ -37,7 +37,6 @@ RPCMethod signmessage()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-            if (!pwallet) return UniValue::VNULL;
 
             LOCK(pwallet->cs_wallet);
 
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index b6cdc8600f..e90e0772fa 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -288,7 +288,6 @@ RPCMethod sendtoaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -392,7 +391,6 @@ RPCMethod sendmany()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -799,7 +797,6 @@ RPCMethod fundrawtransaction()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // parse hex string from parameter
     CMutableTransaction tx;
@@ -900,7 +897,6 @@ RPCMethod signrawtransactionwithwallet()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     CMutableTransaction mtx;
     if (!DecodeHexTx(mtx, request.params[0].get_str())) {
@@ -1033,7 +1029,6 @@ static RPCMethod bumpfee_helper(std::string method_name)
         [want_psbt](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) && !want_psbt) {
         throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead.");
@@ -1263,7 +1258,6 @@ RPCMethod send()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-            if (!pwallet) return UniValue::VNULL;
 
             UniValue options{request.params[4].isNull() ? UniValue::VOBJ : request.params[4]};
             InterpretFeeEstimationInstructions(/*conf_target=*/request.params[1], /*estimate_mode=*/request.params[2], /*fee_rate=*/request.params[3], options);
@@ -1377,7 +1371,6 @@ RPCMethod sendall()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             std::shared_ptr<CWallet> const pwallet{GetWalletForJSONRPCRequest(request)};
-            if (!pwallet) return UniValue::VNULL;
             // Make sure the results are valid at least up to the most recent block
             // the user could have gotten from another RPC command prior to now
             pwallet->BlockUntilSyncedToCurrentChain();
@@ -1623,7 +1616,6 @@ RPCMethod walletprocesspsbt()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     const CWallet& wallet{*pwallet};
     // Make sure the results are valid at least up to the most recent block
@@ -1755,7 +1747,6 @@ RPCMethod walletcreatefundedpsbt()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     CWallet& wallet{*pwallet};
     // Make sure the results are valid at least up to the most recent block
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index 038e30fceb..784f8c2f92 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -225,7 +225,6 @@ RPCMethod listreceivedbyaddress()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -270,7 +269,6 @@ RPCMethod listreceivedbylabel()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -466,7 +464,6 @@ RPCMethod listtransactions()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -577,7 +574,6 @@ RPCMethod listsinceblock()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     const CWallet& wallet = *pwallet;
     // Make sure the results are valid at least up to the most recent block
@@ -719,7 +715,6 @@ RPCMethod gettransaction()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -796,7 +791,6 @@ RPCMethod abandontransaction()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -844,7 +838,6 @@ RPCMethod rescanblockchain()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
     CWallet& wallet{*pwallet};
 
     // Make sure the results are valid at least up to the most recent block
@@ -932,7 +925,6 @@ RPCMethod abortrescan()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     if (!pwallet->IsScanning() || pwallet->IsAbortingRescan()) return false;
     pwallet->AbortRescan();
diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp
index 77a8745ced..80afdf7107 100644
--- a/src/wallet/rpc/util.cpp
+++ b/src/wallet/rpc/util.cpp
@@ -59,7 +59,7 @@ std::optional<std::string> GetWalletNameFromJSONRPCRequest(const JSONRPCRequest&
     return std::nullopt;
 }
 
-std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request)
+util::NotNullSharedPtr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request)
 {
     CHECK_NONFATAL(request.mode == JSONRPCRequest::EXECUTE);
     WalletContext& context = EnsureWalletContext(request.context);
diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h
index 88fdc6639f..0a77c0d906 100644
--- a/src/wallet/rpc/util.h
+++ b/src/wallet/rpc/util.h
@@ -7,6 +7,7 @@
 
 #include <rpc/util.h>
 #include <script/script.h>
+#include <util/pointers.h>
 #include <wallet/wallet.h>
 
 #include <any>
@@ -36,9 +37,9 @@ static const RPCResult RESULT_LAST_PROCESSED_BLOCK { RPCResult::Type::OBJ, "last
  * Figures out what wallet, if any, to use for a JSONRPCRequest.
  *
  * [@param](/bitcoin-bitcoin/contributor/param/)[in] request JSONRPCRequest that wishes to access a wallet
- * [@return](/bitcoin-bitcoin/contributor/return/) nullptr if no wallet should be used, or a pointer to the CWallet
+ * [@return](/bitcoin-bitcoin/contributor/return/) a pointer to the selected CWallet, or throws if no wallet can be selected
  */
-std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request);
+util::NotNullSharedPtr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request);
 std::optional<std::string> GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request);
 /**
  * Ensures that a wallet name is specified across the endpoint and wallet_name.
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 8aa15c7ec5..72beaa6078 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -72,7 +72,6 @@ static RPCMethod getwalletinfo()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     // Make sure the results are valid at least up to the most recent block
     // the user could have gotten from another RPC command prior to now
@@ -304,7 +303,6 @@ static RPCMethod setwalletflag()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-    if (!pwallet) return UniValue::VNULL;
 
     std::string flag_str = request.params[0].get_str();
     bool value = request.params[1].isNull() || request.params[1].get_bool();
@@ -516,7 +514,6 @@ RPCMethod simulaterawtransaction()
     [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> rpc_wallet = GetWalletForJSONRPCRequest(request);
-    if (!rpc_wallet) return UniValue::VNULL;
     const CWallet& wallet = *rpc_wallet;
 
     LOCK(wallet.cs_wallet);
@@ -672,7 +669,6 @@ RPCMethod gethdkeys()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             const std::shared_ptr<const CWallet> wallet = GetWalletForJSONRPCRequest(request);
-            if (!wallet) return UniValue::VNULL;
 
             LOCK(wallet->cs_wallet);
 
@@ -770,7 +766,6 @@ static RPCMethod createwalletdescriptor()
         [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
-            if (!pwallet) return UniValue::VNULL;
 
             std::optional<OutputType> output_type = ParseOutputType(request.params[0].get_str());
             if (!output_type) {
@@ -860,7 +855,6 @@ RPCMethod addhdkey()
         [&](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
         {
             std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
-            if (!wallet) return UniValue::VNULL;
 
             if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
                 throw JSONRPCError(RPC_WALLET_ERROR, "addhdkey is not available for wallets without private keys");

    </details>

  33. in src/test/CMakeLists.txt:1 in fa1e0bb7d0
    l0rinc commented at 12:39 PM on May 23, 2026:

    It can be reviewed by calling curl -fL 'https://raw.githubusercontent.com/microsoft/GSL/688ffcde9018910bef22dae9da9de974803dc982/include/gsl/pointers' -o ./src/util/pointers.h

    Could the first commit stay closer to the imported GSL header, with Bitcoin-specific changes applied in follow-up commits?

    We could even cherry-pick it and add the above curl reproducer as a scripted diff if you want to push the boundaries of #35275 :D

  34. in src/util/pointers.h:354 in fa1e0bb7d0 
     349 | +} // namespace std
 350 | +
 351 | +namespace util {
 352 | +
 353 | +template <class T>
 354 | +struct NotNull : public gsl_detail::strict_not_null<T> {
    l0rinc commented at 3:17 PM on May 23, 2026:

    fa1e0bb util: Add util::NotNull<AnyPtrType>:

    Seeing the comments in e.g. #24423 (comment), maybe we could document when this is preferred over other solutions, e.g. that ordinary references remain preferred for simple non-owning access.

  35. in src/test/util_pointers_tests.cpp:25 in fa1e0bb7d0 
      20 | +
  21 | +    // This unit test only works with a type that may throw on a move
  22 | +    // construction
  23 | +    struct ThrowingMoveNullPtr {
  24 | +        ThrowingMoveNullPtr() = default;
  25 | +        ThrowingMoveNullPtr(ThrowingMoveNullPtr&& other) {};
    l0rinc commented at 3:51 PM on May 23, 2026:

    fa1e0bb util: Add util::NotNull<AnyPtrType>:

    Nit, we could make this explicit to avoid warnings:

            ThrowingMoveNullPtr(ThrowingMoveNullPtr&&) noexcept(false) {};
  36. in src/util/pointers.h:295 in fa1e0bb7d0 
     290 | +    template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
 291 | +    constexpr strict_not_null(const strict_not_null<U>& other) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(other)
 292 | +    {}
 293 | +
 294 | +    // To avoid invalidating the "not null" invariant, the contained pointer is actually copied
 295 | +    // instead of moved. If it is a custom pointer, its constructor could in theory throw exceptions.
    l0rinc commented at 4:41 PM on May 23, 2026:

    fa1e0bb util: Add util::NotNull<AnyPtrType>:

    NotNullUniquePtr can look move-constructible to traits even though actual move construction fails. I understand if we keep this, but could we avoid exposing move construction for util::NotNull? Explicitly deleting move construction/assignment in the public util::NotNull wrapper would make the API surface match the intended contract more directly.

    diff --git a/src/test/util_pointers_tests.cpp b/src/test/util_pointers_tests.cpp
index 85807f8e73..3640e6ccfd 100644
--- a/src/test/util_pointers_tests.cpp
+++ b/src/test/util_pointers_tests.cpp
@@ -14,6 +14,11 @@
 
 static_assert(std::is_same_v<util::NotNullUniquePtr<int>, util::NotNull<std::unique_ptr<int>>>);
 static_assert(std::is_same_v<util::NotNullSharedPtr<int>, util::NotNull<std::shared_ptr<int>>>);
+static_assert(std::is_copy_constructible_v<util::NotNullSharedPtr<int>>);
+static_assert(std::is_copy_assignable_v<util::NotNullSharedPtr<int>>);
+static_assert(!std::is_move_constructible_v<util::NotNullUniquePtr<int>>);
+static_assert(!std::is_move_constructible_v<util::NotNullSharedPtr<int>>);
+static_assert(!std::is_move_assignable_v<util::NotNullSharedPtr<int>>);
 
 BOOST_AUTO_TEST_SUITE(util_pointers_tests)
 
diff --git a/src/util/pointers.h b/src/util/pointers.h
index 73bb45e3e7..948f7021f7 100644
--- a/src/util/pointers.h
+++ b/src/util/pointers.h
@@ -19,6 +19,8 @@
 // * Add NotNullUniquePtr and NotNullSharedPtr aliases to keep smart-pointer
 //   call sites readable.
 // * Add a forwarding constructor for concise non-null smart-pointer returns.
+// * Delete util::NotNull moves so the wrapper does not advertise misleading
+//   move operations.
 //
 // All original code is covered by:
 
@@ -361,6 +363,20 @@ struct NotNull : gsl_detail::strict_not_null<T> {
     template <typename U>
     requires (!std::is_pointer_v<T> && std::is_convertible_v<U, T>)
     constexpr NotNull(U&& u) : Base{std::forward<U>(u)} {}
+
+    NotNull(const NotNull&) = default;
+    NotNull& operator=(const NotNull&) = default;
+    template <typename U, std::enable_if_t<!std::is_same_v<U, T> && std::is_convertible_v<U, T>, bool> = true>
+    NotNull& operator=(const NotNull<U>& other)
+    {
+        Base::operator=(Base{other});
+        return *this;
+    }
+    // Delete move operations so NotNull does not expose misleading move syntax.
+    // Store it where the wrapper itself does not need to be moved; use the
+    // underlying smart pointer at transfer boundaries.
+    NotNull(NotNull&&) = delete;
+    NotNull& operator=(NotNull&&) = delete;
 };
 template <typename T>
 NotNull(T) -> NotNull<T>;
  37. in src/dbwrapper.h:206 in faef857079 
     202 | @@ -202,7 +203,7 @@ class CDBWrapper
 203 |      std::optional<std::string> ReadImpl(std::span<const std::byte> key) const;
 204 |      bool ExistsImpl(std::span<const std::byte> key) const;
 205 |      size_t EstimateSizeImpl(std::span<const std::byte> key1, std::span<const std::byte> key2) const;
 206 | -    auto& DBContext() const LIFETIMEBOUND { return *Assert(m_db_context); }
 207 | +    auto& DBContext() const LIFETIMEBOUND { return *m_db_context; }
    l0rinc commented at 4:47 PM on May 23, 2026:

    faef857 refactor: Use util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context:

    Another similar one is BlockTemplateImpl, it could even simplify passing it as reference:

    diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 16db8692a1..299fa21c8f 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -58,6 +58,7 @@
 #include <uint256.h>
 #include <univalue.h>
 #include <util/check.h>
+#include <util/pointers.h>
 #include <util/result.h>
 #include <util/signalinterrupt.h>
 #include <util/string.h>
@@ -873,7 +874,6 @@ public:
                                                     m_block_template(std::move(block_template)),
                                                     m_node(node)
     {
-        assert(m_block_template);
     }
 
     CBlockHeader getBlockHeader() override
@@ -914,7 +914,7 @@ public:
 
     std::unique_ptr<BlockTemplate> waitNext(BlockWaitOptions options) override
     {
-        auto new_template = WaitAndCreateNewBlock(chainman(), notifications(), m_node.mempool.get(), m_block_template, options, m_assemble_options, m_interrupt_wait);
+        auto new_template = WaitAndCreateNewBlock(chainman(), notifications(), m_node.mempool.get(), *m_block_template, options, m_assemble_options, m_interrupt_wait);
         if (new_template) return std::make_unique<BlockTemplateImpl>(m_assemble_options, std::move(new_template), m_node);
         return nullptr;
     }
@@ -926,7 +926,7 @@ public:
 
     const BlockAssembler::Options m_assemble_options;
 
-    const std::unique_ptr<CBlockTemplate> m_block_template;
+    const util::NotNullUniquePtr<CBlockTemplate> m_block_template;
 
     bool m_interrupt_wait{false};
     ChainstateManager& chainman() { return *Assert(m_node.chainman); }
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index c9a491ef23..d9cd325a41 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -364,7 +364,7 @@ void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wa
 std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainman,
                                                       KernelNotifications& kernel_notifications,
                                                       CTxMemPool* mempool,
-                                                      const std::unique_ptr<CBlockTemplate>& block_template,
+                                                      const CBlockTemplate& block_template,
                                                       const BlockWaitOptions& options,
                                                       const BlockAssembler::Options& assemble_options,
                                                       bool& interrupt_wait)
@@ -391,7 +391,7 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
                 // We assume tip_block is set, because this is an instance
                 // method on BlockTemplate and no template could have been
                 // generated before a tip exists.
-                tip_changed = Assume(tip_block) && tip_block != block_template->block.hashPrevBlock;
+                tip_changed = Assume(tip_block) && tip_block != block_template.block.hashPrevBlock;
                 return tip_changed || chainman.m_interrupt || interrupt_wait;
             });
             if (interrupt_wait) {
@@ -435,7 +435,7 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
 
             // Calculate the original template total fees if we haven't already
             if (current_fees == -1) {
-                current_fees = std::accumulate(block_template->vTxFees.begin(), block_template->vTxFees.end(), CAmount{0});
+                current_fees = std::accumulate(block_template.vTxFees.begin(), block_template.vTxFees.end(), CAmount{0});
             }
 
             // Check if fees increased enough to return the new template
diff --git a/src/node/miner.h b/src/node/miner.h
index 5c8668771f..1a0e3f8d3d 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -150,7 +150,7 @@ void InterruptWait(KernelNotifications& kernel_notifications, bool& interrupt_wa
 std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainman,
                                                       KernelNotifications& kernel_notifications,
                                                       CTxMemPool* mempool,
-                                                      const std::unique_ptr<CBlockTemplate>& block_template,
+                                                      const CBlockTemplate& block_template,
                                                       const BlockWaitOptions& options,
                                                       const BlockAssembler::Options& assemble_options,
                                                       bool& interrupt_wait);
    l0rinc commented at 4:52 PM on May 23, 2026:

    faef857 refactor: Use util::NotNull<std::unique_ptr<LevelDBContext>> m_db_context:

    And a few other ones that we're already treating as non-null implicitly: their constructors initialize m_impl with std::make_unique, and the members are const, so the pointer cannot later be reset or reassigned:

    diff --git a/src/addrman.h b/src/addrman.h
index 94e7d3e653..7a1888d38e 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -10,6 +10,7 @@
 #include <netgroup.h>
 #include <protocol.h>
 #include <streams.h>
+#include <util/pointers.h>
 #include <util/time.h>
 
 #include <cstdint>
@@ -109,7 +110,7 @@ struct AddressPosition {
 class AddrMan
 {
 protected:
-    const std::unique_ptr<AddrManImpl> m_impl;
+    const util::NotNullUniquePtr<AddrManImpl> m_impl;
 
 public:
     explicit AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio);
diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h
index 68deeabaf6..97bf1bfd44 100644
--- a/src/node/txreconciliation.h
+++ b/src/node/txreconciliation.h
@@ -7,6 +7,7 @@
 
 #include <net.h>
 #include <sync.h>
+#include <util/pointers.h>
 
 #include <memory>
 #include <tuple>
@@ -52,7 +53,7 @@ class TxReconciliationTracker
 {
 private:
     class Impl;
-    const std::unique_ptr<Impl> m_impl;
+    const util::NotNullUniquePtr<Impl> m_impl;
 
 public:
     explicit TxReconciliationTracker(uint32_t recon_version);
diff --git a/src/txrequest.h b/src/txrequest.h
index 93972a88c2..6fd92c0ec8 100644
--- a/src/txrequest.h
+++ b/src/txrequest.h
@@ -8,6 +8,7 @@
 #include <primitives/transaction.h>
 #include <net.h>
 #include <uint256.h>
+#include <util/pointers.h>
 
 #include <chrono>
 #include <cstdint>
@@ -100,7 +101,7 @@
 class TxRequestTracker {
     // Avoid littering this header file with implementation details.
     class Impl;
-    const std::unique_ptr<Impl> m_impl;
+    const util::NotNullUniquePtr<Impl> m_impl;
 
 public:
     //! Construct a TxRequestTracker.
  38. l0rinc changes_requested
  39. l0rinc commented at 5:18 PM on May 23, 2026: contributor

    Concept ACK, I think this could help in a few cases where the pointer-like type is part of the contract and a plain reference would not preserve the ownership/storage semantics. This also feels aligned with ongoing efforts like #35229 (cc: @optout21).

    The part that bothers me most is that util::NotNull<std::unique_ptr<T>> does not quite feel like a first-class type yet. The construction and call sites are fairly verbose, which makes the stronger invariant harder to adopt/read. I left a few suggestions around making common smart-pointer usage more compact, adding a few assertions/tests so the API is less surprising, and pointing out a few other places where this pattern may fit.

Contributors
maflckoDrahtBotstickies-vl0rinc
Labels
Utils/log/libs
Linked (view graph)
#24423 Add `not_null<T>` from the Guidelines Support Library
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-23 20:51 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me