💬 maflcko commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3009004461)
only small style changes
re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555 🍪
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment:
...
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3009004461)
only small style changes
re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555 🍪
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment:
...
💬 maflcko commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3009013387)
> you'll just be unable to dump at that height.
Could do a manual `invalidateblock` of the low-work forks to work around the error, but yea, could be annoying if the forks are more than 1, or longer than 1 block.
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3009013387)
> you'll just be unable to dump at that height.
Could do a manual `invalidateblock` of the low-work forks to work around the error, but yea, could be annoying if the forks are more than 1, or longer than 1 block.
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2169429571)
Yeah.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2169429571)
Yeah.
💬 maflcko commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2169431059)
If a `Commit` is not possible without closing the file, then `Commit` should close the file. Requiring all call sites of `Commit` to manually close the file seems verbose and less safe to me.
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2169431059)
If a `Commit` is not possible without closing the file, then `Commit` should close the file. Requiring all call sites of `Commit` to manually close the file seems verbose and less safe to me.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009050890)
> Whiteboarded in person. Hopefully this explanation is clear and explains it in a more permanent location. If this ends up being correct we can adapt the benchmark to be more realistic.
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009050890)
> Whiteboarded in person. Hopefully this explanation is clear and explains it in a more permanent location. If this ends up being correct we can adapt the benchmark to be more realistic.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169450138)
Done! Thanks for the suggestion it was realy insightful.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169450138)
Done! Thanks for the suggestion it was realy insightful.
🚀 fanquake merged a pull request: "depends: Build `qt` package for FreeBSD hosts"
(https://github.com/bitcoin/bitcoin/pull/32731)
(https://github.com/bitcoin/bitcoin/pull/32731)
💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009069508)
> For the CI docs, see https://github.com/bitcoin/bitcoin/tree/master/ci#ci-scripts, but you'll likely have to run that on a 64 core system to recreate the system error
Thanks, maflcko. The thing is this patchset is to extend numbers of scriptcheck threads to nCores-1, so I guess it bound to fail the CI tests. What should I do now.
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009069508)
> For the CI docs, see https://github.com/bitcoin/bitcoin/tree/master/ci#ci-scripts, but you'll likely have to run that on a 64 core system to recreate the system error
Thanks, maflcko. The thing is this patchset is to extend numbers of scriptcheck threads to nCores-1, so I guess it bound to fail the CI tests. What should I do now.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169453387)
Fixed
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169453387)
Fixed
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169457387)
You are right it should be scalled down and rounded up as done in ` return (std::max(nWeight, nSigOpCost * bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; `. Fixed in [d6f4caf](https://github.com/bitcoin/bitcoin/pull/32800/commits/d6f4caf851ad7a4d9df7a8dc67215870ba9bfe14)
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169457387)
You are right it should be scalled down and rounded up as done in ` return (std::max(nWeight, nSigOpCost * bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; `. Fixed in [d6f4caf](https://github.com/bitcoin/bitcoin/pull/32800/commits/d6f4caf851ad7a4d9df7a8dc67215870ba9bfe14)
🚀 fanquake merged a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530)
(https://github.com/bitcoin/bitcoin/pull/32530)
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169463734)
Added in [d6f4caf](https://github.com/bitcoin/bitcoin/pull/32800/commits/d6f4caf851ad7a4d9df7a8dc67215870ba9bfe14). Thanks.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169463734)
Added in [d6f4caf](https://github.com/bitcoin/bitcoin/pull/32800/commits/d6f4caf851ad7a4d9df7a8dc67215870ba9bfe14). Thanks.
💬 fanquake commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-3009086333)
Backported to 29.x in #32810.
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-3009086333)
Backported to 29.x in #32810.
👍 brunoerg approved a pull request: "test: Turn util/test_runner into functional test"
(https://github.com/bitcoin/bitcoin/pull/32697#pullrequestreview-2962749484)
re-ACK fa2163159511a099ecffde2bfddf9cfe33eb9c76
Verified minor changes since my last review: `self.log`usage, catch-throw removal and error message into `Exception`.
(https://github.com/bitcoin/bitcoin/pull/32697#pullrequestreview-2962749484)
re-ACK fa2163159511a099ecffde2bfddf9cfe33eb9c76
Verified minor changes since my last review: `self.log`usage, catch-throw removal and error message into `Exception`.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169464510)
Fixed
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169464510)
Fixed
💬 maflcko commented on pull request "clang-format: modernize and realign clang-format configuration":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2169464696)
Again, as said in my previous comment this doesn't work, because older clang-formats will just error out:
```
$ clang-format-16 -i src/bench/prevector.cpp
/b-c/src/.clang-format:12:3: error: unknown key 'AlignFunctionPointers'
AlignFunctionPointers: false
^~~~~~~~~~~~~~~~~~~~~
Error reading /b-c/src/.clang-format: Invalid argument
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2169464696)
Again, as said in my previous comment this doesn't work, because older clang-formats will just error out:
```
$ clang-format-16 -i src/bench/prevector.cpp
/b-c/src/.clang-format:12:3: error: unknown key 'AlignFunctionPointers'
AlignFunctionPointers: false
^~~~~~~~~~~~~~~~~~~~~
Error reading /b-c/src/.clang-format: Invalid argument
💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009089895)
> Hint: Call /ci_container_base/test/functional/combine_logs.py '/ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20250626_150635/wallet_fundrawtransaction_252'
In my local run, I can do something like this to see the bitcoind log (node log as I said), but how can I see the bitcoind log on Cirrus CI, seems no way to do that since that is not exposed.
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009089895)
> Hint: Call /ci_container_base/test/functional/combine_logs.py '/ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20250626_150635/wallet_fundrawtransaction_252'
In my local run, I can do something like this to see the bitcoind log (node log as I said), but how can I see the bitcoind log on Cirrus CI, seems no way to do that since that is not exposed.
📝 darosior opened a pull request: "Add release note for #32530"
(https://github.com/bitcoin/bitcoin/pull/32819)
(https://github.com/bitcoin/bitcoin/pull/32819)
💬 HowHsu commented on pull request "validation: fetch block inputs on parallel threads 10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3009101750)
Hi folks, this looks great, since if all the `prevout coins` of all transactions of a block are loaded in advance, then the optimization in #32791 makes sense.
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3009101750)
Hi folks, this looks great, since if all the `prevout coins` of all transactions of a block are loaded in advance, then the optimization in #32791 makes sense.
💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3009102393)
I think I'll drop bf63317ae7395b0509f86e2c188b1c6cacf995de and revert to only db63d5bf81 (unless someone sees a blocking reason and the need to use uint32 instead of int32).
Changing to uint32 needs many changes in FeeFrac (which I would like to avoid touching). In FeeFrac size is defined as an int32_t so using uint32 doesn't give us any "advantage".
Friendly ping @davidgumberg @Eunovo @sipa to know your opinion :)
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3009102393)
I think I'll drop bf63317ae7395b0509f86e2c188b1c6cacf995de and revert to only db63d5bf81 (unless someone sees a blocking reason and the need to use uint32 instead of int32).
Changing to uint32 needs many changes in FeeFrac (which I would like to avoid touching). In FeeFrac size is defined as an int32_t so using uint32 doesn't give us any "advantage".
Friendly ping @davidgumberg @Eunovo @sipa to know your opinion :)