💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2846328501)
Concept ACK, but I think approach NACK.
Instead of creating a whole new set of output, why not just take the summary and print it as results arrive rather than holding them all until the end? I put together a quick POC which produces the output below in real time (rather than waiting until end):
```bash
(1/221) addition_overflow #60 DONE cov: 265 ft: 320 corp: 47/1674b lim: 62 exec/s: 0 rss: 115Mb
(2/221) addr_info_deserialize #110 DO
...
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2846328501)
Concept ACK, but I think approach NACK.
Instead of creating a whole new set of output, why not just take the summary and print it as results arrive rather than holding them all until the end? I put together a quick POC which produces the output below in real time (rather than waiting until end):
```bash
(1/221) addition_overflow #60 DONE cov: 265 ft: 320 corp: 47/1674b lim: 62 exec/s: 0 rss: 115Mb
(2/221) addr_info_deserialize #110 DO
...
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2846425432)
`7ef9661fb0...503791ed57`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2846425432)
`7ef9661fb0...503791ed57`: rebase due to conflicts
💬 Sjors commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2846433582)
That sounds reasonable at first glance. Each commit has to pass the tests. Ideally anytime you introduce a new function you would also add at least one test, and/or use that function in existing code (to show that it doesn't break any test).
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2846433582)
That sounds reasonable at first glance. Each commit has to pass the tests. Ideally anytime you introduce a new function you would also add at least one test, and/or use that function in existing code (to show that it doesn't break any test).
💬 rebroad commented on pull request "Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence":
(https://github.com/bitcoin-core/gui/pull/866#issuecomment-2846478431)
I realise this PR is starting to look a bit messy with all the updates. I also realise I ought to separate various functionality changes into separate commits. Is it better to raise a new PR once this is done, force push to this one?
(https://github.com/bitcoin-core/gui/pull/866#issuecomment-2846478431)
I realise this PR is starting to look a bit messy with all the updates. I also realise I ought to separate various functionality changes into separate commits. Is it better to raise a new PR once this is done, force push to this one?
⚠️ Sjors opened an issue: "rfc: separate relay from mining policy"
(https://github.com/bitcoin/bitcoin/issues/32401)
Greg Maxwell brought up an idea on the mailinglist that I think is worth preserving here, although I don't think it's currently needed:
> That said, Bitcoin core has generally not
had knobs to adjust relay policy as distinct from mining policy in large
part because of the design assumption that the two need to be the same.
But in this case if there were a knob here I think would make more sense
for it to control **mining policy rather than relay policy**, since it would
actually have some
...
(https://github.com/bitcoin/bitcoin/issues/32401)
Greg Maxwell brought up an idea on the mailinglist that I think is worth preserving here, although I don't think it's currently needed:
> That said, Bitcoin core has generally not
had knobs to adjust relay policy as distinct from mining policy in large
part because of the design assumption that the two need to be the same.
But in this case if there were a knob here I think would make more sense
for it to control **mining policy rather than relay policy**, since it would
actually have some
...
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2846604494)
This is low prio (for me) and I haven't had the time to PR it for a few months, so I am leaving it as it it. If anybody does PR the diff above or similar, then I would review.
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2846604494)
This is low prio (for me) and I haven't had the time to PR it for a few months, so I am leaving it as it it. If anybody does PR the diff above or similar, then I would review.
⚠️ mrberlinorg opened an issue: "Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:"
(https://github.com/bitcoin/bitcoin/issues/32402)
Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:
Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might not be able to properly process or relay these transactions.
Network congestion: More OP_RETURN outputs
...
(https://github.com/bitcoin/bitcoin/issues/32402)
Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:
Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might not be able to properly process or relay these transactions.
Network congestion: More OP_RETURN outputs
...
💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2846705290)
Feel free to tag me in such a PR.
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2846705290)
Feel free to tag me in such a PR.
✅ maflcko closed an issue: "test: functional test failures under -DENABLE_WALLET=OFF"
(https://github.com/bitcoin/bitcoin/issues/32347)
(https://github.com/bitcoin/bitcoin/issues/32347)
💬 maflcko commented on issue "test: functional test failures under -DENABLE_WALLET=OFF":
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2846803315)
> genuinely fixed this
I agree: With the removal of `is_specified_wallet_compiled` this should no longer happen. Closing for now, but feel free to re-open if the issue still exists.
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2846803315)
> genuinely fixed this
I agree: With the removal of `is_specified_wallet_compiled` this should no longer happen. Closing for now, but feel free to re-open if the issue still exists.
💬 0xB10C commented on pull request "cli: add -usefile option":
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2846837427)
> This might also be helpful for users outside of the functional tests (although there is always the option to use the rpc interface directly instead of bitcoin-cli).
I've been using `-stdin` (e.g. `cat block.hex | bitcoin-cli -stdin submitblock`) for this. Would this work here too?
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2846837427)
> This might also be helpful for users outside of the functional tests (although there is always the option to use the rpc interface directly instead of bitcoin-cli).
I've been using `-stdin` (e.g. `cat block.hex | bitcoin-cli -stdin submitblock`) for this. Would this work here too?
📝 fanquake opened a pull request: "test: remove Boost SIGCHLD workaround."
(https://github.com/bitcoin/bitcoin/pull/32403)
The related code was removed from Boost in https://github.com/boostorg/test/commit/2e3bd1025d417f661a7edbf7b7dbcb801118033d.
(https://github.com/bitcoin/bitcoin/pull/32403)
The related code was removed from Boost in https://github.com/boostorg/test/commit/2e3bd1025d417f661a7edbf7b7dbcb801118033d.
💬 Sjors commented on 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#issuecomment-2846879709)
Concept ACK
If it wasn't for the hardcode minimum `-blockreservedweight=2000`, then if the user set it to zero this loop might never exit? In any case, it's indeed an unrelated concept so deserves its own constant.
(https://github.com/bitcoin/bitcoin/pull/32355#issuecomment-2846879709)
Concept ACK
If it wasn't for the hardcode minimum `-blockreservedweight=2000`, then if the user set it to zero this loop might never exit? In any case, it's indeed an unrelated concept so deserves its own constant.
💬 laanwj commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2071415173)
Usage looks correct according to documentation:
- `hAlgorithm`=`NULL`: required (see below)
- `pbBuffer`=`ent32`: "The address of a buffer that receives the random number. "
- `cbBuffer`=`NUM_OS_RANDOM_BYTES`: "The size, in bytes, of the pbBuffer buffer."
- `dwFlags`=`BCRYPT_USE_SYSTEM_PREFERRED_RNG`: "Use the system-preferred random number generator algorithm. The hAlgorithm parameter must be NULL."
Nit: Might want to use named arguments here.
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2071415173)
Usage looks correct according to documentation:
- `hAlgorithm`=`NULL`: required (see below)
- `pbBuffer`=`ent32`: "The address of a buffer that receives the random number. "
- `cbBuffer`=`NUM_OS_RANDOM_BYTES`: "The size, in bytes, of the pbBuffer buffer."
- `dwFlags`=`BCRYPT_USE_SYSTEM_PREFERRED_RNG`: "Use the system-preferred random number generator algorithm. The hAlgorithm parameter must be NULL."
Nit: Might want to use named arguments here.
💬 laanwj commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2071420415)
We might need to update https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/symbol-check.py#L156
(likely add a DLL, possibly even remove one, though i doubt this was all we use from ADVAPI32).
The guix build will tell.
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2071420415)
We might need to update https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/symbol-check.py#L156
(likely add a DLL, possibly even remove one, though i doubt this was all we use from ADVAPI32).
The guix build will tell.
💬 Sjors commented on issue "rfc: separate relay from mining policy":
(https://github.com/bitcoin/bitcoin/issues/32401#issuecomment-2846924406)
@ajtowns brought up one potential objection in the above thread, or at least something that increases implementation complexity:
> accepting txs into your mempool that you'll still relay but won't mine might block you from accepting other (conflicting) txs that you would mine, eg.
(https://github.com/bitcoin/bitcoin/issues/32401#issuecomment-2846924406)
@ajtowns brought up one potential objection in the above thread, or at least something that increases implementation complexity:
> accepting txs into your mempool that you'll still relay but won't mine might block you from accepting other (conflicting) txs that you would mine, eg.
💬 hebasto commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846925861)
Fetched the updated Polish (pl) translation. Thanks to @jesterhodl !
@maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833289993) has also been addressed.
I believe that any further translation improvements should be made through communication with the language coordinators, which I'm currently trying to arrange.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846925861)
Fetched the updated Polish (pl) translation. Thanks to @jesterhodl !
@maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2833289993) has also been addressed.
I believe that any further translation improvements should be made through communication with the language coordinators, which I'm currently trying to arrange.
💬 0xB10C commented on issue "test: interface_usdt_net.py failure under --valgrind":
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2846931412)
Trying it with `pfrom->GetId()` as the sole tracepoint argument produces the same failure. I don't think it's related to `ConnectionTypeAsString`.
It seems to me that all interface_usdt_*.py tests fail under valgrind?
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2846931412)
Trying it with `pfrom->GetId()` as the sole tracepoint argument produces the same failure. I don't think it's related to `ConnectionTypeAsString`.
It seems to me that all interface_usdt_*.py tests fail under valgrind?
👍 laanwj approved a pull request: "test: remove Boost SIGCHLD workaround."
(https://github.com/bitcoin/bitcoin/pull/32403#pullrequestreview-2811726146)
ACK 3add6ab9adcd722d57c6d488581358ae9b377f1a
(https://github.com/bitcoin/bitcoin/pull/32403#pullrequestreview-2811726146)
ACK 3add6ab9adcd722d57c6d488581358ae9b377f1a
💬 jesterhodl commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846937440)
Going forward:
- who do I contact if there's an issue with source labels?
- do I have to do anything when I just maintain new or updated labels? eg. when I align wording across the app
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846937440)
Going forward:
- who do I contact if there's an issue with source labels?
- do I have to do anything when I just maintain new or updated labels? eg. when I align wording across the app
💬 hebasto commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846937836)
cc @stickies-v @pablomartin4btc @johnny9 as regular reviewers of similar previous PRs.
cc @fanquake as the 29.1 release manager.
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2846937836)
cc @stickies-v @pablomartin4btc @johnny9 as regular reviewers of similar previous PRs.
cc @fanquake as the 29.1 release manager.