Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#issuecomment-2830264751)
Rebased, fixed typo found by LLM, clarified non behavior change for `proposal` mode.

I added a `debug` argument to pass along more detailed failure reasons.

I'm a bit on the fence regarding this suggestion: https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2035713597

I also still need to look at the sigops check.
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059839540)
nit in 20a9173717b1aa0d0706894f8bda47492e1d71a9: Not that it matters, but shouldn't this commit set `self.add_wallet_options(parser, legacy=False)`?
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059892742)
in c847dee1488a294c9a9632a00ba1134b21e41947: I don't think this comment is correct. It is true that `wallet_names` are ignored in `setup_nodes`, but it is still possible to manually call `import_deterministic_coinbase_privkeys` or `init_wallet`.

My recommendation would be to remove the comment, or replace `we` with `setup_nodes`.
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059973381)
This test doesn't seem wallet related? Why is this set?
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059965247)
why is the call removed? Either the function should be removed along with the call, or the call should be kept
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059961431)
forgot to remove `enable_wallet_if_possible`?
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2060029160)
forgot to remove this?
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2059977759)
unrelated in c847dee1488a294c9a9632a00ba1134b21e41947: However, while touching, it could make sense to force named args for literal args, especially integral (boolean) ones:

```py
def __init__(self, rpc, *, cli=False):
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2060139068)
Also, you forgot to remove `REQUIRE_WALLET_TYPE_SET` in this commit.
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2060010792)
why remove bdbro and swap-bdb-endian? The options remain, so either they should be tested, or fully removed.
πŸ’¬ maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2060088046)
forgot to remove LEGACY_WALLET_MSG?
πŸ’¬ vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2059993955)
This change assumes their tip is higher than our tip. What if their tip is lower? Maybe worth advancing `pindexLastCommonBlock` in that case as well. Something like this:

```cpp
// If our tip has advanced beyond pindexLastCommonBlock, move it ahead to the tip. We don't need to
// download any blocks in between, and skipping ahead here allows us to determine nWindowEnd better.
const auto our_tip{m_chainman.ActiveTip()};
const auto peer_best_known{state->pindexBestKnownBlock};
if (our_tip-
...
πŸ’¬ vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2060107128)
nit: could use an array here:

```py
stall_blocks = [blocks[0].sha256, blocks[500].sha256]
```

which would make it possible to have the following code like:

```diff
- peers.append(node.add_outbound_p2p_connection(P2PStaller([stall_block, second_stall]), p2p_idx=id, connection_type="outbound-full-relay"))
+ peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_blocks), p2p_idx=id, connection_type="outbound-full-relay"))

- self.wait_until(lambda: sum(len(peer['inflight']) f
...
πŸ‘ vasild approved a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180#pullrequestreview-2793701909)
ACK c69ee2d5b93296d3008d6978182b2bc29bbeb457

The code changes look reasonable. I tested that `p2p_ibd_stalling.py` fails without commit 1d8504c5e6 `p2p: During block download, adjust pindexLastCommonBlock right away`. It failed 7 times and passed 1 time. Is it expected to pass sometimes?
I also checked that the code inside the modified `if`s is executed during `p2p_ibd_stalling.py`.
πŸ’¬ vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2060110804)
nit: could use a variable `second_stall_block_index = 500` or something like that to avoid repeating the number `500`.
⚠️ Pimpim11 opened an issue: "Double Poisson Sum: Enhancing Energy Distribution in Bitcoin Proof of Work with Main Formula Ξ³^(i/R) + βˆ‘(S[0:i]·τ·φ^j)"
(https://github.com/bitcoin/bitcoin/issues/32348)
Title: Double Poisson Sum: Enhancing Energy Distribution in Bitcoin Proof of Work with Main Formula Ξ³^(i/R) + βˆ‘(S[0:i]·τ·φ^j)
Author: Farrel Al Feshal
Created: April 25, 2025

Abstract
Introduces a novel mathematical framework, Double Poisson Sum, as a potential enhancement to Bitcoin’s Proof of Work (PoW) consensus mechanism. The proposed main formula, Ξ³^(i/R) + βˆ‘(S[0:i]·τ·φ^j), aims to achieve a more equitable distribution of computational energy across the network while providing robust crypt
...
βœ… fanquake closed an issue: "Double Poisson Sum: Enhancing Energy Distribution in Bitcoin Proof of Work with Main Formula Ξ³^(i/R) + βˆ‘(S[0:i]·τ·φ^j)"
(https://github.com/bitcoin/bitcoin/issues/32348)
πŸ’¬ willcl-ark commented on issue "Double Poisson Sum: Enhancing Energy Distribution in Bitcoin Proof of Work with Main Formula Ξ³^(i/R) + βˆ‘(S[0:i]·τ·φ^j)":
(https://github.com/bitcoin/bitcoin/issues/32348#issuecomment-2830289998)
For proposed protocol changes you can post to the [bitcoindev mailing list](https://groups.google.com/g/bitcoindev) or the [Delving Bitcoin](https://delvingbitcoin.org/) Discorse forum. However as this appears to be largely about "Syamailcoin" I am not sure it's particularly appropriate for there either.

For general bitcoin discussion you can try [bitcointalk](https://bitcointalk.org) or [https://reddit.com/r/bitcoin](reddit.com/r/bitcoin).
πŸ’¬ Sjors commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2060159606)
As explained in https://github.com/bitcoin/bitcoin/pull/31981#issuecomment-2830264751 this is fine. `TestBlockValidity` also calls the more thorough method.

I don't know why this check is here, perhaps just a way to quickly reject a block with lots of sigops?
πŸ“ hebasto opened a pull request: "test: Increase stack size for "Debug" builds with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32349)
This PR accommodates the deep recursion in the `FindChallenges()` function used in `test/miniscript_tests.cpp`.

Fixes https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2829441596.

CI log: https://github.com/hebasto/bitcoin/actions/runs/14664806617/job/41156972137