📝 dimitaracev opened a pull request: "wallet: add meaningful error message and fix test"
(https://github.com/bitcoin/bitcoin/pull/29143)
Fixes https://github.com/bitcoin/bitcoin/issues/29073
Calling `unloadwallet` first marks that wallet for unloading and then unloads it. If the unloading does not finish in time and `unloadwallet` is called again for the same wallet an assertion error is thrown `wallet/wallet.cpp:246: void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertionit.second` which is not descriptive. The test in issue https://github.com/bitcoin/bitcoin/issues/29073 is sometimes failing due to that assertion. Th
...
(https://github.com/bitcoin/bitcoin/pull/29143)
Fixes https://github.com/bitcoin/bitcoin/issues/29073
Calling `unloadwallet` first marks that wallet for unloading and then unloads it. If the unloading does not finish in time and `unloadwallet` is called again for the same wallet an assertion error is thrown `wallet/wallet.cpp:246: void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertionit.second` which is not descriptive. The test in issue https://github.com/bitcoin/bitcoin/issues/29073 is sometimes failing due to that assertion. Th
...
💬 dimitaracev commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869707247)
cc @maflcko @fanquake
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869707247)
cc @maflcko @fanquake
💬 brunoerg commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869709929)
```sh
test/functional/wallet_multiwallet.py:38:9: F841 local variable 'e' is assigned to but never used
test/functional/wallet_multiwallet.py:39:50: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:39:104: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:40:63: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:40:130: F821 undefined name 'e'
```
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869709929)
```sh
test/functional/wallet_multiwallet.py:38:9: F841 local variable 'e' is assigned to but never used
test/functional/wallet_multiwallet.py:39:50: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:39:104: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:40:63: F821 undefined name 'e'
test/functional/wallet_multiwallet.py:40:130: F821 undefined name 'e'
```
✅ 22388o closed a pull request: "doc: upgrade Bitcoin Core license to 2024"
(https://github.com/bitcoin/bitcoin/pull/29142)
(https://github.com/bitcoin/bitcoin/pull/29142)
🤔 furszy reviewed a pull request: "wallet: add meaningful error message and fix test"
(https://github.com/bitcoin/bitcoin/pull/29143#pullrequestreview-1796616389)
How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
(https://github.com/bitcoin/bitcoin/pull/29143#pullrequestreview-1796616389)
How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
📝 furszy opened a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144)
Small and simple issue reported [here](https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144).
Instead of throwing the 'Unable to parse settings file' error when the file is just empty.
We can continue execution as if nothing happened.
(https://github.com/bitcoin/bitcoin/pull/29144)
Small and simple issue reported [here](https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144).
Instead of throwing the 'Unable to parse settings file' error when the file is just empty.
We can continue execution as if nothing happened.
💬 luke-jr commented on pull request "test: Add test case for speding bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1869811438)
Title has a typo. Maybe should test that it works with both values for `-permitbaremultisig`?
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1869811438)
Title has a typo. Maybe should test that it works with both values for `-permitbaremultisig`?
💬 dimitaracev commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869811694)
> How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
From my understanding, the statements
```c++
auto it = g_unloading_wallet_set.insert(name);
assert(it.second);
```
assume that the wallet will always be correctly inserted into `g_unloading_wallet_set` where `g_unloading_wallet_set.insert(name)` returns a pair (`it.second` is 1 if the wallet is successfully inserted into the set to be used by `R
...
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869811694)
> How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
From my understanding, the statements
```c++
auto it = g_unloading_wallet_set.insert(name);
assert(it.second);
```
assume that the wallet will always be correctly inserted into `g_unloading_wallet_set` where `g_unloading_wallet_set.insert(name)` returns a pair (`it.second` is 1 if the wallet is successfully inserted into the set to be used by `R
...
💬 luke-jr commented on pull request "Use hardened runtime on macOS release builds.":
(https://github.com/bitcoin/bitcoin/pull/29127#issuecomment-1869813403)
>The release maintainer, or any authorized Apple Developer, will need to run xcrun notarytool to prevent gatekeeper warnings on macOS.
Isn't that just to fully close #15774? In other words, this PR seems like a strict step forward even if we don't go the rest of the way?
> Using xcrun staple to generate a binary that doesn't call home on first launch would be bonus, but at least this would massively improve the user experience.
I was under the impression that *not* doing the notary thin
...
(https://github.com/bitcoin/bitcoin/pull/29127#issuecomment-1869813403)
>The release maintainer, or any authorized Apple Developer, will need to run xcrun notarytool to prevent gatekeeper warnings on macOS.
Isn't that just to fully close #15774? In other words, this PR seems like a strict step forward even if we don't go the rest of the way?
> Using xcrun staple to generate a binary that doesn't call home on first launch would be bonus, but at least this would massively improve the user experience.
I was under the impression that *not* doing the notary thin
...
💬 luke-jr commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869813973)
Feels like this could be confused with real/beancounter accounts like we used to support years ago. Maybe `hd_account` or something?
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869813973)
Feels like this could be confused with real/beancounter accounts like we used to support years ago. Maybe `hd_account` or something?
💬 furszy commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869817009)
> > How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
>
> From my understanding, the statements
>
> ```c++
> auto it = g_unloading_wallet_set.insert(name);
> assert(it.second);
> ```
>
> assume that the wallet will always be correctly inserted into `g_unloading_wallet_set` where `g_unloading_wallet_set.insert(name)` returns a pair (`it.second` is 1 if the wallet is successfully inserted into
...
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869817009)
> > How is this possible if `RemoveWallet()` locks the context wallets' mutex, checks if the wallet is there (failing if not) and then removes it.
>
> From my understanding, the statements
>
> ```c++
> auto it = g_unloading_wallet_set.insert(name);
> assert(it.second);
> ```
>
> assume that the wallet will always be correctly inserted into `g_unloading_wallet_set` where `g_unloading_wallet_set.insert(name)` returns a pair (`it.second` is 1 if the wallet is successfully inserted into
...
💬 dimitaracev commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869823524)
@furszy Oh I see, will continue to look into this further. Thanks.
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1869823524)
@furszy Oh I see, will continue to look into this further. Thanks.
💬 ybaidiuk commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1869829376)
> Величезна кількість спаму, спричиненого написами в блокчейні біткойн, насправді викликає тривогу, його слід вирішувати комплексно та таким чином, щоб не завдати шкоди ринку BRC-20 або засобам існування людей.
>
> Деякі особи не враховують природу вразливості, вона не має нічого спільного з позбавленням від написів. Це пов’язано з перевантаженістю мережі, спричиненою такими типами транзакцій. Справа не лише у вищі комісії чи повільніший час обробки; ця ситуація створює серйозний ризик для ці
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1869829376)
> Величезна кількість спаму, спричиненого написами в блокчейні біткойн, насправді викликає тривогу, його слід вирішувати комплексно та таким чином, щоб не завдати шкоди ринку BRC-20 або засобам існування людей.
>
> Деякі особи не враховують природу вразливості, вона не має нічого спільного з позбавленням від написів. Це пов’язано з перевантаженістю мережі, спричиненою такими типами транзакцій. Справа не лише у вищі комісії чи повільніший час обробки; ця ситуація створює серйозний ризик для ці
...
📝 luke-jr opened a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us"
(https://github.com/bitcoin/bitcoin/pull/29145)
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
Like #28936
I've chosen a domain name that is explicitly verbose about its purpose and the possibility of malware on resolved IPs, to go an extra mile in helping avoid any attempts to abuse it.
(https://github.com/bitcoin/bitcoin/pull/29145)
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
Like #28936
I've chosen a domain name that is explicitly verbose about its purpose and the possibility of malware on resolved IPs, to go an extra mile in helping avoid any attempts to abuse it.
💬 luke-jr commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1869837128)
Note that the current domain name continues to resolve for now; I'm just preemptively doing this in case it becomes a bigger issue in the future.
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1869837128)
Note that the current domain name continues to resolve for now; I'm just preemptively doing this in case it becomes a bigger issue in the future.
💬 achow101 commented on pull request "Compressed Bitcoin Transactions":
(https://github.com/bitcoin/bitcoin/pull/29134#issuecomment-1869858505)
Is this intended to be used for storage, p2p relay, or something else? If for our storage, I would be interested in seeing benchmarks as this would have an effect on how quickly things can be pulled from disk to be relayed. If for relay, then I think there should first be a BIP that describes the protocol for using compression and how that is negotiated. And if neither of those, then I don't think it's within the scope of Bitcoin Core. It seems to be a pretty significant maintenance burden for s
...
(https://github.com/bitcoin/bitcoin/pull/29134#issuecomment-1869858505)
Is this intended to be used for storage, p2p relay, or something else? If for our storage, I would be interested in seeing benchmarks as this would have an effect on how quickly things can be pulled from disk to be relayed. If for relay, then I think there should first be a BIP that describes the protocol for using compression and how that is negotiated. And if neither of those, then I don't think it's within the scope of Bitcoin Core. It seems to be a pretty significant maintenance burden for s
...
💬 achow101 commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859432)
I think this might be a better candidate for a `createwalletdescriptor` (#29130) equivalent for external signers rather than continuing to jam more arguments into `createwallet`.
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859432)
I think this might be a better candidate for a `createwalletdescriptor` (#29130) equivalent for external signers rather than continuing to jam more arguments into `createwallet`.
💬 brunoerg commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859555)
> Feels like this could be confused with real/beancounter accounts like we used to support years ago. Maybe hd_account or something?
I agree, had same thought. Just changed in latest push.
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859555)
> Feels like this could be confused with real/beancounter accounts like we used to support years ago. Maybe hd_account or something?
I agree, had same thought. Just changed in latest push.
💬 brunoerg commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869860600)
> I think this might be a better candidate for a createwalletdescriptor (https://github.com/bitcoin/bitcoin/pull/29130) equivalent for external signers rather than continuing to jam more arguments into createwallet.
You mean have a specific RPC for external signers or extending `createwalletdescriptor`?
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869860600)
> I think this might be a better candidate for a createwalletdescriptor (https://github.com/bitcoin/bitcoin/pull/29130) equivalent for external signers rather than continuing to jam more arguments into createwallet.
You mean have a specific RPC for external signers or extending `createwalletdescriptor`?
🤔 ryanofsky reviewed a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1796742782)
Updated c80bd16c826d564086006cff67b7b9ade9b9f38c -> 91dc48c14825a9075a57c1eefda202b83b6346ba ([`pr/ipcdoc.10`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.10) -> [`pr/ipcdoc.11`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipcdoc.10..pr/ipcdoc.11)) fixing table tag and reference heading.
I think this PR gets another ACK it could be merged to add the new documentation, and improvements could be implemented in
...
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1796742782)
Updated c80bd16c826d564086006cff67b7b9ade9b9f38c -> 91dc48c14825a9075a57c1eefda202b83b6346ba ([`pr/ipcdoc.10`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.10) -> [`pr/ipcdoc.11`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipcdoc.10..pr/ipcdoc.11)) fixing table tag and reference heading.
I think this PR gets another ACK it could be merged to add the new documentation, and improvements could be implemented in
...