💬 maflcko commented on issue "Newer libevent causes `http_request` fuzz target failure":
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110022854)
(I deleted my comment, because the fuzz CI config does not use depends, but the `libevent-dev` from Ubuntu)
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110022854)
(I deleted my comment, because the fuzz CI config does not use depends, but the `libevent-dev` from Ubuntu)
💬 fanquake commented on issue "Newer libevent causes `http_request` fuzz target failure":
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110029212)
Ok. Edited my comment as well. This can still be closed. Maybe an issue can be filed with vcpkg to only ship stable code in production.
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110029212)
Ok. Edited my comment as well. This can still be closed. Maybe an issue can be filed with vcpkg to only ship stable code in production.
👍 sdaftuar approved a pull request: "test: add conflicting topology test case"
(https://github.com/bitcoin/bitcoin/pull/30066#pullrequestreview-2055185685)
utACK, one style preference/nit for you to consider
(https://github.com/bitcoin/bitcoin/pull/30066#pullrequestreview-2055185685)
utACK, one style preference/nit for you to consider
💬 sdaftuar commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599922463)
This may be just a matter of personal preference, so feel free to take it or leave it -- but if I were writing this I'd drop this line which tests the reason for failure, and also drop line 269 which tests that RBF doesn't work, in favor of just having the assertion at line 270 below that the child doesn't make it in. That way, someone changing the behavior in the future is less likely to rip the whole test out (eg if "not child with unconfirmed parents" stops being a rejection reason, or if we
...
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599922463)
This may be just a matter of personal preference, so feel free to take it or leave it -- but if I were writing this I'd drop this line which tests the reason for failure, and also drop line 269 which tests that RBF doesn't work, in favor of just having the assertion at line 270 below that the child doesn't make it in. That way, someone changing the behavior in the future is less likely to rip the whole test out (eg if "not child with unconfirmed parents" stops being a rejection reason, or if we
...
💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1599947252)
Nice, will add if I end up needing to retouch!
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1599947252)
Nice, will add if I end up needing to retouch!
💬 hebasto commented on issue "Newer libevent causes `http_request` fuzz target failure":
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110112892)
> Ok, then I think this can be closed. The problem here primarily seems to be that Microsoft/vcpkg is shipping unreleased code into production.
I agree. I opened this issue to document https://github.com/hebasto/bitcoin/pull/199.
Closing.
(https://github.com/bitcoin/bitcoin/issues/30096#issuecomment-2110112892)
> Ok, then I think this can be closed. The problem here primarily seems to be that Microsoft/vcpkg is shipping unreleased code into production.
I agree. I opened this issue to document https://github.com/hebasto/bitcoin/pull/199.
Closing.
✅ hebasto closed an issue: "Newer libevent causes `http_request` fuzz target failure"
(https://github.com/bitcoin/bitcoin/issues/30096)
(https://github.com/bitcoin/bitcoin/issues/30096)
💬 instagibbs commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599957582)
I'm a little conflicted since without the string checking it's a little harder to tell what the two cases being checked are there? I do agree that dropping the check for the parent RBF is better.
I pushed a small update to make it a bit harder to have the whole case ripped out by someone reading the test, let me know what you think.
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599957582)
I'm a little conflicted since without the string checking it's a little harder to tell what the two cases being checked are there? I do agree that dropping the check for the parent RBF is better.
I pushed a small update to make it a bit harder to have the whole case ripped out by someone reading the test, let me know what you think.
💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1599959087)
This was somewhat answered in https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1465152180 above but let me elaborate - the thing that prompted me to do this change is this text from the `fread(3)` manual page:
> The function fread() does not distinguish between end-of-file and error,
> and callers must use feof(3) and ferror(3) to determine which occurred.
I think checking for `ferror(3)` after `fread(3)` is a kind of harmless "belt-and-suspenders". If you are sure that if `detai
...
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1599959087)
This was somewhat answered in https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1465152180 above but let me elaborate - the thing that prompted me to do this change is this text from the `fread(3)` manual page:
> The function fread() does not distinguish between end-of-file and error,
> and callers must use feof(3) and ferror(3) to determine which occurred.
I think checking for `ferror(3)` after `fread(3)` is a kind of harmless "belt-and-suspenders". If you are sure that if `detai
...
👍1
💬 instagibbs commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599959394)
done
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1599959394)
done
💬 willcl-ark commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2110153539)
@theuni FWIW I do see some _increased_ RSS measurements on your current branch vs master (with my one, single test using listtransactions rpc on a large wallet):

