💬 fjahr commented on pull request "test: p2p: check that connecting to ourself leads to disconnect":
(https://github.com/bitcoin/bitcoin/pull/30362#discussion_r1659388331)
nit: Could pull this up two lines and use it to check the exact log line I think, but not a blocker for me

💬 ajtowns commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2197781492)
I think both entries should continue to include examples.

I think removing "severe" from the LogWarning description doesn't make sense -- anything the admin might have to address is a severe (potential) problem, isn't it?

Having LogError mean "the node admin **will** need to address" vs LogWarning mean "the node admin **may** need to address" seems to argue towards retaining the "LogError is fatal" interpretation to me -- fatal errors are ones the admin does need address; anything else the
...

💬 ajtowns commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197782050)
> > Touching every single line of logging code for a "nice to have" doesn't make much sense to me.
>
> AJ I know we are disagreeing about a lot of things, and I do not fault you in any way for this, but your reactions seem very strong relative the actual scope of changes we would like to make.

I don't know what you want me to say here? I didn't say "THAT'S CRAZY YOU SHOULD ALL BE PUT IN AN ASYLUM"; I said "it doesn't make much sense to me". The only way to be less strong about my opinion i
...

💬 theStack commented on pull request "test: p2p: check that connecting to ourself leads to disconnect":
(https://github.com/bitcoin/bitcoin/pull/30362#discussion_r1659421007)
Tried that initially, but the log line shows not the destination, but the source address of the connection, which contains an ephemeral port (i.e. randomly assigned by the OS) and hence can't be predicted.

💬 djkarmi commented on issue "Slow sync ":
(https://github.com/bitcoin/bitcoin/issues/30360#issuecomment-2197790240)
So what should I do now?

💬 mzumsande commented on issue "Slow sync ":
(https://github.com/bitcoin/bitcoin/issues/30360#issuecomment-2197795804)
you could just restart the node and see whether the problem persists. If it's the problem I mentioned above, you should connect to new peers and the problem should go away. If it persists, it's probably another problem, e.g. networking issues.

💬 kevkevinpal commented on pull request "test: p2p: check that connecting to ourself leads to disconnect":
(https://github.com/bitcoin/bitcoin/pull/30362#issuecomment-2197886240)
tACK [5d2fb14](https://github.com/bitcoin/bitcoin/pull/30362/commits/5d2fb14bafe4e80c0a482d99e5ebde07c477f000)

looks good to me and I ran it locally on my machine and looks to work well

💬 azazar commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/29398#issuecomment-2198052531)
I didn't find a way to reproduce it after the restart.

💬 djkarmi commented on issue "Slow sync ":
(https://github.com/bitcoin/bitcoin/issues/30360#issuecomment-2198074781)
When using the 32 bit version there was no such problem with synchronization.

💬 alfonsoromanz commented on pull request "Assumeutxo: bugfix on loadtxoutset with a divergent chain + tests":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2198179328)
Thanks @mzumsande. I have reverted the change regarding the commit order and have also addressed the feedback from @ryanofsky by calling `LastCommonAncestor()` before `TryDownloadingHistoricalBlocks()`.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659831027)
There's no accessor to `m_prev` though. This is checked below in the random deletion test.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659831168)
Hmm well the `AddFlags` function is in production code, so this is constructing the linked list via production code. The `std::list` we create is a test harness helper that we use to check that our linked list behaves the same.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659835029)
It just checks that removing the last element sets head to `nullptr`. I suppose just for completeness, since this case is the same as when we delete `n4` above.

👍 tdb3 approved a pull request: "test: p2p: check that connecting to ourself leads to disconnect"
(https://github.com/bitcoin/bitcoin/pull/30362#pullrequestreview-2149639061)
ACK 5d2fb14bafe4e80c0a482d99e5ebde07c477f000
Good addition.
Tested locally with the following:
1) Running the p2p_handshake test variants with `test_runner` (passed)
2) Running a purposefully separate node (`src/bitcoind -regtest -bind=127.0.0.1:1234`), and modifying `p2p_handshake` to run the test against it (`node_listen_addr = f"127.0.0.1:{1234}"`). Test failed as expected (expected messages weren't found).
3) Stopping the node at 1234 and running the modified test. Test failed as expe
...

💬 tdb3 commented on pull request "test: p2p: check that connecting to ourself leads to disconnect":
(https://github.com/bitcoin/bitcoin/pull/30362#discussion_r1659838824)
I like how this is using `self.options.v2transport` as the argument, enabling the script list in `test_runner` to specify whether or not v2transport should be used for the test.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659847033)
This checks that the linked list nodes get removed from the linked list due to the list iterator deleting them, not due to clearing the flags in `Next`.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659852814)
Done

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659852854)
Done

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659853094)
Done.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659853149)
Updated the comment.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659853210)
Done.