💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248721755)
Now that there is `send` and `sendall` support, is it just transactions received by the wallet?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248694635)
9b478f1f68c00b5c514a30a53d19cd58fe813795

integers can just be passed by value

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248729121)
Missing test coverage for this condition I think? Need `user_input_weight_not_overwritten` test for something spending unconfirmed v3

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2248722112)
Maybe mention TRUC and/or BIP 431?

📝 ishaanam opened a pull request: "test: fix anti-fee-sniping off-by-one error"
(https://github.com/bitcoin/bitcoin/pull/33118)
This fixes the off-by-one error in the anti-fee-sniping tests for `send` and `sendall`. `assert_greater_than` fails if the two values are equal.

Closes #33114

💬 glozow commented on issue "intermittent issue in wallet_sendall.py", line 440, in sendall_anti_fee_sniping assert_greater_than(tx_from_wallet["decoded"]["locktime"], tx_from_wallet["blockheight"]":
(https://github.com/bitcoin/bitcoin/issues/33114#issuecomment-3145590340)
#33118

💬 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
...