Bitcoin Core Github
43 subscribers
124K links
Download Telegram
💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108460621)
I looked into this again. The underlying problem seems to be that the kernel library still depends on the `ArgsManager`, which in turn depends on the global. I'd rather not make this change devolve into changing `ArgsManager` internals and behaviour, so I'll defer refactoring this global to once the `ArgsManager` is removed from the kernel library.
💬 furszy commented on pull request "wallet: Refactor and document CoinControl":
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1433087966)
While I would like to merge this one (it's good as is), I think that would be better to check #25273 first. Mainly because this PR is about improving the `CoinControl` class documentation and #25273 modifies a big part of it (which, if we merge this one first, will make us re-do this doc improvements in another follow-up later).
💬 furszy commented on pull request "wallet: Refactor and document CoinControl":
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1433091681)
@MarcoFalke, guess that Drahtbot shouldn't have requested review to me.
💬 MarcoFalke commented on pull request "wallet: Refactor and document CoinControl":
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1433123977)
Yeah, if it was live 3 weeks ago, it would have requested a review from you before your comment. Which, I guess, would have made sense.

Also, it will only request reviews when there is an email notification sent out anyway, so the harm/risk should be minimal. But let me know if it should be tamed down or turned off.
💬 ajtowns commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108469843)
Seems more ergonomic to write:

```c++
CChainParams::SigNetOptions ReadSigNetArgs(const ArgsManager& args)
{
...
```

but maybe this is just deliberately keeping with the same pattern as per https://github.com/bitcoin/bitcoin/pull/26883/files#r1068247937
💬 ajtowns commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108499523)
Rename to `GetBuriedDeployment()` ? This function should be part of deploymentinfo.cpp, shouldn't it?
💬 ajtowns commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108500764)
`name` is really `deployment_id` (`dep` for short)?
💬 ajtowns commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108508687)
Using optionals for signet but not regtest makes sense to me.
🚀 MarcoFalke merged a pull request: "test: previous releases: add v24.0.1"
(https://github.com/bitcoin/bitcoin/pull/26586)
🚀 MarcoFalke merged a pull request: "doc: remove mention of "proper signing key""
(https://github.com/bitcoin/bitcoin/pull/27107)
🚀 MarcoFalke merged a pull request: "test: add coverage for unparsable `-maxuploadtarget`"
(https://github.com/bitcoin/bitcoin/pull/26714)
⚠️ yanagawak opened an issue: "Why is Bitcoin Core structured as a wallet? I know that it began as a wallet, but is it possible to make it like a GUI block explorer instead?"
(https://github.com/bitcoin/bitcoin/issues/27110)
I'm always frustrated when I open up Bitcoin Core, and then all I get is an empty wallet GUI. I see no information about the block-chain itself. When I want to see on-chain data, I have to go to centralized websites instead.

I would like to see Bitcoin Core be restructured as a GUI block explorer. Delete all the wallet features. Bitcoin Core is a really, really bad wallet, to be honest. Didn't Core developer Luke Dash Junior just lose his entire stash of 300 coins because he was using a inter
...
fanquake closed an issue: "Why is Bitcoin Core structured as a wallet? I know that it began as a wallet, but is it possible to make it like a GUI block explorer instead?"
(https://github.com/bitcoin/bitcoin/issues/27110)
💬 MarcoFalke commented on issue "Why is Bitcoin Core structured as a wallet? I know that it began as a wallet, but is it possible to make it like a GUI block explorer instead?":
(https://github.com/bitcoin/bitcoin/issues/27110#issuecomment-1433194111)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base. General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat.
🚀 fanquake merged a pull request: "doc: FreeBSD build doc updates to reflect removal of install_db4.sh"
(https://github.com/bitcoin/bitcoin/pull/26773)
💬 yancyribbens commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433228883)
@achow101

> Yes, it could be a problem if the test is removed, but it's unlikely that the test will be removed, and if it is removed, the line can be just as easily moved at that time.

I did not mean to imply that a test might be removed. I was only pointing out that this kind of coding practice is confusing because the variable definition doesn't belong _only_ to the test.

> While this isn't incorrect, it's not meaningfully helpful. It doesn't really add anything useful to the codeba
...
💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108602508)
Yes, also keeping it the same as in: https://github.com/bitcoin/bitcoin/pull/25862/files#diff-4e268aeb074a176689aace31957a889c1f39c909e6678ca7be8707bda1a7be56R11
💬 prusnak commented on pull request "doc: remove mention of "proper signing key"":
(https://github.com/bitcoin/bitcoin/pull/27107#issuecomment-1433230441)
ACK 304ae6d
💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1108608610)
I see, I'll use `dep`, because that's what is used elsewhere in the code.