💬 fanquake commented on pull request "build: Fix `macdeployqtplus` after switching to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2060882568)
Same Q here as above.
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2060882568)
Same Q here as above.
💬 glozow commented on issue "estimateSmartFee error: "Insufficient data or no feerate found":
(https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2831440563)
I think there have been a few debates about whether it's safer to return nothing ("insufficient data") vs a potentially bad guess (mempool min could also be artificially low if we're just starting out). But also, lowballing isn't the end of the world since you should be able to replace transactions that didn't pay enough.
(https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2831440563)
I think there have been a few debates about whether it's safer to return nothing ("insufficient data") vs a potentially bad guess (mempool min could also be artificially low if we're just starting out). But also, lowballing isn't the end of the world since you should be able to replace transactions that didn't pay enough.
💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831451724)
Needed to reorg the commits a bit to get the test-every-commit CI to pass
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831451724)
Needed to reorg the commits a bit to get the test-every-commit CI to pass
💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831459829)
I guess this could use a bug label too if someone with permission can get around to that :)
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831459829)
I guess this could use a bug label too if someone with permission can get around to that :)
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2060900943)
(Done in latest push from ~8 hours ago).
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2060900943)
(Done in latest push from ~8 hours ago).
📝 kevkevinpal opened a pull request: "tests: Added progress tracker when running fuzz test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/32354)
Right now if you run `test_runner.py` on all the targets it is hard to tell how far you are in the progress even with `--loglevel=DEBUG` on. As it just spits out the target that was completed an no info on how many targets are left.
This change gives you a percentage in 5% increments on how far along you are finishing the targets you've selected
This also applies when you use `-g/--generate` and `--m_dir`
I can append this change to either change the percent increments or move it to th
...
(https://github.com/bitcoin/bitcoin/pull/32354)
Right now if you run `test_runner.py` on all the targets it is hard to tell how far you are in the progress even with `--loglevel=DEBUG` on. As it just spits out the target that was completed an no info on how many targets are left.
This change gives you a percentage in 5% increments on how far along you are finishing the targets you've selected
This also applies when you use `-g/--generate` and `--m_dir`
I can append this change to either change the percent increments or move it to th
...
💬 jonatack commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#issuecomment-2831574347)
ACK 4b6171982a20d736b2c627ab9b7ea788b06af457
(https://github.com/bitcoin/bitcoin/pull/31895#issuecomment-2831574347)
ACK 4b6171982a20d736b2c627ab9b7ea788b06af457
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061100509)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
This is not correct. The code (and comment) above indicate that `buffer` is being interpreted using 1 bit of asmap code per byte. The line here is interpreting it as 1 byte per byte.
You need some conversion function, I think:
```c++
namespace {
std::vector<std::byte> BitsToBytes(std::span<const uint8_t> input) noexcept
{
std::vector<std::byte> ret;
uint8_t next_byte{0};
int next_byte_bits{0};
...
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061100509)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
This is not correct. The code (and comment) above indicate that `buffer` is being interpreted using 1 bit of asmap code per byte. The line here is interpreting it as 1 byte per byte.
You need some conversion function, I think:
```c++
namespace {
std::vector<std::byte> BitsToBytes(std::span<const uint8_t> input) noexcept
{
std::vector<std::byte> ret;
uint8_t next_byte{0};
int next_byte_bits{0};
...
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061112524)
In commit "refactor: Use span instead of vector for data in util/asmap"
No need for the explicit conversion:
```diff
-if (SanityCheckASMap(std::span<const std::byte>(asmap), buffer.size() - 1 - sep_pos)) {
+if (SanityCheckASMap(asmap, buffer.size() - 1 - sep_pos)) {
```
Also twice the same further in this function.
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061112524)
In commit "refactor: Use span instead of vector for data in util/asmap"
No need for the explicit conversion:
```diff
-if (SanityCheckASMap(std::span<const std::byte>(asmap), buffer.size() - 1 - sep_pos)) {
+if (SanityCheckASMap(asmap, buffer.size() - 1 - sep_pos)) {
```
Also twice the same further in this function.
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096624)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
I don't think this is necessary? The code above could operate on `asmap` instead of `asmap_vec` directly?
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096624)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
I don't think this is necessary? The code above could operate on `asmap` instead of `asmap_vec` directly?
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096369)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
Simpler:
```c++
asmap_vec.push_back(std::byte(buffer[1 + i]));
```
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096369)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
Simpler:
```c++
asmap_vec.push_back(std::byte(buffer[1 + i]));
```
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061101276)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
This doesn't make sense anymore. You can't slice off individual bits from an `std::byte`.
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061101276)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
This doesn't make sense anymore. You can't slice off individual bits from an `std::byte`.
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061109899)
"In commit "refactor: Operate on bytes instead of bits in Asmap code":
I think it would be helpful to have a bit getter helper function:
```c++
inline bool GetBit(std::span<const std::byte> bytes, uint32_t bitpos) noexcept
{
return (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
}
```
And then for example:
```c++
...
bit = GetBit(data, bitpos++);
...
```
and
```c++
...
if (GetBit(ip, ip.size() * 8 - bits)) {
--bits;
...
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061109899)
"In commit "refactor: Operate on bytes instead of bits in Asmap code":
I think it would be helpful to have a bit getter helper function:
```c++
inline bool GetBit(std::span<const std::byte> bytes, uint32_t bitpos) noexcept
{
return (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
}
```
And then for example:
```c++
...
bit = GetBit(data, bitpos++);
...
```
and
```c++
...
if (GetBit(ip, ip.size() * 8 - bits)) {
--bits;
...
💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061094811)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
Simpler:
```diff
--- a/src/netgroup.cpp
+++ b/src/netgroup.cpp
@@ -90,18 +90,18 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
if (address.HasLinkedIPv4()) {
// For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
- ip_bits[byte_i] = static_cast<std::byte>(IPV4_IN_IPV6_PREFIX[by
...
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061094811)
In commit "refactor: Operate on bytes instead of bits in Asmap code"
Simpler:
```diff
--- a/src/netgroup.cpp
+++ b/src/netgroup.cpp
@@ -90,18 +90,18 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
if (address.HasLinkedIPv4()) {
// For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
- ip_bits[byte_i] = static_cast<std::byte>(IPV4_IN_IPV6_PREFIX[by
...
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114715)
I don't think this is needed. `TxGraph` doesn't promise a specific ordering of equal-feerate diagram segments. The implementation is allowed to be more specific than the interface promises.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114715)
I don't think this is needed. `TxGraph` doesn't promise a specific ordering of equal-feerate diagram segments. The implementation is allowed to be more specific than the interface promises.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114932)
No, because it's computing the diagram for the linearization `vec1`, which is constructed using the real graph's transaction comparison function.
It's just using `sims[0].graph` to pull the feerate information from.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114932)
No, because it's computing the diagram for the linearization `vec1`, which is constructed using the real graph's transaction comparison function.
It's just using `sims[0].graph` to pull the feerate information from.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061115163)
Sure, the caller is responsible for not violating the contract of the interface.
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061115163)
Sure, the caller is responsible for not violating the contract of the interface.
🚀 hebasto merged a pull request: "Replace stray tfm::format to cerr with qWarning"
(https://github.com/bitcoin-core/gui/pull/868)
(https://github.com/bitcoin-core/gui/pull/868)
💬 hebasto commented on pull request "build: Fix `macdeployqtplus` after switching to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2061202483)
Thanks! Adjusted as well.
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2061202483)
Thanks! Adjusted as well.
📝 luke-jr opened a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355)
PR #30356 incorrectly changed a constant of `4000` to `m_options.coinbase_max_additional_weight` in the check for when to give up finding another transaction to fill the block:
```diff
if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight >
- m_options.nBlockMaxWeight - 4000) {
+ m_options.nBlockMaxWeight - m_options.block_reserved_weight) {
// Give up if we're close to full and haven't succeeded in a while
...
(https://github.com/bitcoin/bitcoin/pull/32355)
PR #30356 incorrectly changed a constant of `4000` to `m_options.coinbase_max_additional_weight` in the check for when to give up finding another transaction to fill the block:
```diff
if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight >
- m_options.nBlockMaxWeight - 4000) {
+ m_options.nBlockMaxWeight - m_options.block_reserved_weight) {
// Give up if we're close to full and haven't succeeded in a while
...