🤔 optout21 reviewed a pull request: "refactor: unify container presence checks (without PR conflicts)"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512)
ACK f70d2c7faa8f7d724e146e4c409de9c6778b7299
Re-reviewed; this is a subset of #33097, comments from [there](https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3093962887) still apply:
- The changes increase code readability, as 'contains' expresses the code logic / intent more specifically
- It also results in higher performance, due to potential early exit. The improvement is probably negligible though.
- Changes are localized (each to a single line), local impact only
(
...
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512)
ACK f70d2c7faa8f7d724e146e4c409de9c6778b7299
Re-reviewed; this is a subset of #33097, comments from [there](https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3093962887) still apply:
- The changes increase code readability, as 'contains' expresses the code logic / intent more specifically
- It also results in higher performance, due to potential early exit. The improvement is probably negligible though.
- Changes are localized (each to a single line), local impact only
(
...
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312290776)
https://github.com/bitcoin/bitcoin/pull/32541/commits/94389c28e1068ffcc116614d16ac3047eb3068e3 seems to be passing (after a rebase over `master`) :thinking:
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312290776)
https://github.com/bitcoin/bitcoin/pull/32541/commits/94389c28e1068ffcc116614d16ac3047eb3068e3 seems to be passing (after a rebase over `master`) :thinking:
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312297480)
Thanks - updated the comment.
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312297480)
Thanks - updated the comment.
💬 naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2312399866)
I’d suggest removing this instead of commenting it out.
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2312399866)
I’d suggest removing this instead of commenting it out.
🤔 naiyoma reviewed a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3171569181)
ACK fa3f682032a3292604f363a5ee4557937f3d8950
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3171569181)
ACK fa3f682032a3292604f363a5ee4557937f3d8950
✅ martinus closed a pull request: "Draft: CCoinMap Experiments"
(https://github.com/bitcoin/bitcoin/pull/32128)
(https://github.com/bitcoin/bitcoin/pull/32128)
💬 martinus commented on pull request "Draft: CCoinMap Experiments":
(https://github.com/bitcoin/bitcoin/pull/32128#issuecomment-3240102028)
Up for grabs, if anyone is interested
(https://github.com/bitcoin/bitcoin/pull/32128#issuecomment-3240102028)
Up for grabs, if anyone is interested
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312514122)
@hodlinator would it be OK to squash https://github.com/bitcoin/bitcoin/commit/94389c28e1068ffcc116614d16ac3047eb3068e3 into https://github.com/bitcoin/bitcoin/pull/32541/commits/bdd90ab25f5f3b05c03a8dfb7177399264bed377?
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312514122)
@hodlinator would it be OK to squash https://github.com/bitcoin/bitcoin/commit/94389c28e1068ffcc116614d16ac3047eb3068e3 into https://github.com/bitcoin/bitcoin/pull/32541/commits/bdd90ab25f5f3b05c03a8dfb7177399264bed377?
🤔 hebasto reviewed a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171703063)
I’ve obtained matching hashes for Guix builds on the `aarch64` and `riscv64` hardware:
```
835b80bca08284974c5300768fb03be2c50143510548f8c1c03c737d6b22e473 guix-build-91560160a011/output/aarch64-linux-gnu/SHA256SUMS.part
8b0d7a7447ec50624528e42d04c318da20d755dd0e61758a77f2ebfcd74c4a0f guix-build-91560160a011/output/aarch64-linux-gnu/bitcoin-91560160a011-aarch64-linux-gnu-debug.tar.gz
95632363960c1db8aafaab8523f35d3d41961455bf075c5e73bf4f7903136563 guix-build-91560160a011/output/aarch64-li
...
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171703063)
I’ve obtained matching hashes for Guix builds on the `aarch64` and `riscv64` hardware:
```
835b80bca08284974c5300768fb03be2c50143510548f8c1c03c737d6b22e473 guix-build-91560160a011/output/aarch64-linux-gnu/SHA256SUMS.part
8b0d7a7447ec50624528e42d04c318da20d755dd0e61758a77f2ebfcd74c4a0f guix-build-91560160a011/output/aarch64-linux-gnu/bitcoin-91560160a011-aarch64-linux-gnu-debug.tar.gz
95632363960c1db8aafaab8523f35d3d41961455bf075c5e73bf4f7903136563 guix-build-91560160a011/output/aarch64-li
...
👍 hebasto approved a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171710452)
ACK 91560160a011fe3b2f472aa9144d4072e9e369cd, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171710452)
ACK 91560160a011fe3b2f472aa9144d4072e9e369cd, I have reviewed the code and it looks OK.
🤔 furszy reviewed a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3171730621)
q: is `IsFromMe()` method still relevant? It seems we could directly call `IsMine()` on the inputs, which is also cached now and should be slightly safer for migration.
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3171730621)
q: is `IsFromMe()` method still relevant? It seems we could directly call `IsMine()` on the inputs, which is also cached now and should be slightly safer for migration.
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240325938)
> is `IsFromMe()` method still relevant?
Yes, transactions that we make are still treated specially.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240325938)
> is `IsFromMe()` method still relevant?
Yes, transactions that we make are still treated specially.
💬 furszy commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240345028)
> > is `IsFromMe()` method still relevant?
>
> Yes, transactions that we make are still treated specially.
Where?. `IsFromMe()` seems to be only ever called in `AddToWalletIfInvolvingMe()` and `ApplyMigrationData()` next to the `IsMine()` call. In both places we effectively treat it as `is_mine = IsMine(outputs) || IsFromMe(inputs)`.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240345028)
> > is `IsFromMe()` method still relevant?
>
> Yes, transactions that we make are still treated specially.
Where?. `IsFromMe()` seems to be only ever called in `AddToWalletIfInvolvingMe()` and `ApplyMigrationData()` next to the `IsMine()` call. In both places we effectively treat it as `is_mine = IsMine(outputs) || IsFromMe(inputs)`.
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240361084)
> Where?
Hmm, I forgot that `CachedTxIsFromMe` does not use `IsFromMe`. Might need to change that in this PR as well.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240361084)
> Where?
Hmm, I forgot that `CachedTxIsFromMe` does not use `IsFromMe`. Might need to change that in this PR as well.
💬 hebasto commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3240432791)
> Could be used for #32764.
That PR would benefit from being updated a bit further, to https://codeberg.org/guix/guix/commit/2877c75dc5db0dbf664fb6170d5754068e941d91.
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3240432791)
> Could be used for #32764.
That PR would benefit from being updated a bit further, to https://codeberg.org/guix/guix/commit/2877c75dc5db0dbf664fb6170d5754068e941d91.
⚠️ nwokentathankgod-cell opened an issue: "Bug Report: [Provide a concise summary] bc1qq8m35x6sfhva08vzrr5qr75ec9dj000gq8e2mj"
(https://github.com/bitcoin/bitcoin/issues/33272)
**Describe the bug:**
A clear and concise description of what the bug is.
**To Reproduce:**
Steps to reproduce the behavior:
1. Go to '...'
2. Click on '...'
3. Scroll down to '...'
4. See error
**Expected behavior:**
A clear and concise description of what you expected to happen.
**Screenshots:**
If applicable, add screenshots to help explain your problem.
**Additional context:**
Add any other context about the problem here.
(https://github.com/bitcoin/bitcoin/issues/33272)
**Describe the bug:**
A clear and concise description of what the bug is.
**To Reproduce:**
Steps to reproduce the behavior:
1. Go to '...'
2. Click on '...'
3. Scroll down to '...'
4. See error
**Expected behavior:**
A clear and concise description of what you expected to happen.
**Screenshots:**
If applicable, add screenshots to help explain your problem.
**Additional context:**
Add any other context about the problem here.
✅ pinheadmz closed an issue: "Bug Report: [Provide a concise summary] bc1qq8m35x6sfhva08vzrr5qr75ec9dj000gq8e2mj"
(https://github.com/bitcoin/bitcoin/issues/33272)
(https://github.com/bitcoin/bitcoin/issues/33272)
💬 maflcko commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3240910468)
re-ACK b49a4f17aba76d2f2d7f1109d7e68b02303947bf 📦
<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: re-ACK b49a4f17aba76d2f2d7f
...
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3240910468)
re-ACK b49a4f17aba76d2f2d7f1109d7e68b02303947bf 📦
<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: re-ACK b49a4f17aba76d2f2d7f
...
⚠️ ellenkampguus opened an issue: "Old wallet support (Berkeley 4.8)"
(https://github.com/bitcoin/bitcoin/issues/33273)
### Please describe the feature you'd like to see added.
I can imagine the developer's side of removing support for the old wallets using Berkeley 4.8. However, isn't one of the things people holding Bitcoin want is hold it for a long time? I think it is important to make sure older wallets can just be opened in the default Bitcoin node and wallet software.
This issue relates to "The Bitcoin Devs" not supporting "Bitcoin as it is supposed to be" according to the more purist Bitcoiners, as it s
...
(https://github.com/bitcoin/bitcoin/issues/33273)
### Please describe the feature you'd like to see added.
I can imagine the developer's side of removing support for the old wallets using Berkeley 4.8. However, isn't one of the things people holding Bitcoin want is hold it for a long time? I think it is important to make sure older wallets can just be opened in the default Bitcoin node and wallet software.
This issue relates to "The Bitcoin Devs" not supporting "Bitcoin as it is supposed to be" according to the more purist Bitcoiners, as it s
...
🤔 maflcko reviewed a pull request: "test: Remove polling loop from test_runner (take 2)"
(https://github.com/bitcoin/bitcoin/pull/33141#pullrequestreview-3172171987)
force pushed with small formatting changes. Should be trivial to re-review via `--ignore-all-space`.
(https://github.com/bitcoin/bitcoin/pull/33141#pullrequestreview-3172171987)
force pushed with small formatting changes. Should be trivial to re-review via `--ignore-all-space`.
💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2312988889)
> I can imagine this will result in new test failures we haven't seen before.
I'd doubt that test will fail due to scheduling them more tightly without sleeps in-between. Though, if they do, that is a good thing, because it hints at a bug that we should be aware of and ideally should fix.
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2312988889)
> I can imagine this will result in new test failures we haven't seen before.
I'd doubt that test will fail due to scheduling them more tightly without sleeps in-between. Though, if they do, that is a good thing, because it hints at a bug that we should be aware of and ideally should fix.