💬 Sjors commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1797121223)
Rebased to fix those two new typos.

💬 Sjors commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1384244572)
`redeemScrtipt` is a typo

💬 Sjors commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1797145061)
Fixing the test is hopefully a matter of just adding the new field(s) to these tests.

Also make sure to [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits).

🤔 mzumsande reviewed a pull request: "Test: followups to #27823"
(https://github.com/bitcoin/bitcoin/pull/28612#pullrequestreview-1716628073)
I ran this locally a couple 100 times and the test failed for me maybe 1% of the time - haven't done any deeper analysis though.

👍 pablomartin4btc approved a pull request: "Fix typos"
(https://github.com/bitcoin/bitcoin/pull/28605#pullrequestreview-1716628156)
re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4

💬 mzumsande commented on issue "getaddrmaninfo RPC: add Transport v1/v2 to `tried` for ipv4 & ipv6":
(https://github.com/bitcoin/bitcoin/issues/28807#issuecomment-1797178738)
I think that this is not so straightforward because we don't save historic connection details in addrman (and I'm not sure if we want to start doing that). What we do save is the service flags , which could be dumped with the debug-only `getrawaddrman` rpc.

However, a BIP324 service flag for an addr in `tried` does not necessarily indicate that we connected to them via v2. The service flag may just be wrong (and we connected to them but downgraded the conection to v1), or maybe it is true now
...

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