💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2325028945)
Switched back to a native approach in `cmake` without `python` or `xxd`.

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741105265)
now that we have two clocks here, this doesn't seem to work anymore (this seems to be the build failure's reason). Do we really need the `- nNow` part?
```suggestion
int64_t{Ticks<std::chrono::microseconds>(SteadyClock::now())},
```
or if we do:
```suggestion
int64_t{Ticks<std::chrono::microseconds>(SteadyClock::now()) - Ticks<std::chrono::microseconds>(nNow)},
```

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741105267)
I'm not touching any of those lines in this PR so I'd rather keep this PR a bit narrower.

```diff
- BOOST_CHECK_EQUAL((R1L & arith_uint256{0xffffffffffffffff}), arith_uint256{R1LLow64});
+ BOOST_CHECK_EQUAL(R1L & arith_uint256{0xffffffffffffffff}, arith_uint256{R1LLow64});
```

Thanks, I'll patch this if I have to force-push.

(note: you can hide long code blocks with a [`<details>...</details>` tag](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-f
...

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741107409)
Using SteadyClock in https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741105265 seems to have stabilized the tests.
Approach ACK

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741109081)
if you decide to keep this structure instead of the exhaustive one:
```suggestion
// Time to periodically flush is between 50 and 70 minutes (inclusive)
```

📝 hebasto converted_to_draft a pull request: "build: Use correct variable name"
(https://github.com/bitcoin/bitcoin/pull/30791)
This was overlooked after https://github.com/bitcoin-core/secp256k1/pull/1546.

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741110745)
if you decide to keep this structure, please add boundary test for `50min` turning `m_did_flush` to true.
Would also cover `+ 71min` for the same reason.

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741111508)
I don't think it would, because it's random.

💬 maflcko 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-2325061180)
> Regtest

Haven't followed any of this in detail, but on regtest, modifying the consensus rule on the command line should be harmless and is already being done to closer mirror main-net, where the defaults don't achieve it. It should be easy to apply in this case as well, if there is need to.

💬 l0rinc commented on pull request "refactor: Extend CScript with `operator<<` for `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1741115644)
I'll investigate these tomorrow, thanks for the comments!

💬 l0rinc commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1741116936)
> you can hide long code blocks

Ah, I don't see them anymore, `Refined Github` extension does it automatically. I'll do it next time.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2325087575)
`31d841ddb8...08c9a8254a`: rebase to pick CMake

💬 ryanofsky commented on pull request "refactor: Extend CScript with `operator<<` for `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1741129350)
> Would it make sense to drop operator<< from CScript, and instead have explicit "Append" and "Push" member functions (that take spans), and do the expected thing?

I think it makes sense, especially because cscripts are serializable, so it is confusing that `x << script` does an entirely different operation than `script << x`. But making this change would expand the scope of the PR beyond what it is currently doing which is just making cscript work a little better with hex literals from #3037
...

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2325113562)
reACK 100e1b9ecb2 via range-diff

⚠️ fanquake opened an issue: "ci: failure in win64 unit tests"
(https://github.com/bitcoin/bitcoin/issues/30792)
Seen in https://cirrus-ci.com/task/6387064294342656?logs=ci#L2492:
```bash
127/135 Test #129: spend_tests .......................... Passed 9.85 sec
128/135 Test #135: db_tests ............................. Passed 3.32 sec
129/135 Test #1: util_test_runner .....................***Failed 154.46 sec
2024-09-02 15:49:05,155 - ERROR - Error parsing command output as hex: non-hexadecimal number found in fromhex() arg at position 1
2024-09-02 15:49:30,298 - ERROR - FAILED_TESTCASES:
...

✅ fjahr closed an issue: "Embedded ASMap data Tracking Issue"
(https://github.com/bitcoin/bitcoin/issues/28794)

💬 fjahr commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2325120705)
This issue hasn't seen much love but that's because there isn't much else to do by now in my mind, aside from #28792 and also most of the interesting action on ASMapt takes place outside of this repo, like in https://github.com/fjahr/asmap-data/ and https://github.com/fjahr/kartograf. So I am going to close this for now, the focus is on #28792 which should hopefully let us take the next step in adoption of this feature.

If we end up needing to put more work into ASMap than expected, I am happ
...

💬 pablomartin4btc commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1741151732)
I do agree unless there's another use case that invalidates it.

💬 MarcusMWilliams commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2325156600)
1-225-316-0850

On Mon, Sep 2, 2024, 12:47 PM pablomartin4btc ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/rpc/blockchain.cpp
> <https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1741151732>:
>
> > + // data is available before starting to roll back.
> + if (node.chainman->m_blockman.IsPruneMode()) {
> + LOCK(node.chainman->GetMutex());
> + const CBlockIndex* current_tip{n
...

💬 pablomartin4btc commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2325158560)
> > As @Sjors mentioned above please consider adding -rpcclienttimeout=0 to the calls also in the documentation. Furthermore, these example outputs could be useful there too.
>
> Can you say which examples you mean? I added it to the RPC help examples in my last push but I guess you might be referring to the README examples where I didn't do it yet. Or is there another place I am still missing?

Thanks! I've checked the places where you added the `-rpcclienttimeout=0`, also it could be adde
...