💬 maflcko commented on pull request "refactor: rpc: Remove unnecessary uses of ParseNonRFCJSONValue() and rename it":
(https://github.com/bitcoin/bitcoin/pull/27256#discussion_r1567489469)
Fixed in #29892

💬 Sjors commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2059278946)
utACK cbcb2c82669deaad71e739c64b1baf687e76e604

Holding on to `cs_main` in this particular spot, when index catches up to the tip, is not a big deal performance wise. The other, much more frequently called `Commit()` in `Sync()` was already done without holding `cs_main`.

💬 ryanofsky commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1567499444)
In commit "indexes: Rewrite chain sync logic, simplify index sync code" (ffa9e452ac7cfe08deecaae409eca3369dcccb76)

This commit has the same race condition reported in #29767 (which was introduced in #14121 and fixed in #29776), so when this PR is rebased it needs to incorporate a fix similar to #29776.

The problem is that index is considered synced before the last block is commited so new blocks could attached from the validationinterface thread while this thread is still committing. Proba
...

💬 Sjors commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-2059296060)
Now that we have a `createwalletdescriptor` RPC call #29130, this could be expanded to take private key material, as suggested by @ryanofsky: https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572

codex32 then be one of the import formats.

💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-2059322038)
Thanks! The next time I am working on codex32 wallet support I will try that approach instead.

💬 stickies-v commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-2059323658)
> Does it work with `OBJ_NAMED_PARAMS`?

Nope. I think we should just address the general use case of getting `OBJ`/`ARR` args with the `Arg` helpers, and then that should help with `OBJ_NAMED_PARAMS` too?

💬 maflcko commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#discussion_r1567534464)
It could make sense to remove the index-based access and require the name-based access?

💬 sipa commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2059335680)
utACK 9d76d77ba04450e8b5b5a528b448b61c61346193

💬 Sjors commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#discussion_r1567544335)
Yes I left that out, because the command will tell you to.

(also agree that we should at least wait until someone else runs into this)

💬 glozow commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2059346502)
Ah so (900 + 172 * 483 * 2 + 224) / 4 = 41819vB?

So maybe a better number is 42KvB or 50KvB.

✅ Sjors closed an issue: "depens: bdb build fails on Intel macOS 13.6.6 "
(https://github.com/bitcoin/bitcoin/issues/29792)

💬 fanquake commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#discussion_r1567554130)
> (also agree that we should at least wait until someone else runs into this - hopefully BDB will be gone before then)

Ok, well then lets close this for now, given there's not really anything to review or do here, other than wait indefinitely for someone else to report the same thing (prior to BDB removal / maintainence end). If that does happen, I'm sure we'll remember this PR.

✅ Sjors closed a pull request: "depends: suggest GNU patch for macOS <= 13"
(https://github.com/bitcoin/bitcoin/pull/29814)

👍 ryanofsky approved a pull request: "index: race fix, lock cs_main while 'm_synced' is subject to change"
(https://github.com/bitcoin/bitcoin/pull/29867#pullrequestreview-2003980602)
Code review ACK 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e. I think I have the opposite opinion as andrewtoth, and that original bugfix c395b26df37b3839a9c76abbd4d5fb48e79b3208 is more direct and simple than current 99afb9d15a08d2f46739f4d2b66c63dbabd7a44e since the original fix leaves the Rewind code alone.

re: https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2059115444

> Not a blocker, but I think it would be cleaner if this PR was the following 3 commits:
>
> ```
> revert bb
...

💬 instagibbs commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2059368920)
pushed a fixup since I think it could have resulted in UB when selecting direct conflicts from the `mempool_txs` vector.

👍 stickies-v approved a pull request: "test: Fix failing univalue float test"
(https://github.com/bitcoin/bitcoin/pull/29892#pullrequestreview-2004014087)
ACK fa4c69669e079c38844ecea1ad3394aae3702ae1 , thanks for fixing!

💬 Sjors commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2059400017)
@ryanofsky's suggestion looks good to me as well.

💬 maflcko commented on pull request "test: Fix failing univalue float test":
(https://github.com/bitcoin/bitcoin/pull/29892#issuecomment-2059403538)
For testing, one can use gcc-13 (or later) on i386.

For example, running the centos CI task on `fedora:40`:

```diff
diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_i686_centos.sh
index 5f8391c..dc80e98 100755
--- a/ci/test/00_setup_env_i686_centos.sh
+++ b/ci/test/00_setup_env_i686_centos.sh
@@ -12,6 +12,7 @@ export CI_IMAGE_NAME_TAG="quay.io/centos/amd64:stream9"
export CI_BASE_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libs
...

💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-2059436483)
Rebased after #28981 (I previously already tested on top of that).

Briefly tested with HWI 3.0 as well on a Ledger Nano X.

🤔 tdb3 reviewed a pull request: "netbase: clean up Proxy logging"
(https://github.com/bitcoin/bitcoin/pull/29882#pullrequestreview-2004067074)
CR ACK for fb4cc5f423ce587c1e97377e8afdf92fb4850f59