🤔 instagibbs reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2015296846)
reviewed through 7d220c6a5c0e0c5e8cfe79ebd2eae6e845d1d983

tested and confirmed fuzz coverage is hitting meaningful `GetChildrenFrom*` results

continuing longer range testing

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1575059642)
nit
```Suggestion
BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent1, node2).empty());
BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent2, node2).empty());
```

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1575061630)
nit
```Suggestion
// There shouldn't be any children of this tx in orphanage
```

💬 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