๐ค brunoerg reviewed a pull request: "wallet, test: Replace MockableDatabase with in-memory SQLiteDatabase"
(https://github.com/bitcoin/bitcoin/pull/33032#pullrequestreview-3053056674)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33032#pullrequestreview-3053056674)
Concept ACK
๐ฌ maflcko commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2229432505)
calling call block_template->waitNext() -> calling block_template->waitNext() [duplicate โcallโ makes the phrase awkward and potentially confusing]
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2229432505)
calling call block_template->waitNext() -> calling block_template->waitNext() [duplicate โcallโ makes the phrase awkward and potentially confusing]
๐ค brunoerg reviewed a pull request: "test: Add functional tests for blockreconstructionextratxn parameter"
(https://github.com/bitcoin/bitcoin/pull/33023#pullrequestreview-3053070561)
I think the description is not clear about what you're trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn't show any related new coverage.
(https://github.com/bitcoin/bitcoin/pull/33023#pullrequestreview-3053070561)
I think the description is not clear about what you're trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn't show any related new coverage.
๐ฌ maflcko commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2229441917)
I see. Though, it may still be good to make this one `explicit` to avoid silent uint256 conversions in the future?
Also, `Txid::data` is `std::byte*`, so `value_type` should probably be `std::byte` as well.
I haven't tried, but I presume both modifications should compile with afe99b3b7df6c658fb4068c6418dfd7ebf151460
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2229441917)
I see. Though, it may still be good to make this one `explicit` to avoid silent uint256 conversions in the future?
Also, `Txid::data` is `std::byte*`, so `value_type` should probably be `std::byte` as well.
I haven't tried, but I presume both modifications should compile with afe99b3b7df6c658fb4068c6418dfd7ebf151460
๐ brunoerg approved a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3053079846)
Concept & light cr ACK d89c6fa4a71810cdb28395d4609632e1b22249b3
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3053079846)
Concept & light cr ACK d89c6fa4a71810cdb28395d4609632e1b22249b3
๐ฌ glozow commented on pull request "[29.x] final changes for v29.1rc1":
(https://github.com/bitcoin/bitcoin/pull/33056#issuecomment-3114741444)
thanks! done
(https://github.com/bitcoin/bitcoin/pull/33056#issuecomment-3114741444)
thanks! done
๐ฌ glozow commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2229453544)
Comment above this declaration needs to be changed
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2229453544)
Comment above this declaration needs to be changed
๐ฌ glozow commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2229461893)
Hm, is there a reason why `m_tx_inventory_known_filter` should contain sometimes-txids instead of wtxids?
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2229461893)
Hm, is there a reason why `m_tx_inventory_known_filter` should contain sometimes-txids instead of wtxids?
๐ brunoerg approved a pull request: "doc: add note for watch-only wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32866#pullrequestreview-3053110010)
ACK 9d25880bb720bc675a533098268b9e02f86e17ce
Happy to re-ack if you touch it again to address the LLM linter suggestion.
(https://github.com/bitcoin/bitcoin/pull/32866#pullrequestreview-3053110010)
ACK 9d25880bb720bc675a533098268b9e02f86e17ce
Happy to re-ack if you touch it again to address the LLM linter suggestion.
๐ฌ maflcko commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3114779229)
[18:13:33.790] ./wallet/test/coinselector_tests.cpp(1247): [1;31;49merror: in "coinselector_tests/srd_tests": check EquivalentResult(expected_result, *res) has failed [0;39;49m
https://api.cirrus-ci.com/v1/task/6251945985310720/logs/ci.log
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3114779229)
[18:13:33.790] ./wallet/test/coinselector_tests.cpp(1247): [1;31;49merror: in "coinselector_tests/srd_tests": check EquivalentResult(expected_result, *res) has failed [0;39;49m
https://api.cirrus-ci.com/v1/task/6251945985310720/logs/ci.log
๐ฌ bigshiny90 commented on pull request "test: Add functional tests for blockreconstructionextratxn parameter":
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3114783342)
> I think the description is not clear about what you're trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn't show any related new coverage.
This tests the -blockreconstructionextratxn parameter extra pool memory used specifically for compact block reconstruction. The "extra transaction pool" stores transactions that were rejected from the mempool for policy reasons (dust, low fees, non-standard scripts). Also, rbf original transactions get stuffed in here
...
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3114783342)
> I think the description is not clear about what you're trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn't show any related new coverage.
This tests the -blockreconstructionextratxn parameter extra pool memory used specifically for compact block reconstruction. The "extra transaction pool" stores transactions that were rejected from the mempool for policy reasons (dust, low fees, non-standard scripts). Also, rbf original transactions get stuffed in here
...
๐ฌ w0xlt commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3114785492)
The emphasis of this PR is on backup functionality: a backup feature isnโt very useful unless you can also restore the wallet reliably.
By contrast, the other PRs aim only to import a Codex32 secretโpresumably from Core Lightning into Bitcoin Coreโand each uses a different method.
The approach here creates a fresh wallet and rescans the chain, whereas PR #32652 appears to convert a Codex32 secret into an xpub via the proposed `addhdkey` RPC.
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3114785492)
The emphasis of this PR is on backup functionality: a backup feature isnโt very useful unless you can also restore the wallet reliably.
By contrast, the other PRs aim only to import a Codex32 secretโpresumably from Core Lightning into Bitcoin Coreโand each uses a different method.
The approach here creates a fresh wallet and rescans the chain, whereas PR #32652 appears to convert a Codex32 secret into an xpub via the proposed `addhdkey` RPC.
๐ฌ martinatime commented on issue "getbestblockhash is sometimes taking a very long time":
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3114802238)
My RPi 4 only has 4GB of RAM and I am only running off of the SSD with no SD card installed.
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3114802238)
My RPi 4 only has 4GB of RAM and I am only running off of the SSD with no SD card installed.
๐ฌ maflcko commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3114802810)
For reference, the lint task still fails. Not sure if this is intentional or if you missed it.
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3114802810)
For reference, the lint task still fails. Not sure if this is intentional or if you missed it.
๐ฌ maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2229493624)
(tested podman buildx on a vanilla Ubuntu 24.04, fwiw)
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2229493624)
(tested podman buildx on a vanilla Ubuntu 24.04, fwiw)
โ
maflcko closed a pull request: "NOMERGE DEBUG WIP ignore"
(https://github.com/bitcoin/bitcoin/pull/33028)
(https://github.com/bitcoin/bitcoin/pull/33028)
๐ฌ maflcko commented on pull request "NOMERGE DEBUG WIP ignore":
(https://github.com/bitcoin/bitcoin/pull/33028#issuecomment-3114821576)
this is just too difficult to reproduce
(https://github.com/bitcoin/bitcoin/pull/33028#issuecomment-3114821576)
this is just too difficult to reproduce
๐ฌ jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229516213)
Review feedback was unfavorable to adding test coverage for -netinfo. I'm not against it and happy to review proposals.
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229516213)
Review feedback was unfavorable to adding test coverage for -netinfo. I'm not against it and happy to review proposals.
๐ฌ pinheadmz commented on issue "getbestblockhash is sometimes taking a very long time":
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3114844144)
Running v29 on my Pi4 has been fine so far. Even with 8 available I rarely ever see it use more than 2, and this is with lnd, LndHub, tor, and a few other services. I will note that I am still running the 32-bit PiOS however.
```
$ free -mh total used free shared buff/cache available Mem: 7.7Gi 1.2Gi 490Mi 41Mi
...
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3114844144)
Running v29 on my Pi4 has been fine so far. Even with 8 available I rarely ever see it use more than 2, and this is with lnd, LndHub, tor, and a few other services. I will note that I am still running the 32-bit PiOS however.
```
$ free -mh total used free shared buff/cache available Mem: 7.7Gi 1.2Gi 490Mi 41Mi
...
๐ฌ maflcko commented on pull request "NOMERGE DEBUG WIP ignore":
(https://github.com/bitcoin/bitcoin/pull/33028#discussion_r2229528575)
It just reproduced. i guess the deadlock happens inside `Popen`, as there are no logs lines after this one:
```
./test/functional/combine_logs.py /ci_container_base/ci/scratch/test_runner/test_runner_โฟ_๐_20250724_203021/rpc_signer_0/ | grep -C3 'warning'
test 2025-07-24T20:30:49.464060Z TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-datadir=/ci_container_base/ci/scratch/test_runner/test_runner_โฟ_๐_20250724_203021/rpc_signer_0/node3', 'enumeratesigners']
node3 2025-07-24T2
...
(https://github.com/bitcoin/bitcoin/pull/33028#discussion_r2229528575)
It just reproduced. i guess the deadlock happens inside `Popen`, as there are no logs lines after this one:
```
./test/functional/combine_logs.py /ci_container_base/ci/scratch/test_runner/test_runner_โฟ_๐_20250724_203021/rpc_signer_0/ | grep -C3 'warning'
test 2025-07-24T20:30:49.464060Z TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-datadir=/ci_container_base/ci/scratch/test_runner/test_runner_โฟ_๐_20250724_203021/rpc_signer_0/node3', 'enumeratesigners']
node3 2025-07-24T2
...