💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3043873716)
This may also be useful for Silent Payments, cc @josibake.
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3043873716)
This may also be useful for Silent Payments, cc @josibake.
💬 eval-exec commented on pull request "rest: replace `rf_names[0].rf` by `RESTResponseFormat::UNDEF` for code clarity":
(https://github.com/bitcoin/bitcoin/pull/32884#issuecomment-3043891057)
Friendly request @fanquake to review this PR.
(https://github.com/bitcoin/bitcoin/pull/32884#issuecomment-3043891057)
Friendly request @fanquake to review this PR.
📝 maflcko opened a pull request: "ci: Use optimized Debug build type in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/32888)
An optimized debug build is mostly as fast as a release build, because hot loops of heavy debug-only code are rare. So use that setting in the test-each-commit CI, to enable more checks almost "for free".
(https://github.com/bitcoin/bitcoin/pull/32888)
An optimized debug build is mostly as fast as a release build, because hot loops of heavy debug-only code are rare. So use that setting in the test-each-commit CI, to enable more checks almost "for free".
💬 stratospher commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3043958524)
reACK a53d43b.
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3043958524)
reACK a53d43b.
💬 maflcko commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3043971964)
I'd say it would be useful if the `mocks` scripts had more debug logging enabled; maybe some command tracing when entering the script and when exiting it?
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3043971964)
I'd say it would be useful if the `mocks` scripts had more debug logging enabled; maybe some command tracing when entering the script and when exiting it?
💬 rkrux commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2189441597)
I kind of liked the separation of concerns here but done this because multiple people are interested.
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2189441597)
I kind of liked the separation of concerns here but done this because multiple people are interested.
💬 maflcko commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3044138868)
Just to clarify this is just a refactor/cleanup and doesn't affect end-to-end IBD performance in a measurable way?
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3044138868)
Just to clarify this is just a refactor/cleanup and doesn't affect end-to-end IBD performance in a measurable way?
💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3044199252)
d3b8a54a81209420ef6447dd4581e1b6b8550647
Addressed some of the comments. Mostly nits.
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3044199252)
d3b8a54a81209420ef6447dd4581e1b6b8550647
Addressed some of the comments. Mostly nits.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3044285606)
`4a873c2a67...6d697bba16`: rebase and address [the above suggestion about `BroadcastTransaction()`](https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-2972293733).
> I think there is a bug in `BroadcastTransaction()` ...
Right, good catch! I fixed it like this:
```diff
// Transaction is not already in the mempool.
- if (max_tx_fee > 0 || broadcast_method == NO_MEMPOOL_PRIVATE_BROADCAST) {
+ const bool check_max_fee{max_tx_fee > 0};
+
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3044285606)
`4a873c2a67...6d697bba16`: rebase and address [the above suggestion about `BroadcastTransaction()`](https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-2972293733).
> I think there is a bug in `BroadcastTransaction()` ...
Right, good catch! I fixed it like this:
```diff
// Transaction is not already in the mempool.
- if (max_tx_fee > 0 || broadcast_method == NO_MEMPOOL_PRIVATE_BROADCAST) {
+ const bool check_max_fee{max_tx_fee > 0};
+
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2189573061)
Which doc? This is in the output of `bitcoin-cli help sendrawtransaction`.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2189573061)
Which doc? This is in the output of `bitcoin-cli help sendrawtransaction`.
✅ maflcko closed a pull request: "ci: Catch tests corrupting the source directory"
(https://github.com/bitcoin/bitcoin/pull/32874)
(https://github.com/bitcoin/bitcoin/pull/32874)
💬 maflcko commented on pull request "ci: Catch tests corrupting the source directory":
(https://github.com/bitcoin/bitcoin/pull/32874#issuecomment-3044435815)
Closing for now. Probably best to move this to a dedicated nightly ci, than to try to force it into the existing ci infra.
(https://github.com/bitcoin/bitcoin/pull/32874#issuecomment-3044435815)
Closing for now. Probably best to move this to a dedicated nightly ci, than to try to force it into the existing ci infra.
📝 maflcko opened a pull request: "bench: Avoid tmp files in pwd"
(https://github.com/bitcoin/bitcoin/pull/32890)
It is a bit confusing that one bench run, when aborted, could leave behind temp files in the current working directory. It is similarly confusing to delete those files in the next run of bench.
Fix all issues by using `BasicTestingSetup`, which provides a proper temp folder to use and also cleans up after itself.
Can be tested via:
```
( echo 'my file content' > streams_tmp ) && ls streams_tmp && ./bld-cmake/bin/bench_bitcoin --filter=FindByte && ls streams_tmp
```
Previously the f
...
(https://github.com/bitcoin/bitcoin/pull/32890)
It is a bit confusing that one bench run, when aborted, could leave behind temp files in the current working directory. It is similarly confusing to delete those files in the next run of bench.
Fix all issues by using `BasicTestingSetup`, which provides a proper temp folder to use and also cleans up after itself.
Can be tested via:
```
( echo 'my file content' > streams_tmp ) && ls streams_tmp && ./bld-cmake/bin/bench_bitcoin --filter=FindByte && ls streams_tmp
```
Previously the f
...
💬 maflcko commented on pull request "threading: use correct mutex name in reverse_lock fatal error messages":
(https://github.com/bitcoin/bitcoin/pull/32829#issuecomment-3044551062)
Done in https://github.com/bitcoin/bitcoin/pull/32888
(https://github.com/bitcoin/bitcoin/pull/32829#issuecomment-3044551062)
Done in https://github.com/bitcoin/bitcoin/pull/32888
💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3044614613)
@davidgumberg might be talking past each other, but what's happening here is that locally as soon as the block is reconstructed (including merkle checks), prior to script checks, the block is advertised forward via compact blocks to its peers. This is inline with the protocol description, and helps speed up block propagation.
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3044614613)
@davidgumberg might be talking past each other, but what's happening here is that locally as soon as the block is reconstructed (including merkle checks), prior to script checks, the block is advertised forward via compact blocks to its peers. This is inline with the protocol description, and helps speed up block propagation.
💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189796353)
will do on touchup
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189796353)
will do on touchup
🤔 Prabhat1308 reviewed a pull request: "ci: Use optimized Debug build type in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/32888#pullrequestreview-2993523631)
ACK [`fa21d3c`](https://github.com/bitcoin/bitcoin/pull/32888/commits/fa21d3c7781f36d96aa13fb32cad40aad5b5131f)
changes seem good as discussed in #32829 . Can we use `mold` as a linker in other Linux based system workflows ? dependencies we have seem to satisfy the deps here https://github.com/rui314/mold?tab=readme-ov-file#how-to-build
(https://github.com/bitcoin/bitcoin/pull/32888#pullrequestreview-2993523631)
ACK [`fa21d3c`](https://github.com/bitcoin/bitcoin/pull/32888/commits/fa21d3c7781f36d96aa13fb32cad40aad5b5131f)
changes seem good as discussed in #32829 . Can we use `mold` as a linker in other Linux based system workflows ? dependencies we have seem to satisfy the deps here https://github.com/rui314/mold?tab=readme-ov-file#how-to-build
💬 maflcko commented on pull request "ci: Use optimized Debug build type in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3044773485)
> Can we use `mold` as a linker in other Linux based system workflows ?
Sure, happy to review a follow-up. Only place to avoid it would probably the ci tasks that mirror the guix build (win-cross, mac-cross)
(https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3044773485)
> Can we use `mold` as a linker in other Linux based system workflows ?
Sure, happy to review a follow-up. Only place to avoid it would probably the ci tasks that mirror the guix build (win-cross, mac-cross)
💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897570)
fixed
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897570)
fixed
💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897816)
done
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897816)
done