Repository: bitcoin/bitcoin
File: src/uint256.h (C/C++)

Code before:
/** 160-bit opaque blob.
 * @note This type is called uint160 for historical reasons only. It is an opaque
 * blob of 160 bits and has no integer operations.
 */
class uint160 : public base_blob<160> {
public:
    constexpr uint160() = default;
    constexpr explicit uint160(Span<const unsigned char> vch) : base_blob<160>(vch) {}
};

/** 256-bit opaque blob.
 * @note This type is called uint256 for historical reasons only. It is an
 * opaque blob of 256 bits and has no integer operations. Use arith_uint256 if
 * those are required.
 */
class uint256 : public base_blob<256> {
public:
    constexpr uint256() = default;
    constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {}
    constexpr explicit uint256(Span<const unsigned char> vch) : base_blob<256>(vch) {}
    static const uint256 ZERO;
    static const uint256 ONE;
};

/* uint256 from const char *.
 * This is a separate function because the constructor uint256(const char*) can result
 * in dangerously catching uint256(0).
 */
inline uint256 uint256S(const char *str)
{
    uint256 rv;
    rv.SetHex(str);
    return rv;
}
/* uint256 from std::string.
 * This is a separate function because the constructor uint256(const std::string &str) can result
 * in dangerously catching uint256(0) via std::string(const char*).
 */
inline uint256 uint256S(const std::string& str)
{
    uint256 rv;
    rv.SetHex(str);
    return rv;
}

#endif // BITCOIN_UINT256_H


Review comment (bug):
nit: I interpret "historical fears" as "the fears are no longer applicable", in which case I think it should just be a constructor and this helper function removed. But, it [seems](https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675775175) like until C++23 this is indeed an issue (even if the ambiguous overload with the `uint8_t` should prevent compilation), so the current wording is confusing to me.

Suggested alternative:

```
/* uint256 from std::string_view.
 * This is not a uint256 constructor to avoid the ambiguity of
 * uint256(0) being interpreted as initialization from a nullptr.
 *
 * This helper can be replaced with a constructor when we require c++23,
 * which prohibits constructing a string_view from a nullptr, see
 * https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2166r1.html
 */
```

Code after:
/** 160-bit opaque blob.
 * @note This type is called uint160 for historical reasons only. It is an opaque
 * blob of 160 bits and has no integer operations.
 */
class uint160 : public base_blob<160> {
public:
    constexpr uint160() = default;
    constexpr explicit uint160(Span<const unsigned char> vch) : base_blob<160>(vch) {}
};

/** 256-bit opaque blob.
 * @note This type is called uint256 for historical reasons only. It is an
 * opaque blob of 256 bits and has no integer operations. Use arith_uint256 if
 * those are required.
 */
class uint256 : public base_blob<256> {
public:
    constexpr uint256() = default;
    constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {}
    constexpr explicit uint256(Span<const unsigned char> vch) : base_blob<256>(vch) {}
    static const uint256 ZERO;
    static const uint256 ONE;
};

/* uint256 from std::string_view, treated as little-endian.
 * This is not a uint256 constructor because of historical fears of uint256(0)
 * resolving to a NULL string and crashing.
 */
inline uint256 uint256S(std::string_view str)
{
    uint256 rv;
    rv.SetHex(str);
    return rv;
}

#endif // BITCOIN_UINT256_H