💬 jonatack commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#issuecomment-1788065706)
Concept ACK.

(Maybe unrelated, but the Win64 CI task is one I've often wished for more debug info from when a unit test fails.)

💬 kevkevinpal commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#discussion_r1378227825)
why do we only have the first few digits here, I tried with `046d6b657901000000` and it worked fine

`assert_equal(all([not line == "046d6b657901000000" for line in f]), True)`

💬 jonatack commented on pull request "build: remove duplicate `-lminiupnpc` linking":
(https://github.com/bitcoin/bitcoin/pull/28755#issuecomment-1788174027)
ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18

Build output diff is the same as my previous https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1705089533, and with the latest push also see a similar improvement as reported by @hebasto in https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1706070279:

master
```
checking for miniupnpc/miniupnpc.h... yes
checking for upnpDiscover in -lminiupnpc... yes
checking for miniupnpc/upnpcommands.h... yes
checking for upnpD
...

💬 ajtowns commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378307864)
Could alternatively make these member functions of `Package` ?

👍 pablomartin4btc approved a pull request: "[do not merge] validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-1707472149)
tACK 0800be24bc6de4154653be75b8fa9902a4a8c3a7

```
./contrib/devtools/utxo_snapshot.sh 800000 - ./src/bitcoin-cli -datadir=${AU_DATADIR}
Rewinding chain back to height 800000 (by invalidating 00000000000000000000e26b239cf19ec7ace5edd9694d51a3f6933247720947); this may take a while
Generating txoutset info...
{
"height": 800000,
"bestblock": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
"txouts": 111535121,
"bogosize": 8443325590,
"hash_serialized_3": "6e
...

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1788285499)
I would expect that a wallet that connects to us over (untrusted) p2p will get the exact same info regardless of background validation progress.

💬 Sjors commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1788287616)
@pablomartin4btc thanks for checking that you get the same snapshot. In addition it would be good to test if you're able to load the snapshot and complete the background sync.

💬 achow101 commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#discussion_r1378328199)
This prefix matches all possible `mkey` records so it can make sure that there are no `mkey`s slipping in elsewhere.

💬 pablomartin4btc commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1788298888)
> @pablomartin4btc thanks for checking that you get the same snapshot. In addition it would be good to test if you're able to load the snapshot and complete the background sync.

I've already checked IBD completed fine with`[background validation]`finished, but with the previous`hash_serialised`, as I wanted to verify that #28698 was only happening before the IBD completion.

I'll run all the process again with the new`hash_serialised`value.

💬 toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788341767)
https://github.com/bitcoin/bitcoin/issues/12115
<img width="921" alt="image" src="https://github.com/bitcoin/bitcoin/assets/21998212/eb9c1ef3-bb33-4600-a5d7-f9fd0dcddfbe">

@achow101 @maflcko @willcl-ark @gregwebs I saw that this issue was raised in 2018, but no attention was paid to it

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378361925)
I think the usage of virtual size to build the ancestor score of a package might come with incentive suboptimal block template if you have a significant number of segwit spends constituting it (sounds 90% as of 2023). A 500 vbyte transaction A paying 100 sats with 100 vbytes of witnesses isn’t equivalent to a 500 vbyte transaction B paying 100 sats with 20 vbytes of witnesses considering block max size is checked against `MAX_BLOCK_WEIGHT` in `CheckBlock()`.

🤔 ariard reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1707520236)
Review at commit “[txpackages] add AncestorPackage for linearizing packages”.

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378371630)
If my understanding of the guarantees of the `PackageEntry` comparison operator is correct, a breakage of the guarantee would be a yield `AncestorPackage` that produce a consensus-invalid block template, e.g a more incentive-compatible subset of descendants where the parents are sorted after. This comment to understand how test coverage should be built for this PR.

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378372594)
I don’t know if it has been discussed before in the review of this PR, though in case of a tie sounds the breaker should not be on the absolute number of in-package ancestor though on their accumulated vsize ? E.g package entry A might have 1 ancestor of 1 kvB and package entry B might have 3 ancestors of 200 vbytes each.

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378363626)
I think one limitation of the partial set of ancestors being used to evaluate the ancestor-score of a package _could_ be the edge-case where unconfirmed ancestors are replaced in the mempool by better same-nonwitness-data-in-mempool candidates, with increased or diminished witness size. This is not an issue right now as we don’t allow wtxid-replacement in `PreChecks()`.

💬 maflcko commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788505868)
Again, it is hard or impossible to fix your problem if it is wasn't properly diagnosed. The delay *could* be entirely unrelated to the wallet.

However, if the delay is caused by storage speed limitations in combination with the BDB flush behavior, a solution could be to migrate your wallet to sqlite. The (experimental) `migratewallet` RPC is only available in 25.x or the upcoming 26.x.

👋 maflcko's pull request is ready for review: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735)

💬 maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1788558389)
Taken out of draft. Future improvements to `chaincodelabs/libmultiprocess` can be done in the future.

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1378500641)
636d927c4a9cd5cbf4d0c22e1c25dc44ecaf7a69

Is it still fair to call this state *initial* block download?

📝 naumenkogs opened a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765)
Keep track of per-peer reconciliation sets containing transactions to be exchanged efficiently. The remaining transactions are announced via usual flooding.

Erlay Project Tracking: #28646

✅ naumenkogs closed a pull request: "p2p: Fill reconciliation sets and request reconciliation (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/26283)