💬 glozow commented on pull request "test: fix anti-fee-sniping off-by-one error":
(https://github.com/bitcoin/bitcoin/pull/33118#discussion_r2248749935)
Trailing whitespace

🤔 glozow reviewed a pull request: "test: fix anti-fee-sniping off-by-one error"
(https://github.com/bitcoin/bitcoin/pull/33118#pullrequestreview-3080386253)
You could also `assert_greater_than_or_equal`?

💬 ishaanam commented on pull request "test: fix anti-fee-sniping off-by-one error":
(https://github.com/bitcoin/bitcoin/pull/33118#discussion_r2248761466)
Fixed

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2248763843)
I didn't use this patch, but I put some effort into making the code more readable.

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2248764330)
Thanks for this. I've updated the comment.

💬 murchandamus commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248767523)
Thanks for the clarification and adding it to the commit message.

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3145623139)
> Mostly happy with [d82dcf2](https://github.com/bitcoin/bitcoin/commit/d82dcf2eaa7efb3809ae559c8f97f74403a2ccd6), but in [b32c2b2](https://github.com/bitcoin/bitcoin/commit/b32c2b290f8e4dcb7fe54ee4c81714a3785920a7) _descriptor: ToPrivateString() pass if at least 1 priv key exists_ I find the miniscript change hard to follow (mostly because I find the miniscript code itself hard to follow). Can you split that out in a seperate commit that doesn't change behaviour?

I split the original commit
...

💬 l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33094#issuecomment-3145625422)
Thanks, rebased to solve conflict in `validations.cpp` and migrated a few new values - thanks for the hint @janb84.
You can re-review via `git range-diff master 71911a59 53461dc5`

💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2248786530)
`has_priv_key` is only relevant when `ToPrivateString` is called on an actual Descriptor instance. IIUC, this test doesn't do that, so the value of `has_priv_key` does not matter here.
I'm not sure we should set it here, apart from the fact that it doesn't affect the test, the string returned doesn't have a private key.

💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3145748442)
> I plan to test this soon(tm).

Thanks @Sjors !

💬 pablomartin4btc commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2248869900)
We have the assert above so we don't need to check if it's a descriptor and legacy can't be created anymore.
```suggestion
walletInstance->SetupDescriptorScriptPubKeyMans();
```

💬 pablomartin4btc commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2248871373)
Please consider #32977

📝 Christewart opened a pull request: "doc: Fix 'getdescriptoractivity' RPCHelpMan"
(https://github.com/bitcoin/bitcoin/pull/33119)
Fixes bug in `getdescriptoractivity` RPC help manual.

Here is the line that pushes `spend_vin` field, there is no `spend_vout` json field.

https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2757

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-3145843454)
I've rebased to account for the recent `GenTxid` changes by @marcofleon.

I've also decided to clean the design while I was at it, to make things easier to follow and test. As part of this, the file structure has been changed from two files (`txreconciliation.h` and `txreconciliation.cpp`) to three (`txreconciliation.h`, `txreconciliation_impl.h` and `txreconciliation_impl.cpp`), following a similar approach as `txdownloadman`.

The code is split as follows:

- `txreconciliation.h` contain
...

📝 TheTyg opened a pull request: "rpc auth .py"
(https://github.com/bitcoin/bitcoin/pull/33120)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 pablomartin4btc commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#issuecomment-3145863988)
Shouldn't `src/wallet/walletdb.cpp` be part of this PR? `DBErrors WalletBatch::LoadWallet(CWallet* pwallet)` is being called during wallet creation I think. If so, please also consider the **note** at #33082's description.

💬 ryanofsky commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2248934018)
In commit "interfaces: expose UTXO snapshot loading in node interface" (de11e3b89f8aba9a90a3419086807e5fee0f941d)

Note with #10102 if the GUI & node are running in separate processes, it could be a awkward for the gui process to create an AutoFile object and pass it to the node like this. Technically it could probably work: the AutoFile could be passed as a file descriptor or a (path, position) tuple. But it doesn't seem like a clean separation of responsibilities if the GUI process is readin
...

💬 pstratem commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3145893262)
We're calling CBlockHeader::GetHash hundreds of times for the same CBlockHeader object from ActivateBestChain.

I chased the calls with this commit and found all the results where from ProcessNewBlock ActivateBestChain

https://github.com/pstratem/bitcoin/commit/7de23309167d3f713f90cc552e1bd431464e4c49

📝 mzumsande opened a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121)
This fixes two issues with `p2p_leak_tx.py`:

1.) #33090: As far as I can see, this is just the randomness of `NextInvToInbounds`/ `rand_exp_duration`, which has a probability of `e^-(60s/5s) = 6.14×10^−6` to result in a period > 60s (our waiting time), so that the test would fail every 160k runs... Doubling the timeout should be sufficient to lower the probability drastically.

2.) The subtest `test_notfound_on_unannounced_tx` has some (w)txid confusion: we send a `MSG_TX`-type getdata with
...

💬 mzumsande commented on issue "test: spurious failure in p2p_leak_tx.py --v1transport":
(https://github.com/bitcoin/bitcoin/issues/33090#issuecomment-3145915708)
See #33121 for a fix and an explanation attempt.