fix: Make TxidFromString() respect string_view length #30436

hodlinator · 2024-07-12T09:07:31Z

Problem

Prior to this, TxidFromString() was passing string_view::data() into uint256S() which meant it would only receive the a naked char* pointer and potentially scan past the string_view::length() until it found a null terminator (or some other non-hex character).

Appears to have been a fully dormant bug as callers were either passing a string literal or std::string directly to TxidFromFromString(), meaning a null terminator always existed at pointer[length()]. Bug existed since original merge of TxidFromString().

Solution

Make uint256S() (and base_blob::SetHex()) take and operate on std::string_view instead of const char* and have TxidFromString() pass that in.

(PR was prompted by comment in #30377 (comment) (referring to #28922 (comment))).

DrahtBot · 2024-07-12T09:07:34Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	paplorinc, maflcko, ryanofsky
Stale ACK	stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30482 (rest: Reject truncated hex txid early in getutxos parsing by maflcko)
#30377 (refactor: Add consteval uint256("str") and ParseHex("str") by hodlinator)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko

left some comments

src/util/transaction_identifier.h

src/uint256.cpp

src/uint256.h

src/rest.cpp

src/uint256.h

src/rest.cpp

src/util/string.h

maflcko

ACK c3a9c71 🐞

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c3a9c71c7077324bf0aa40f834f7537a14619340 🐞
0oe/WjDO2mSwVc5Blutf21rEvpYjnqCdQdO76x2UhqHObkUZghAnaWmbpcKtyptM6F8bw8dIl8IU3+TJAkcnBg==

stickies-v

Approach ACK c3a9c71

src/uint256.h

ryanofsky

Code review ACK c3a9c71

I think the changes are all good, but the PR and the most important commit "fix: Make TxidFromString() respect string length" (c3a9c71) are lacking a good description that actually says what the bug is.

I think they should say that currently TxidFromString takes a string_view parameter, but internally it is calling the uint256S function which expects a nul-terminated string. If TxidFromString is called with a string that not contain \0, the implementation will read the string and potentially uninitialized memory after it until it finds any character which is not a hex digit.

src/uint256.cpp

stickies-v

ACK c3a9c71,

are lacking a good description that actually says what the bug is.

I agree, I spent quite a bit of time getting to the bottom of this and more documentation would have sped that up

but internally it is calling the uint256S function which expects a nul-terminated string

Maybe I'm being too pedantic (or wrong), but I'm not sure this is 100% correct? I don't think the uint256S(std::string_view str) str needs to be null-terminated. Rather, I think the problem is that by removing the string length data in TxidFromstring (by passing std::string_view::data instead of std::string_view), uint256S has to construct a std::string_view from the const char* without length information, which is dangerous (as you describe) when the initial string_view was not null-terminated, e.g. because it is a substring?

For example, this test (with a non-null terminated string) fails before c3a9c71 and succeeds after:

//txid "0x74d681e0e03bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20"
const char arr[66] = {
    '7', '4', 'd', '6', '8', '1', 'e', '0', 'e', '0', '3', 'b', 'a', 'f', 'a', '8',
    '0', '2', 'c', '8', 'a', 'a', '0', '8', '4', '3', '7', '9', 'a', 'a', '9', '8',
    'd', '9', 'f', 'c', 'd', '6', '3', '2', 'd', 'd', 'c', '2', 'e', 'd', '9', '7',
    '8', '2', 'b', '5', '8', '6', 'e', 'c', '8', '7', '4', '5', '1', 'f', '2', '0',
    'a', 'b' // extra 2 characters
};
const char* ptr = arr;  // not null-terminated
std::string_view view{ptr, 64};
BOOST_CHECK_EQUAL(TxidFromString(view).ToString(), view);

src/uint256.cpp

ryanofsky

re: #30436 (review)

but internally it is calling the uint256S function which expects a nul-terminated string

Maybe I'm being too pedantic (or wrong), but I'm not sure this is 100% correct? I don't think the uint256S(std::string_view str) str needs to be null-terminated.

Yes, both explanations are saying the same thing but mine is unnecessarily confusing. The uint256S function "expects" a nul-terminated string because it accepts a bare char* argument, and in C bare char* strings are almost always nul-terminated. But it does not "need" a nul-terminated string. The string can be terminated with any character other than 0123456789AaBbCcDdEeFf, as mentioned in the next sentence.

src/uint256.cpp

maflcko · 2024-07-19T05:42:00Z

are lacking a good description that actually says what the bug is.

I agree, I spent quite a bit of time getting to the bottom of this and more documentation would have sped that up

I agree as well. Thanks for brining it up! This should be fixed before merge, other than that, I'd say it is ready.

stickies-v · 2024-07-19T14:14:23Z

Side note: we can use bugprone-suspicious-stringview-data-usage (from unreleased clang-19) to catch this going forward, when we have clang-19. I tried running clang-tidy-19 locally from master branch for longer than I probably should have, and eventually gave up.

As a poor man's alternative, I looked for other stringview data misusage based on some rough grepping, and couldn't find anything, but that was definitely not an exhaustive search either.

hodlinator · 2024-07-19T20:47:31Z

Thanks for the feedback! Helped me realize I was being too terse. (I think it was a mistake alone just to not have string_view in the title for the sake of keeping under 51 chars).

(Edit: where/were typo)

Only modified commit message:

--- c3a9c71c7077324bf0aa40f834f7537a14619340	2024-07-19 22:42:56.019823786 +0200
+++ 788fe9cc9ab979c5e14f544ae05dc46436892b81	2024-07-19 22:48:26.785770275 +0200
@@ -1,3 +1,5 @@
-fix: Make TxidFromString() respect string length
+fix: Make TxidFromString() respect string_view length
 
-Appears to have been a fully dormant bug. Introduced since inception of TxidFromString(), discussed in https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378.
+Prior to this, passing in string_view::data() meant uint256S() would only receive the a naked char* pointer and scan past the string_view::length() until it found a null terminator.
+
+Appears to have been a fully dormant bug as callers were either passing a string literal or std::string directly to TxidFromFromString(), meaning null terminator always existed at pointer[length()]. Bug existed since original merge of TxidFromString(), discussed in https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378.

Prior to this, passing string_view::data() into uint256S() meant the latter would only receive the a naked char* pointer and potentially scan past the string_view::length() until it found a null terminator (or some other non-hex character). Appears to have been a fully dormant bug as callers were either passing a string literal or std::string directly to TxidFromFromString(), meaning null terminator always existed at pointer[length()]. Bug existed since original merge of TxidFromString(), discussed in bitcoin#28922 (comment).

hodlinator · 2024-07-23T13:06:01Z

Credit @ryanofsky as author of TxidFromString() test with a minor transformation.

Use BOOST_CHECK_EQUAL_COLLECTIONS().

SetHex() - Largely reset back to previously reviewed implementation with 3 ACKs. Sacrificing both my find_if() and @paplorinc & mine's very nice final loop. Keeping the trimmed string_view variable and consted the parameter as a hat-tip to @paplorinc.

As I was changing SetHex() I initially forgot to bring back std::fill(m_data.begin(), m_data.end(), 0);, but unit tests still succeeded, so I added a test to catch that.

Hope this strikes a good balance. Thank you again for your patience and hopefully my intuition in doing async review online has been refined.

src/test/uint256_tests.cpp

src/uint256.cpp

l0rinc

ACK 09ce350

Note: adding co-authorship for the commits that reviewers have contributed to is a good way to make sure all participants are motivated to continue doing it.

maflcko · 2024-07-23T15:34:58Z

re-ACK 09ce350 🕓

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 09ce3501fa2ea2885a857e380eddb74605f7038c 🕓
nckkzWnlpOSIdef04sGXd+awRj6JrntK6TIzwwCm2K3hhSnHCUyW+8VcOS4IzMWnefV3XMunu4gyoYA/oKIwCg==

ryanofsky

Code review ACK 09ce350. I think the current code changes are about as small as you could make to fix the bug without introducing a string copy, and the surrounding test improvements are all very nice and welcome.

It seems like may be some suggestions from paplorinc that could be responded to, but they are pretty minor. So this looks ready for merge.

ryanofsky · 2024-07-23T15:39:29Z

src/test/uint256_tests.cpp

-    return rv;
-}
-inline uint160 uint160S(const std::string& str)
+inline uint160 uint160S(std::string_view str)


