💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2607242803)
TIL!
done.
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2607242803)
TIL!
done.
💬 andrewtoth commented on pull request "coins: remove SetFresh method from CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2607248709)
> What if instead both `fresh` and `dirty` are parameters to a general `SetState` method?
But the goal is to make it impossible to set `fresh` on its own. So why have a separate `dirty` parameter? I think the way it is now is optimal.
(https://github.com/bitcoin/bitcoin/pull/33018#discussion_r2607248709)
> What if instead both `fresh` and `dirty` are parameters to a general `SetState` method?
But the goal is to make it impossible to set `fresh` on its own. So why have a separate `dirty` parameter? I think the way it is now is optimal.
📝 th30neee opened a pull request: "Merge"
(https://github.com/bitcoin/bitcoin/pull/34043)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/34043)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
🤔 mzumsande reviewed a pull request: "p2p: reduce false-positives in addr rate-limiting"
(https://github.com/bitcoin/bitcoin/pull/33699#pullrequestreview-3563344611)
I think the suggestion by @ajtowns (https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3462287763) to send the first self-advertisement separately would make sense on its own - currently it will usually (I think it depends a bit on timing) get mixed up with the GETADDR answer and replaces one of the 1000 addrs from it, which isn't clean.
(https://github.com/bitcoin/bitcoin/pull/33699#pullrequestreview-3563344611)
I think the suggestion by @ajtowns (https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3462287763) to send the first self-advertisement separately would make sense on its own - currently it will usually (I think it depends a bit on timing) get mixed up with the GETADDR answer and replaces one of the 1000 addrs from it, which isn't clean.
🤔 Sjors reviewed a pull request: "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet"
(https://github.com/bitcoin/bitcoin/pull/32489#pullrequestreview-3563175683)
Reviewed 6fafa9dd48a814a929efb31b746dce343a3c96dc and tested it, though not with hardened derivation.
(https://github.com/bitcoin/bitcoin/pull/32489#pullrequestreview-3563175683)
Reviewed 6fafa9dd48a814a929efb31b746dce343a3c96dc and tested it, though not with hardened derivation.
💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607155345)
In 67216700ce8ec5e84bca5f83860091f6ac4f67f4 _wallet: Move listdescriptors retrieving from RPC to CWallet_:
Could try the new `util::Expected<<std::vector<WalletDescInfo>>,std::string>` from #34006 here.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607155345)
In 67216700ce8ec5e84bca5f83860091f6ac4f67f4 _wallet: Move listdescriptors retrieving from RPC to CWallet_:
Could try the new `util::Expected<<std::vector<WalletDescInfo>>,std::string>` from #34006 here.
💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607263677)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: if you take my earlier suggestion https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2606838777 for having this return `false` for non-ranged descriptors, you'll need to add `w_desc.descriptor->IsRange() &&`
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607263677)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: if you take my earlier suggestion https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2606838777 for having this return `false` for non-ranged descriptors, you'll need to add `w_desc.descriptor->IsRange() &&`
💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607293085)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: at the moment `DescriptorCache` doesn't contain any private key material, but if that ever changes this line would put that into the watch-only wallet.
Maybe add a note to the `DescriptorCache` description that if private key material is added, `ExportWatchOnlyWallet` needs to be modified to avoid it.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607293085)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: at the moment `DescriptorCache` doesn't contain any private key material, but if that ever changes this line would put that into the watch-only wallet.
Maybe add a note to the `DescriptorCache` description that if private key material is added, `ExportWatchOnlyWallet` needs to be modified to avoid it.
💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607201881)
After #33032 this intermediate watch-only wallet could in-memory.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607201881)
After #33032 this intermediate watch-only wallet could in-memory.
💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607241544)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: nit `*Assert(exported)`
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2607241544)
In a453ea6ff06797224adbb1dd839299fe3d8636ae _wallet: Add CWallet::ExportWatchOnly_: nit `*Assert(exported)`
💬 sedited commented on pull request "doc: clarify libbitcoinkernel usage in libraries design":
(https://github.com/bitcoin/bitcoin/pull/34042#issuecomment-3637868117)
This has been discussed in https://github.com/bitcoin/bitcoin/pull/28690, which introduces a libbitcoin_kernel target. In the past the design doc was understood as a goal to work towards. I think we should finally decide whether to move forward with #28690, and if not, accept the changes you laid out here.
(https://github.com/bitcoin/bitcoin/pull/34042#issuecomment-3637868117)
This has been discussed in https://github.com/bitcoin/bitcoin/pull/28690, which introduces a libbitcoin_kernel target. In the past the design doc was understood as a goal to work towards. I think we should finally decide whether to move forward with #28690, and if not, accept the changes you laid out here.
👋 fanquake's pull request is ready for review: "depends: Boost 1.90.0"
(https://github.com/bitcoin/bitcoin/pull/33428)
(https://github.com/bitcoin/bitcoin/pull/33428)
💬 fanquake commented on pull request "depends: Boost 1.90.0":
(https://github.com/bitcoin/bitcoin/pull/33428#issuecomment-3637893169)
Updated to `1.90.0`.
(https://github.com/bitcoin/bitcoin/pull/33428#issuecomment-3637893169)
Updated to `1.90.0`.
💬 billymcbip commented on pull request "test: Improve STRICTENC/DERSIG unit tests in script_tests.json":
(https://github.com/bitcoin/bitcoin/pull/33973#issuecomment-3637941000)
@darosior I added the STRICTENC tests back. Maintainers can we run CI?
(https://github.com/bitcoin/bitcoin/pull/33973#issuecomment-3637941000)
@darosior I added the STRICTENC tests back. Maintainers can we run CI?
💬 maflcko commented on pull request "util: Add some more Unexpected and Expected methods":
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376375)
thx, done
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376375)
thx, done
💬 maflcko commented on pull request "util: Add some more Unexpected and Expected methods":
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376833)
thx, done
(https://github.com/bitcoin/bitcoin/pull/34032#discussion_r2607376833)
thx, done
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607401275)
I am not touching those lines, so I'll leave this as-is for now
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607401275)
I am not touching those lines, so I'll leave this as-is for now
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607405653)
will leave as-is for now
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607405653)
will leave as-is for now
💬 maflcko commented on pull request "test: Detect truncated download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607407049)
`black` prefers this, so I'll leave this as-is for now
(https://github.com/bitcoin/bitcoin/pull/34040#discussion_r2607407049)
`black` prefers this, so I'll leave this as-is for now
💬 stickies-v commented on pull request "kernel: Improve logging API":
(https://github.com/bitcoin/bitcoin/pull/33847#discussion_r2607414440)
I was actually wrong about my earlier example about chainman owning context: it is [passed](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.h#L967) as a non-owning (`const` ptr) view, and lifetime then ensured via [`std::shared_ptr`](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.cpp#L472). Whether we prevent destroying or automatically extend lifetimes is not really my concern he
...
(https://github.com/bitcoin/bitcoin/pull/33847#discussion_r2607414440)
I was actually wrong about my earlier example about chainman owning context: it is [passed](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.h#L967) as a non-owning (`const` ptr) view, and lifetime then ensured via [`std::shared_ptr`](https://github.com/bitcoin/bitcoin/blob/56ce78d5f62c9eb9353d33eedd15495c1205465f/src/kernel/bitcoinkernel.cpp#L472). Whether we prevent destroying or automatically extend lifetimes is not really my concern he
...