Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 Sjors commented on pull request "[DO NOT MERGE] testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2070283429)
> it breaks the use case of someone testing being able to mine a block on-demand without actual mining hardware

I doubt many people do that. You can still set `nProofOfWorkLimit` to a lower value. We could add a code comment for that (or a setting). That way you can mine locally as fast as you want, without causing mayhem for others.
💬 hebasto commented on pull request "doc: suggest only necessary Qt packages for installation on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/29932#discussion_r1575115643)
1. Why `qt5-qmake`?

2. The build system (`build-aux/m4/bitcoin_qt.m4`) checks for the `qt5-network` and `qt-dbus` packages, which both are dependencies of the `qt5-gui`. However, only `qt5-network` is listed explicitly. For consistency sake, I suggest to list either both packages or none.
💬 mzumsande commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-2070355480)
re-ACK fd81a37239541d0d508402cd4eeb28f38128c1f2 (just looked at diff, only small logging change)
💬 mjdietzx commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-2070481115)
Thank you for the thorough review @achow101! I'll update this PR addressing all of your comments and ping you when I've pushed (I'll need to find a chunk of time to do this so it may be a week or two)
💬 sr-gi commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2070486303)
Rebased to address merge conflicts
💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2070491742)
Rebased to fix merge conflicts
🤔 mzumsande reviewed a pull request: "bugfix: update chainman best_header after block reconsideration"
(https://github.com/bitcoin/bitcoin/pull/29913#pullrequestreview-2015537357)
IIn my opinion setting it to `ActiveTip()` is not correct because `m_best_header` was not necessarily equal to `ActiveTip()` in the first place (before `invalidateblock`) :

During IBD they differ a lot (and `invalidateblock` / `resonsiderblock` is allowed then, too!). So after `resonsiderblock` it would still be set to the wrong value. I could verify that on signet via `getblockchaininfo` shortly after downloading the headers.

I think that the correct approach would be to iterate through h
...
⚠️ laanwj opened an issue: "guix: SOURCE_DATE_EPOCH is already set in some environments"
(https://github.com/bitcoin/bitcoin/issues/29935)
The environment variable `SOURCE_DATE_EPOCH` allows overriding the date that will be used inside the archives for guix-built binaries. This is an intentional feature, as documented in `contrib/guix/README.md`:

> * _**SOURCE_DATE_EPOCH**_
>
> Override the reference UNIX timestamp used for bit-for-bit reproducibility,
> the variable name conforms to [standard][r12e/source-date-epoch].
>
> _(defaults to the output of `$(git log --format=%at -1)`)_

However, some environments, as a
...
💬 hebasto commented on issue "guix: SOURCE_DATE_EPOCH is already set in some environments":
(https://github.com/bitcoin/bitcoin/issues/29935#issuecomment-2070764504)
> * Rename our `SOURCE_DATE_EPOCH` to something non-standard that doesn't conflict with Nix.

As we run Guix shell in a container, it seems reasonable to rename `SOURCE_DATE_EPOCH` in the `guix-build` script, and pass it to the container using its original name:
```
--env SOURCE_DATE_EPOCH="${BITCOIN_SOURCE_DATE_EPOCH:?unable to determine value}"
```
📝 brunoerg opened a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936)
This PR adds a fuzz target for the `CreateTransaction` function. It is a regression target for https://github.com/bitcoin/bitcoin/pull/27271 and can be testing by applying:
```diff
@@ -1110,7 +1110,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN)
// and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways

...
💬 brunoerg commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2070810257)
Just added https://github.com/bitcoin/bitcoin/pull/29936 to the list. It's a regression to #27271.
💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1575302127)
Good catch, fixed
👍 brunoerg approved a pull request: "rpc: Reword SighashFromStr error message"
(https://github.com/bitcoin/bitcoin/pull/29870#pullrequestreview-2015715112)
utACK fa6ab0d020d0b1492203f7eb2ccb8051812de086
💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2070869416)
For testing big endian wallets, you can use `db_dump wallet.dat | db_load -c db_lorder=4321 wallet_big_endian.dat` to make big endian databasese using BDB's tools.

However, I also realized that BDB lets us set the endianness to use for a new database, so I've added a `bdb_swap` format to `createfromdump` that will make a BDB database with the other endianness. This will flip it depending on the system that you use, so on little endian, it will make a big endian wallet, and on big endian, it w
...
💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1575308063)
> Is this used in this pull request?

No, it's used in the followup.

> I wonder if it could make sense to return the error string to the caller, assuming this is called by RPC eventually?

`Backup` is a part of the `WalletDatabase` parent class, so changing this would also mean changing the `Backup` of existing database classes, and all of their callers. I think that's something that would be worth doing in a followup, but not this PR.
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2070973893)
cc: @maflcko
💬 ariard commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2070974638)
> Still, what are TRUCs ? Do you have documentation ?

TRUC = Topologically Restricted Until Confirmation, see https://github.com/bitcoin/bips/pull/1541
💬 achow101 commented on issue "Release schedule for 27.0":
(https://github.com/bitcoin/bitcoin/issues/29028#issuecomment-2071021674)
@levantah Contributions are certainly welcome and desired, you can take a look at any of the open issues, particularly ones tagged as "Good first issue". However, as Bitcoin Core is security software, we only want to link to our own official sources for releases. While we appreciate the gesture you did by hosting a mirror of the release, it is still something we cannot point people to, and for the safety of our users, the comment is removed. While I don't doubt that you are only trying to help,
...
💬 achow101 commented on pull request "Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type":
(https://github.com/bitcoin/bitcoin/pull/29657#issuecomment-2071028198)
ACK c3e632b44153e314ef946f342c68c2758b1cbc4d
🚀 achow101 merged a pull request: "Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type"
(https://github.com/bitcoin/bitcoin/pull/29657)