Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2130012490)
> I feel like I'm missing something here though. Before the changes of this pull, the fixed seeds would be pulled after a minute of trying DNS seeds and not reaching the desired peer goal. However, after this pull `ProcessFixedSeeds` is called sequentially after `QueryDNSSeeds`. And the latter has no exit logic based on how long that has been running, meaning that if both are set `ProcessFixedSeeds` will never trigger (?).
>
> PS: Well, technically that's not completely accurate. This will fi
...
💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785731)
Done.
💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785818)
Done.
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1613785831)
> The spec recommend trying again with ever shorter intervals, but no less than 4 seconds, as the deadline approached. But it seems fine to not bother.

Right, the retrying is not according to spec-because i had the same thought. It seems overkill to adaptive timing for something that's meant to interact with the local router. If it doesn't have enough capacity to handle a PCP/NAT-PMP packet it sure won't be routing a lot of traffic. i was more afraid that bitcoind (or the system it's running
...
💬 sipa commented on pull request "util: add RingBuffer":
(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.
💬 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!)
💬 sipa commented on pull request "util: add RingBuffer":
(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).
💬 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
💬 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
🚀 achow101 merged a pull request: "fuzz: Fix wallet_bdb_parser stdlib error matching"
(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
💬 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?
💬 epiccurious commented on pull request "refactor: Use type-safe time in txorphanage":
(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!
💬 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.
💬 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/ ?
💬 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?