💬 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

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