re: #30436 (comment)

it's not very important in this particular method, but inline + const could help with the actual inlining - it's also clearer to the reader to understand what happens when mutating the parameter, since after inlining it should ideally avoid the copying (which is easy to do for a const). At least this was my thinking for recommending it, let me know if I'm mistaken.

Responded in the other thread #30436 (comment)

src/uint256.cpp

maflcko · 2024-07-23T15:29:39Z

src/test/uint256_tests.cpp

+    BOOST_CHECK_EQUAL_COLLECTIONS(R1L.begin(), R1L.end(), R1Array, R1Array + R1L.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(TmpL.begin(), TmpL.end(), R1Array, R1Array + TmpL.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(R2L.begin(), R2L.end(), R2Array, R2Array + R2L.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(ZeroL.begin(), ZeroL.end(), ZeroArray, ZeroArray + ZeroL.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(OneL.begin(), OneL.end(), OneArray, OneArray + OneL.size());


style nit in f11f816: Seems confusing to use the left size to calculate the right end. Seems easier to just use std::end(right), if it compiles?

Edit: Or use std::array and the begin() and end() methods.

I originally did that for the +32 case, but wasn't working for the +20

Agree it is bad form to use variable from the left side of the comparison in the right side.

Tested std::end(R1Array) but it would be off by 1 thanks to the null-terminator in the string, resulting in a size/distance-mismatch between the collections/iterators being tested.

Follow-up fixing this issue: #30526

stickies-v

post-merge ACK 09ce350

stickies-v · 2024-07-23T19:41:11Z

src/test/util/setup_common.cpp

@@ -79,6 +79,18 @@ const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */
 static FastRandomContext g_insecure_rand_ctx_temp_path;

+std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
+{
+    os << ArithToUint256(num).ToString();


Is the ArithToUint256 conversion necessary?

Good point, I missed the base_uint::ToString() method. Not as familiar with arith_uint256 as I am with uint256 yet.

Should probably also document endianness of arith_uint256 strings to mirror the added comments to uint256 in this PR.

Follow-up fixing this issue: #30526

Follow-up to bitcoin#30436. uint256 string representation was wrongfully documented as little-endian due to comparison operator giving most significance to the least significant bytes. uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side. setup_common.cpp - Skip needless ArithToUint256-conversion.

Follow-up to bitcoin#30436. uint256 string representation was wrongfully documented as little-endian due to them being reversed by GetHex() etc, and base_blob::Compare giving most significance to the beginning of the internal array. They are closer to "big-endian", but this commit tries to be even more precise than that. uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side. setup_common.cpp - Skip needless ArithToUint256-conversion.

Follow-up to bitcoin#30436. uint256 string representation was wrongfully documented as little-endian due to them being reversed by GetHex() etc, and base_blob::Compare() giving most significance to the beginning of the internal array. They are closer to "big-endian", but this commit tries to be even more precise than that. uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side. setup_common.cpp - Skip needless ArithToUint256-conversion.

Github-Pull: bitcoin#30436 Rebased-From: f0eeee2

Github-Pull: bitcoin#30436 Rebased-From: f11f816

Github-Pull: bitcoin#30436 Rebased-From: 2f5577d

Clarify that hex strings are parsed as little-endian. Github-Pull: bitcoin#30436 Rebased-From: 01e314c

Prior to this, passing string_view::data() into uint256S() meant the latter would only receive the a naked char* pointer and potentially scan past the string_view::length() until it found a null terminator (or some other non-hex character). Appears to have been a fully dormant bug as callers were either passing a string literal or std::string directly to TxidFromFromString(), meaning null terminator always existed at pointer[length()]. Bug existed since original merge of TxidFromString(), discussed in bitcoin#28922 (comment). Github-Pull: bitcoin#30436 Rebased-From: 09ce350

Follow-up to bitcoin#30436. uint256 string representation was wrongfully documented as little-endian due to them being reversed by GetHex() etc, and base_blob::Compare() giving most significance to the beginning of the internal array. They are closer to "big-endian", but this commit tries to be even more precise than that. uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side. setup_common.cpp - Skip needless ArithToUint256-conversion.

@maflcko

73e3fa1 doc + test: Correct uint256 hex string endianness (Hodlinator) Pull request description: This PR is a follow-up to #30436. Only changes test-code and modifies/adds comments. Byte order of hex string representation was wrongfully documented as little-endian, but are in fact closer to "big-endian" (endianness is a memory-order concept rather than a numeric concept). `[arith_]uint256` both store their data in arrays with little-endian byte order (`arith_uint256` has host byte order within each `uint32_t` element). **uint256_tests.cpp** - Avoid using variable from the left side of the condition in the right side. Credits to @maflcko: #30436 (comment) **setup_common.cpp** - Skip needless ArithToUint256-conversion. Credits to @stickies-v: #30436 (comment) --- <details> <summary> ## Logical reasoning for endianness </summary> 1. Comparing an `arith_uint256` (`base_uint<256>`) to a `uint64_t` compares the beginning of the array, and verifies the remaining elements are zero. ```C++ template <unsigned int BITS> bool base_uint<BITS>::EqualTo(uint64_t b) const { for (int i = WIDTH - 1; i >= 2; i--) { if (pn[i]) return false; } if (pn[1] != (b >> 32)) return false; if (pn[0] != (b & 0xfffffffful)) return false; return true; } ``` ...that is consistent with little endian ordering of the array. 2. They have the same endianness (but `arith_*` has host-ordering of each `uint32_t` element): ```C++ arith_uint256 UintToArith256(const uint256 &a) { arith_uint256 b; for(int x=0; x<b.WIDTH; ++x) b.pn[x] = ReadLE32(a.begin() + x*4); return b; } ``` ### String conversions The reversal of order which happens when converting hex-strings <=> uint256 means strings are actually closer to big-endian, see the end of `base_blob<BITS>::SetHexDeprecated`: ```C++ unsigned char* p1 = m_data.data(); unsigned char* pend = p1 + WIDTH; while (digits > 0 && p1 < pend) { *p1 = ::HexDigit(trimmed[--digits]); if (digits > 0) { *p1 |= ((unsigned char)::HexDigit(trimmed[--digits]) << 4); p1++; } } ``` Same reversal here: ```C++ template <unsigned int BITS> std::string base_blob<BITS>::GetHex() const { uint8_t m_data_rev[WIDTH]; for (int i = 0; i < WIDTH; ++i) { m_data_rev[i] = m_data[WIDTH - 1 - i]; } return HexStr(m_data_rev); } ``` It now makes sense to me that `SetHexDeprecated`, upon receiving a shorter hex string that requires zero-padding, would pad as if the missing hex chars where towards the end of the little-endian byte array, as they are the most significant bytes. "Big-endian" string representation is also consistent with the case where `SetHexDeprecated` receives too many hex digits and discards the leftmost ones, as a form of integer narrowing takes place. ### How I got it wrong in #30436 Previously I used the less than (`<`) comparison to prove endianness, but for `uint256` it uses `memcmp` and thereby gives priority to the *lower* bytes at the beginning of the array. ```C++ constexpr int Compare(const base_blob& other) const { return std::memcmp(m_data.data(), other.m_data.data(), WIDTH); } ``` `arith_uint256` is different in that it begins by comparing the bytes from the end, as it is using little endian representation, where the bytes toward the end are more significant. ```C++ template <unsigned int BITS> int base_uint<BITS>::CompareTo(const base_uint<BITS>& b) const { for (int i = WIDTH - 1; i >= 0; i--) { if (pn[i] < b.pn[i]) return -1; if (pn[i] > b.pn[i]) return 1; } return 0; } ``` (The commit documents that `base_blob::Compare()` is doing lexicographic ordering unlike the `arith_*`-variant which is doing numeric ordering). </details> ACKs for top commit: paplorinc: ACK 73e3fa1 ryanofsky: Code review ACK 73e3fa1 Tree-SHA512: 121630c37ab01aa7f7097f10322ab37da3cbc0696a6bbdbf2bbd6db180dc5938c7ed91003aaa2df7cf4a4106f973f5118ba541b5e077cf3588aa641bbd528f4e

8756ccd scripted-diff: Replace ParseHex[<std::byte>]("str") -> "str"_hex[_u8] (Hodlinator) 9cb6873 refactor: Prepare for ParseHex -> ""_hex scripted-diff (Hodlinator) 50bc017 refactor: Hand-replace some ParseHex -> ""_hex (Hodlinator) 5b74a84 util: Add consteval ""_hex[_v][_u8] literals (l0rinc) dc5f6f6 test refactor: util_tests - parse_hex clean up (Hodlinator) 2b5e6ef refactor: Make XOnlyPubKey tolerate constexpr std::arrays (Hodlinator) 403d86f refactor: vector -> span in CCrypter (Hodlinator) bd0830b refactor: de-Hungarianize CCrypter (Hodlinator) d99c816 refactor: Improve CCrypter related lines (Hodlinator) 7e1d9a8 refactor: Enforce lowercase hex digits for consteval uint256 (Hodlinator) Pull request description: Motivation: * Validates and converts the hex string into bytes at compile time instead of at runtime like `ParseHex()`. * Eliminates runtime dependencies: #30377 (comment), #30048 (comment) * Has stricter requirements than `ParseHex()` (disallows whitespace and uppercase hex digits) and replaces it in a bunch of places. * Makes it possible to derive other compile time constants. * Minor: should shave off a few runtime CPU cycles. `""_hex` produces `std::array<std::byte>` as the momentum in the codebase is to use `std::byte` over `uint8_t`. Also makes `uint256` hex string constructor disallow uppercase hex digits. Discussed: #30560 (comment) Surprisingly does not change the size of the Guix **bitcoind** binary (on x86_64-linux-gnu) by 1 single byte. Spawned already merged PRs: #30436, #30482, #30532, #30560. ACKs for top commit: l0rinc: ACK 8756ccd stickies-v: Rebase re-ACK 8756ccd, no changes since a096215 ryanofsky: Code review ACK 8756ccd, just rebasing since last review and taking advantage of CScript constructors in #29369, also tweaking a code comment Tree-SHA512: 9b2011b7c37e0ef004c669f8601270a214b388916316458370f5902c79c2856790b1b2c7c123efa65decad04886ab5eff95644301e0d84358bb265cf1f8ec195

hodlinator mentioned this pull request Jul 12, 2024

refactor: Replace ParseHex with consteval ""_hex literals #30377

Merged

maflcko reviewed Jul 12, 2024

View reviewed changes

src/util/transaction_identifier.h Show resolved Hide resolved

src/uint256.cpp Outdated Show resolved Hide resolved

src/uint256.h Outdated Show resolved Hide resolved

src/uint256.h Outdated Show resolved Hide resolved

hodlinator force-pushed the 2024-07_fix_TxidFromString branch 2 times, most recently from 9b6c6c5 to fcec222 Compare July 12, 2024 12:01

maflcko reviewed Jul 12, 2024

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

maflcko reviewed Jul 12, 2024

View reviewed changes

src/uint256.h Outdated Show resolved Hide resolved

8000

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from fcec222 to 3d0aec1 Compare July 12, 2024 19:25

DrahtBot mentioned this pull request Jul 13, 2024

rest: Reject negative outpoint index early in getutxos parsing #30444

Merged

maflcko reviewed Jul 16, 2024

View reviewed changes

src/rest.cpp Outdated Show resolved Hide resolved

hodlinator force-pushed the 2024-07_fix_TxidFromString branch 2 times, most recently from 735865a to d20fc9c Compare July 16, 2024 09:10

maflcko reviewed Jul 16, 2024

View reviewed changes

src/util/string.h Outdated Show resolved Hide resolved

maflcko reviewed Jul 16, 2024

View reviewed changes

src/util/string.h Outdated Show resolved Hide resolved

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from d20fc9c to c3a9c71 Compare July 16, 2024 10:16

maflcko approved these changes Jul 16, 2024

View reviewed changes

fanquake requested a review from stickies-v July 17, 2024 14:46

stickies-v reviewed Jul 17, 2024

View reviewed changes

src/uint256.h Show resolved Hide resolved

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from c3a9c71 to 4923d90 Compare July 17, 2024 19:04

maflcko reviewed Jul 18, 2024

View reviewed changes

src/uint256.h Outdated Show resolved Hide resolved

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from 4923d90 to c3a9c71 Compare July 18, 2024 07:37

ryanofsky approved these changes Jul 18, 2024

View reviewed changes

src/uint256.cpp Outdated Show resolved Hide resolved

src/uint256.cpp Outdated Show resolved Hide resolved

DrahtBot requested a review from stickies-v July 18, 2024 16:08

stickies-v approved these changes Jul 18, 2024

View reviewed changes

src/uint256.cpp Outdated Show resolved Hide resolved

ryanofsky reviewed Jul 19, 2024

View reviewed changes

src/uint256.cpp Outdated Show resolved Hide resolved

stickies-v mentioned this pull request Jul 19, 2024

rest: Reject truncated hex txid early in getutxos parsing #30482

Merged

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from c3a9c71 to 649c88d Compare July 19, 2024 20:43

hodlinator force-pushed the 2024-07_fix_TxidFromString branch from bfcc016 to 09ce350 Compare July 23, 2024 13:04

l0rinc reviewed Jul 23, 2024

View reviewed changes

src/test/uint256_tests.cpp Show resolved Hide resolved

l0rinc reviewed Jul 23, 2024

View reviewed changes

src/uint256.cpp Show resolved Hide resolved

l0rinc approved these changes Jul 23, 2024

View reviewed changes

DrahtBot requested a review from maflcko July 23, 2024 15:17

ryanofsky approved these changes Jul 23, 2024

View reviewed changes

ryanofsky merged commit 7cc00bf into bitcoin:master Jul 23, 2024
16 checks passed

maflcko reviewed Jul 23, 2024

View reviewed changes

stickies-v reviewed Jul 23, 2024

View reviewed changes

hodlinator deleted the 2024-07_fix_TxidFromString branch July 23, 2024 22:03

maflcko mentioned this pull request Jul 24, 2024

ci: add _LIBCPP_REMOVE_TRANSITIVE_INCLUDES to TSAN (libc++) job #30519

Merged

hodlinator mentioned this pull request Jul 25, 2024

doc: Correct uint256 hex string endianness #30526

Merged

hodlinator mentioned this pull request Aug 1, 2024

refactor: Add consteval uint256 constructor #30560

Merged

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 1, 2024

test: Add test for TxidFromString() behavior

e75283a

Github-Pull: bitcoin#30436 Rebased-From: f0eeee2

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 1, 2024

refactor: Make uint256_tests no longer use deprecated BOOST_CHECK()

7a9f5c0

Github-Pull: bitcoin#30436 Rebased-From: f11f816

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 1, 2024

test: uint256 - Garbage suffixes and zero padding

f299964

Github-Pull: bitcoin#30436 Rebased-From: 2f5577d

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 1, 2024

refactor: Change base_blob::SetHex() to take std::string_view

f027fb9

Clarify that hex strings are parsed as little-endian. Github-Pull: bitcoin#30436 Rebased-From: 01e314c

fix: Make TxidFromString() respect string_view length #30436

fix: Make TxidFromString() respect string_view length #30436

Uh oh!

Conversation

Uh oh!

Problem

Solution

Uh oh!

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!