💬 ismaelsadeeq commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1253237996)
Thank you @Xekyo

🤔 Xekyo reviewed a pull request: "fuzz: improve `coinselection`"
(https://github.com/bitcoin/bitcoin/pull/27585#pullrequestreview-1514739299)
reACK bd6315a2a75d2891a8d3476c492d6b53309b7296

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1253248607)
I've added a FuzzResult enum as suggested by @MarcoFalke, and added some explanation there.

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1253249225)
Done!

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#issuecomment-1621949444)
Rebased and switched from `bool` to an `enum class FuzzResult` which has values `MAYBE_INTERESTING` and `UNINTERESTING`, making it hopefully clearer what the return values correspond to.

📝 glozow opened a pull request: "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module"
(https://github.com/bitcoin/bitcoin/pull/28031)
Milestone 1 of package relay p2p changes. See #27463 for full project tracking.

Please see #27742 for how this PR fits into the big picture. I strongly suggest that reviewers look at that PR first to decide if they are comfortable with the overall approach.

This PR is mainly refactors, with a few behavior changes and improvements:
- Introduces `TxPackageTracker`, which starts out as a wrapper for the `TxOrphanage`. This will be the main "package relay" module as well.
- Adds a new log ca
...

🤔 glozow reviewed a pull request: "[NO MERGE] BIP331 Ancestor Package Relay"
(https://github.com/bitcoin/bitcoin/pull/27742#pullrequestreview-1514439276)
Opened #28031 for milestone 1, addressing the comments here pertaining to those commits.

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253067629)
As in `Announcement::m_is_wtxid`, sorry. Clarified (in other PR).

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253060069)
Done (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253138929)
Changed/clarified (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253139157)
Added param comment (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253068219)
Removed (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253066804)
Ah I mean of the `GenTxid`. Clarified comment (in other PR), thanks.

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253243468)
I've squashed the commits so that it's more clear that this was copied to TxPackageTracker (other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253253635)
Done (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253252501)
Done (in other PR)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253290488)
Fixed here

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253291132)
Done (in other PR)

💬 ajtowns commented on pull request "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime":
(https://github.com/bitcoin/bitcoin/pull/27427#discussion_r1253319265)
I'm not sure what I was trying to suggest in the comment above: having `BeginTime()` return a height doesn't make sense. Maybe I confused myself by using `N` as a block height when the PR at the time was already time based?

I think `BeginTime() { return MinBIP9WarningTime > nPowTargetTimespan ? MinBIP9WarningTime - nPowTargetTimespan : MinBIP9WarningTime; }` might make reasonable sense, with the idea being to check that there aren't any warning reported, then set `MinBIP9WarningTime` to a tim
...

🤔 mzumsande reviewed a pull request: "test: bugfix, synchronize indexes synchronously"
(https://github.com/bitcoin/bitcoin/pull/28026#pullrequestreview-1514862606)
> (An alternative, functionally equivalent to this, might be to just make the timeout infinite, see https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007, but I think this is fine as well.)

I think I'd prefer that option because then we wouldn't need to add a test-only arg to `BaseIndex::Start`, plus having the same thread structure as in production seems more natural and more robust with respect to possible future changes of the init sequence and unit tests.