📝 MarcoFalke opened a pull request: "test: Restore unlimited timeout in IndexWaitSynced"
(https://github.com/bitcoin/bitcoin/pull/28036)
The timeout was unlimited before, so just restore that value for now: https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007

👍 fanquake approved a pull request: "ci: Print full lscpu output"
(https://github.com/bitcoin/bitcoin/pull/28034#pullrequestreview-1516419650)
ACK fa956d20480b21b0f348b83dd451667620010d8e

https://cirrus-ci.com/task/6617265752244224?logs=ci#L180
```bash
+ lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Address sizes: 48 bits physical, 48 bits virtual
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Vendor ID: AuthenticAMD
Model name: AMD EPYC 7B13
...

🚀 fanquake merged a pull request: "ci: Print full lscpu output"
(https://github.com/bitcoin/bitcoin/pull/28034)

💬 MarcoFalke commented on pull request "test: bugfix, synchronize indexes synchronously":
(https://github.com/bitcoin/bitcoin/pull/28026#issuecomment-1623588603)
Went ahead and created the alternative: https://github.com/bitcoin/bitcoin/pull/28036, if reviewers want it. Happy to close it, if this is merged in the meantime.

💬 josibake commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#issuecomment-1623602185)
> > Adds support to the Bitcoin Core wallet for receiving silent payments
>
> Prefer to leave this for a second PR

Can you explain your reasoning for wanting to split it up? I'd prefer to keep them together only because I see no reason to merge sending without receiving support in Bitcoin Core. In terms of work, if the goal is to merge both sending and receiving support, the amount of review doesn't change, and splitting into two PRs seems like unnecessary shuffling of commits

💬 ishaanam commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254378046)
In that test don't all of the pre-selected inputs belong to the wallet?

💬 ismaelsadeeq commented on issue "EstimateMedianVal returns higher fee for higher confTarget":
(https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1623679114)
I try recreating this issue on master HEAD bc4f6b13feb29146b7e10e86f93dc7f6fb6937f2
with the same command `test/functional/feature_fee_estimation.py --randomseed 108574360997204915`

The tests passed.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1254444996)
> sending the TX without doing INV/GETDATA first risks having the TX be ignored as unrequested

I thought about doing `INV` first and waiting for get data but decided to not do that for the following reasons:

1. Bitcoin Core accepts unsolicited transactions. According to https://developer.bitcoin.org/reference/p2p_networking.html#tx there is already some software that sends unsolicited transactions.
2. It would add complexity to the implementation.
3. After `INV` we would have to wait for
...

💬 MarcoFalke commented on issue "EstimateMedianVal returns higher fee for higher confTarget":
(https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1623698708)
@ismaelsadeeq Can you try with the commit specified in OP as well? If it fails, can you try bisecting?

🤔 furszy reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1516581154)
Updated per feedback, [small diff](https://github.com/bitcoin/bitcoin/compare/94c9b1f37e335c43c739b853bb9457737b67d73a..30b2511f39d32e29f9f05859aa8a97b84c22376b). Thanks ryanofsky!

Changes:
* Documented behavior change when tip has no data in 7944974 commit description.
* Removed the introduced `pruneblockchain` "nothing to prune" error.
* Made `getblockchaininfo` "pruneheight" result consistent with the RPC documentation.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254460179)
> I would probably update the code rather than the documentation, since that seems simpler, and this case should only happen when there's an assumeutxo snapshot so there shouldn't be a backwards compatibility concern.

Wouldn't be odd for the user to receive the height of a block that they do not have? (tip + 1).

Isn't really a big deal anyway, this is an edge case and we can stick to the current RPC docs. But maybe would be good to discuss further what "pruneheight" result should be in a f
...

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254464720)
done 👍🏼.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254462432)
done, extended the commit description.

💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254477566)
Pushed the suggested code changes.

🤔 furszy reviewed a pull request: "test: Restore unlimited timeout in IndexWaitSynced"
(https://github.com/bitcoin/bitcoin/pull/28036#pullrequestreview-1516637883)
ACK fabed7eb

I'm a bit more inclined for #28026 but it is fine either way.

Note:
Would be good to mention in the PR description that this is a behavior change for the blockfilterindex and txindex tests. It only restores the coinstatsindex behavior.

💬 MarcoFalke commented on pull request "test: Restore unlimited timeout in IndexWaitSynced":
(https://github.com/bitcoin/bitcoin/pull/28036#issuecomment-1623750494)
Thx, edited OP

💬 ajtowns commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1254506653)
> I don't think so.

You're right -- I missed seeing the call to `ProcessGetData` and assumed it wasn't picked up until the next `ProcessMessages`.

💬 furszy commented on pull request "test: Restore unlimited timeout in IndexWaitSynced":
(https://github.com/bitcoin/bitcoin/pull/28036#issuecomment-1623765081)
Little note: I think that the only downside here with respect to increasing the timeout to a much higher number or #28026 is that the tests could run forever if the thread never finishes. I'm not sure how the CI will behave in this scenario, guess that we will realize that something like this happened just by reading the last executed test name.

💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1623767306)
> What is (was?) the reason for [#27213 (comment)](https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1611194968)?

The unsigned integer flow was caused by https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241362625 (fixed now!) - the counter wasn't incremented for new `pszDest` connections, but still decremented on disconnection, thus the overflow.

💬 MarcoFalke commented on pull request "test: Restore unlimited timeout in IndexWaitSynced":
(https://github.com/bitcoin/bitcoin/pull/28036#issuecomment-1623778841)
We've been plagued by intermittent timeouts locally (for months/years) and now in CI (for days), but never a thread that never finished. So I think it should be evident that this or #https://github.com/bitcoin/bitcoin/pull/28026 should be preferred. After all, it is no different than any other deadlock or infinite loop anywhere else in the code.