💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610635130)
First letter capitalization should match your strings on 304:305, if they are kept.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610635130)
First letter capitalization should match your strings on 304:305, if they are kept.
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610578325)
This will be more work to maintain than not worrying about what part of the string is what and only care about finding matches in SHA256SUMS.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610578325)
This will be more work to maintain than not worrying about what part of the string is what and only care about finding matches in SHA256SUMS.
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610568644)
I don't see the point of bumping the version numbers in the examples. They'll always get old.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610568644)
I don't see the point of bumping the version numbers in the examples. They'll always get old.
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610610813)
The `VERSION_PLATFORM` element `"apple"` will never match a fully specified release version string, it will always match `"apple-darwin"` first:
```
ben@ben:~/guix.sigs$ for file in $(grep all.SHA256SUMS <<< $(find .) | sed '/.asc/d' | uniq); do cat $file; done | grep -c "apple"
4551
ben@ben:~/guix.sigs$ for file in $(grep all.SHA256SUMS <<< $(find .) | sed '/.asc/d' | uniq); do cat $file; done | grep -c "apple-darwin"
4551
```
Including both `"apple"` and `"apple-darwin"` in `VERSION_PLA
...
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610610813)
The `VERSION_PLATFORM` element `"apple"` will never match a fully specified release version string, it will always match `"apple-darwin"` first:
```
ben@ben:~/guix.sigs$ for file in $(grep all.SHA256SUMS <<< $(find .) | sed '/.asc/d' | uniq); do cat $file; done | grep -c "apple"
4551
ben@ben:~/guix.sigs$ for file in $(grep all.SHA256SUMS <<< $(find .) | sed '/.asc/d' | uniq); do cat $file; done | grep -c "apple-darwin"
4551
```
Including both `"apple"` and `"apple-darwin"` in `VERSION_PLA
...
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610571290)
I also added an "-x86_64-linux-gnu" example here as it's likely among the top choices a user wants to specifically download & verify.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610571290)
I also added an "-x86_64-linux-gnu" example here as it's likely among the top choices a user wants to specifically download & verify.
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610627298)
This prevents people from specifying a version string like:
```
./verify.py pub 27.0-win64-setup.exe
./verify.py pub 27.0-win64.zip
```
To fetch one of the specific files to download as we also have diversity in file extensions among -arch-platforms.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610627298)
This prevents people from specifying a version string like:
```
./verify.py pub 27.0-win64-setup.exe
./verify.py pub 27.0-win64.zip
```
To fetch one of the specific files to download as we also have diversity in file extensions among -arch-platforms.
💬 BenWestgate commented on pull request "contrib: verify-binaries accept full arch-platform specifier":
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610635610)
Suggest changing to:
```python3
f"Example architectures: {VERSION_ARCHES}\n"
f"Example platforms: {VERSION_PLATFORMS}\n"
```
or
```python3
f"Suggested architectures: {VERSION_ARCHES}\n"
f"Suggested platforms: {VERSION_PLATFORMS}\n"
```
so it can become out of date but not become wrong.
(https://github.com/bitcoin/bitcoin/pull/28418#discussion_r1610635610)
Suggest changing to:
```python3
f"Example architectures: {VERSION_ARCHES}\n"
f"Example platforms: {VERSION_PLATFORMS}\n"
```
or
```python3
f"Suggested architectures: {VERSION_ARCHES}\n"
f"Suggested platforms: {VERSION_PLATFORMS}\n"
```
so it can become out of date but not become wrong.
💬 BenWestgate commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-2125872340)
crACK https://github.com/bitcoin/bitcoin/commit/84900ac34f6888b7a851d0a6a5885192155f865c
Have not tested on a Windows machine.
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-2125872340)
crACK https://github.com/bitcoin/bitcoin/commit/84900ac34f6888b7a851d0a6a5885192155f865c
Have not tested on a Windows machine.
💬 BenWestgate commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-2125882826)
> The UX for downloading two verification files sucks IMO. I think we should just go for the clearsign option, even if it's exclusive.
>
> Thoughts?
We have [`./contrib/verify-binaries/verify.py`](https://github.com/bitcoin/bitcoin/tree/master/contrib/verify-binaries) in this repository to make verification UX better. I just reviewed a PR and made a PR to it that makes it able to let a single file be downloaded & verified. Changes to here require changes to that script, right?
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-2125882826)
> The UX for downloading two verification files sucks IMO. I think we should just go for the clearsign option, even if it's exclusive.
>
> Thoughts?
We have [`./contrib/verify-binaries/verify.py`](https://github.com/bitcoin/bitcoin/tree/master/contrib/verify-binaries) in this repository to make verification UX better. I just reviewed a PR and made a PR to it that makes it able to let a single file be downloaded & verified. Changes to here require changes to that script, right?
🤔 edilmedeiros reviewed a pull request: "doc: Update mentions of generating bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30154#pullrequestreview-2072542001)
Good catch.
I thought this would not make sense for people getting the binaries, but seems that none of the binary distributions include the `doc` folder. IIUIC, this change affects only people who is getting core from source. If so, the update does make sense.
(https://github.com/bitcoin/bitcoin/pull/30154#pullrequestreview-2072542001)
Good catch.
I thought this would not make sense for people getting the binaries, but seems that none of the binary distributions include the `doc` folder. IIUIC, this change affects only people who is getting core from source. If so, the update does make sense.
💬 edilmedeiros commented on pull request "doc: Update mentions of generating bitcoin.conf":
(https://github.com/bitcoin/bitcoin/pull/30154#discussion_r1610761009)
I don't like this change that much since this hardens the logical link between this file and `contrib/devtools/README.md` without any kind of automation to check if e.g. the section `gen-bitcoin-conf.sh` still exist.
(https://github.com/bitcoin/bitcoin/pull/30154#discussion_r1610761009)
I don't like this change that much since this hardens the logical link between this file and `contrib/devtools/README.md` without any kind of automation to check if e.g. the section `gen-bitcoin-conf.sh` still exist.
💬 edilmedeiros commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2125912025)
> > > Which one? http://miniupnp.free.fr/ without HTTPS?
> >
> >
> > If that does keep the sha256 hash the same then maybe.
>
> It does. Switched to that website.
This is the same URL I used to update the MacOS package in the macports project a couple of months ago, so there will be more people watching it.
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2125912025)
> > > Which one? http://miniupnp.free.fr/ without HTTPS?
> >
> >
> > If that does keep the sha256 hash the same then maybe.
>
> It does. Switched to that website.
This is the same URL I used to update the MacOS package in the macports project a couple of months ago, so there will be more people watching it.
🤔 luke-jr requested changes to a pull request: "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection"
(https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2072577992)
I'm pretty sure you can just modify the existing line in `setWalletActionsEnabled` to:
```c++
historyAction->setEnabled(enabled && !isPrivacyModeActivated());
```
(https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2072577992)
I'm pretty sure you can just modify the existing line in `setWalletActionsEnabled` to:
```c++
historyAction->setEnabled(enabled && !isPrivacyModeActivated());
```
💬 luke-jr commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1610785803)
Why move this up top? Now it's after the signal is connected, so there's a tiny chance of a race...
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1610785803)
Why move this up top? Now it's after the signal is connected, so there's a tiny chance of a race...
💬 luke-jr commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1610789375)
This shouldn't be necessary.
(https://github.com/bitcoin-core/gui/pull/815#discussion_r1610789375)
This shouldn't be necessary.
💬 luke-jr commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1610799043)
nit
```suggestion
virtual std::map<CNetAddr, LocalServiceInfo> getNetLocalAddresses() = 0;
```
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1610799043)
nit
```suggestion
virtual std::map<CNetAddr, LocalServiceInfo> getNetLocalAddresses() = 0;
```
💬 luke-jr commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1610802187)
AFAIK Tor hidden services do still have port numbers
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1610802187)
AFAIK Tor hidden services do still have port numbers
💬 luke-jr commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2125971142)
Also, please split the interfaces changes into a different commit than the GUI changes.
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2125971142)
Also, please split the interfaces changes into a different commit than the GUI changes.
💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2125972389)
> Needs rebase (for some reason Drahtbot isn't detecting this)
Rebased. Also deleted a couple of redundant comment as suggested by @maflcko to retrigger the false negative CI
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2125972389)
> Needs rebase (for some reason Drahtbot isn't detecting this)
Rebased. Also deleted a couple of redundant comment as suggested by @maflcko to retrigger the false negative CI
💬 furszy commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1610809735)
why restart if you aren't doing anything with the node?
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1610809735)
why restart if you aren't doing anything with the node?
💬 theStack commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#issuecomment-2125983180)
@LarryRuane: Interesting problem (and very creative solution with the RBF tx :)). Fwiw, on one of my machines running OpenBSD 7.5, the same behaviour as you described occurs on commit 663e5012d318a7e80a42385ac4e6f71dec46814e: after `fill_mempool`, mempool usage is very close to the max (`'usage': 4998768`), and the subtest `test_mid_package_eviction` fails on the first MiniWallet tx submission with a `"mempool full"` error.
One idea I had to solve this at the `fill_mempool` side is to submit
...
(https://github.com/bitcoin/bitcoin/pull/28531#issuecomment-2125983180)
@LarryRuane: Interesting problem (and very creative solution with the RBF tx :)). Fwiw, on one of my machines running OpenBSD 7.5, the same behaviour as you described occurs on commit 663e5012d318a7e80a42385ac4e6f71dec46814e: after `fill_mempool`, mempool usage is very close to the max (`'usage': 4998768`), and the subtest `test_mid_package_eviction` fails on the first MiniWallet tx submission with a `"mempool full"` error.
One idea I had to solve this at the `fill_mempool` side is to submit
...