💬 Sjors commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1112863805)
This sentence can be dropped with my suggestion.
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1112863805)
This sentence can be dropped with my suggestion.
💬 Sjors commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1112871259)
I'm confused: `!use_legacy` means we have a descriptor wallet. We should never create a `LegacyScriptPubKeyMan` in that case. I don't understand what this `if` block is trying to achieve, but I think it should `throw JSONRPCError(RPC_WALLET_ERROR, "Use 'importdescriptors' instead"`
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1112871259)
I'm confused: `!use_legacy` means we have a descriptor wallet. We should never create a `LegacyScriptPubKeyMan` in that case. I don't understand what this `if` block is trying to achieve, but I think it should `throw JSONRPCError(RPC_WALLET_ERROR, "Use 'importdescriptors' instead"`
💬 glozow commented on pull request "change txid log format specifier from %i to %s":
(https://github.com/bitcoin/bitcoin/pull/27133#issuecomment-1438282156)
https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy
(https://github.com/bitcoin/bitcoin/pull/27133#issuecomment-1438282156)
https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy
✅ glozow closed a pull request: "change txid log format specifier from %i to %s"
(https://github.com/bitcoin/bitcoin/pull/27133)
(https://github.com/bitcoin/bitcoin/pull/27133)
💬 willcl-ark commented on issue "Followups from #19525":
(https://github.com/bitcoin/bitcoin/issues/19623#issuecomment-1438304263)
I think I found the spot in the [ELF spec](https://refspecs.linuxfoundation.org/elf/elf.pdf) that describes what @laanwj observed, in _**Book III, Section 2, Base Address**_ (page 73), emphasis mine:
> An executable or shared object file's base address is calculated during execution from three
values: the virtual memory load address, the maximum page size, and the lowest virtual address
of a program's loadable segment. To compute the base address, one determines the memory
address associat
...
(https://github.com/bitcoin/bitcoin/issues/19623#issuecomment-1438304263)
I think I found the spot in the [ELF spec](https://refspecs.linuxfoundation.org/elf/elf.pdf) that describes what @laanwj observed, in _**Book III, Section 2, Base Address**_ (page 73), emphasis mine:
> An executable or shared object file's base address is calculated during execution from three
values: the virtual memory load address, the maximum page size, and the lowest virtual address
of a program's loadable segment. To compute the base address, one determines the memory
address associat
...
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438317444)
[running Guix build now, will mark as ready for review when that's done and I've tested it]
I added the word "test" to the application name.
Since [Bitcoin Inquisition](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020921.html) uses the default Signet to deploy several proposed soft forks, I think it's useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).
See #8285 for additional context.
I generated the icon fro
...
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438317444)
[running Guix build now, will mark as ready for review when that's done and I've tested it]
I added the word "test" to the application name.
Since [Bitcoin Inquisition](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020921.html) uses the default Signet to deploy several proposed soft forks, I think it's useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).
See #8285 for additional context.
I generated the icon fro
...
💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112930264)
This config option enables the `Bitcoin Core Security Policy` row with the `Open` button which links to our `SECURITY.md`. The `Report a security vulnerability` row, with the big green `Report a vulnerability` button seems to be a GitHub default (which I don't know how to disable).
My preference would be to keep both, as otherwise the link to `SECURITY.md` via the `Report a vulnerability` button is from a small right menu item:

(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112930264)
This config option enables the `Bitcoin Core Security Policy` row with the `Open` button which links to our `SECURITY.md`. The `Report a security vulnerability` row, with the big green `Report a vulnerability` button seems to be a GitHub default (which I don't know how to disable).
My preference would be to keep both, as otherwise the link to `SECURITY.md` via the `Report a vulnerability` button is from a small right menu item:

💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112935813)
Added in 3fa1185dd
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112935813)
Added in 3fa1185dd
💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112936206)
I agree that this is nice-to-have rather than essential (for most bug reports) and so made it optional in 3fa1185dd
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112936206)
I agree that this is nice-to-have rather than essential (for most bug reports) and so made it optional in 3fa1185dd
💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112937018)
I made this change in 3fa1185dd. Seems like files can be added to the textarea with drag & drop
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1112937018)
I made this change in 3fa1185dd. Seems like files can be added to the textarea with drag & drop
💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1438330397)
Thanks for the comments. I have incorporated the changes into 3fa1185dd which is also live on my fork.
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1438330397)
Thanks for the comments. I have incorporated the changes into 3fa1185dd which is also live on my fork.
👋 Sjors's pull request is ready for review: "Add Signet launch shortcut for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334)
(https://github.com/bitcoin/bitcoin/pull/26334)
💬 TheCharlatan commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1438377141)
Concept ACK
This has been annoying me for years. I think there is one more `options` field that could be converted to `OBJ_NAMED_PARAMS` in the `scanblocks` call.
There is also the `template_request` argument in `getblocktemplate` and the `solving_data` argument in `fundrawtransaction` that could be converted as well, but they contain a bunch of array arguments, so I'm not sold if this would actually increase ergonomics.
The tests now all seem to be using the named arguments. Are there
...
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1438377141)
Concept ACK
This has been annoying me for years. I think there is one more `options` field that could be converted to `OBJ_NAMED_PARAMS` in the `scanblocks` call.
There is also the `template_request` argument in `getblocktemplate` and the `solving_data` argument in `fundrawtransaction` that could be converted as well, but they contain a bunch of array arguments, so I'm not sold if this would actually increase ergonomics.
The tests now all seem to be using the named arguments. Are there
...
💬 hebasto commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1438418636)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1438418636)
Concept ACK.
👋 hebasto's pull request is ready for review: "ci: A few fixes of `ccache` issues"
(https://github.com/bitcoin/bitcoin/pull/27084)
(https://github.com/bitcoin/bitcoin/pull/27084)
💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1438441083)
The questionable commit has been dropped.
Ready to re-review.
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1438441083)
The questionable commit has been dropped.
Ready to re-review.
📝 MarcoFalke opened a pull request: "Revert "[contrib] verify-commits: Add MarcoFalke fingerprint""
(https://github.com/bitcoin/bitcoin/pull/27135)
This reverts commit fa243293343eb964bfee5b91cc52b91f16232ab6.
The commit may be signed by my key, but I haven't checked it. Also, I haven't checked the new `contrib/verify-commits/trusted-git-root`.
(https://github.com/bitcoin/bitcoin/pull/27135)
This reverts commit fa243293343eb964bfee5b91cc52b91f16232ab6.
The commit may be signed by my key, but I haven't checked it. Also, I haven't checked the new `contrib/verify-commits/trusted-git-root`.
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438484759)
I think I found the right incantation, using the high res PNG version as input:
```
convert src/qt/res/icons/bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" src/qt/res/icons/bitcoin_signet.ico
```
Will update screenshot and push the updated file after I finish my Guix build.
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438484759)
I think I found the right incantation, using the high res PNG version as input:
```
convert src/qt/res/icons/bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" src/qt/res/icons/bitcoin_signet.ico
```
Will update screenshot and push the updated file after I finish my Guix build.
💬 willcl-ark commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1438496796)
@popkian, see [this previous comment](https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1243253449) on what you can do to help us diagnose the issue.
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1438496796)
@popkian, see [this previous comment](https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1243253449) on what you can do to help us diagnose the issue.
👍 hebasto approved a pull request: "Convert ArgsManager::GetDataDir to a read-only function"
(https://github.com/bitcoin/bitcoin/pull/27073)
ACK 0af17e161d751b15f90615800411f36e11b3fcde, tested on Ubuntu 22.04.
> Fixes https://github.com/bitcoin/bitcoin/issues/20070
Confirming the `bitcoin-cli` does not create a data directory and its subdirectories.
I've noticed another behavior change which looks good for me:
- master:
```
$ ./src/bitcoind rubbish
Error: Command line contains unexpected token 'rubbish', see bitcoind -h for a list of options.
$ tree ~/.bitcoin
/home/hebasto/.bitcoin
└── wallets
1 directory, 0 fi
...
(https://github.com/bitcoin/bitcoin/pull/27073)
ACK 0af17e161d751b15f90615800411f36e11b3fcde, tested on Ubuntu 22.04.
> Fixes https://github.com/bitcoin/bitcoin/issues/20070
Confirming the `bitcoin-cli` does not create a data directory and its subdirectories.
I've noticed another behavior change which looks good for me:
- master:
```
$ ./src/bitcoind rubbish
Error: Command line contains unexpected token 'rubbish', see bitcoind -h for a list of options.
$ tree ~/.bitcoin
/home/hebasto/.bitcoin
└── wallets
1 directory, 0 fi
...
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#discussion_r1113070519)
Forgot to add `test ` here, pushing fix.
(https://github.com/bitcoin/bitcoin/pull/26334#discussion_r1113070519)
Forgot to add `test ` here, pushing fix.