Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#issuecomment-1871716006)
There we go, tests all happy now
💬 luke-jr commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871717493)
> just fix (or explain if there is a legit reason) the issue with only returinng old nodes.

They weren't old nodes when that change was made - they were the latest, intentionally excluding old nodes which didn't enforce Taproot.

I already updated it last night after you brought it to my attention.

However, it should be noted that the nodes returned were perfectly fine, and there wasn't actually a real issue.

>Removal is warranted as explained in PR description or we need to change po
...
💬 1440000bytes commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871732739)
> However, it should be noted that the nodes returned were perfectly fine, and there wasn't actually a real issue.

They were missing a few important bug fixes

> The PR description is pure lies. You are just outing yourself as a bad actor.

You wanted questions in https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871680509 which I shared in https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871700172 and are not answered yet
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438055454)
Good idea, but what do you mean by path correctness?
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438055076)
I can't get this to work.
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438054972)
The `g_printed_dir` guard is only needed because the user may be running more than one test case (like your first example). I don't love this global variable but it seems weird to print this out on every individual test case, and I think it's okay for a test program. I convinced myself that no race condition is possible, but we could use `std::call_once()` here but I didn't think it was worth it.
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438057148)
Good catch, I was actually aware of this but concluded that it is such a small timing window, that it's not a problem in practice. The problem is that on Windows, you can't remove an open file (and having a file lock is a form of having the file open). I could make this conditional on `WIN32` (I think it is), but I'm not sure that's even worth it.
💬 ns-xvrn commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871798839)
I agree with @mzumsande and request the author to close this PR.
💬 1440000bytes commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871799612)
In case its not clear many new devs/users: this is not reddit and up/down vote will be meaningless. Either PR gets merged or not and maintainer decide it.
💬 ns-xvrn commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871801467)
You're not a dev, you're just a troll harassing devs.
💬 anibilthare commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#discussion_r1438073495)
I agree with @shaavan [here](https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1798049445) . What if we simply modify the error message here and keep rest of the code as is to be more user friendly. Referring to https://github.com/bitcoin/bitcoin/pull/23096, https://github.com/bitcoin/bitcoin/pull/22591, and https://github.com/bitcoin/bitcoin/issues/21340#issuecomment-880147010 as mentioned earlier, following change can be added to the error message
```suggestion
errors.
...
maflcko closed a pull request: "[WIP] add a stratum v2 template provider"
(https://github.com/bitcoin/bitcoin/pull/27854)
💬 maflcko commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1871827453)
Closing for now, due to lack of addressing feedback and questions. Please leave a comment if this should be re-opened. In the meantime, it seems better to focus on https://github.com/bitcoin/bitcoin/pull/28983
🤔 BrandonOdiwuor reviewed a pull request: "test: add assumeutxo wallet test"
(https://github.com/bitcoin/bitcoin/pull/28838#pullrequestreview-1798753190)
Concept ACK
💬 kristapsk commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1871874677)
Should the permissions that can be set limited here? There is no reason to ever set +x on an RPC cookie file.
💬 Sjors commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1871897223)
Keep in mind that when a node gets a list of IP addresses from a DNS seed, the first thing it will do upon connection is to ask more peers. While the seed returns about a dozen addresses per query, these initial peers typically return a thousand each. So if subsequent outbound connections are randomly drawn, the odds of connecting to one of the peers initially returned by the seed are quite small.

So it doesn't matter that much what features these initial peers support, as long as they can gi
...
💬 ajtowns commented on pull request "rpc: Remove deprecated -rpcserialversion":
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1871939016)
> The deprecation was covered in https://bitcoinops.org/en/newsletters/2023/09/20/ and 26.0 was released a few weeks ago. Unless anyone heard someone complain, this seems good to move forward now?

Given we're doing a short cycle, probably should either merge this soon for 27.0 or defer it to 28.0 and merge it shortly after branch off. No opinion either way on that personally.

crACK fa46cc22bc696e6845915ae91d6b68e36bf4c242
🤔 shaavan reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1798843843)
I get your point, @furszy, and I can see your approach helping with smoother handling of the "file empty" situation.

However, pointing back again to ryanofsky comment

> A zero-size settings file is a corrupt settings file.

The interpretation I draw from this is if a file is empty (and hence corrupted once) there is no telling what could be the reason for that happening or whether such corruption could happen again.
So, I believe being very critical and clear with the handling will be b
...
💬 shaavan commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#discussion_r1438165814)
I see your point here. But wording like this provides a very unclear reason for failure.
Don't you think we would be throwing users in too many directions here?