Bitcoin Core Github
43 subscribers
122K links
Download Telegram
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464506)
[Same](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464459). Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464538)
[Same](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464459). Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464782)
@andrewtoth, now I've included `std::set::contains` as [you](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1383787269) and @jonatack [recommended](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1476755003).
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573465383)
It was also [suggested](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1383787269) by @andrewtoth, but C++20 wasn't supported at that time. Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573465430)
It makes more sense, thanks! Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573465501)
True. Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573465552)
I like it. Done!
๐Ÿ’ฌ fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#issuecomment-2067818798)
Hm, still seems to be flakey at least, giving it one more shot with longer bump
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573465667)
Nice. Done!
๐Ÿ’ฌ pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2067822211)
Updates:
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/26990#pullrequestreview-1860423961) provided by @jonatack regarding first commit 6898219043f42ae42e4703c549e656f56d276d13.
- Updated second commit cf7dd3564a3ace4153a32930f36bd78432b59097 following some directions [given](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1483546926) by @jonatack and [discussed](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1573464319) with @andrewtoth.
- Fixed incorr
...
๐Ÿ’ฌ fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#issuecomment-2067826359)
#28313 / #28321 may be related
๐Ÿ’ฌ luke-jr commented on pull request "[WIP] libevent @ master + use CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2067837088)
This doesn't seem like a good enough reason to use untested/unreleased code... :/
๐Ÿ’ฌ luke-jr commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2067841261)
Seems like this goes beyond the expected behaviour. Maybe make it `=2` at least?
๐Ÿ‘ luke-jr approved a pull request: "net: Decrease nMaxIPs when learning from DNS seeds"
(https://github.com/bitcoin/bitcoin/pull/29850#pullrequestreview-2013209083)
utACK
๐Ÿ’ฌ luke-jr commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2067857920)
This seems likely to have a high maintenance cost. What's the benefit?
๐Ÿ’ฌ luke-jr commented on pull request "tracing: cast block_connected duration to ยตs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2067867597)
So 23.x and 24.x used microseconds, while 25.x, 26.x, and 27.x currently use nanoseconds?

Seems like changing this back will disrupt more than it will fix...? Any reason not to just stick to nanoseconds and harden it against future accidents?
๐Ÿ’ฌ showtime73 commented on pull request "tracing: cast block_connected duration to ยตs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2067870839)
Oh geez
๐Ÿ’ฌ luke-jr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2067871490)
Concept NACK. Embedding copies of shared code just makes things more fragile, not safer. It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.
๐Ÿ’ฌ luke-jr commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2067873707)
Instead of Qt's dependencies, maybe we should wrap Qt itself this way? Or do C++ templates break that too much?
๐Ÿ’ฌ Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2067879382)
Putting in draft while I fix falling test