π¬ pablomartin4btc commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2686310566)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2686310566)
Concept ACK
π l0rinc approved a pull request: "doc: update fuzz instructions when on macOS"
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2646056999)
ACK 725c0fd95b15157bd0ec141cc24d27818f536656
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2646056999)
ACK 725c0fd95b15157bd0ec141cc24d27818f536656
π¬ l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972486916)
It's not exactly *full*, we still need:
```bash
$ cmake --build build_fuzz -j$(nproc)
$ FUZZ=process_message build_fuzz/src/test/fuzz/fuzz
```
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972486916)
It's not exactly *full*, we still need:
```bash
$ cmake --build build_fuzz -j$(nproc)
$ FUZZ=process_message build_fuzz/src/test/fuzz/fuzz
```
π¬ pablomartin4btc commented on pull request "doc: Update translation generation instructions":
(https://github.com/bitcoin/bitcoin/pull/31930#issuecomment-2686323908)
> I think after #31741 we should be able to drop the extra argument.
I thought we'd need to replace it by `-DENABLE_IPC=OFF` (as it will be `ON` in `CMakePresets.json`) and I was getting errors of missing config files (`CapnProtoConfig.cmake` & `capnproto-config.cmake`) when I run the instructions in #31741, but the problem was that I needed to install the Cap'n Proto for multiprocess (as in the documentation), so your statement is correct, I'm updating this PR's description accordingly. Than
...
(https://github.com/bitcoin/bitcoin/pull/31930#issuecomment-2686323908)
> I think after #31741 we should be able to drop the extra argument.
I thought we'd need to replace it by `-DENABLE_IPC=OFF` (as it will be `ON` in `CMakePresets.json`) and I was getting errors of missing config files (`CapnProtoConfig.cmake` & `capnproto-config.cmake`) when I run the instructions in #31741, but the problem was that I needed to install the Cap'n Proto for multiprocess (as in the documentation), so your statement is correct, I'm updating this PR's description accordingly. Than
...
π¬ m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972501719)
Technically, it is the full _configuration_ step :)
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972501719)
Technically, it is the full _configuration_ step :)
π¬ Prabhat1308 commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#issuecomment-2686348268)
tACK [`725c0fd`](https://github.com/bitcoin/bitcoin/pull/31954/commits/725c0fd95b15157bd0ec141cc24d27818f536656)
-Checked by running the fuzz tests on several fuzz targets . No warnings displayed now.
(https://github.com/bitcoin/bitcoin/pull/31954#issuecomment-2686348268)
tACK [`725c0fd`](https://github.com/bitcoin/bitcoin/pull/31954/commits/725c0fd95b15157bd0ec141cc24d27818f536656)
-Checked by running the fuzz tests on several fuzz targets . No warnings displayed now.
π¬ yancyribbens commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1972515521)
```suggestion
- `m_assumeutxo_data` array can be appended to with the (partial) output of the `gettxoutsetinfo "hash_serialized_3 <target_block_height>"` RPC. The same height considerations for `defaultAssumeValid` apply.
```
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1972515521)
```suggestion
- `m_assumeutxo_data` array can be appended to with the (partial) output of the `gettxoutsetinfo "hash_serialized_3 <target_block_height>"` RPC. The same height considerations for `defaultAssumeValid` apply.
```
π¬ yancyribbens commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r1972522204)
```suggestion
// 2) Maintain all busy threads except one.
```
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r1972522204)
```suggestion
// 2) Maintain all busy threads except one.
```
π¬ hugohn commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2686702465)
> if no one can be bothered to test locally
Maybe Iβm reading too much into it, but it feels like thereβs a bit of a blame game going on in this thread, which doesn't seem constructive. People _did_ test the changes more than once. Let me share my own post-mortem, in case it helps. (If I misread the tone, I apologize.)
There were four main actors in this incident:
**Reporter**
My colleague at Nunchuk, @giahuy98, investigated this issue after several users reported they couldnβt open ou
...
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2686702465)
> if no one can be bothered to test locally
Maybe Iβm reading too much into it, but it feels like thereβs a bit of a blame game going on in this thread, which doesn't seem constructive. People _did_ test the changes more than once. Let me share my own post-mortem, in case it helps. (If I misread the tone, I apologize.)
There were four main actors in this incident:
**Reporter**
My colleague at Nunchuk, @giahuy98, investigated this issue after several users reported they couldnβt open ou
...
π curiosityz opened a pull request: "Codespace ideal space yodel 977946qwx57gc74q7"
(https://github.com/bitcoin-core/gui/pull/855)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin-core/gui/pull/855)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
β
curiosityz closed a pull request: "Codespace ideal space yodel 977946qwx57gc74q7"
(https://github.com/bitcoin-core/gui/pull/855)
(https://github.com/bitcoin-core/gui/pull/855)
π fanquake locked a pull request: "Codespace ideal space yodel 977946qwx57gc74q7"
(https://github.com/bitcoin-core/gui/pull/855)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin-core/gui/pull/855)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
π¬ m3dwards commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2686935466)
> I also would have expected `-DAPPEND_LDFLAGS='-fuse-ld=lld'` instead
Good spot, this would probably work. I updated the doc change in #31954 to use the flag `CMAKE_EXE_LINKER_FLAG` which also works.
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2686935466)
> I also would have expected `-DAPPEND_LDFLAGS='-fuse-ld=lld'` instead
Good spot, this would probably work. I updated the doc change in #31954 to use the flag `CMAKE_EXE_LINKER_FLAG` which also works.
π¬ i-am-yuvi commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2687058943)
Tested ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
- I've tested all the functional tests and it passes (5/5)
- interface_usdt_coinselection.py β
- interface_usdt_mempool.py β
- interface_usdt_net.py β
- interface_usdt_utxo.py β
- interface_usdt_validation.py β
- All tracing tools works fine (3/3)
- Did 5-6 runs of CI similar to this [one](https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656), which can be seen [here](https://github.com/i-am-yuvi/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2687058943)
Tested ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
- I've tested all the functional tests and it passes (5/5)
- interface_usdt_coinselection.py β
- interface_usdt_mempool.py β
- interface_usdt_net.py β
- interface_usdt_utxo.py β
- interface_usdt_validation.py β
- All tracing tools works fine (3/3)
- Did 5-6 runs of CI similar to this [one](https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656), which can be seen [here](https://github.com/i-am-yuvi/bitcoin
...
π¬ Talej commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2687090846)
> Please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits). You can improve the commit message at the same time. i.e "doc: ...."
Done!
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-2687090846)
> Please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits). You can improve the commit message at the same time. i.e "doc: ...."
Done!
π¬ hodlinator commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2687169428)
Maybe one could have a `I_KNOW_WHAT_I_AM_DOING` environment variable for developers knowledgeable with the intricacies of the build process, that disables the default-enabled error for this?
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2687169428)
Maybe one could have a `I_KNOW_WHAT_I_AM_DOING` environment variable for developers knowledgeable with the intricacies of the build process, that disables the default-enabled error for this?
π€ i-am-yuvi reviewed a pull request: "test, tracing: don't use problematic `bpf_usdt_readarg_p()`"
(https://github.com/bitcoin/bitcoin/pull/31848#pullrequestreview-2647016062)
Tested ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
- I've tested all the functional tests and it passes (5/5)
- interface_usdt_coinselection.py β
- interface_usdt_mempool.py β
- interface_usdt_net.py β
- interface_usdt_utxo.py β
- interface_usdt_validation.py β
- All tracing tools works fine (3/3)
- Did 5-6 runs of CI similar to this [one](https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656), which can be seen [here](https://github.com/i-am-yuvi/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/31848#pullrequestreview-2647016062)
Tested ACK a0b66b4bffaa6bc354c293785b785c2da2cef4de
- I've tested all the functional tests and it passes (5/5)
- interface_usdt_coinselection.py β
- interface_usdt_mempool.py β
- interface_usdt_net.py β
- interface_usdt_utxo.py β
- interface_usdt_validation.py β
- All tracing tools works fine (3/3)
- Did 5-6 runs of CI similar to this [one](https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656), which can be seen [here](https://github.com/i-am-yuvi/bitcoin
...
π rkrux approved a pull request: "rpc: Allow fullrbf fee bump in (psbt)bumpfee"
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2647075553)
tACK fa2068c58905d027918e2d3917deeeec020681f3
Definitely in favour of this because it cleans up remnants of the recently outdated `BIP 125 signalling` flag in transaction creation and propagation. The PR description can be updated though to mention the case of BIP 125 signalling.
```
The RPCs (psbt)bumpfee, and the GUI, reject fee bumps when BIP 125 signalling is absent in the transaction even when the mempool and other RPCs allow them.
```
Besides the tests, I tested the scenario in
...
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2647075553)
tACK fa2068c58905d027918e2d3917deeeec020681f3
Definitely in favour of this because it cleans up remnants of the recently outdated `BIP 125 signalling` flag in transaction creation and propagation. The PR description can be updated though to mention the case of BIP 125 signalling.
```
The RPCs (psbt)bumpfee, and the GUI, reject fee bumps when BIP 125 signalling is absent in the transaction even when the mempool and other RPCs allow them.
```
Besides the tests, I tested the scenario in
...
π¬ rkrux commented on pull request "rpc: Allow fullrbf fee bump in (psbt)bumpfee":
(https://github.com/bitcoin/bitcoin/pull/31953#discussion_r1973277370)
```diff
- // Call bumpfee. Test disabled, canceled, enabled, then failing cases.
+ // Call bumpfee. Test canceled non replaceable, canceled replaceable, enabled replaceable, and then failing cases.
```
Or something like this.
(https://github.com/bitcoin/bitcoin/pull/31953#discussion_r1973277370)
```diff
- // Call bumpfee. Test disabled, canceled, enabled, then failing cases.
+ // Call bumpfee. Test canceled non replaceable, canceled replaceable, enabled replaceable, and then failing cases.
```
Or something like this.
π¬ rkrux commented on pull request "rpc: Allow fullrbf fee bump in (psbt)bumpfee":
(https://github.com/bitcoin/bitcoin/pull/31953#discussion_r1973235877)
Looking forward to seeing this argument going away in future PRs.
(https://github.com/bitcoin/bitcoin/pull/31953#discussion_r1973235877)
Looking forward to seeing this argument going away in future PRs.