💬 furszy commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2116475460)
> nit: on 2nd. commit (https://github.com/bitcoin-core/gui/commit/95aaaa4f023c6a6b629c36cfb9d8abd4bab1cb66), you forgot to remove the change from the makefile as it's not longer needed.
Removed. Thanks.
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2116475460)
> nit: on 2nd. commit (https://github.com/bitcoin-core/gui/commit/95aaaa4f023c6a6b629c36cfb9d8abd4bab1cb66), you forgot to remove the change from the makefile as it's not longer needed.
Removed. Thanks.
💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2116484602)
> There seems to be a conflict with #30098. Maybe rebase?
Thanks! Rebased now.
> If other reviewers or maintainers are not happy with introducing a bash script, I could volunteer some time to translate it.
Yes this could be a good idea. The script started out just being a few lines of bash listing symbols exported by one library and imported by another one. But then it grew as it was extended to support multiple libraries and suppress known errors. So now it's no longer a small script,
...
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2116484602)
> There seems to be a conflict with #30098. Maybe rebase?
Thanks! Rebased now.
> If other reviewers or maintainers are not happy with introducing a bash script, I could volunteer some time to translate it.
Yes this could be a good idea. The script started out just being a few lines of bash listing symbols exported by one library and imported by another one. But then it grew as it was extended to support multiple libraries and suppress known errors. So now it's no longer a small script,
...
💬 edilmedeiros commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116495430)
Newer version of bitcoin core use [descriptor wallets](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md).
You can see them using:
```
bitcoin-cli listdescriptors true
```
The `true` argument mean to export the private descriptors, the equivalent of private keys.
I usually use the tool `jq` to extract them to an environment variable like so:
```
DESCRIPTORS=$(bitcoin-cli listdescriptors true | jq -r .descriptors)
```
I import the descriptors to my wallet with:
```
bitcoin-cl
...
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116495430)
Newer version of bitcoin core use [descriptor wallets](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md).
You can see them using:
```
bitcoin-cli listdescriptors true
```
The `true` argument mean to export the private descriptors, the equivalent of private keys.
I usually use the tool `jq` to extract them to an environment variable like so:
```
DESCRIPTORS=$(bitcoin-cli listdescriptors true | jq -r .descriptors)
```
I import the descriptors to my wallet with:
```
bitcoin-cl
...
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1604259198)
done!
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1604259198)
done!
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1604259301)
done!
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1604259301)
done!
💬 ygcool commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116520822)
@edilmedeiros Hi, I tried and it still doesn't work, which version of bitcoin core still works with dumpprivkey?
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116520822)
@edilmedeiros Hi, I tried and it still doesn't work, which version of bitcoin core still works with dumpprivkey?
💬 edilmedeiros commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116524820)
V27 works with `dumpprivkey`.
The problem is that you are creating a newer descriptor wallet and are trying to use an rpc which is compatible with legacy wallets only.
Can you be more specific about what you tried?
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116524820)
V27 works with `dumpprivkey`.
The problem is that you are creating a newer descriptor wallet and are trying to use an rpc which is compatible with legacy wallets only.
Can you be more specific about what you tried?
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-2116530496)
Updates:
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1566860922) from @fjahr by amending the logging info description.
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1597147718) from @furszy by using a better RPC error code (`RPC_DESERIALIZATION_ERROR` instead of `RPC_INTERNAL_ERROR`).
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-2116530496)
Updates:
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1566860922) from @fjahr by amending the logging info description.
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1597147718) from @furszy by using a better RPC error code (`RPC_DESERIALIZATION_ERROR` instead of `RPC_INTERNAL_ERROR`).
✅ achow101 closed an issue: "dumpprivkey error"
(https://github.com/bitcoin/bitcoin/issues/30129)
(https://github.com/bitcoin/bitcoin/issues/30129)
💬 achow101 commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116532819)
As the error message says, your wallet is not compatible with the `dumpprivkey` command. @edilmedeiros has already laid out your alternatives, but there is no way to make your specific wallet work with `dumpprivkey`.
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116532819)
As the error message says, your wallet is not compatible with the `dumpprivkey` command. @edilmedeiros has already laid out your alternatives, but there is no way to make your specific wallet work with `dumpprivkey`.
💬 ygcool commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116537647)
thank.
Solved it:
`bitcoind -deprecatedrpc=create_bdb -daemon`
`bitcoin-cli -named createwallet "walletname" descriptors=false`
`bitcoin-cli getnewaddress`
`bitcoin-cli dumpprivkey "address"`
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116537647)
thank.
Solved it:
`bitcoind -deprecatedrpc=create_bdb -daemon`
`bitcoin-cli -named createwallet "walletname" descriptors=false`
`bitcoin-cli getnewaddress`
`bitcoin-cli dumpprivkey "address"`
🤔 tdb3 reviewed a pull request: "rpc: introduce getversion RPC"
(https://github.com/bitcoin/bitcoin/pull/30112#pullrequestreview-2062178448)
Concept ACK.
Great work. This seems like an improvement over whitelisting `getnetworkinfo` and reduces exposed info.
https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2114646750
> getrpcversion maybe? The reason i bring it up is that it needs to be restrictive enough in scope to just the RPC interface version, not wallet version, not P2P versions, etc.
I generally agree with this comment that the scope of this new RPC should be constrained where it makes sense. A more specif
...
(https://github.com/bitcoin/bitcoin/pull/30112#pullrequestreview-2062178448)
Concept ACK.
Great work. This seems like an improvement over whitelisting `getnetworkinfo` and reduces exposed info.
https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2114646750
> getrpcversion maybe? The reason i bring it up is that it needs to be restrictive enough in scope to just the RPC interface version, not wallet version, not P2P versions, etc.
I generally agree with this comment that the scope of this new RPC should be constrained where it makes sense. A more specif
...
💬 tdb3 commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#discussion_r1604241946)
Thinking out loud about this one: Are there instances where `long` would be insufficient for determining if the client/node is a release? In the examples provided in the opening post, it seems like this could be determined by lack of commit hash and rc designation. It definitely would be convenient for the RPC user to have a bool provided (and prevents burdening the user with extra parsing or interpretation), so I see value in keeping this field.
(https://github.com/bitcoin/bitcoin/pull/30112#discussion_r1604241946)
Thinking out loud about this one: Are there instances where `long` would be insufficient for determining if the client/node is a release? In the examples provided in the opening post, it seems like this could be determined by lack of commit hash and rc designation. It definitely would be convenient for the RPC user to have a bool provided (and prevents burdening the user with extra parsing or interpretation), so I see value in keeping this field.
💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-2116717090)
> I think it'd be better to have multiple classes that build from v2_p2p and modify what's needed, even if the file gets larger.
that's a great suggestion @sr-gi! it's possible to design those classes in such a way that it avoids code duplication and the file is almost the same size!
> Turns out if you removed both changes, the tests still passes, but I'm not sure why.
because we don't send a version message to ensure that the disconnection happens in the v2 handshake phase. in your ca
...
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-2116717090)
> I think it'd be better to have multiple classes that build from v2_p2p and modify what's needed, even if the file gets larger.
that's a great suggestion @sr-gi! it's possible to design those classes in such a way that it avoids code duplication and the file is almost the same size!
> Turns out if you removed both changes, the tests still passes, but I'm not sure why.
because we don't send a version message to ensure that the disconnection happens in the v2 handshake phase. in your ca
...
💬 josibake commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2116882706)
reACK https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2116882706)
reACK https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2117004469)
Added a (mostly untested for now) Windows implementation of `QueryDefaultGateway`, if someone could test this it'd be very helpful. i will try in WINE later.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2117004469)
Added a (mostly untested for now) Windows implementation of `QueryDefaultGateway`, if someone could test this it'd be very helpful. i will try in WINE later.
💬 josibake commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1604550196)
ah, my bad. I had even read that comment that @glozow linked to earlier but it didn't click for me that by the time we get to the block we've already processed transactions and excluded the child transaction. I think a comment explaining what you just explained here would help a lot, maybe at the call site of `removeParentTxs`?
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1604550196)
ah, my bad. I had even read that comment that @glozow linked to earlier but it didn't click for me that by the time we get to the block we've already processed transactions and excluded the child transaction. I think a comment explaining what you just explained here would help a lot, maybe at the call site of `removeParentTxs`?
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1604554237)
Done
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1604554237)
Done
👍 TheCharlatan approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062665306)
Re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062665306)
Re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a
✅ laanwj closed a pull request: "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps"
(https://github.com/bitcoin/bitcoin/pull/29959)
(https://github.com/bitcoin/bitcoin/pull/29959)