💬 bigshiny90 commented on pull request "test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)":
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3237148592)
needed to rebase to current master and fix tests for the new lower fee-rate policy.
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3237148592)
needed to rebase to current master and fix tests for the new lower fee-rate policy.
📝 fanquake opened a pull request: "[29.x] rc3 or final"
(https://github.com/bitcoin/bitcoin/pull/33271)
Backports:
* #33236
Might include #33268.
Since `rc2` #33212 was also backported in #33251.
(https://github.com/bitcoin/bitcoin/pull/33271)
Backports:
* #33236
Might include #33268.
Since `rc2` #33212 was also backported in #33251.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310498917)
Done
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310498917)
Done
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501279)
I moved `MockMempoolMinFee` to mempool utils to use it here and in the unit tests. I had to add one more parameter which is the mempool to be mocked.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501279)
I moved `MockMempoolMinFee` to mempool utils to use it here and in the unit tests. I had to add one more parameter which is the mempool to be mocked.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501512)
Done.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310501512)
Done.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310502672)
Make sense, will leave it for a possible follow-up.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310502672)
Make sense, will leave it for a possible follow-up.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310503817)
I think so, I can change it if I have to touch it again or in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2310503817)
I think so, I can change it if I have to touch it again or in a follow-up.
💬 0xB10C commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3237765557)
> Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?
Yes. Just the nonce in the block header changes. The signet commitment commits to the header version, prevhash, merkleroot, and timestamp. It does not commit (and can not commit) to the nonce. The underlying block does not change. The commitment remains valid.
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3237765557)
> Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?
Yes. Just the nonce in the block header changes. The signet commitment commits to the header version, prevhash, merkleroot, and timestamp. It does not commit (and can not commit) to the nonce. The underlying block does not change. The commitment remains valid.
💬 achow101 commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3237941668)
> Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add `-locations none` to the LCONVERT invocation in translate.cmake?
I don't think line numbers is the problem. The actual source location of the string can change and the ids in the `.xlf` won't change. It's the sort order in the `.ts` file that `lupdate` generates that seems to cause the id change. It's unclear to me why the sort order would ch
...
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3237941668)
> Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add `-locations none` to the LCONVERT invocation in translate.cmake?
I don't think line numbers is the problem. The actual source location of the string can change and the ids in the `.xlf` won't change. It's the sort order in the `.ts` file that `lupdate` generates that seems to cause the id change. It's unclear to me why the sort order would ch
...
👍 l0rinc approved a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3169923720)
ACK fa3f682032a3292604f363a5ee4557937f3d8950
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3169923720)
ACK fa3f682032a3292604f363a5ee4557937f3d8950
💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2311028761)
Done
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2311028761)
Done
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048525)
Updated the commit message.
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048525)
Updated the commit message.
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048812)
Clarified the test name
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048812)
Clarified the test name
👍 l0rinc approved a pull request: "build: add `-Wleading-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482#pullrequestreview-3169950067)
Core review ACK 8c60dc39a353c9d66cbe3a5a0a934eca6b2a287a
(https://github.com/bitcoin/bitcoin/pull/32482#pullrequestreview-3169950067)
Core review ACK 8c60dc39a353c9d66cbe3a5a0a934eca6b2a287a
💬 l0rinc commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311047700)
nit: there are a few remaining trailing spaces e.g. in https://github.com/bitcoin/bitcoin/blob/07f0a61ef711a2f75ded3d73545bfabdf2a64fef/src/minisketch/src/fields/clmul_common_impl.h#L39, we could add a PR to https://github.com/bitcoin-core/minisketch to get rid of those as well
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311047700)
nit: there are a few remaining trailing spaces e.g. in https://github.com/bitcoin/bitcoin/blob/07f0a61ef711a2f75ded3d73545bfabdf2a64fef/src/minisketch/src/fields/clmul_common_impl.h#L39, we could add a PR to https://github.com/bitcoin-core/minisketch to get rid of those as well
💬 l0rinc commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311037992)
nit: given that we're fixing non-trailing tabs as well, the only remaining .cpp/.h file that still contains tabs is https://github.com/bitcoin/bitcoin/blob/75a5c8258ec5309fe506438aa3815608430b53d6/src/util/subprocess.h#L1149, if you edit again, consider including that
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311037992)
nit: given that we're fixing non-trailing tabs as well, the only remaining .cpp/.h file that still contains tabs is https://github.com/bitcoin/bitcoin/blob/75a5c8258ec5309fe506438aa3815608430b53d6/src/util/subprocess.h#L1149, if you edit again, consider including that
💬 l0rinc commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2311103606)
> Admittedly they are imprecise
I'd say ~85000 vs ~400 is a bit more than imprecise :)
> almost double on same line
That's not how we usually decide outcomes, rather by better explanations - especially since many of those aren't counting declarations, e.g
* https://github.com/bitcoin/bitcoin/blob/f7d2db28e90297664390d527881ebbbf2c6ce6f0/src/bitcoin-cli.cpp#L241
* https://github.com/bitcoin/bitcoin/blob/face8123fdc10549676c6679ee3225c178a7f30c/src/httpserver.cpp#L565
* https://github.
...
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2311103606)
> Admittedly they are imprecise
I'd say ~85000 vs ~400 is a bit more than imprecise :)
> almost double on same line
That's not how we usually decide outcomes, rather by better explanations - especially since many of those aren't counting declarations, e.g
* https://github.com/bitcoin/bitcoin/blob/f7d2db28e90297664390d527881ebbbf2c6ce6f0/src/bitcoin-cli.cpp#L241
* https://github.com/bitcoin/bitcoin/blob/face8123fdc10549676c6679ee3225c178a7f30c/src/httpserver.cpp#L565
* https://github.
...
👋 l0rinc's pull request is ready for review: "stabilize translations by reverting old ids by text content"
(https://github.com/bitcoin/bitcoin/pull/33270)
(https://github.com/bitcoin/bitcoin/pull/33270)
🤔 enirox001 reviewed a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3170325100)
ACK fa3f682 - docstring cleanup, looks good.
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3170325100)
ACK fa3f682 - docstring cleanup, looks good.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3238341125)
> Are there any stats on increased coverage, or is this just checking more values in the same paths?
I'm going to share a coverage report but yes, it increases the coverage and reaches 100% of `wallet/fees.cpp`. One of the motivations for this PR is that I noticed that the following code was uncovered due to mempool min fee being always zero:
```cpp
// Obey mempool min fee when using smart fee estimation
CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee();
if (feerate_needed <
...
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3238341125)
> Are there any stats on increased coverage, or is this just checking more values in the same paths?
I'm going to share a coverage report but yes, it increases the coverage and reaches 100% of `wallet/fees.cpp`. One of the motivations for this PR is that I noticed that the following code was uncovered due to mempool min fee being always zero:
```cpp
// Obey mempool min fee when using smart fee estimation
CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee();
if (feerate_needed <
...