💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1567469129)
Thanks! I agree, I'll update it if I have to retouch it and if not, could be done in a follow-up.

📝 maflcko opened a pull request: "test: Fix failing univalue float test"
(https://github.com/bitcoin/bitcoin/pull/29892)
Currently the test may fail for some compilers, because `1e-8` may not be possible to represent exactly/consistently.

```
$ ./src/univalue/test/object
object: univalue/test/object.cpp:424: void univalue_readwrite(): Assertion `v.read("0.00000000000000000000000000000000000001e+30 ") && v.get_real() == 1e-8' failed.
Aborted (core dumped)
```

Fixes https://github.com/bitcoin/bitcoin/pull/27256#discussion_r1567356943

💬 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
...