💬 stickies-v commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253179575)
doc nit:
```suggestion
targets = [(t, {}) for t in targets] # expand to add dictionary for target-specific env variables
```
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253179575)
doc nit:
```suggestion
targets = [(t, {}) for t in targets] # expand to add dictionary for target-specific env variables
```
💬 stickies-v commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253178781)
nit: Would add a small docstring here:
```suggestion
# Instead of a single rpc target, replace it with a a target for each rpc function"""
rpc_target = "rpc"
```
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253178781)
nit: Would add a small docstring here:
```suggestion
# Instead of a single rpc target, replace it with a a target for each rpc function"""
rpc_target = "rpc"
```
💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208733)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208733)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.
💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208865)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208865)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.
💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#issuecomment-1621899833)
> Fuzzing is still a bit too magical to me to comment on whether or not this approach is the best we can take.
For reference, within the suggested solutions (either compile into a separate binary for each fuzz target, or select via env var), all should be identical in the eyes of the fuzz engine. Obviously there are other improvements that can be done, but they are orthogonal to this pull.
(https://github.com/bitcoin/bitcoin/pull/28015#issuecomment-1621899833)
> Fuzzing is still a bit too magical to me to comment on whether or not this approach is the best we can take.
For reference, within the suggested solutions (either compile into a separate binary for each fuzz target, or select via env var), all should be identical in the eyes of the fuzz engine. Obviously there are other improvements that can be done, but they are orthogonal to this pull.
💬 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
(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
(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.
(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!
(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.
(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
...
(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.
(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).
(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)
(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)
(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)
(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)
(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.
(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)
(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)
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1253253635)
Done (in other PR)