Bitcoin Core Github
44 subscribers
121K links
Download Telegram
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?
💬 maflcko commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-1871985349)
> it only _enables_ us to do so if desired.

It would be good to list at least one benefit, otherwise the benefits of this change are unclear.
💬 kristapsk commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1871995364)
> It would probably be useful to introduce support for comments. This way, we can write something at the beginning of the file, ensuring that users and other software developers don't clean it up manually, thinking that it will be regenerated automatically.

This will not help if cause was full disk. https://github.com/bitcoin/bitcoin/issues/21340#issuecomment-876779561
🤔 glozow reviewed a pull request: "test: doc: follow-up #28368"
(https://github.com/bitcoin/bitcoin/pull/29013#pullrequestreview-1798855735)
concept ACK, thanks for fixing the issue
💬 glozow commented on pull request "test: doc: follow-up #28368":
(https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1438169687)
75f0478e0c435c1ee9242007ee1b391d3175519e this doc seems unnecessary
💬 anibilthare commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#discussion_r1438182115)
Can you please point out exactly what part of these statements might confuse the user?
Although we'll be asking the user to check for 2 things in the comment but I believe they are pretty straight forward, right?

1. Is there any space left on the disk.
2. Is settings.json empty or not?

If none of the above conditions hold true and user is still seeing the issue then we are indicating the possibility of corruption of the settings file itself.
achow101 closed a pull request: "Remove `dnsseed.bitcoin.dashjr.org` temporarily"
(https://github.com/bitcoin/bitcoin/pull/29149)
💬 furszy commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1872060582)
> 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 handl
...
💬 1440000bytes commented on pull request "Remove `dnsseed.bitcoin.dashjr.org` temporarily":
(https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1872063421)
@achow101 why is this pull request closed?
💬 1440000bytes commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#discussion_r1438254888)
NACK for this PR

Core does not works for umbrel

This is external and need to maintain outside of core: https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144
💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438256160)
```c++
fs::path dir = m_node.args->GetPathArg("-testdatadir");
// todo: check if path is valid
m_path_root = dir / "test_temp";
```
💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1438257281)
> Good idea, but what do you mean by path correctness?

Probably we should fail when `-testdatadir` is an empty string.