💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901906865)
deleted
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901906865)
deleted
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901895281)
The conditions aren't exactly the same since `AddAnnouncer` also returns false if we already HaveTxFromPeer. We'd need to make the return result tri-state.
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901895281)
The conditions aren't exactly the same since `AddAnnouncer` also returns false if we already HaveTxFromPeer. We'd need to make the return result tri-state.
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901883025)
I've moved that test to its own commit
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901883025)
I've moved that test to its own commit
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887146)
good point, added
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887146)
good point, added
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901867183)
removed
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901867183)
removed
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887320)
deleted now that parent_txids is gone
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887320)
deleted now that parent_txids is gone
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901905478)
Fwiw makes sense, but this function doesn't exist anymore, so marking resolved
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901905478)
Fwiw makes sense, but this function doesn't exist anymore, so marking resolved
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901882259)
Done
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901882259)
Done
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901891303)
added
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901891303)
added
💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#issuecomment-2569511364)
Thanks for the reviews @mzumsande @instagibbs! Just got back from holiday, addressed all the comments. Biggest change was recalculating missing parents each time we add a new orphan reso candidate. I also wrote a test for https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1881018172
(https://github.com/bitcoin/bitcoin/pull/31397#issuecomment-2569511364)
Thanks for the reviews @mzumsande @instagibbs! Just got back from holiday, addressed all the comments. Biggest change was recalculating missing parents each time we add a new orphan reso candidate. I also wrote a test for https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1881018172
💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901970599)
> Is it really a histogram at this point or just a vector of fee rates/fracs?
It is a vector of package fee frac's, which can be used to build a histogram.
> Might be helpful to add a comment here telling that the values in this vector are sorted. That's what I assumed while going through the `CalculatePercentiles` function here.
Yes, it is sorted by the order in which the packages are selected in the block template. I believe this is obvious because we are emplacing the fee fr
...
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901970599)
> Is it really a histogram at this point or just a vector of fee rates/fracs?
It is a vector of package fee frac's, which can be used to build a histogram.
> Might be helpful to add a comment here telling that the values in this vector are sorted. That's what I assumed while going through the `CalculatePercentiles` function here.
Yes, it is sorted by the order in which the packages are selected in the block template. I believe this is obvious because we are emplacing the fee fr
...
✅ achow101 closed a pull request: "[28.x] Finalize 28.1"
(https://github.com/bitcoin/bitcoin/pull/31582)
(https://github.com/bitcoin/bitcoin/pull/31582)
💬 achow101 commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2569523096)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/31594
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2569523096)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/31594
💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2569523210)
> Nit: In the PR description, a draft PR is mentioned as the project. Can add this link for project issue: #30392
Added, and addressed your comments!
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2569523210)
> Nit: In the PR description, a draft PR is mentioned as the project. Can add this link for project issue: #30392
Added, and addressed your comments!
💬 achow101 commented on pull request "[28.x] 28.1 backports (final?)":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2569535615)
ACK 1451c56cce3e3df784e1dac969ffb0165df313c7
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2569535615)
ACK 1451c56cce3e3df784e1dac969ffb0165df313c7
💬 l0rinc commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901981925)
```suggestion
* treated as little-endian numbers and sorted in ascending numeric order.
```
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901981925)
```suggestion
* treated as little-endian numbers and sorted in ascending numeric order.
```
💬 remcoros commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901988639)
'can be used to used to display' -> 'can be used to display'
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901988639)
'can be used to used to display' -> 'can be used to display'
💬 l0rinc commented on pull request "refactor: Allow std::byte in (Read/Write)(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1901989166)
I just haven't really seen this being used in the project, but I don't mind if we do.
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1901989166)
I just haven't really seen this being used in the project, but I don't mind if we do.
💬 edilmedeiros commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1901995035)
Agree with this, even though I have never encountered any problems with the wallet files on a Mac with a FAT formatted disk, only on the blocks dir.
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1901995035)
Agree with this, even though I have never encountered any problems with the wallet files on a Mac with a FAT formatted disk, only on the blocks dir.
💬 l0rinc commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901996392)
This was already fixed [`13464c0c44645e0ed88d9d72c3ad697dca800e10..3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40`#diff-800776e2dd](https://github.com/bitcoin/bitcoin/compare/13464c0c44645e0ed88d9d72c3ad697dca800e10..3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40#diff-800776e2dda39116e889839f69409571a5d397de048a141da7e4003bc099e3e2)L88
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901996392)
This was already fixed [`13464c0c44645e0ed88d9d72c3ad697dca800e10..3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40`#diff-800776e2dd](https://github.com/bitcoin/bitcoin/compare/13464c0c44645e0ed88d9d72c3ad697dca800e10..3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40#diff-800776e2dda39116e889839f69409571a5d397de048a141da7e4003bc099e3e2)L88