Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 stickies-v approved a pull request: "mempool: Don't sort in entryAll"
(https://github.com/bitcoin/bitcoin/pull/29019#pullrequestreview-1770813594)
ACK 87fa444a04854a5d3a9ff283a8b6c34587e8430f
💬 mzumsande commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1419470209)
I agree that it makes no sense, if you want just one connection you should pick a node you trust and do `-connect`. Using automatic connections for that means eclipsing yourself - maybe we should warn about that in general?
That being said, this PR does log disconnection to `-blocksonly` peers, although only in `BCLog::NET` - do you suggest to make it unconditional based on `-maxconnections`?
💬 murchandamus commented on pull request "Avoid changeless input sets when SFFO is active":
(https://github.com/bitcoin/bitcoin/pull/28985#issuecomment-1845984159)
Rebased on top of #28994
🤔 sipa reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1770812972)
Halfway through.
💬 sipa commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1419512254)
In commit "[test] Introduce EncryptedP2PState object in P2PConnection"

Nit: I'd find it a bit cleaner to *not* give this variable such an accurate name for reasons of abstractions. The code here shouldn't care about *what* it is that the `EncryptedP2PState` tells us to send; it should just be sent. Maybe call it "send_handshake_bytes" ?
💬 sipa commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1419473256)
In commit "[test] Construct class to handle v2 P2P protocol functions"

It seems a bit strange to always send exactly 10 decoy packets before the version packet. Maybe randomize that too?
💬 sipa commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1419447076)
In commit "[test] Construct class to handle v2 P2P protocol functions"

It would be helpful to explain what the return value of the various functions in this class means. In several cases the return value seems to be bytes-that-should-be-sent-to-the-peer, but there are exceptions (-1 meaning downgrade for `respond_v2_handshake`, and `v2_ecdh` returning the shared secret).
💬 sipa commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1419502857)
In commit "[test] Construct class to handle v2 P2P protocol functions"

As I understand it, this function does not distinguish between "valid packet with 0-byte contents" and "decoy packet", which I believe is incorrect (the version packet can have 0-byte contents, but if it's a decoy, we should still listen for more packets). This isn't caught because Bitcoin Core's V2Transport implementation never sends decoys, but if it did, I believe the tests would break.

My suggestion would be to retu
...
💬 sipa commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1419509492)
In commit "[test] Construct class to handle v2 P2P protocol functions"

I don't think we should be hardcoding this assumption. Decoy packets with 0-byte contents are very much correct and possibly useful. See my other comment on `v2_receive_packet` about distinguishing decoys from 0-byte content packets.
💬 achow101 commented on pull request "doc: add historical release notes for 26.0":
(https://github.com/bitcoin/bitcoin/pull/29023#issuecomment-1846005450)
ACK ca5937553b4b4dde53995d0b66e30150401023eb
🚀 achow101 merged a pull request: "doc: add historical release notes for 26.0"
(https://github.com/bitcoin/bitcoin/pull/29023)
💬 brunoerg commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1846105723)
Force-pushed addressing ASMap health check. cc: @fjahr
💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1846169419)
I understand your argument.

However, now `OP_RETURN`, baremultisig, and other flaws such as `OP_0` `OP_IF`, are used to spam the chain. So what do you think we should do? Let them spam the chain by what we can't give them a toy on which to spam?
🤔 achow101 reviewed a pull request: "wallet, rpc: add anti-fee-sniping to `send` and `sendall`"
(https://github.com/bitcoin/bitcoin/pull/28944#pullrequestreview-1771199234)
ACK f348eadadd6099e7763f04eb3d42cfd8bba33146
💬 achow101 commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1419702642)
In f348eadadd6099e7763f04eb3d42cfd8bba33146 "test: test sendall and send do anti-fee-sniping"

nit: `verbose` here too.
💬 ajtowns commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1846204744)
> > What does this mean? I'm seeing file sizes of 448MB (with fuzzing enabled) and 146MB (with a normal build) for test/fuzz/fuzz.
>
> If you build this pull request you should see a binary per fuzz harness in `src/test/fuzz` (e.g. `src/test/fuzz/process_message`), as well as the usual fuzz binary `src/test/fuzz/fuzz`. If you sum up the size of the individual per harness binaries (assuming you compile with LTO) you should end up with roughly 3.6GB (maybe this depends on compiler version etc.)
...
⚠️ prusnak opened an issue: "DB_RUNRECOVERY: Fatal error, run database recovery"
(https://github.com/bitcoin/bitcoin/issues/29026)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

We are experiencing the build failures in NixPkgs both on x86_64-darwin and aarch64-darwin:

```
> 2023-12-07T14:45:33.311681Z [test] [wallet/bdb.cpp:189] [Open] BerkeleyEnvironment::Open: Error -30974 opening database environment: DB_RUNRECOVERY: Fatal error, run database recovery
> make[3]: *** [Makefile:22524: wallet/test/wallet_tests.cpp.test] Error 1
```

The full
...
🤔 ishaanam reviewed a pull request: "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction"
(https://github.com/bitcoin/bitcoin/pull/25273#pullrequestreview-1771245748)
ACK 1d1a6ff03215cf3aadd5fd1118f354b1ba8ffa5a
💬 1440000bytes commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1846220961)
Approach NACK

1. `DatacarrierBytes()` will need an update every few weeks as explained by @Sjors in https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1844981799 and could affect some other use cases.
2. `datacarriersize` is used for OP_RETURN and should not be mixed with other things. We should remove default limits for OP_RETURN because it does not affect UTXO set and there is no witness discount involved.
3. I do not see any issues with inscriptions. Neither they are spam nor an
...
💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1846256921)
Updated 0e647454b49274ee82978c06b606f3fd9c02b779 -> 778c0214d25e5a012c61251d592257ae19805255 ([`pr/rmutil.1`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.1) -> [`pr/rmutil.2`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.1..pr/rmutil.2)) fixing CI errors from forgitting to add new files to git

re: https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1844180519

> > Motivation for this change is t
...