👋 murchandamus's pull request is ready for review: "Slay BnB Mutants"
(https://github.com/bitcoin/bitcoin/pull/33060)
(https://github.com/bitcoin/bitcoin/pull/33060)
💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3119906723)
I cherry picked https://github.com/monlovesmango/bitcoin/commit/174827285797794928cd87732e2587fea70c0157 and fixed some of the incorrect comments.
> [doc: Fixes GetChildrenFromSamePeer comments](https://github.com/monlovesmango/bitcoin/commit/2a25c28aec3773bdf9a17148e45ce38ee95a7342) - Updates comments to reflect that non-reconsiderable announcements are sorted before reconsiderable announcements.
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3119906723)
I cherry picked https://github.com/monlovesmango/bitcoin/commit/174827285797794928cd87732e2587fea70c0157 and fixed some of the incorrect comments.
> [doc: Fixes GetChildrenFromSamePeer comments](https://github.com/monlovesmango/bitcoin/commit/2a25c28aec3773bdf9a17148e45ce38ee95a7342) - Updates comments to reflect that non-reconsiderable announcements are sorted before reconsiderable announcements.
💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2231808946)
They can be imported with `importdescriptors`.
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2231808946)
They can be imported with `importdescriptors`.
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2231837660)
Why is it safe to assume that the transaction doesn't already have a child here?
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2231837660)
Why is it safe to assume that the transaction doesn't already have a child here?
💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231847269)
added these and `serialize.h`, hope that'll do. (never got iwyu running locally and the tidy job didn't cover the new file for some reason).
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231847269)
added these and `serialize.h`, hope that'll do. (never got iwyu running locally and the tidy job didn't cover the new file for some reason).
💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231849239)
yes, I think the part with the "three items" is still correct, it talks about the values of the map.
I replaced the part about the key from blockfilterindex.cpp by a reference to db_key.h
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231849239)
yes, I think the part with the "three items" is still correct, it talks about the values of the map.
I replaced the part about the key from blockfilterindex.cpp by a reference to db_key.h
💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231851121)
Done - I added a comment to the file, using parts from `blockfilterindex.cpp`.
I agree that `db_key.h` is nicer, changed it to that.
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2231851121)
Done - I added a comment to the file, using parts from `blockfilterindex.cpp`.
I agree that `db_key.h` is nicer, changed it to that.
💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3120065955)
[03b67a2](https://github.com/bitcoin/bitcoin/commit/03b67a27d013e55891df69b1aef9425574b88f5e) to [936bd87](https://github.com/bitcoin/bitcoin/commit/936bd874ac1581de532317741f5ab26e875935b3):
Addressed feedback by @fjahr.
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3120065955)
[03b67a2](https://github.com/bitcoin/bitcoin/commit/03b67a27d013e55891df69b1aef9425574b88f5e) to [936bd87](https://github.com/bitcoin/bitcoin/commit/936bd874ac1581de532317741f5ab26e875935b3):
Addressed feedback by @fjahr.
💬 achow101 commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3120080986)
ACK c5c1960f9350d6315cadbdc95fface5f85f25806
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3120080986)
ACK c5c1960f9350d6315cadbdc95fface5f85f25806
💬 achow101 commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3120084517)
ACK d89c6fa4a71810cdb28395d4609632e1b22249b3
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3120084517)
ACK d89c6fa4a71810cdb28395d4609632e1b22249b3
💬 macgyver13 commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2231890541)
Shouldn't the coin_type be different for regtest/signet? /352h/1h instead of /352h/0h
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2231890541)
Shouldn't the coin_type be different for regtest/signet? /352h/1h instead of /352h/0h
🤔 macgyver13 reviewed a pull request: "Silent Payments: Receiving"
(https://github.com/bitcoin/bitcoin/pull/32966#pullrequestreview-3056616801)
working on backup / restore testing I noticed a few issues.
(https://github.com/bitcoin/bitcoin/pull/32966#pullrequestreview-3056616801)
working on backup / restore testing I noticed a few issues.
💬 macgyver13 commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2231883781)
I think this change is breaking the ability to import sp descriptor with private keys defined for scan and spend
This is the warning I receive when using importdescriptors:
"Not all private keys provided. Some wallet functionality may return unexpected errors"
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2231883781)
I think this change is breaking the ability to import sp descriptor with private keys defined for scan and spend
This is the warning I receive when using importdescriptors:
"Not all private keys provided. Some wallet functionality may return unexpected errors"
🚀 achow101 merged a pull request: "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs"
(https://github.com/bitcoin/bitcoin/pull/32845)
(https://github.com/bitcoin/bitcoin/pull/32845)
💬 jonatack commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#discussion_r2231906796)
- For users who don't follow changes closely, perhaps mention when legacy wallets became no longer supported
- Perhaps briefly remind users how to migrate legacy wallets to descriptor ones
(https://github.com/bitcoin/bitcoin/pull/32944#discussion_r2231906796)
- For users who don't follow changes closely, perhaps mention when legacy wallets became no longer supported
- Perhaps briefly remind users how to migrate legacy wallets to descriptor ones
🚀 achow101 merged a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944)
(https://github.com/bitcoin/bitcoin/pull/32944)
👍 pinheadmz approved a pull request: "index: initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3052111555)
code review ACK a3ddbe6bde6c296948cd963f396b35bb6a906b22
Built and tested on macos/arm64 as well as Debian/x86. Left several questions and suggestions. I really like ThreadPool as a util and look forward to using it for http workers as well, to share the code.
I'm currently rebuilding block filter and tx indexes with this branch to compare against master, which after 48 hours was only about 70% complete...
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE
...
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3052111555)
code review ACK a3ddbe6bde6c296948cd963f396b35bb6a906b22
Built and tested on macos/arm64 as well as Debian/x86. Left several questions and suggestions. I really like ThreadPool as a util and look forward to using it for http workers as well, to share the code.
I'm currently rebuilding block filter and tx indexes with this branch to compare against master, which after 48 hours was only about 70% complete...
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE
...
💬 pinheadmz commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228839630)
d2138bef76ae7c25679571f9c07ecb44e99d4ecc
is this sleep to give tasks a chance to execute if the blocking breaks?
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228839630)
d2138bef76ae7c25679571f9c07ecb44e99d4ecc
is this sleep to give tasks a chance to execute if the blocking breaks?
💬 pinheadmz commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228819659)
d2138bef76ae7c25679571f9c07ecb44e99d4ecc
would there be any benefit here to incrementing the counter at the end of each blocking task and check that they executed properly when unblocked?
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228819659)
d2138bef76ae7c25679571f9c07ecb44e99d4ecc
would there be any benefit here to incrementing the counter at the end of each blocking task and check that they executed properly when unblocked?
💬 pinheadmz commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228891058)
b36010261921adc74e61bdcbc91ba6b7778bad9a
"must be in-between 1 and..." but you allow `0`
also nit, could drop the "in-" and say "number between 1 and ..."
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2228891058)
b36010261921adc74e61bdcbc91ba6b7778bad9a
"must be in-between 1 and..." but you allow `0`
also nit, could drop the "in-" and say "number between 1 and ..."