💬 davidrobinsonau commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1976743171)
What do you suggest as the return value for the function when never is passed? So that this can be easily checked against for rescan? it can't be 0 as "0 can be specified to scan the entire blockchain." -1, null?

static int64_t GetImportTimestamp(const UniValue& data, int64_t now)
```
{
if (data.exists("timestamp")) {
const UniValue& timestamp = data["timestamp"];
if (timestamp.isNum()) {
return timestamp.getInt<int64_t>();
} else if (timestamp.i
...

💬 Crypt-iQ commented on issue "p2p: lingering entries in `mapBlockSource`":
(https://github.com/bitcoin/bitcoin/issues/29410#issuecomment-2693020111)
I left some details out of both the original PR and this issue and I think the missing information is needed to come to a solution:
- Node A is currently at height `h` and receives a `BLOCK` message from an _honest_ peer B for a stale block at height `h`. There are a couple ways that a `BLOCK` message could be sent instead of a `CMPCTBLOCK` message. For example, the compact-block slots could all be taken by an attacker or maybe there was a [collision for short ids](https://github.com/bitcoin/bit
...

💬 rkrux commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1977046435)
Yes, I see the same issue. This is as per the code: https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L1019-L1021

Following commands work for me instead:
```
bitcoincli gettxoutsetinfo
bitcoincli gettxoutsetinfo muhash <block-height>
bitcoincli gettxoutsetinfo none <block-height>
```

👍 rkrux approved a pull request: "Add mainnet assumeutxo param at height 880,000"
(https://github.com/bitcoin/bitcoin/pull/31969#pullrequestreview-2653545385)
Concept ACK 14f16748557faf57cf4b0f4c91c162592557434c

I have not tested this because I run a pruned node and my block's prune height is more than the 880k.

In the PR description, the last command should pass `true` instead.

> Once the snapshot is loaded:
> ```
> bitcoin-cli setnetworkactive true
> ```
----------

> I'll upload a torrent later.

Any idea why don't we upload snapshots in [bitcoincore.org/bin](https://bitcoincore.org/bin/) as well?

💬 rkrux commented on pull request "rpc: add cli examples, update docs":
(https://github.com/bitcoin/bitcoin/pull/31958#discussion_r1977187442)
I tend to agree, updated.

💬 rkrux commented on pull request "rpc: add cli examples, update docs":
(https://github.com/bitcoin/bitcoin/pull/31958#issuecomment-2693800784)
Updated example output of `walletcreatefundedpsbt` RPC:

```
Examples:

Create a PSBT with automatically picked inputs that sends 0.5 BTC to an address and has a fee rate of 2 sat/vB:
> bitcoin-cli walletcreatefundedpsbt "[]" "[{\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\":0.5}]" 0 "{\"add_inputs\":true,\"fee_rate\":2}"

Create the same PSBT as the above one instead using named arguments:
> bitcoin-cli -named walletcreatefundedpsbt outputs="[{\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34huf
...

💬 mprenditore commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1977231841)
@davidrobinsonau I think both could be good, but `-1` seems the best option here

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2694007427)
Rebased to refresh the CI.

💬 Sjors commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#issuecomment-2694168087)
Concept ACK

Every major release seems plenty, same as assumevalid.

👋 saikiran57's pull request is ready for review: "Added rescan option for import descriptors"
(https://github.com/bitcoin/bitcoin/pull/31668)

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1977417409)
Updated the code

💬 Sjors commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2694189118)
Added torrent to the description. @achow101, if you agree we should host it, can you add it to bitcoincore.org and give me the link? I'll add it as a binary seed.

💬 laanwj commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977456821)
The regexp doesn't match the entire string until the end, so "0.19.0.1" and such already match. This change will only make it that "0.19.1" no longer matches.

💬 laanwj commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977459441)
We shouldn't have `|` at the end of the last clause, as this will make it match the empty string too (so effectively everything starting with `Satoshi:` matches).

💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2694290202)
Concept ACK.

💬 dergoegge commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2694345636)
Discussions to host the actual chainstate as the project itself (i.e. on bitcoincore.org) should not be had on some unrelated pull request. I think this deserves a full discussion in a separate issue or at the irc meeting.

Iirc there were discussions in the past about this and the sentiment (as I remember it) was not to host the actual snapshot.

👍 rkrux approved a pull request: "test: Fix authproxy named args debug logging"
(https://github.com/bitcoin/bitcoin/pull/31955#pullrequestreview-2654107325)
tACK fac1dd9dffba1033245c283bc0468e801c14e910

I ran the `wallet_timelock` test by passing both positional and named arguments in the first `listunspent` RPC.

```
coin_before = node.listunspent(1, maxconf=1, include_unsafe=True)
```

I can see both kinds of arguments being logged.

```
-28-> listunspent [1] {"maxconf": 1, "include_unsafe": true}
<-- [0.000553] {"jsonrpc":"2.0","result":[{"txid":"c0bbea0efd29a1c72e264efeb4eb53096741ebf3d309cabcec343b96e21a59ed","vout":0,"address":"bc
...

💬 rkrux commented on pull request "test: Fix authproxy named args debug logging":
(https://github.com/bitcoin/bitcoin/pull/31955#discussion_r1977508527)
The RPC request seems to be logged with the `AuthServiceProxy.__id_count` but the corresponding non erroneous response doesn't seem to be. @maflcko Do you know the reason for it or can we improve it later?

```
-30-> getreceivedbyaddress
<-- [0.000553] {"jsonrpc":"2.0"...
```

💬 rkrux commented on pull request "test: Fix authproxy named args debug logging":
(https://github.com/bitcoin/bitcoin/pull/31955#discussion_r1977514451)
Also it would be easier on the eyes to log the suffix `s` after the elapsed time but something maybe for another PR.

💬 rkrux commented on pull request "test: Fix authproxy named args debug logging":
(https://github.com/bitcoin/bitcoin/pull/31955#discussion_r1977504465)
Older code and doesn't need to be fixed in this PR but this line seems buggy to me, Idk how these 2 conditions can be true at the same time.

💬 Sjors commented on pull request "cmake: Revamp handling of data files":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2694402685)
re-tACK ecf54a32ed26a50e861fca559e43ec1f9dee93b7

The range-diff shows that we persevere the `DEPENDS_EXPLICIT_OPT` check added in #30911.

Checked the same thing as last time: https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2598371523

> My cmake knowledge is also limited, but I like that this removes duplication.

It still is, and this PR still does.