Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 davidgumberg commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#issuecomment-2846099273)
Rebase to address style feedback.
🤔 shahsb reviewed a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2811079433)
Concept ACK
💬 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
...
💬 vasild commented on pull request "Split CConnman":
(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).
💬 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?
⚠️ 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
...
💬 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.
⚠️ 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
...
💬 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.
maflcko closed an issue: "test: functional test failures under -DENABLE_WALLET=OFF"
(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.
💬 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?
📝 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.
💬 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.
💬 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.
💬 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.
💬 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.
💬 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.