Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ stratospher commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1719789856)
ea7dbfb:
```suggestion
self.nodes[0].replace_in_config([("#bind", "bind="), ("#dnsseed=", "dnsseed=")])
```
πŸ’¬ stratospher commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1710072169)
d079370: nit: maybe use the term outbound connections instead of addrman connections?
πŸ’¬ Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2309672460)
Rebased after #30658.
πŸ’¬ Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1730914174)
@TheCharlatan do you want to make a separate PR for the kernel notification changes, or should I just include them here?
βœ… Sjors closed a pull request: "Move log messages: tx enqueue to mempool, allocation to blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27277)
πŸ’¬ Sjors commented on pull request "Move log messages: tx enqueue to mempool, allocation to blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27277#issuecomment-2309682650)
Let's mark this up for grabs. Though I might eventually grab it.
πŸ’¬ TheCharlatan commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1730917837)
I think it would be good to integrate them here, and I'll try and follow up with removing the `m_best_header` global from validation.
πŸ’¬ Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2309718338)
Rebased, ran `clang-format-diff` and updated includes and forward declarations based on https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1729875816

Except that I kept `#include <node/types.h>` around because with `namespace node { struct BlockCreateOptions; }` my clang 15.0.0 compiler would complain:

```
initialization of incomplete type 'const node::BlockCreateOptions'
virtual std::unique_ptr<BlockTemplate> createNewBlock(const CScript& script_pub_key, const node::BlockCr
...
πŸ’¬ Sjors commented on pull request "test: Add time-timewarp-attack boundary cases":
(https://github.com/bitcoin/bitcoin/pull/30698#discussion_r1730946634)
Let's use `block` here, since it's not actually bad.
πŸ’¬ Sjors commented on pull request "test: Add time-timewarp-attack boundary cases":
(https://github.com/bitcoin/bitcoin/pull/30698#issuecomment-2309730075)
Concept ACK
πŸ’¬ Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1730954753)
I ran into that issue with the bash script, not this PR. This PR actually fixes the problem. The bash script would first do the rollback and only then call `dumptxoutset`, which is where the check is. So it's fine now.
πŸ’¬ Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1730958148)
It can wait until some bully actually wastes $100K+ to produce such a stale block at an existing assume utxo height :-)
πŸ’¬ Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2309750697)
re-utACK ac9d53d0dee26db58ab125aa369f476c232da660
πŸ’¬ maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1730971231)
Ok, I've decided to pull in the doc update to the line of codes that I am modifying, to avoid having to touch them again. But the other refactoring and documentation will be done in the follow-up, as it needs a new commit anyway and I have a few follow-up queued up anyway.
πŸ’¬ maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2309766315)
Pushed trivial doc-only update. Should be easy to re-review.
πŸ’¬ Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2309786222)
I incorporated https://github.com/TheCharlatan/bitcoin/commit/3c665065f132fd80ea3b1733785b98e45c8b371e so the `waitTipChanged()` implementation uses the kernel signal instead of `g_best_block`. Also rebased.
πŸ’¬ Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1730997386)
Done
πŸ’¬ vasild commented on issue "-proxy does not work for addresses like 10.x.y.z":
(https://github.com/bitcoin/bitcoin/issues/25684#issuecomment-2309809495)
> Would it be enough just to document this change?

There is no central decision maker to answer that question. My advice would be to pick the solution you think is best, open a pull request and see if there will be a rough consensus around it (enough people to review and approve it and no objections).
πŸ’¬ maaku commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#issuecomment-2309834241)
@Sjors I have brought this up on the mailing list, 4 months ago: https://groups.google.com/g/bitcoindev/c/CAfm7D5ppjo/m/Jiurx8AiAgAJ
πŸ€” ismaelsadeeq reviewed a pull request: "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface"
(https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2260433636)
This force-push [diff](https://github.com/bitcoin/bitcoin/compare/245dfa20bd37f78783adbd03897b7f144d71a7b8..528d4e67e68350f451180a2356a635f67f02287f) is a complete change in approach from the previous one, which was simpler and more straightforward 245dfa20bd37f78783adbd03897b7f144d71a7b8 (i.e., locking the mutex before starting the settings update sequence). The commit was reviewed and acked by @stickies-v thanks

The new approach was written by @furszy which modifies the chain interface’s `
...