Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 MarcoFalke approved a pull request: "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs"
(https://github.com/bitcoin/bitcoin/pull/27444#pullrequestreview-1381080456)
lgtm
💬 MarcoFalke commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1163983711)
You can use gcc and drop it, if you want
💬 MarcoFalke commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#issuecomment-1505093759)
> to improve support for aarch64

What is the error message on aarch64? Does it happen with gcc and clang?
🤔 stickies-v reviewed a pull request: "Allow configuring target block time for a signet"
(https://github.com/bitcoin/bitcoin/pull/27446#pullrequestreview-1381041535)
Concept ACK, I've seen the need for signet custom block times come up quite often and I think it makes sense.
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163966920)
I think this can be simplified with `GetIntArg` which returns a `std::optional`:

<details>
<summary>git diff</summary>

```diff
diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index f41d2aaf8..02d51039e 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -27,19 +27,14 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
}
options.challenge.emplace(ParseHex(signet_challenge[0]));
}
- if (args.IsArgSet("-signe
...
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163982117)
I think this can be simplified quite a bit by just initializing `SigNetOptions::nPowTargetSpacing` with a default value:

<details>
<summary>git diff</summary>

```diff
diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
index 7f66a6b8e..32905af48 100644
--- a/src/kernel/chainparams.cpp
+++ b/src/kernel/chainparams.cpp
@@ -290,7 +290,6 @@ public:
explicit SigNetParams(const SigNetOptions& options)
{
std::vector<uint8_t> bin;
- int64_t nPow
...
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163968385)
nit: we use snake case now ([developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c)), I think it's fine to have this one be snake case and `Consensus::Params::nPowTargetSpacing` be camel case.

```suggestion
std::optional<int64_t> pow_target_spacing{};
```
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163988830)
I think the doc also needs to specify that `-signetchallenge` is required when using this option
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163975357)
nit: I'd define the default in a variable and use it here to avoid hardcoding the default here so it doesn't go out of sync
💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1163959320)
Would `InitParameterInteraction()` (`init.cpp`) be a better place for this?
⚠️ BataBoom opened an issue: "cli not returning data"
(https://github.com/bitcoin/bitcoin/issues/27450)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I created a new wallet, then deleted the new wallet returning it to what it was (1 wallet) or so I think. The logs look good from what I can tell but when I run bitcoin-cli getbalance, getnewaddress, etc it just stalls and doesnt return anything?

I should have made a new wallet over litecoin, id much rather have that down than a bitcoin gateway. ooooF. Any ideas/help is greatly apprecia
...
BataBoom closed an issue: "cli not returning data"
(https://github.com/bitcoin/bitcoin/issues/27450)
💬 carnhofdaki commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1505220494)
How does it work in Elements (Blockstream Liquid)? Is the 1-minute block time hardwired there?

I agree with @ajtowns this may cause inconsistencies (and I would add "confusion"). For example nodes that will not set the parameter to the same value as miners will simply refuse the blocks which do not fit their point-of-view. So in the end the parameter would need to be distributed the same way as `signetchallenge` (which is a once and for all setting) and can not be changed on-the-fly on an alr
...
📝 tarunraheja84 opened a pull request: "updated"
(https://github.com/bitcoin/bitcoin/pull/27451)
Steps to compile Bitcoin Core in Windows (most used OS) inserted.
fanquake closed a pull request: "updated"
(https://github.com/bitcoin/bitcoin/pull/27451)
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1505248502)
> if the `HTTPRequest` object is created successfully (once `replySent` is removed and the test does `try/catch`). If the input is such that `HTTPRequest:HTTPRequest()` throws, then it does not have to test the above because it will never be executed in the real code.

I'm sorry, I said nonsense, ofc that code won't be executed because the `uri_parsed` will be null and so on, my concern is that we'll lose the test coverage by the current fuzz, even it's not a real case scenario, the test is pe
...
💬 glozow commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1164171540)
> From the ephemeral anchors review club:

>> `<glozow> ok hm. i kinda need to get rid of blockmintxfee for pakcage relay tho`

> Can you elaborate on that?

My thinking there was that we needed to address this DoS issue in order to proceed with package relay, and this PR seemed like the most likely approach (hence the "kinda"). Now, maybe #26933 seems more likely?

> You can just drop the blockminfee commit from this PR, and it seems fine, no?
> The only result of setting -blockmin
...
💬 tarunraheja84 commented on pull request "updated":
(https://github.com/bitcoin/bitcoin/pull/27451#issuecomment-1505330949)
Hello, I am completely new to open source. This was my first PR.
Compiling Bitcoin Core into pc is the first step for a new developer to get started with which is not mentioned in Readme anywhere. I thought this very process should have been mentioned in Readme itself in a clean way instead of the link to doc folder.
Can you please highlight reasons of closing this PR understanding I am a beginner having great zeal to learn and contribute.
💬 pinheadmz commented on pull request "updated":
(https://github.com/bitcoin/bitcoin/pull/27451#issuecomment-1505333867)
Thanks for contributing and for your excitement -- you could have saved a little time by opening an issue first or asking on IRC or stackexchange for example where the windows build directions are, before putting in the work you did.