Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972870)
Done.
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972961)
Done.
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1546636677)
I've made a number of changes.

* (Done earlier) Made the `GE` class represent infinity explicitly.
* The new classes / code are in a new module `test_framework.secp256k1`, which removes the implementation details from `test_framework.key`, and also feels a bit better namespace-wise (`secp256k1.GE` is probably more informative to an unfamiliar reviewer than just `GE`, at least indicating it has something to do with elliptic curve cryptography).
* Dropped a number of unused functions / operat
...
💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1546651567)
I just downloaded v24rc3 for testing, but when I do:

`gpg --verify SHA256SUMS.asc SHA256SUMS`

it fails (exitcode 2). Normally on official releases this always worked fine.

So is it normal that release candidates are not signed? Or what could be the problem?
💬 martinus commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546658892)
Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).
💬 sangaman commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546666636)
> Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).

Thanks! I'll try that branch out and see if resolves the underlying issue without this env var, in which case I'd agree this PR is probably not necessary.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546714913)
Thank you for the suggestions @ryanofsky!

Updated 0b159b9256c214a289135ca7bfaf70e48ab28e20 -> 63e915af5a437d36abdd79a1b90fed2395086589 ([chainstateRmKernelUiInterface_0](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_0) -> [chainstateRmKernelUiInterface_1](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_0..chainstateRmKernelUiInterface_1)).
...
⚠️ kroese opened an issue: "Allow getblockfrompeer to use any peer"
(https://github.com/bitcoin/bitcoin/issues/27652)
### Please describe the feature you'd like to see added.

When you use `getblockfrompeer` you have to specify an index of a specific peer. But it would be much easier if it allowed you to use ANY peer.

For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.

### Is your feature related to a problem, if so please des
...
💬 kroese commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546731833)
Maybe @Sjors can look into this?
💬 furszy commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546733957)
> For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.

Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.
💬 kroese commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546736533)

> Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
> You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.

I'm aware you could workaround this issue by implementing a bunch of manual logic on the client-side. But even if you skip the pruned nodes and this full node couldn't provide you the block, it would be handy if the call would just try the next one.
💬 furszy commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546741712)
> I'm aware you could workaround this issue by implementing a bunch of manual logic on the client-side. But even if you skip the pruned nodes and this full node couldn't provide you the block, it would be handy if the call would just try the next one.

Sure. Was providing you a simple workaround to the described issue.

I actually have done something similar for #21267 (in that case is a block filter tracker but here will be for blocks only).
So, I'm happy to work on it. Will tackle it soon
...
💬 AdamISZ commented on issue "Could the wallet count unconfirmed non-mempool change?":
(https://github.com/bitcoin/bitcoin/issues/11887#issuecomment-1546817951)
Thanks to @harding for pinging us on this, but: in Joinmarket, we have had a number of user reports (via our Telegram support chat and a couple other places) for the last several weeks (while mempool sizes were trending higher ofc) of missing balance. It seems that it is related to what @morcos has described above here.

Context: Joinmarket uses an unusual, nowadays (and probably bad!) model for tracking its wallet: starting from a normal BIP84 seed, we are using address imports into a **watch
...
💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193099662)
Any reason for the cast?
💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193099637)
This is fine, but I think for documentation purposes, the static globals are initialized in `initialize_setup` in the fuzz tests
💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193100074)
nit for less code (feel free to ignore):

```suggestion
FeeCalculation fee_calculation;
FeeCalculation* maybe_fee_calculation{ fuzzed_data_provider.ConsumeBool()?nullptr:&fee_calculation};
(void)GetMinimumFeeRate(wallet, coin_control, maybe_fee_calculation);
(void)GetMinimumFee(wallet, tx_bytes, coin_control, maybe_fee_calculation);
```
🚀 fanquake merged a pull request: "build: Fix shared lib linking for darwin with lld"
(https://github.com/bitcoin/bitcoin/pull/27628)
🚀 fanquake merged a pull request: "depends: no-longer nuke libc++abi.so* in native_clang package"
(https://github.com/bitcoin/bitcoin/pull/27493)
💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546869753)
Concept ACK.
🤔 hebasto reviewed a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1425535676)
Approach ACK 63e915af5a437d36abdd79a1b90fed2395086589.

> The approach implemented here introduces some indirection, which makes the code a bit harder to read.

As for me, it does not look like a problem at all. Good function naming works well :D

In commit 73556797ab9c44038994954b945db996642656d2:
```diff
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4659,7 +4659,7 @@ void Chainstate::LoadExternalBlockFile(
}
}

- Notif
...