💬 achow101 commented on pull request "wallet, rpc, gui: List legacy wallets with a message about migration":
(https://github.com/bitcoin/bitcoin/pull/32619#discussion_r2107887370)
Indeed, done.
(https://github.com/bitcoin/bitcoin/pull/32619#discussion_r2107887370)
Indeed, done.
💬 adyshimony commented on pull request "wallet: Always set descriptor cache upgraded flag for new wallets":
(https://github.com/bitcoin/bitcoin/pull/32597#issuecomment-2910699910)
ACK
[47237cd](https://github.com/bitcoin/bitcoin/pull/32597/commits/47237cd1938058b29fdec242c3a37611e255fda0)
Manual tests:
```
./bitcoin-cli createwallet "test_32597_wallet" false false "" false true
{
"name": "test_32597_wallet",
"warnings": [
"Empty string given as passphrase, wallet will not be encrypted."
]
}
./bitcoin-cli -rpcwallet=test_32597_wallet getwalletinfo
{
"walletname": "test_32597_wallet",
"walletversion": 169900,
"format": "sqlite
...
(https://github.com/bitcoin/bitcoin/pull/32597#issuecomment-2910699910)
ACK
[47237cd](https://github.com/bitcoin/bitcoin/pull/32597/commits/47237cd1938058b29fdec242c3a37611e255fda0)
Manual tests:
```
./bitcoin-cli createwallet "test_32597_wallet" false false "" false true
{
"name": "test_32597_wallet",
"warnings": [
"Empty string given as passphrase, wallet will not be encrypted."
]
}
./bitcoin-cli -rpcwallet=test_32597_wallet getwalletinfo
{
"walletname": "test_32597_wallet",
"walletversion": 169900,
"format": "sqlite
...
📝 theStack opened a pull request: "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs"
(https://github.com/bitcoin/bitcoin/pull/32621)
This PR is a late follow-up to https://github.com/bitcoin/bitcoin/pull/27432, introducing an option for the utxo-to-sqlite script to store the txid/scriptPubKey columns as bytes (= `BLOB` storage class in sqlite, see e.g. https://www.sqlite.org/datatype3.html in sqlite) rather than hex strings. This was proposed in earlier reviews (https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1516857024, https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1653739351) and has the obvious adva
...
(https://github.com/bitcoin/bitcoin/pull/32621)
This PR is a late follow-up to https://github.com/bitcoin/bitcoin/pull/27432, introducing an option for the utxo-to-sqlite script to store the txid/scriptPubKey columns as bytes (= `BLOB` storage class in sqlite, see e.g. https://www.sqlite.org/datatype3.html in sqlite) rather than hex strings. This was proposed in earlier reviews (https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1516857024, https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1653739351) and has the obvious adva
...
💬 Krellan commented on pull request "gui: Add a menu action to restore then migrate a legacy wallet":
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2910981771)
Good idea. I had this happen to me just recently. I encountered a Catch-22: couldn't restore an old wallet file because it wasn't in a currently recognized format (it told me to use the migrate wallet option instead), but I also couldn't access the wallet in the migrate menu because it hadn't successfully been loaded yet!
I tried manually copying the `wallet.dat` file into a new directory created in my Bitcoin datadir, but that just produced an error at startup saying the wallet was not in a
...
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2910981771)
Good idea. I had this happen to me just recently. I encountered a Catch-22: couldn't restore an old wallet file because it wasn't in a currently recognized format (it told me to use the migrate wallet option instead), but I also couldn't access the wallet in the migrate menu because it hadn't successfully been loaded yet!
I tried manually copying the `wallet.dat` file into a new directory created in my Bitcoin datadir, but that just produced an error at startup saying the wallet was not in a
...
💬 Krellan commented on pull request "Including exception what() in Runaway dialog box":
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-2910984899)
As for the CI, it failed the Windows build because the URL https://github.com/boostorg/move/archive/boost-1.87.0.tar.gz: is no longer found. It's not related to this pull request.
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-2910984899)
As for the CI, it failed the Windows build because the URL https://github.com/boostorg/move/archive/boost-1.87.0.tar.gz: is no longer found. It's not related to this pull request.
💬 Krellan commented on pull request "Including exception what() in Runaway dialog box":
(https://github.com/bitcoin-core/gui/pull/876#discussion_r2108084452)
Good catch, made "what" a const ref. The string "message" is unchanged because it gets modified later in the function.
(https://github.com/bitcoin-core/gui/pull/876#discussion_r2108084452)
Good catch, made "what" a const ref. The string "message" is unchanged because it gets modified later in the function.
💬 Krellan commented on pull request "Including exception what() in Runaway dialog box":
(https://github.com/bitcoin-core/gui/pull/876#discussion_r2108084527)
Good catch, made "what" a const ref. The string "message" is unchanged because it gets modified later in the function.
(https://github.com/bitcoin-core/gui/pull/876#discussion_r2108084527)
Good catch, made "what" a const ref. The string "message" is unchanged because it gets modified later in the function.
💬 Krellan commented on pull request "Including exception what() in Runaway dialog box":
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-2911011903)
> Have you verified that we won't output the same error message twice?
I looked at the `Warning` class, and looked at its usages, and the messages that are passed in differ from the text that would arise if an exception were to be thrown (they come from different areas of the code).
(https://github.com/bitcoin-core/gui/pull/876#issuecomment-2911011903)
> Have you verified that we won't output the same error message twice?
I looked at the `Warning` class, and looked at its usages, and the messages that are passed in differ from the text that would arise if an exception were to be thrown (they come from different areas of the code).
💬 ajtowns commented on pull request "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs":
(https://github.com/bitcoin/bitcoin/pull/32621#issuecomment-2911074418)
> The approach taken is introducing a `-m`/`--mode` parameter which can either have the values "hex" (default) or "bytes". Happy to take suggestions on naming and thoughts on future extensibility etc.
I wonder if separate options would be better, eg `--spk=raw --txid=raw`?
> I think there is no ambiguity on byte-string-serialization of txids; they are ultimately just hash results and hence, they should be stored as such, and adding a big/little endian knob wouldn't make much sense.
I t
...
(https://github.com/bitcoin/bitcoin/pull/32621#issuecomment-2911074418)
> The approach taken is introducing a `-m`/`--mode` parameter which can either have the values "hex" (default) or "bytes". Happy to take suggestions on naming and thoughts on future extensibility etc.
I wonder if separate options would be better, eg `--spk=raw --txid=raw`?
> I think there is no ambiguity on byte-string-serialization of txids; they are ultimately just hash results and hence, they should be stored as such, and adding a big/little endian knob wouldn't make much sense.
I t
...
💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-2911077066)
Concept ACK. LLM linter's punctuation advice seems right to me. :)
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-2911077066)
Concept ACK. LLM linter's punctuation advice seems right to me. :)
💬 encloinc commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2911095927)
Concept ACK, arbitrary restrictions won't stop how people are currently using the network already.
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2911095927)
Concept ACK, arbitrary restrictions won't stop how people are currently using the network already.
⚠️ ou-tai opened an issue: "bitcoin core初次打开无法生成新的收款地址"
(https://github.com/bitcoin/bitcoin/issues/32622)
尝试过了重启客户端,切换界面等多种方法无法解决。
只有通过控制台get new address命令取得新地址后才能在收款界面创建新地址。
(https://github.com/bitcoin/bitcoin/issues/32622)
尝试过了重启客户端,切换界面等多种方法无法解决。
只有通过控制台get new address命令取得新地址后才能在收款界面创建新地址。
💬 maflcko commented on pull request "wallet, rpc, gui: List legacy wallets with a message about migration":
(https://github.com/bitcoin/bitcoin/pull/32619#issuecomment-2911221570)
lgtm ACK f3a444c45fb4bf4e51d53ebf1cf4c2631ded481c
(https://github.com/bitcoin/bitcoin/pull/32619#issuecomment-2911221570)
lgtm ACK f3a444c45fb4bf4e51d53ebf1cf4c2631ded481c
💬 maflcko commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2911226436)
lgtm ACK 9ee8b7f278627b917f0784adf23cbc76cae5fa0
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2911226436)
lgtm ACK 9ee8b7f278627b917f0784adf23cbc76cae5fa0
✅ achow101 closed an issue: "bitcoin core初次打开无法生成新的收款地址"
(https://github.com/bitcoin/bitcoin/issues/32622)
(https://github.com/bitcoin/bitcoin/issues/32622)
💬 maflcko commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2106681301)
nit: can remove the casts
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2106681301)
nit: can remove the casts
💬 maflcko commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2108280953)
I wonder if we really want to commit to the storage serialization on the rest interface. Currently in this pull, it will be re-serialized, so there is no dependency. However, with this suggestion, and a future change in the storage serialization, there will be a breaking change on the rest interface, or alternatively a re-serialization again (back to what this pull is doing).
No strong opinion, just mentioning it here.
Maybe a benchmark could help to decide if it is worth it?
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2108280953)
I wonder if we really want to commit to the storage serialization on the rest interface. Currently in this pull, it will be re-serialized, so there is no dependency. However, with this suggestion, and a future change in the storage serialization, there will be a breaking change on the rest interface, or alternatively a re-serialization again (back to what this pull is doing).
No strong opinion, just mentioning it here.
Maybe a benchmark could help to decide if it is worth it?
💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2108287018)
> GHA
I don't know about GHA, but the Cirrus tasks require manual setup, see https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/.cirrus.yml#L8.
(Closing conversation for now)
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2108287018)
> GHA
I don't know about GHA, but the Cirrus tasks require manual setup, see https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/.cirrus.yml#L8.
(Closing conversation for now)
🤔 furszy reviewed a pull request: "wallet, rpc, gui: List legacy wallets with a message about migration"
(https://github.com/bitcoin/bitcoin/pull/32619#pullrequestreview-2869966132)
Code review ACK f3a444c45fb4bf4e51d53ebf1cf4c2631ded481c
(https://github.com/bitcoin/bitcoin/pull/32619#pullrequestreview-2869966132)
Code review ACK f3a444c45fb4bf4e51d53ebf1cf4c2631ded481c
📝 maflcko opened a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623)
The subtree docs and lint checks list the subtrees in three places, making it hard to follow and update and easy to miss one.
Fix all issues by including the missing one and removing the list in one place.
(https://github.com/bitcoin/bitcoin/pull/32623)
The subtree docs and lint checks list the subtrees in three places, making it hard to follow and update and easy to miss one.
Fix all issues by including the missing one and removing the list in one place.