💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1797253502)
I get between 0 and 2 of these unsolicited blocks per day:

```
2023-11-07T02:19:56.507378Z Saw new header via unsolicited block hash=00000000000000000003310c8a9104bc38473171de3d2628ce608b7418384402 peer=0 peeraddr=x.x.x.x:8333
```

📝 kevkevinpal opened a pull request: "refactor: Miniminer package linearization followups"
(https://github.com/bitcoin/bitcoin/pull/28808)
### Motivation
In https://github.com/bitcoin/bitcoin/pull/28762 there were some post merge comments which are being addressed in this PR with the following commits

### [8d4c46f](https://github.com/kevkevinpal/bitcoin/pull/5/commits/8d4c46f54d10fb67d20d7a9a6afa37ecfd2bdc18) Reorganizing `MiniMinerMempoolEntry` to match the order we have elsewhere
* https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381775670

### [7505ec2](https://github.com/kevkevinpal/bitcoin/pull/5/commits/7505e
...

🤔 cacrowley reviewed a pull request: "bugfix: Make `CCheckQueue` RAII-styled (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/26762#pullrequestreview-1716829393)
#26762

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1384403599)
82f19c7e97281bbd66a25e53f30fe2f80a4f9d11: I found this while loading an existing wallet into a fresh (signet) node, while it was loading doing the background sync. That's not a very common scenario, so I haven't bothered looking into deeper causes...

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1384405176)
82f19c7e97281bbd66a25e53f30fe2f80a4f9d11: such a test seems hard to write without a (background sync aware) `stopatheight` RPC. So far I just tested manually that the value is correct: absent before loading a snapshot, `true` while background sync is below it, `false` once background sync passed the confirmation height.

⚠️ Sjors opened an issue: "Make -stopatheight work with background sync"
(https://github.com/bitcoin/bitcoin/issues/28809)
### Please describe the feature you'd like to see added.

When a background chainstate is present and `-stopatheight` is set below the assumeutxo height, stop when the background sync reaches that height.

### Is your feature related to a problem, if so please describe it.

Currently the node will stop as soon as the tip grows, because of `m_stop_at_height && index.nHeight >= m_stop_at_height`.

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

...

⚠️ jamesob opened an issue: "Wallet won't load in 26.x"
(https://github.com/bitcoin/bitcoin/issues/28810)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I have a wallet that (I guess) is berkeleydb format that as of 26.x (or master) refuses to load. I get the following message on `loadwallet`:
```
"Wallet file verification failed. Failed to open database path '/home/james/.bitcoin/wallet-name'. Build does not support Berkeley DB database format." code: -18
```

I suppose I somehow have to upgrade this old wallet to sqlite, but I'm no
...

✅ jamesob closed an issue: "Wallet won't load in 26.x"
(https://github.com/bitcoin/bitcoin/issues/28810)

💬 jamesob commented on issue "Wallet won't load in 26.x":
(https://github.com/bitcoin/bitcoin/issues/28810#issuecomment-1797920978)
Oops, nevermind - looks like my bdb inclusion started failing for some reason during my configure step.

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1797931541)
@Sjors Do you have any suggestion to make the code more cleaner? I think it's not a convenient way to call `txin.scriptSig.GetOp(pc, opcode, vch);` three times.

💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1384521390)
perhaps two separate lists (or a mapping to bool) could be added when it's needed, and not now that it's unused?

💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1384523820)
This could yield false positives: sometimes we need just this flags for a different reason than speeding up, right? I also don't think we can reliably expect a test code reader to figure out his answer ("why noban") is in the test framework.

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1384553675)
The word `initial` here is indeed confusing... Thinking about something happening only at node bootstrap, which is a more obvious version i think. Can we find a different word now that you're changing it to certainly not mean that?

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1384550193)
I'm wondering, does it even make sense to keep such flag at this point? What it would take to adjust all code to going back-and-forth w.r.t. superstale?

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1384547974)
ef4a1a44dca1af20adcc8fb0c14e8a31683a9d99

nit: this enum could be better... First, `StalePast` also qualifies as `Stale` (just english-wise). Second, it's a negation (non-synced).

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798060231)
> I would expect that a wallet that connects to us over (untrusted) p2p will get the exact same info regardless of background validation progress.

assumeutxo allows blocks and the mempool to be based on top of the assumed utxo set, so transactions from those blocks and that mempool can be sent on p2p. I am pretty sure that this mempool does not have "the exact same info regardless of background validation progress."

🤔 maflcko reviewed a pull request: "rpc: Add script verification flags to getdeploymentinfo"
(https://github.com/bitcoin/bitcoin/pull/28806#pullrequestreview-1717157751)
left some nits/questions. Feel free to ignore.

💬 maflcko commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1384586970)
```suggestion
#define FLAG_NAME(flag) {std::string{#flag}, uint32_t{SCRIPT_VERIFY_##flag}}
```

No need to cast, they are already unsigned, see fa621ededdf. Also, could omit the comma for clarity and to allow the use of clang-format on your code?

💬 maflcko commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1384597041)
unrelated nit:

```
deploymentinfo.h should add these lines:
#include <cassert> // for assert
#include <cstdint> // for uint32_t
#include <string_view> // for string_view
```

```

💬 maflcko commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1384589476)
Is this ever hit? If not, maybe it can be removed, as the remaining code should reach the same conclusion anyway in the rare case that this is hit?