💬 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3059330260)
> Why?
Padding (not margin) has been removed because it is unnecessary and visually disruptive. Padding serves no purpose and only worsens the layout.
> Bitcoin Core has its own logo for years. I don't see your reference as a justification for this change.
I discussed this PR with @jonasschnelli on IRC a few days ago. As the creator of the original logo files, he understands the rationale behind replacing them.
> I believe this constitutes a misuse of the Bitcoin Core codebase.
This assumpt
...
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3059330260)
> Why?
Padding (not margin) has been removed because it is unnecessary and visually disruptive. Padding serves no purpose and only worsens the layout.
> Bitcoin Core has its own logo for years. I don't see your reference as a justification for this change.
I discussed this PR with @jonasschnelli on IRC a few days ago. As the creator of the original logo files, he understands the rationale behind replacing them.
> I believe this constitutes a misuse of the Bitcoin Core codebase.
This assumpt
...
💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#issuecomment-3059542310)
I'll take a closer look at this, although some quick feedback since I've implemented something similar.
> This fuzz test compares a brute force search with the BnB result to ensure that BnB succeeds.
I recently removed a similar automated test that used brute force. The issue with using a brute solution I had was a) brute force is slow b) only works on a limited pool size of say 20 UTXOs (see a). Instead of using brute force, I adopted a strategy where I generate two random UTXO sets. T
...
(https://github.com/bitcoin/bitcoin/pull/32894#issuecomment-3059542310)
I'll take a closer look at this, although some quick feedback since I've implemented something similar.
> This fuzz test compares a brute force search with the BnB result to ensure that BnB succeeds.
I recently removed a similar automated test that used brute force. The issue with using a brute solution I had was a) brute force is slow b) only works on a limited pool size of say 20 UTXOs (see a). Instead of using brute force, I adopted a strategy where I generate two random UTXO sets. T
...
🤔 pablomartin4btc reviewed a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3008114956)
ACK 85eb29605ef4051ba23cd32e54a3d5a3ee203f6c
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3008114956)
ACK 85eb29605ef4051ba23cd32e54a3d5a3ee203f6c
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2199344214)
Done! Thanks!
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2199344214)
Done! Thanks!
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2199345678)
Applied a very tiny tweak to it. Thanks!
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2199345678)
Applied a very tiny tweak to it. Thanks!
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3060160125)
<ins>_Updates_</ins>
- Addressed @stickies-v's feedback:
- converted the `GetWalletNameFromJSONRPCRequest` overload to `EnsureUniqueWalletName` as [proposed](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197447487) - no changes in logic;
- [corrected](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197391680) the `getdescriptoractivity` fix making both blockhashes and descriptors arrays as mandatory, returning the RPC Help for the command when any of them or both a
...
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3060160125)
<ins>_Updates_</ins>
- Addressed @stickies-v's feedback:
- converted the `GetWalletNameFromJSONRPCRequest` overload to `EnsureUniqueWalletName` as [proposed](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197447487) - no changes in logic;
- [corrected](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197391680) the `getdescriptoractivity` fix making both blockhashes and descriptors arrays as mandatory, returning the RPC Help for the command when any of them or both a
...
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199418773)
Right, they only have one valid linearization anyway, so any number of iterations suffices to find optimal (though I guess it could require *some* iterations to figure out that it is actually optimal).
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199418773)
Right, they only have one valid linearization anyway, so any number of iterations suffices to find optimal (though I guess it could require *some* iterations to figure out that it is actually optimal).
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199429024)
Done.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199429024)
Done.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428976)
Done.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428976)
Done.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199429462)
Nice, indeed. With `PostLinearize` I think even 3 may work, but changed to 2 now.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199429462)
Nice, indeed. With `PostLinearize` I think even 3 may work, but changed to 2 now.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428590)
Done.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428590)
Done.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428428)
Will think about. I haven't done this yet, so leaving open.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2199428428)
Will think about. I haven't done this yet, so leaving open.
💬 maflcko commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3060581268)
needs release note?
(https://github.com/bitcoin/bitcoin/pull/32944#issuecomment-3060581268)
needs release note?
💬 Sjors commented on issue "Guix build fails on M4 macOS host with Ubuntu in UTM":
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3060808833)
The new machine also gives me the "cannot set armhf-linux personality" error.
It seems to happen after successfully building one package (with `--max-jobs=1`). Each time you do `guix build` it'll build another package and throw this error.
If I switch back to c8abd972818f83d90bf50b250131f338034460ef then `guix build` just works. At least after I fix the app armor issue:
In `/etc/apparmor.d/guix`:
```
abi <abi/4.0>,
include <tunables/global>
# Profile for the guix binary
profile guix /usr/bi
...
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3060808833)
The new machine also gives me the "cannot set armhf-linux personality" error.
It seems to happen after successfully building one package (with `--max-jobs=1`). Each time you do `guix build` it'll build another package and throw this error.
If I switch back to c8abd972818f83d90bf50b250131f338034460ef then `guix build` just works. At least after I fix the app armor issue:
In `/etc/apparmor.d/guix`:
```
abi <abi/4.0>,
include <tunables/global>
# Profile for the guix binary
profile guix /usr/bi
...
💬 Prabhat1308 commented on pull request "test: add functional test for upgradewallet rpc":
(https://github.com/bitcoin/bitcoin/pull/32803#issuecomment-3060846745)
closing in favour of #32944
(https://github.com/bitcoin/bitcoin/pull/32803#issuecomment-3060846745)
closing in favour of #32944
💬 Prabhat1308 commented on pull request "test: add functional test for upgradewallet rpc":
(https://github.com/bitcoin/bitcoin/pull/32803#issuecomment-3060848294)
closing in favour of #32944
(https://github.com/bitcoin/bitcoin/pull/32803#issuecomment-3060848294)
closing in favour of #32944
✅ Prabhat1308 closed a pull request: "test: add functional test for upgradewallet rpc"
(https://github.com/bitcoin/bitcoin/pull/32803)
(https://github.com/bitcoin/bitcoin/pull/32803)
💬 Sjors commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2199811385)
765922d8022d3addeb80b5f3f6e041f7fd2ad3ed: do you know why https://codeberg.org/guix/guix/pulls/353/files only needs `python-pydantic-2` and not `python-pydantic-core`?
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2199811385)
765922d8022d3addeb80b5f3f6e041f7fd2ad3ed: do you know why https://codeberg.org/guix/guix/pulls/353/files only needs `python-pydantic-2` and not `python-pydantic-core`?
🤔 Prabhat1308 reviewed a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3008894533)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3008894533)
Concept ACK
💬 Prabhat1308 commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#discussion_r2199814332)
There is another instance of `SetMinVersion` in this file . Since `Createwallet` will create a wallet with the newest version , that use is also a dead code . Might as well remove this function too.
(https://github.com/bitcoin/bitcoin/pull/32944#discussion_r2199814332)
There is another instance of `SetMinVersion` in this file . Since `Createwallet` will create a wallet with the newest version , that use is also a dead code . Might as well remove this function too.