💬 furszy commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1471209737)
Done as suggested.

💬 sipa commented on pull request "net: enable v2transport by default":
(https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1916865507)
@willcl-ark I believe @mzumsande is planning to open a PR to enable it everywhere in the tests.

👍 willcl-ark approved a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1851387736)
crACK 0bef1042ce6c459acb1de965cbccd98867a417f1

👍 BrandonOdiwuor approved a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1851430061)
ACK 0bef1042ce6c459acb1de965cbccd98867a417f1

💬 glozow commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1916993905)
Let me know if I should reopen this issue?

💬 maflcko commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1916996716)
The test fails for me:

```
146/294 - p2p_v2_earlykeyresponse.py failed, Duration: 1 s

stdout:
2024-01-30T13:33:10.051000Z TestFramework (INFO): PRNG seed is: 5518845862592986813
2024-01-30T13:33:10.052000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20240130_142948/p2p_v2_earlykeyresponse_133
2024-01-30T13:33:10.370000Z TestFramework (INFO): Sending ellswift bytes in parts to ensure that response from responder is received only when
2024-01-30T13:33:10.3700
...

💬 furszy commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1471245338)
To not add `policy/policy.h` dependency, which comes with other dependencies (`script/interpreter.h`, `script/solver.h`, etc..), would suggest to make this field optional. Treating `std::nullopt` as `MAX_STANDARD_TX_WEIGHT` internally. This is what we already do for other default values.

💬 furszy commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1471333943)
@instagibbs, see https://github.com/furszy/bitcoin-core/commit/b2d3ac32cb51a1f350b5a4005e6da39d712b8fa9. It makes the process consistent, returning the same "weight exceeded" error message when such scenario occurs.

Still, I would like to revisit this PR further after https://github.com/bitcoin/bitcoin/pull/29264/commits/7bfc80b70e3955c2c3b6f820f51a7d8d45aaff8e#r1464525381 and most other comments.

💬 furszy commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1471278057)
@murchandamus, I'm not entirely sure about this naming change. At this point, we are placed at the coin selection algos level, which are situated a level below the transaction creation process. There is no concept of transaction weight here; only selection weight.

💬 furszy commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1471255355)
Same as above, the `policy.h` dependency generates some noise on me. Would suggest to not default initialize `m_max_tx_weight`. Callers are already forced to provide the weight in the constructor.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1471340914)
Oof oops :facepalm: fixed

💬 willcl-ark commented on issue "bitcoin core v.26 shuts down without warning - Doesnt save blocks downloaded":
(https://github.com/bitcoin/bitcoin/issues/29348#issuecomment-1917041373)
This sounds most likely like a hardware malfunction to me too.

In any case you could try rotating your debug log (to start a fresh one) and this time be sure to use v26.0, then upload _that_ debug log here if the problem persists? Something like:

```bash
# rotate debug log
$ mv ~/.bitcoin/debug.log ~/.bitcoin/debug.log.1

# check that you are going to run v26.0
$ bitcoind --version
Bitcoin Core version v26.0
Copyright (C) 2009-2023 The Bitcoin Core developers

# Start v26.0
$ bit
...

🤔 mzumsande reviewed a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1851551229)
Concept ACK

> @willcl-ark I believe @mzumsande is planning to open a PR to enable it everywhere in the tests.

Yes, I'll open a PR later this week (current branch https://github.com/mzumsande/bitcoin/tree/202401_bip324_alltests which still needs some cleanups)!

> If we are now defaulting v2 to enabled, do we want to enable v2transport on a few more of the p2p functional tests

It's more than adding more tests to the list, the `-v2transport` option currently doesn't make the python `P2P
...

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1917230742)
hmm weird, looking into it.

i also have a [WIP branch](https://github.com/stratospher/bitcoin/commits/more-v2-tests/) which uses this test file and adds more v2 tests for sending excess garbage bytes, wrong garbage terminator, incorrect ellswift and non-empty version packet.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1471455135)
true, will clean it up in [this branch](https://github.com/stratospher/bitcoin/commits/more-v2-tests/).

💬 virtu commented on issue "ASN-based bucketing of the network nodes":
(https://github.com/bitcoin/bitcoin/issues/16599#issuecomment-1917362538)
Seeing some of my previous work already got linked, I wanted to share an updated view on one major and one minor concern I came across during my research on ASMAP.

The major concern relates to second- and third-order effects of establishing only one outbound per AS that lead to several negative outcomes for the P2P network graph. To demonstrate the effects, consider a clearnet node opening ten outbound connections: given that Hetzner's AS comprises around 1k out of a total of 8k reachable cl
...

💬 theStack commented on pull request "rpc: Do not wait for headers inside loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29345#issuecomment-1917386986)
Concept ACK

I tend to agree that it's best to remove the waiting loop and have instant feedback about missing headers instead, as I think that avoids frustration and/or confusion for users. Curious about reasonable arguments to keep it though (maybe I'm missing something).

💬 maflcko commented on pull request "assumeutxo, rpc: Add 'start' parameter to loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28659#issuecomment-1917432586)
Are you still working on this?

💬 hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1917433706)
> Is this fixed in a later version of gcc?

After cross-compiling on Ubuntu 23.10 the issue still remains.

```
$ x86_64-w64-mingw32-g++-posix --version
x86_64-w64-mingw32-g++-posix (GCC) 12-posix
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

````

💬 theuni commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1917436655)
@achow101/ @ajtowns Looking again, I agree the order of operations here probably looks a little strange.

For context, most of this discussion came out of the discussion about porting this to CMake. We went in circles for months trying to decide how to best port this over to CMake, but we eventually got together and asked ourselves: should we even bother? Does anyone actually use this?

This PR was our way of fact-finding. It's a pretty resounding "no". The lib has been around since 2014 an
...