💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785897)
Done.
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785897)
Done.
💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785977)
Done.
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785977)
Done.
💬 theStack commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786034)
In Rust they call it [`VecDeque`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html), maybe that's a naming option? (It even rhymes!)
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786034)
In Rust they call it [`VecDeque`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html), maybe that's a naming option? (It even rhymes!)
💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786106)
Done.
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786106)
Done.
💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786217)
Done (and more).
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786217)
Done (and more).
💬 BenWestgate commented on issue "Welcome screen uses GiB values as GB, risking running out of space":
(https://github.com/bitcoin-core/gui/issues/821#issuecomment-2130014340)
On further analysis, the code comments in chainparams.h also think these values should be GB so I understand the origin of the gui error. It seems the release-process.md is the odd one out of the 3. This issue can be closed if the doc: PR below is ACK'd.
https://github.com/bitcoin/bitcoin/pull/30171
(https://github.com/bitcoin-core/gui/issues/821#issuecomment-2130014340)
On further analysis, the code comments in chainparams.h also think these values should be GB so I understand the origin of the gui error. It seems the release-process.md is the odd one out of the 3. This issue can be closed if the doc: PR below is ACK'd.
https://github.com/bitcoin/bitcoin/pull/30171
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2130020267)
[6430bea ](https://github.com/bitcoin/bitcoin/commit/6430bea749d2c5832faeae12e69fe078680e01d3)to [c97bd16](https://github.com/bitcoin/bitcoin/commit/c97bd161212d56c167d3552da3b13db5ab2bc75e):
Addressed feedback by @sr-gi, thanks!
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2130020267)
[6430bea ](https://github.com/bitcoin/bitcoin/commit/6430bea749d2c5832faeae12e69fe078680e01d3)to [c97bd16](https://github.com/bitcoin/bitcoin/commit/c97bd161212d56c167d3552da3b13db5ab2bc75e):
Addressed feedback by @sr-gi, thanks!
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2130021361)
1cbbcba42cf1c050022a73cb02b9d385fa184f2e: squashed all fixups to clean up the list of commits, no other changes
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2130021361)
1cbbcba42cf1c050022a73cb02b9d385fa184f2e: squashed all fixups to clean up the list of commits, no other changes
🚀 achow101 merged a pull request: "fuzz: Fix wallet_bdb_parser stdlib error matching"
(https://github.com/bitcoin/bitcoin/pull/30169)
(https://github.com/bitcoin/bitcoin/pull/30169)
📝 achow101 opened a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172)
Adds error messages that were not being handled. Also removes error messages that no longer exist.
Fixes #30166
(https://github.com/bitcoin/bitcoin/pull/30172)
Adds error messages that were not being handled. Also removes error messages that no longer exist.
Fixes #30166
💬 epiccurious commented on pull request "doc: Change GiB to GB in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130032118)
Concept ACK c04edce83db41cfce7a0a2f3418cf830c2e8afb5.
This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130032118)
Concept ACK c04edce83db41cfce7a0a2f3418cf830c2e8afb5.
This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?
💬 epiccurious commented on pull request "refactor: Use type-safe time in txorphanage":
(https://github.com/bitcoin/bitcoin/pull/30170#issuecomment-2130040435)
utACK fa6d4891c7815025ab1cd58d0906466af31bb648.
(https://github.com/bitcoin/bitcoin/pull/30170#issuecomment-2130040435)
utACK fa6d4891c7815025ab1cd58d0906466af31bb648.
💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613810244)
Renamed!
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613810244)
Renamed!
💬 BenWestgate commented on pull request "doc: Change GiB to GB in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130058903)
> Concept ACK [c04edce](https://github.com/bitcoin/bitcoin/commit/c04edce83db41cfce7a0a2f3418cf830c2e8afb5).
>
> This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?
Software usually displays large sizes in GB because that's what the operating system and storage manufacturers use.
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130058903)
> Concept ACK [c04edce](https://github.com/bitcoin/bitcoin/commit/c04edce83db41cfce7a0a2f3418cf830c2e8afb5).
>
> This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?
Software usually displays large sizes in GB because that's what the operating system and storage manufacturers use.
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613803860)
My comment is just about the commit message for this commit, not this line of code (sorry):
> Relax assumptions aabout in-mempool children of in-mempool
"aabout" --> "about"
> TxA (in mempool) <- TxB' (in package, conflicts with TxB) <-
> TxD (in package)
s/TxD/TxC/ ?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613803860)
My comment is just about the commit message for this commit, not this line of code (sorry):
> Relax assumptions aabout in-mempool children of in-mempool
"aabout" --> "about"
> TxA (in mempool) <- TxB' (in package, conflicts with TxB) <-
> TxD (in package)
s/TxD/TxC/ ?
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613810585)
Seems weird to loop through all the workspaces, when `GetEntriesForConflicts` is doing all the work that needs to be done on the first invocation? Maybe just invoke once with the child's txid, as is done further down?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613810585)
Seems weird to loop through all the workspaces, when `GetEntriesForConflicts` is doing all the work that needs to be done on the first invocation? Maybe just invoke once with the child's txid, as is done further down?
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613820116)
s/V3 transactions/TRUC transactions/, perhaps?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613820116)
s/V3 transactions/TRUC transactions/, perhaps?
💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613817141)
Can you just add a comment here that references the comment on line 1520, as a reminder that we can't relax this without revisiting how we track available inputs when processing a package?
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613817141)
Can you just add a comment here that references the comment on line 1520, as a reminder that we can't relax this without revisiting how we track available inputs when processing a package?
💬 hebasto commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2130100585)
> @Sjors According to https://github.com/vmactions/freebsd-vm?tab=readme-ov-file#under-the-hood you can use `with: release:` to pick one.
By default, the latest release is used, which is 14.0. See https://github.com/hebasto/bitcoin/actions/runs/9215593397/job/25354268473#step:3:7032:
```
Config file: freebsd-14.0.conf
```
On my FreeBSD 14.0 machine, the default compiler is
```
$ c++ --version
FreeBSD clang version 16.0.6 (https://github.com/llvm/llvm-project.git llvmorg-16.0.6-0-g7cb
...
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2130100585)
> @Sjors According to https://github.com/vmactions/freebsd-vm?tab=readme-ov-file#under-the-hood you can use `with: release:` to pick one.
By default, the latest release is used, which is 14.0. See https://github.com/hebasto/bitcoin/actions/runs/9215593397/job/25354268473#step:3:7032:
```
Config file: freebsd-14.0.conf
```
On my FreeBSD 14.0 machine, the default compiler is
```
$ c++ --version
FreeBSD clang version 16.0.6 (https://github.com/llvm/llvm-project.git llvmorg-16.0.6-0-g7cb
...
💬 hebasto commented on pull request "[PoC] ci: Add FreeBSD GitHub Actions job":
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2130109577)
> > IMPORTANT. The `vmactions/*` needs to be added to the "Actions permissions" section of the repository settings.
>
> Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
I think, this repository granted on Read permissions to the GH Actions workflows as their logs include:
```
GITHUB_TOKEN Permission
...
(https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2130109577)
> > IMPORTANT. The `vmactions/*` needs to be added to the "Actions permissions" section of the repository settings.
>
> Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
I think, this repository granted on Read permissions to the GH Actions workflows as their logs include:
```
GITHUB_TOKEN Permission
...