🚀 achow101 merged a pull request: "doc: Add example of mixing private and public keys in descriptors"
(https://github.com/bitcoin/bitcoin/pull/28373)
(https://github.com/bitcoin/bitcoin/pull/28373)
💬 fanquake commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1569045988)
> Would it be possible to change these to values 0/1 instead of defined/undefined?
These are currently checked via: `AC_CHECK_HEADERS([sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h])`. If we wanted, to we could change this.
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1569045988)
> Would it be possible to change these to values 0/1 instead of defined/undefined?
These are currently checked via: `AC_CHECK_HEADERS([sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h])`. If we wanted, to we could change this.
✅ maflcko closed a pull request: "ci: Run everything in Nulgrind or Valgrind"
(https://github.com/bitcoin/bitcoin/pull/29900)
(https://github.com/bitcoin/bitcoin/pull/29900)
💬 maflcko commented on pull request "ci: Run everything in Nulgrind or Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29900#issuecomment-2061593496)
I guess this is too fragile to be useful
(https://github.com/bitcoin/bitcoin/pull/29900#issuecomment-2061593496)
I guess this is too fragile to be useful
💬 maflcko commented on issue "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust":
(https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2061604188)
I guess it could make more sense if someone tried to reproduce this in `rr`.
(https://github.com/bitcoin/bitcoin/issues/29889#issuecomment-2061604188)
I guess it could make more sense if someone tried to reproduce this in `rr`.
💬 theStack commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#issuecomment-2061659300)
Concept ACK, thanks for fixing!
(https://github.com/bitcoin/bitcoin/pull/29898#issuecomment-2061659300)
Concept ACK, thanks for fixing!
⚠️ brunoerg opened an issue: "Wallet fuzzing tracking issue"
(https://github.com/bitcoin/bitcoin/issues/29901)
The wallet has poor fuzz coverage. Hopefully, some work is being done to improve it. The goal of this issue is to actively track current work and work that needs to be done to improve fuzz coverage for the wallet.
## Open PRs / Ready to review
- [ ] https://github.com/bitcoin/bitcoin/pull/29694
- [ ] https://github.com/bitcoin/bitcoin/pull/28236
- [ ] https://github.com/bitcoin/bitcoin/pull/28074
## Current wallet targets
We currently have 6 specific targets for wallet, which cover
...
(https://github.com/bitcoin/bitcoin/issues/29901)
The wallet has poor fuzz coverage. Hopefully, some work is being done to improve it. The goal of this issue is to actively track current work and work that needs to be done to improve fuzz coverage for the wallet.
## Open PRs / Ready to review
- [ ] https://github.com/bitcoin/bitcoin/pull/29694
- [ ] https://github.com/bitcoin/bitcoin/pull/28236
- [ ] https://github.com/bitcoin/bitcoin/pull/28074
## Current wallet targets
We currently have 6 specific targets for wallet, which cover
...
💬 m3dwards commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2061714375)
I did get a bit of variation but hard to know what's important. `BlockAssemblerAddPackageTxns` was about 10-15% slower on `27.0rc1` vs `26.0`. The bench also throws a filesystem error on 27.0rc1 but completes on 26.0.
[Full bench results](`BlockAssemblerAddPackageTxns`)
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2061714375)
I did get a bit of variation but hard to know what's important. `BlockAssemblerAddPackageTxns` was about 10-15% slower on `27.0rc1` vs `26.0`. The bench also throws a filesystem error on 27.0rc1 but completes on 26.0.
[Full bench results](`BlockAssemblerAddPackageTxns`)
💬 stratospher commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2061766412)
seen in #29898 too - https://cirrus-ci.com/task/5457627730149376?logs=ci#L3718
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2061766412)
seen in #29898 too - https://cirrus-ci.com/task/5457627730149376?logs=ci#L3718
🚀 ryanofsky merged a pull request: "security: restrict abis in bitcoind.service"
(https://github.com/bitcoin/bitcoin/pull/28340)
(https://github.com/bitcoin/bitcoin/pull/28340)
🤔 murchandamus reviewed a pull request: "fuzz: wallet, add target for Spend"
(https://github.com/bitcoin/bitcoin/pull/28236#pullrequestreview-2006620250)
Hey,
Just did a quick pass. What is the status of this PR?
(https://github.com/bitcoin/bitcoin/pull/28236#pullrequestreview-2006620250)
Hey,
Just did a quick pass. What is the status of this PR?
💬 murchandamus commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569162422)
This comment is inconsistent:
```suggestion
// Add 50 spendable UTXO, 50 BTC each, to the wallet (total balance 2500 BTC)
```
A variable set of available coins might be desirable. Smaller amounts could for example trigger behavior regarding negative effective value or having insufficient funds to create a transaction.
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569162422)
This comment is inconsistent:
```suggestion
// Add 50 spendable UTXO, 50 BTC each, to the wallet (total balance 2500 BTC)
```
A variable set of available coins might be desirable. Smaller amounts could for example trigger behavior regarding negative effective value or having insufficient funds to create a transaction.
💬 murchandamus commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569167950)
If you removed the Singleton, did you perhaps not push the latest state of this PR?
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569167950)
If you removed the Singleton, did you perhaps not push the latest state of this PR?
💬 murchandamus commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569173834)
Agree wtih @maflcko, since the seeds are created per target, it would be better to have a separate target for `CoinsResult`.
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1569173834)
Agree wtih @maflcko, since the seeds are created per target, it would be better to have a separate target for `CoinsResult`.
💬 maflcko commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2061781675)
Coverage is nice, but if the fuzz test never finds any real user-facing issues, it will be useless. I don't think any wallet fuzz target found a real issue yet? Obviously coverage is the first step toward adding regression fuzz tests, so here are some ideas:
* #23444 tried to add a regression fuzz test, but it was picked up as part of https://github.com/bitcoin/bitcoin/pull/28933, and I couldn't reproduce the regression crash by re-introducing it.
* https://github.com/bitcoin/bitcoin/pull/27
...
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2061781675)
Coverage is nice, but if the fuzz test never finds any real user-facing issues, it will be useless. I don't think any wallet fuzz target found a real issue yet? Obviously coverage is the first step toward adding regression fuzz tests, so here are some ideas:
* #23444 tried to add a regression fuzz test, but it was picked up as part of https://github.com/bitcoin/bitcoin/pull/28933, and I couldn't reproduce the regression crash by re-introducing it.
* https://github.com/bitcoin/bitcoin/pull/27
...
💬 petertodd commented on pull request "tests: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2061785140)
On Wed, Apr 17, 2024 at 04:32:14AM -0700, Chris Stewart wrote:
> @dgpv @petertodd what do you think is the path forward for this?
Seems to me that one reasonable path forward is to just do nothing and not
merge these changes.
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2061785140)
On Wed, Apr 17, 2024 at 04:32:14AM -0700, Chris Stewart wrote:
> @dgpv @petertodd what do you think is the path forward for this?
Seems to me that one reasonable path forward is to just do nothing and not
merge these changes.
🤔 glozow reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2006530259)
I think we had some docs written for doc/policy/packages.md and doc/policy/mempool_replacements.md?
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2006530259)
I think we had some docs written for doc/policy/packages.md and doc/policy/mempool_replacements.md?
💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1569130553)
This seems like a merge conflict with #28970, maybe add the fix here instead?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1569130553)
This seems like a merge conflict with #28970, maybe add the fix here instead?
💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1569134261)
34c513554c7be97d91446126bb3ef0de2884fc19
This seems slightly inaccurate, as it wouldn't currently be in a package if `ReplacementChecks` is happening?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1569134261)
34c513554c7be97d91446126bb3ef0de2884fc19
This seems slightly inaccurate, as it wouldn't currently be in a package if `ReplacementChecks` is happening?
💬 brunoerg commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-2061802631)
> Tested with HWI 2.4.0
>
> When a Ledger Nano X is in screen saver mode, I now get the following error:
>
> ```
> '...hwi' error: Could not open client or get fingerprint information: ('0x5515', 'Error in <DefaultInsType.GET_VERSION: 1> command', '')
> ```
>
> That's what HWI returns, so can't make it much better...
>
> When the Bitcoin app is not opened on the device the error is more clear:
>
> ```
> '...hwi' error: Could not open client or get fingerprint information: Ledge
...
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-2061802631)
> Tested with HWI 2.4.0
>
> When a Ledger Nano X is in screen saver mode, I now get the following error:
>
> ```
> '...hwi' error: Could not open client or get fingerprint information: ('0x5515', 'Error in <DefaultInsType.GET_VERSION: 1> command', '')
> ```
>
> That's what HWI returns, so can't make it much better...
>
> When the Bitcoin app is not opened on the device the error is more clear:
>
> ```
> '...hwi' error: Could not open client or get fingerprint information: Ledge
...