💬 achow101 commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1516902528)
ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1516902528)
ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635
📝 fanquake opened a pull request: "[25.x] Bump version to 25.0rc1"
(https://github.com/bitcoin/bitcoin/pull/27504)
Bump the version number.
Generate the man pages.
Generate example bitcoin.conf.
Point release-notes.md to the wiki: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-Draft.
Bump the version number in bips.md.
(https://github.com/bitcoin/bitcoin/pull/27504)
Bump the version number.
Generate the man pages.
Generate example bitcoin.conf.
Point release-notes.md to the wiki: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-Draft.
Bump the version number in bips.md.
💬 achow101 commented on pull request "[25.x] Bump version to 25.0rc1":
(https://github.com/bitcoin/bitcoin/pull/27504#issuecomment-1516909292)
ACK cf5850688d431e663e01a595b8547ff0d2902b69
(https://github.com/bitcoin/bitcoin/pull/27504#issuecomment-1516909292)
ACK cf5850688d431e663e01a595b8547ff0d2902b69
💬 sipa commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1516910306)
Posthumous utACK 9f947fc3d4b779f017332135323b34e8f216f613
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1516910306)
Posthumous utACK 9f947fc3d4b779f017332135323b34e8f216f613
🚀 achow101 merged a pull request: "wallet: coin selection, don't return results that exceed the max allowed weight"
(https://github.com/bitcoin/bitcoin/pull/26720)
(https://github.com/bitcoin/bitcoin/pull/26720)
👍 fanquake approved a pull request: "test: Remove unused sanitizer suppressions"
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1394726376)
ACK fa5af4d510a8f129157db8b69a9a387036efb206
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1394726376)
ACK fa5af4d510a8f129157db8b69a9a387036efb206
🚀 achow101 merged a pull request: "[25.x] Bump version to 25.0rc1"
(https://github.com/bitcoin/bitcoin/pull/27504)
(https://github.com/bitcoin/bitcoin/pull/27504)
💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1516922824)
> > the resulting file has roughly 2x the size of the compact-serialized UTXO set (this is mostly due to encoding txids and scriptpubkeys as hex-strings rather than bytes)
>
> What is the rationale for encoding as text rather than bytes? SQLite can store byte values as BLOBs.
Fair question. There was already some discussion in #24952 about whether to store txids/scriptPubKeys as TEXT or BLOB, see https://github.com/bitcoin/bitcoin/pull/24952#pullrequestreview-952102140, https://github.com/
...
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1516922824)
> > the resulting file has roughly 2x the size of the compact-serialized UTXO set (this is mostly due to encoding txids and scriptpubkeys as hex-strings rather than bytes)
>
> What is the rationale for encoding as text rather than bytes? SQLite can store byte values as BLOBs.
Fair question. There was already some discussion in #24952 about whether to store txids/scriptPubKeys as TEXT or BLOB, see https://github.com/bitcoin/bitcoin/pull/24952#pullrequestreview-952102140, https://github.com/
...
💬 fanquake commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1516926753)
Ubuntu 23.04 has been released: https://lists.ubuntu.com/archives/ubuntu-announce/2023-April/000289.html. So the Google Cloud images should be available soon.
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1516926753)
Ubuntu 23.04 has been released: https://lists.ubuntu.com/archives/ubuntu-announce/2023-April/000289.html. So the Google Cloud images should be available soon.
👋 fanquake's pull request is ready for review: "ci: use LLVM/clang-16 in native_asan job"
(https://github.com/bitcoin/bitcoin/pull/27360)
(https://github.com/bitcoin/bitcoin/pull/27360)
💬 TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1516940697)
Thank you for the reviews!
Updated bbe05915adad45560907b5a9b58fc55052838c08 -> bbe05915adad45560907b5a9b58fc55052838c08 ([kernelChainType_0](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_0) -> [kernelChainType_1](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_0..kernelChainType_1))
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171175379),
...
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1516940697)
Thank you for the reviews!
Updated bbe05915adad45560907b5a9b58fc55052838c08 -> bbe05915adad45560907b5a9b58fc55052838c08 ([kernelChainType_0](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_0) -> [kernelChainType_1](https://github.com/TheCharlatan/bitcoin/tree/kernelChainType_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelChainType_0..kernelChainType_1))
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1171175379),
...
💬 TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1173086418)
The style guide is vague on enums (though I guess you could follow the recommendation for classes)
```
- Braces on new lines for classes, functions, methods.
- Braces on the same line for everything else.
```
and our code (even the most recent additions) has a mix between putting braces on the same line and a new line for enums.
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1173086418)
The style guide is vague on enums (though I guess you could follow the recommendation for classes)
```
- Braces on new lines for classes, functions, methods.
- Braces on the same line for everything else.
```
and our code (even the most recent additions) has a mix between putting braces on the same line and a new line for enums.
💬 TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1173086733)
I'm not sure on this one. Granted, `std::string ChainName(ChainType chain)` would make it clear for the caller that this is most likely a label associated with the `ChainType`. However, for consistency I would also have to rename the helper functions to `GetChainName()`, and that seems less clear, because it does not have a typed argument allowing the caller to infer the relationship with `ChainType`.
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1173086733)
I'm not sure on this one. Granted, `std::string ChainName(ChainType chain)` would make it clear for the caller that this is most likely a label associated with the `ChainType`. However, for consistency I would also have to rename the helper functions to `GetChainName()`, and that seems less clear, because it does not have a typed argument allowing the caller to infer the relationship with `ChainType`.
📝 pinheadmz opened a pull request: "net: use interruptible async getaddrinfo wrapper from libevent for DNS"
(https://github.com/bitcoin/bitcoin/pull/27505)
Closes https://github.com/bitcoin/bitcoin/issues/16778
Bitcoin uses `getaddrinfo` to make DNS requests for DNS seed servers and for adding peers with `-addnode`, `-seednode` and `-connect`. Depending on the platform this can be clunky and a system issue could prevent name resolution from completing at all, blocking the thread and in some cases preventing a clean shutdown.
An attempt was made to switch to the asynchronous `getaddrinfo_a` in https://github.com/bitcoin/bitcoin/pull/4421 but t
...
(https://github.com/bitcoin/bitcoin/pull/27505)
Closes https://github.com/bitcoin/bitcoin/issues/16778
Bitcoin uses `getaddrinfo` to make DNS requests for DNS seed servers and for adding peers with `-addnode`, `-seednode` and `-connect`. Depending on the platform this can be clunky and a system issue could prevent name resolution from completing at all, blocking the thread and in some cases preventing a clean shutdown.
An attempt was made to switch to the asynchronous `getaddrinfo_a` in https://github.com/bitcoin/bitcoin/pull/4421 but t
...
💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1173095947)
> But my comment isn't really about the accuracy of the timestamp, but rather about the possibility of skipping a block after we have already started scanning. A block's timestamp can be before its parent. The code as written would let us skip scanning a block even if we had scanned its parent. The reason to use MTP is because it can only move forward. The MTP of a block can never be less than the MTP of its parent, it can be equal, but that's fine for us.
Yeah, where I was going is that we r
...
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1173095947)
> But my comment isn't really about the accuracy of the timestamp, but rather about the possibility of skipping a block after we have already started scanning. A block's timestamp can be before its parent. The code as written would let us skip scanning a block even if we had scanned its parent. The reason to use MTP is because it can only move forward. The MTP of a block can never be less than the MTP of its parent, it can be equal, but that's fine for us.
Yeah, where I was going is that we r
...
💬 achow101 commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1516954704)
ACK 0076bed45eb2b42111fa3f4c95181393c685a42e
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1516954704)
ACK 0076bed45eb2b42111fa3f4c95181393c685a42e
🚀 achow101 merged a pull request: "logging, net: add ASN from peers on logs"
(https://github.com/bitcoin/bitcoin/pull/27412)
(https://github.com/bitcoin/bitcoin/pull/27412)
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-1516996244)
Rebased and added a refactor to split up `ThreadAddressSeed` into separate functions for DNS and fixed seeds - I will keep in draft a bit longer, until the issue below is resolved.
> While implementing and testing the `ThreadAddressSeed` code organization changes, I ran into a small issue/ behavior change caused by the movement of the fixed seeding into another thread - I need to analyze this closer, marking as Draft for now.
The issue was that if one chooses `-seednode=X`, the idea is to
...
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-1516996244)
Rebased and added a refactor to split up `ThreadAddressSeed` into separate functions for DNS and fixed seeds - I will keep in draft a bit longer, until the issue below is resolved.
> While implementing and testing the `ThreadAddressSeed` code organization changes, I ran into a small issue/ behavior change caused by the movement of the fixed seeding into another thread - I need to analyze this closer, marking as Draft for now.
The issue was that if one chooses `-seednode=X`, the idea is to
...
💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1517007742)
Updated per feedback, thanks achow.
Changes:
* Replaced `BLANK_BIRTH_TIME` constant by setting the `m_birth_time` to its max numeric value.
* Tackled a block time variability concern by using the maximum time in the chain (a moving forward only timestamp) instead of the raw block time.
* Extended the grace period to be twice as big as the one used in the rescan process. Even when this is not strictly needed, the rationale is to be a bit more conservative during IBD scanning because it's an
...
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1517007742)
Updated per feedback, thanks achow.
Changes:
* Replaced `BLANK_BIRTH_TIME` constant by setting the `m_birth_time` to its max numeric value.
* Tackled a block time variability concern by using the maximum time in the chain (a moving forward only timestamp) instead of the raw block time.
* Extended the grace period to be twice as big as the one used in the rescan process. Even when this is not strictly needed, the rationale is to be a bit more conservative during IBD scanning because it's an
...
📝 amitiuttarwar opened a pull request: "test: prevent intermittent failures"
(https://github.com/bitcoin/bitcoin/pull/27506)
Follow up to #27214 - add an address to the tried table before the new table to make sure a new table collision is not possible.
(https://github.com/bitcoin/bitcoin/pull/27506)
Follow up to #27214 - add an address to the tried table before the new table to make sure a new table collision is not possible.
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1517186540)
opened #27506 for the test fix
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1517186540)
opened #27506 for the test fix