👍 ryanofsky approved a pull request: "refactor: Move chain names to the util library"
(https://github.com/bitcoin/bitcoin/pull/27294#pullrequestreview-1376476610)
Code review ACK f3c57bf4654599a4343cee3a1d2ffa7026a8ade7. This breaks dependency between server parameters (CChainParams in libbitcoin_kernel) and client parameters (CBaseChainParams in libbitcoin_common), so client parameters aren't dragged into the kernel.

However I think as long as every usage of the constants is changing anyway, it would be better to turn this into to enum and not introduce an odd namespace with string symbols. Suggestion would be to add a new `src/util/chain_types.h` fil
...

🤔 Xekyo reviewed a pull request: "wallet: Avoid underpaying transaction fees when signing taproot spends"
(https://github.com/bitcoin/bitcoin/pull/23502#pullrequestreview-1376476830)
Concept ACK

How would this behave in the context of changeless transactions? Would we be overestimating the target and dropping an excess to the fees? Does that get recognized when we compare coin selection solutions?

💬 Xekyo commented on pull request "wallet: Avoid underpaying transaction fees when signing taproot spends":
(https://github.com/bitcoin/bitcoin/pull/23502#discussion_r1160890949)
These make sense to me, but if we correctly estimated the maximum and minimum, how would they ever get used?

💬 Sjors commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1500550465)
> Silent watch-only wallets have not yet been implemented. Some architectural improvements are needed first.

This would be nice though, maybe in a followup. I was trying to test it with `bitcoin-cli -signet -named createwallet wallet_name=SilentWatcher silent_payment=true disable_private_keys=true` and then import a descriptor like `sp(…)`.

I guess one thing you need for that is the ability to dump the watch key. You could add an argument `scan_privkey` to `decodesilentaddress` which would
...

💬 Tracachang commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1500592284)
> Hm, seems to work for me on master branch. The taproot addresses are identical from each wallet.

Did not try the master branch, I am on v.24.0.1

⚠️ Muzeyfi opened an issue: "Muzeyfi "
(https://github.com/bitcoin/bitcoin/issues/27438)

✅ hebasto closed an issue: "Muzeyfi "
(https://github.com/bitcoin/bitcoin/issues/27438)

:lock: hebasto locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/27438)

💬 Xekyo commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#issuecomment-1500618548)
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98

💬 kevkevinpal commented on pull request "test: add coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27422#issuecomment-1500649484)
utACK by visually looking at the code I can see 3 different options
`status` `abort` and `start` which requires 1 argument

but if it doesn't get any of the above options it calls `JSONRPCError` link below
[Link to blockchain.cpp Line #2243](https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2243)

💬 ismaelsadeeq commented on pull request "test: add coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27422#issuecomment-1500679698)
Thank you @kevkevinpal Yes if it gets an argument that is not `status` `abort` or `start`
[ blockchain.cpp#L2243 ](https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2243) is called.

But
` scantxoutset `action argument is specified as
RPCArg::Optional::NO [blockchain.cpp#L2062](https://github.com/bitcoin/bitcoin/blob/5a8bd4505687a7ec76d731b1a8249ee04d641990/src/rpc/blockchain.cpp#L2062).
If there are no arguments scantxoutset won't be called at all.
It returns the
...

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062770)
I've updated it to this, this is much more clear.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062811)
I've added a check for this.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062833)
Done

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062927)
I agree, the naming here was a bit confusing. It should be fixed now.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062954)
Done both here and one other place.

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1161062971)
Fixed

💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1500792114)
Thanks for the review @glozow, I have addressed your comments and also made a modification to `RecursiveUpdateTxState` which will allow it to be more extensible.

⚠️ geenutts opened an issue: "Gee"
(https://github.com/bitcoin/bitcoin/issues/27439)
https://github.com/geenutts/consensus-specs/commit/e1ae7ec2ad141cf18940566d5070c1f146a7c2a9

✅ fanquake closed an issue: "Gee"
(https://github.com/bitcoin/bitcoin/issues/27439)