💬 stickies-v commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689596319)
nit: should we add this docstring to `uint256S` too?

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#issuecomment-2248269822)
Rebased to deal with CI failing.

c76f4c6dab267b8f83c75e44939f77d237251c1a has been amended to include `__file__` in the `ChainTiebreaksTest` constructor, as required by any subclass of `BitcoinTestFramework` since https://github.com/bitcoin/bitcoin/pull/30463

💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2248276315)
Rebased to deal with CI failing.

c76f4c6dab267b8f83c75e44939f77d237251c1a has been amended to include `__file__` in the `P2PSeedNodes` constructor, as required by any subclass of `BitcoinTestFramework` since https://github.com/bitcoin/bitcoin/pull/30463

💬 fanquake commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#discussion_r1690009227)
Thanks, had a look at this, and ran into issues with at least leveldb, which we'd first have to fix in the subtree, i.e:
```bash
In file included from leveldb/util/comparator.cc:14:
./leveldb/util/no_destructor.h:40:17: error: 'aligned_storage<8, 8>' is deprecated [-Werror,-Wdeprecated-declarations]
40 | typename std::aligned_storage<sizeof(InstanceType),
| ^
leveldb/util/comparator.cc:71:47: note: in instantiation of template class 'leveldb::NoDestructor<leveldb
...

💬 fanquake commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#discussion_r1690012798)
Thanks, added.

💬 fanquake commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#discussion_r1690022414)
There is an up-upstream issue / PR for this. https://github.com/google/leveldb/issues/1013 & https://github.com/google/leveldb/pull/896.

💬 ryanofsky commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2248316407)
Mentioning the history of this change in the PR description is helpful, but it would be clearer if it said what the change is and what effects it has.

From code comments in this PR and https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2188840483, it seems like the main intended effect of this change is to prevent github tasks from running twice in PRs and causing the following output:

> <img width="1038" alt="Scherm­afbeelding 2024-06-25 om 14 48 34" src="https://github.com/bitcoi
...

💬 fanquake commented on pull request "cleanse: switch to SecureZeroMemory for Windows cross-compile":
(https://github.com/bitcoin/bitcoin/pull/26950#issuecomment-2248331845)
Guix Build (aarch64):
```bash
b2617f05ed438479493daac5d6faa637d9bc8368186f41db270392ae8bcc2ff1 guix-build-c399c80a09a3/output/aarch64-linux-gnu/SHA256SUMS.part
255827ffdae24ce4099885caa4517a0d05e883adc49f92bf36226afa910d1b6b guix-build-c399c80a09a3/output/aarch64-linux-gnu/bitcoin-c399c80a09a3-aarch64-linux-gnu-debug.tar.gz
299e1401c19e901c30096b870b34c342318d08c89bc3b6ac02a82e718ddc41ad guix-build-c399c80a09a3/output/aarch64-linux-gnu/bitcoin-c399c80a09a3-aarch64-linux-gnu.tar.gz
dea5a7
...

💬 maflcko commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#issuecomment-2248337297)
re-ACK e3edaccd9deb2da50be70d2d8768eca8821785c7

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690047209)
Sure, done for both pulls.

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690048284)
Yes, done.

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690049630)
Good catch. Fixed the typo!

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690051167)
Ok, done in both places

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690055634)
Reordered, because I changed the comment anyway in another push.

👍 theStack approved a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2197161263)
lgtm ACK 1b732dbc3bd9c59303a774cf2095c2d6639dbe1d
(happy to re-ACK if the [`Assert` suggestion](https://github.com/bitcoin/bitcoin/pull/30507#discussion_r1689880324) for `pindexNewTip` is taken)

🤔 ismaelsadeeq reviewed a pull request: "crypto, refactor: add new KeyPair class"
(https://github.com/bitcoin/bitcoin/pull/30051#pullrequestreview-2197175891)
Concept ACK

clean refactor IMO.

💬 stickies-v commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#issuecomment-2248405674)
re-ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a - only doc and test updates to address review comments, thanks!

👍 TheCharlatan approved a pull request: "cleanse: switch to SecureZeroMemory for Windows cross-compile"
(https://github.com/bitcoin/bitcoin/pull/26950#pullrequestreview-2197209786)
ACK c399c80a09a393d38368a44ef04753e9f62350f0

💬 ryanofsky commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1690107471)
re: https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1672501712

> nit: Maybe make a reference to a PR consistent with the [previous one](https://github.com/bitcoin/bitcoin/blob/9adebe145557ef410964dd48a02f3d239f488cd0/contrib/devtools/check-deps.sh#L55)?

Thanks, updated

🤔 ryanofsky reviewed a pull request: "contrib: fix check-deps.sh to check for weak symbols"
(https://github.com/bitcoin/bitcoin/pull/30415#pullrequestreview-2197231263)
Updated 5a8b9432cde11f6140855717af195d8b7e798d75 -> 114b2a406e604747bd856f566aa8c7ad84dd8f15 ([`pr/weakcheck.1`](https://github.com/ryanofsky/bitcoin/commits/pr/weakcheck.1) -> [`pr/weakcheck.2`](https://github.com/ryanofsky/bitcoin/commits/pr/weakcheck.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/weakcheck.1..pr/weakcheck.2)) making comment more consistent