This is also what I saw on my linked "univalue-move" branch above. I didn't investigate why exactly this happens, yet. It seems like `move` might not always be advantageous for us...
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2110153539)
@theuni FWIW I do see some _increased_ RSS measurements on your current branch vs master (with my one, single test using listtransactions rpc on a large wallet):

This is also what I saw on my linked "univalue-move" branch above. I didn't investigate why exactly this happens, yet. It seems like `move` might not always be advantageous for us...
💬 vasild commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1599981699)
My understanding is that it is safe to use `thread_local` on FreeBSD for variables that do not have a destructor even in the presence of the above bug (or bugs if they are two separate bugs).
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1599981699)
My understanding is that it is safe to use `thread_local` on FreeBSD for variables that do not have a destructor even in the presence of the above bug (or bugs if they are two separate bugs).
👍 ryanofsky approved a pull request: "kernel: Remove batchpriority from kernel library"
(https://github.com/bitcoin/bitcoin/pull/30083#pullrequestreview-2055370796)
Code review ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, just added suggested comment since last review
(https://github.com/bitcoin/bitcoin/pull/30083#pullrequestreview-2055370796)
Code review ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, just added suggested comment since last review
💬 glozow commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2110260609)
backport for 26.x in #29899
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2110260609)
backport for 26.x in #29899
👋 glozow's pull request is ready for review: "[26.x] archive 26.1 release notes + backports"
(https://github.com/bitcoin/bitcoin/pull/29899)
(https://github.com/bitcoin/bitcoin/pull/29899)
💬 naiyoma commented on pull request "test: Refactor fee/feerate calculations in feature_fee_estimation.py to use Decimal instead of float":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600064674)
Thanks for the breakdown, seems ideal to also use satoshi_round here and to use round down for min and round up for max, helps with greater precision
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600064674)
Thanks for the breakdown, seems ideal to also use satoshi_round here and to use round down for min and round up for max, helps with greater precision
💬 ajtowns commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2110275325)
> Another thing, it doesn't seem to exist an option to control the miner's internal "timer". That is, suppose I want to mine every minute instead of every 10 minutes.
The nbits options exist to control the miner's internal timer -- if you were to mine every minute instead of every 10 minutes, that will increase the difficulty until you run out of hashpower, at which point you'll be running your computer at full power 24/7 and end up getting blocks every 10 minutes. The "no matter how much has
...
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2110275325)
> Another thing, it doesn't seem to exist an option to control the miner's internal "timer". That is, suppose I want to mine every minute instead of every 10 minutes.
The nbits options exist to control the miner's internal timer -- if you were to mine every minute instead of every 10 minutes, that will increase the difficulty until you run out of hashpower, at which point you'll be running your computer at full power 24/7 and end up getting blocks every 10 minutes. The "no matter how much has
...
💬 sdaftuar commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1600064969)
Looks good, thanks!
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1600064969)
Looks good, thanks!
💬 sdaftuar commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#issuecomment-2110283778)
re-utACK
(https://github.com/bitcoin/bitcoin/pull/30066#issuecomment-2110283778)
re-utACK
💬 rkrux commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1600088583)
Oh nevermind, reviewing this PR gave me the answer: https://github.com/bitcoin/bitcoin/pull/29982
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1600088583)
Oh nevermind, reviewing this PR gave me the answer: https://github.com/bitcoin/bitcoin/pull/29982
💬 naiyoma commented on pull request "test: Refactor fee/feerate calculations in feature_fee_estimation.py to use Decimal instead of float":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600089403)
resolved
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600089403)
resolved