Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 maflcko commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#issuecomment-2309536360)
ACK e1d5dd732d5dc641faf1dde316275c84b6bb224b
💬 TheCharlatan commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#issuecomment-2309536891)
Concept ACK, but needs rebase and the title and description should be updated, since this is no longer replacing the height, but is instead adding logic to the start time of a period.
📝 hebasto opened a pull request: "qt: 28.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/30715)
The 28.x branching off is [scheduled](https://github.com/bitcoin/bitcoin/issues/29891) for today, so it's [time](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off) to fetch the recent translations from [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool.

A similar PR from the previous release
...
💬 hebasto commented on pull request "qt: 28.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/30715#issuecomment-2309568245)
cc @stickies-v @pablomartin4btc @jarolrod
💬 Sjors 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-2309596835)
@maaku this would be better to discuss on the mailinglist and/or Delving.

There's ongoing discussion what addition / other mitigation is needed to handle another variant of the timewarp that is currently not fixed: https://delvingbitcoin.org/t/zawy-s-alternating-timestamp-attack/1062

Maybe that can be combined with what you have in mind.

We can always launch a testnet5 with completely different rules.
👍 theStack approved a pull request: "test: XORed blocks test follow up"
(https://github.com/bitcoin/bitcoin/pull/30669#pullrequestreview-2260068418)
re-ACK e1d5dd732d5dc641faf1dde316275c84b6bb224b
🤔 stratospher reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2228592166)
Concept ACK. nice to have more consistent behaviour for what to expect with the negated version of the config flags compared to on master.
💬 stratospher commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1730774338)
d079370: (feel free to ignore/not a blocker since `noconnect=0` isn't recommended)

would `IsArgSet` be simpler compared to `GetArgs()/IsArgNegated()` here? or is something else meant by preferring `GetArgs()/IsArgNegated()` in most cases (but not all) over `IsArgSet`.


i just found this behaviour of `noconnect` confusing - even though both do the same thing, `GetArgs()` returns empty in one case and not empty in the other case. (and similarly for `IsArgNegated`)

- when `noconnect=1`

...
💬 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