Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 theuni approved a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903#pullrequestreview-2395817738)
Concept ACK and quick review ACK.

Tested with depends and verified that the fallback isn't used:
https://github.com/theuni/bitcoin/commit/d98c76c69e01ddacc79974fef6f7fd0f9df5ddcc

@hebasto want to pull that in here?
💬 theuni commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#discussion_r1817015584)
Doubtful it matters here, but external libs after internal libs please.
💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438279436)
> People can be asked to add benchmarks for performance improvements or to verify that a change to existing code doesn't degrade performance

Imo, performance improvements (and accompanying benchmarks) should only be made to code that makes critical things slower than they need to be. Otherwise we're just improving performance for the sake of doing it, which is not something we have the resources for. We should aim our resources with intention, and the benchmarks can act as the scope.

For e
...
💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2438280776)
Will add the doc changes next week
💬 jonatack commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2438284288)
Approach ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
👍 instagibbs approved a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2395838425)
reACK 0f4bc635854597e15ea6968767fc4e5cf5bdd790

could use some fuzzing c @marcofleon
💬 sipa commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2438324004)
Concept ACK. While we don't actually know how many systems are out there which could be made reachable through UPnP but not through NAT-PMP/PCP, I suspect the number of them actually doing that is very low (given it being default off, and not widely advertized).
💬 sdaftuar commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#issuecomment-2438357266)
re-ACK f32c34d0c3d4041a301822b27e88d6db4cbf631e
💬 hebasto commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2438376673)
> Concept ACK and quick review ACK.
>
> Tested with depends and verified that the fallback isn't used: [theuni@d98c76c](https://github.com/theuni/bitcoin/commit/d98c76c69e01ddacc79974fef6f7fd0f9df5ddcc)
>
> @hebasto want to pull that in here?

Thanks! Pulled in.
💬 hebasto commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#discussion_r1817070022)
Sure. Reordered.
⚠️ petertodd opened an issue: "MIN_STANDARD_TX_NONWITNESS_SIZE prevents efficient spending of P2A outputs"
(https://github.com/bitcoin/bitcoin/issues/31155)
### Please describe the feature you'd like to see added.

The most efficient way to spend a single P2A output to an `OP_RETURN` results in a transaction 61 bytes in size, below the 65 byte limit enforced by `MIN_STANDARD_TX_NONWITNESS_SIZE`. For example:

```01000000017989c51d11e9e1e6aa634d471e31900141fe44bd491bcb5996f948b9039b232e0000000000ffffffff010000000000000000016a00000000```

This transaction should be standard.

### Is your feature related to a problem, if so please describe it.

_No
...
💬 edilmedeiros commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2438488332)
utACK b7b24352906f1dba64826e7a093069b5bfc504dc.

Agree with @fanquake comments.
📝 mzumsande opened a pull request: "test: Don't enforce BIP94 on regtest unless specified by arg"
(https://github.com/bitcoin/bitcoin/pull/31156)
The added arg `-enforcebip94` is only used in a functional test for BIP94. This is done because the default regtest consensus rules should be close to mainnet, not testnet.

Fixes #31137.
💬 Davidson-Souza commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2438501129)
In 7864871. If I fire a `getblocktemplate` **before** sync is done, I get a `segfault`.

<details>

<summary>Valgrind stack trace</summary>

```
==1025895== Thread 7 b-httpworker.3:
==1025895== Invalid read of size 4
==1025895== at 0x36A95F: node::(anonymous namespace)::MinerImpl::rollbackTestnet4() (interfaces.cpp:997)
==1025895== by 0x42F759: getblocktemplate()::{lambda(RPCHelpMan const&, JSONRPCRequest const&)#1}::operator()(RPCHelpMan const&, JSONRPCRequest const&) const [clo
...
💬 mzumsande commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2438504815)
I opened #31156 which just adds an arg. This seemed the least invasive solution to me.

>A pre-mined testnet4 chain can possibly be used for the test as well, instead.

That should be possible too, but would require enabling `setmocktime` for testnet, which I didn't love.
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817154923)
aside from reorgs?
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817158265)
when would removals or additions happen outside of a changeset?
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817154002)

```suggestion
* set of new transactions and compare with the existing mempool.
```
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817158983)
?
```suggestion
* Apply() will apply the removals and additions that are staged into the mempool.
```
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817161237)
logically seems to work, probably not worth nitting