💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#discussion_r1451556682)
nit: Maybe add a comment that this line might be revised in the future considering https://github.com/miniupnp/miniupnp/pull/659?

💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#discussion_r1451557398)
```suggestion
@@ -116,7 +116,7 @@ if (UPNPC_BUILD_STATIC)
```
to avoid `Hunk #1 succeeded at 116 (offset 17 lines).`

💬 kevkevinpal commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1890593620)
looks like we can replace the [comment to run all the lint commands](https://github.com/bitcoin/bitcoin/blob/master/test/README.md)


we can swap this
`test/lint/all-lint.py`
for
`test/lint/lint-*.py`

💬 achow101 commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890593625)
> Is this for backport?

This is a problem that goes back several major versions (it is possible to hit without assumeutxo), so probably not.

💬 kevkevinpal commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1890595882)
Concept ACK [eb78ea4](https://github.com/bitcoin/bitcoin/pull/29227/commits/eb78ea4eebfe150bc1746282bfdad6eb0f764e3c)

💬 fanquake commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1890627039)
> Or are we adding it to the linker elsewhere?

We are passing the min version as part of `-platform_version` in `darwin_LDFLAGS`. Note that we also check that the release bins have this min os set in `symbol-check.py` (along with sdk & linker version).

🤔 murchandamus reviewed a pull request: "wallet: Reset chain notifications handler if AttachChain fails"
(https://github.com/bitcoin/bitcoin/pull/29243#pullrequestreview-1820086857)
ACK ea2551e55d260854a5cca8aa95034970d4adca1c

👍 TheCharlatan approved a pull request: "wallet: Reset chain notifications handler if AttachChain fails"
(https://github.com/bitcoin/bitcoin/pull/29243#pullrequestreview-1820104671)
ACK ea2551e55d260854a5cca8aa95034970d4adca1c

💬 TheCharlatan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1451613773)
This check seems like the right thing to do, but this could be handled by the caller, by checking `if (!(pindex->nStatus & BLOCK_HAVE_DATA))`, like done in `net_processing.cpp:2292`. Maybe still add this in a separate commit to clearly explain what is going and why being defensive here is an improvement?

👍 furszy approved a pull request: "wallet: Reset chain notifications handler if AttachChain fails"
(https://github.com/bitcoin/bitcoin/pull/29243#pullrequestreview-1820111062)
Code review ACK ea2551e5

Plus, made a quick test replicating the behavior https://github.com/furszy/bitcoin-core/commit/b6aefde6bdb9bf52e43c54d98921d8da2f0a6b5f (it is not meant to be merged, just to trigger the failure locally). It fails without the ea2551e5, and passes with it.

💬 fjahr commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1890789321)
utACK cdc6ac4126b31426261605a757c52ea2dbfb2a81

This seems to be the best solution for this issue.

💬 cecuabin commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1890829990)
(cfdm^+1/2e+lt1)* = dtt^2 +1/2-4/3.15.dmb
GPI + K3.105^n = R^2*.rsc
dt= 2/3 -icsg8 . pi^2*.int
gu=dd1t1cs^2d2-DT9t'*.lk
Ka = rc^muv^sh2-pu^2/Eccp^8''*.zip
W1^3/2 + d1t1 . w^3/2 = $LUNC + target 300dsh ($0.023)
RB + MAF = Eo - d1t1 . wrs^2|/^o,313

💬 cecuabin commented on issue "`-maxtxfee` is used as a fee and a feerate":
(https://github.com/bitcoin/bitcoin/issues/29220#issuecomment-1890830538)
FR + dac = -At^2sds + Wo^3/1.223
358 dt^2sv = F271 +dr^3 / 1:121234^3/2 r^2 -cfv^2

🤔 jonesk7734 reviewed a pull request: "Update doc/policy/README.md"
(https://github.com/bitcoin/bitcoin/pull/29095#pullrequestreview-1820177649)
Reviewed, and terms agreed to.

📝 BrandonOdiwuor opened a pull request: "doc: update encryptwallet passphrase doc"
(https://github.com/bitcoin/bitcoin/pull/29245)
Follow up to https://github.com/bitcoin/bitcoin/pull/28974, coming from https://github.com/bitcoin/bitcoin/pull/28974#issuecomment-1855661692

🤔 jonesk7734 reviewed a pull request: "doc: refer to "Node relay options" in policy/README"
(https://github.com/bitcoin/bitcoin/pull/29235#pullrequestreview-1820183065)
Reviewed, and accepted terms.

✅ BrandonOdiwuor closed a pull request: "wallet: Refactor DumpWallet function to accept -dumpfile path argument"
(https://github.com/bitcoin/bitcoin/pull/29223)

💬 hsjoberg commented on pull request "Implement 64 bit arithmetic op codes in the Script interpreter":
(https://github.com/bitcoin/bitcoin/pull/29221#issuecomment-1890905163)
Concept ACK. This is great!

📝 fanquake locked a pull request: "doc: refer to "Node relay options" in policy/README"
(https://github.com/bitcoin/bitcoin/pull/29235)
Fixed up #29095, to refer to `-help`, rather than listing every option.

💬 hebasto commented on pull request "Revert "build: Fix regression in "ARMv8 CRC32 intrinsics" test"":
(https://github.com/bitcoin/bitcoin/pull/29226#issuecomment-1890934252)
> We could alternatively add a suppression but I prefer reverting unless someone can explain how this is not an actual bug.

Assuming that this is a bug, disabling the buggy feature should be documented properly, no?