💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1900772624)
Updated 492ff2c504d1da3f4ae8704a5485a286ee76f0d9 -> f442a3a5b2a0a58bc263fbb9c87e8e4715de103a ([noGlobalSignals_6](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_6) -> [noGlobalSignals_7](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_6..noGlobalSignals_7))

* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1459116286), using appropriate variable f
...

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1900781426)
CI is green btw :green_circle:

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459376293)
as much a sense as "1". Could have minimal relay size here instead?

🤔 glozow reviewed a pull request: "refactor: remove CTxMemPool::queryHashes()"
(https://github.com/bitcoin/bitcoin/pull/29260#pullrequestreview-1833296999)
ACK 282b12ddb0aeb1f0991dd9f45c6b5c5c079652ec. Looks like there's no conflicts.

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459420366)
should be mitigated?

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459420447)
done

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459420556)
done

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459420625)
taken

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459420739)
did the minimal suggested work to give knapsack a fighting chance

💬 murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459477415)
At second glance, lemme retract my comment. It doesn’t really hurt to allow values down to 0, even if that is less than a sensible use of that parameter. Might be helpful in tests to be able to restrict it fairly low.

💬 mzumsande commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1900883607)
> I do not fully get it. Can somebody elaborate what is that? With an example of what could go wrong if this "safety buffer" is ignored.

I think one reason is that the estimation in `IsInitialBlockDownload()` (or now `ApproximateBestBlockDepth()`) is not precise because block header timestamps could be wrong (within limits).
So if we used something very close to 2 days, we could think we are just out of IBD and connect to lots of `NODE_NETWORK_LIMITED` peers, which would not serve us the nex
...

🤔 murchandamus reviewed a pull request: "Add max_tx_weight to transaction funding options"
(https://github.com/bitcoin/bitcoin/pull/29264#pullrequestreview-1833400688)
ACK 069c1652ed22b5e22537f63946cedd6c68487b22

Thanks for cleanly distinguishing `max_tx_weight` and `max_input_weight`!

💬 murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459483691)
Ah, yeah `max_input_weight` is a much better name. Thanks!

💬 murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459539171)
I don’t think this condition can be hit.

AFAICT, there is only be a single scenario in which `vfBest` can contain an input selection that is in excess of the `max_input_weight`: `ApproximateBestSubset` initializes `vfBest` with all of `applicable_groups` without checking the weight. If it then never finds a better input selection throughout the exploration, it would be possible that it returns an overweight `vfBest`. Since `lowest_larger` is only set when it adheres to the weight limit, we e
...

💬 murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459526141)
I don’t think we can ever hit this `if` block since we only set `lowest_larger` when it’s weight is less than the max_input_weight.

🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1833561066)
I implemented @mzumsande's proposed changes because they maintain the existing workflow and document the previously obscure behavior. The objective of this PR is to fix the post-IBD stalling scenario. It would be good to leave the discussion about the "safety buffer" number for a follow-up.

That being said, I agree with @vasild that the number is quite over-conservative for no apparent reason and could be increased.

> 144 blocks may still be very conservative / excessive, but it doesn't se
...

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665430)
makes sense, removed

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665529)
makes sense, removed

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#issuecomment-1901026758)
fixed up the fuzz test, which was attempting larger than `MAX_STANDARD_TX_WEIGHT` coins and not actually rejecting them as they should have previously, causing fuzz failure. Invariants in test should work now.

📝 hebasto opened a pull request: "Avoid non-self-contained Windows header"
(https://github.com/bitcoin-core/gui/pull/789)
Using the `windows.h` header guarantees correctness regardless of the content of other headers.

For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio

Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager.

Related to https://github.com/hebasto/bitcoin/pull/77.