💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166947957)
thx, done

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166948122)
thx, done

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166948363)
thx, done

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166948827)
Might do next week/month, unless someone beats me to it

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1166950408)
Just using the current time seems good enough. Might implement next week.

💬 furszy commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1508740170)
Not really a fix if there is something else going on but.. could use https://github.com/furszy/bitcoin-core/commit/8495c852ebe199a96a4e205937861b76f54227eb (quick and dirty) to remove the active-wait timeout error.

💬 MarcoFalke commented on pull request "depends: fix compiling bdb with clang-16":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508758659)
This is only broken on aarch64, right?

💬 fanquake commented on pull request "depends: fix compiling bdb with clang-16":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508763753)
> This is only broken on aarch64, right?

Yea. Otherwise the CI should be failing. I haven't looked into the x86_64 vs aarch64 code in bdb.

💬 sipa commented on pull request "Update src/secp256k1 subtree to release v0.3.1":
(https://github.com/bitcoin/bitcoin/pull/27445#issuecomment-1508763839)
Updated, and also improved the linter logic to decide which files to lint.

💬 sipa commented on pull request "Update src/secp256k1 subtree to release v0.3.1":
(https://github.com/bitcoin/bitcoin/pull/27445#discussion_r1166963400)
Note for reviewers: this should just be excluding the following files:
* src/crc32c/.ycm_extra_conf.py
* src/minisketch/tests/pyminisketch.py
* src/secp256k1/tools/tests_wycheproof_generate.py

💬 theuni commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508784669)
Concept ACK. To @MarcoFalke's point, how nasty would a patch be to actually fix the casts?

💬 fanquake commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508790120)
> how nasty would a patch be to actually fix the casts?

The first failures occur in configure, so probably nasty.

💬 fanquake commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508818821)
Guix Build:
```bash
ed1f83a77d00:/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
05fb39e87c26ce80c12c9a1e054eb5b558a454120c19cc555d7ab1d6508c0749 guix-build-0cc90822d502/output/aarch64-linux-gnu/SHA256SUMS.part
c488ff0d8eb0e1ec605f5ca6836b3dba4ab468f2f51ad2356ec9a632f6aee4c5 guix-build-0cc90822d502/output/aarch64-linux-gnu/bitcoin-0cc90822d502-aarch64-linux-gnu-debug.tar.gz
32c33bdb964d7eb3fe16f30f00b5eb3582e6
...

👍 real-or-random approved a pull request: "Update src/secp256k1 subtree to release v0.3.1"
(https://github.com/bitcoin/bitcoin/pull/27445#pullrequestreview-1385704456)
utACK 621c17869d3754559c03e4f2bee73885659e0c68 subtree matches. diff to linter script looks good

💬 fanquake commented on pull request "Update src/secp256k1 subtree to release v0.3.1":
(https://github.com/bitcoin/bitcoin/pull/27445#issuecomment-1508834870)
cc @jonasnick too

👍 ryanofsky approved a pull request: "blockstorage: Adjust fastprune limit if block exceeds blockfile size"
(https://github.com/bitcoin/bitcoin/pull/27191#pullrequestreview-1385705001)
Code review ACK e930814fdb9261f180d5c436cefe52e9cf38fd67, but I think it would be great to add the test written by @pinheadmz https://github.com/bitcoin/bitcoin/pull/27191#pullrequestreview-1353171052 because the blockmanager code is fragile and a regression could happen, and because the test setup is very clean so it could probably be used to branch out and test other things.

💬 ryanofsky commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1166995654)
In commit "blockstorage: Adjust fastprune limit if block exceeds blockfile size" (e930814fdb9261f180d5c436cefe52e9cf38fd67)

What is the +1 for? Would be helpful to add a code comment saying what the extra byte is.

💬 ryanofsky commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1167004409)
In commit "blockstorage: Adjust fastprune limit if block exceeds blockfile size" (e930814fdb9261f180d5c436cefe52e9cf38fd67)

Would be good to add to add `assert(nAddSize <= max_blockfile_size);` here to make it clear what this function is assuming and catch the problem instead of going into an infinite loop if outside code changes.

💬 ryanofsky commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1508895445)
> I think assert is the appropriate move here to prevent the infinite memory loop!

I think throwing an exception or calling abort() would be ok, but better to reserve the assert macro for conditions which are actually impossible and not use it for error handling, because that would water down the meaning of other asserts.

💬 theuni commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1508919253)
May be interesting: I don't hit any issues when cross building for aarch64-linux-gnu from x86_64:
```
make NO_QT=1 HOST=aarch64-linux-gnu CC="clang --target=aarch64-linux-gnu" CXX='clang++ --target=aarch64-linux-gnu'
```
^^ (Where clang/clang++ are clang16 via PATH) builds fine.