💬 MarcoFalke commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137368863)
Thx, can be marked resolved
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137368863)
Thx, can be marked resolved
💬 MarcoFalke commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137369578)
Thx, can be marked resolved
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137369578)
Thx, can be marked resolved
💬 MarcoFalke commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137369716)
Thx, can be marked resolved
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137369716)
Thx, can be marked resolved
💬 MarcoFalke commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137370113)
Thx, can be marked resolved
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1137370113)
Thx, can be marked resolved
💬 stratospher commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/19860#issuecomment-1470325525)
revived this in #27264.
(https://github.com/bitcoin/bitcoin/pull/19860#issuecomment-1470325525)
revived this in #27264.
💬 MarcoFalke commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1470327165)
re-ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6 🛁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK b3e78dc91d01e364b77a
...
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1470327165)
re-ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6 🛁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK b3e78dc91d01e364b77a
...
💬 achow101 commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#issuecomment-1470331104)
ACK dacd3316ca0279c56d12372957633b73a999b5b2
(https://github.com/bitcoin/bitcoin/pull/19690#issuecomment-1470331104)
ACK dacd3316ca0279c56d12372957633b73a999b5b2
💬 TheCharlatan commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1137383018)
Right on, can be marked as resolved.
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1137383018)
Right on, can be marked as resolved.
💬 desirepl commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470344927)
@ryanofsky "Various ways this could be handled"
Let's think about situation:
1) we have external dir/disk with blocks only (blk* and rev* for now are stick together :|)
2) we have datadir with indexes/chainstate and other files currently belongs to datadir (peers/mempool etc) at another second/disk
3) we have configuration files at third dir/disk
4) we have logdir for everything at fourth dir/disk
It's a good solution to divide them all.
Now we do some magic - place bitcoin.conf at defa
...
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470344927)
@ryanofsky "Various ways this could be handled"
Let's think about situation:
1) we have external dir/disk with blocks only (blk* and rev* for now are stick together :|)
2) we have datadir with indexes/chainstate and other files currently belongs to datadir (peers/mempool etc) at another second/disk
3) we have configuration files at third dir/disk
4) we have logdir for everything at fourth dir/disk
It's a good solution to divide them all.
Now we do some magic - place bitcoin.conf at defa
...
💬 achow101 commented on pull request "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`":
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1470364821)
ACK fabb95e7bf02f3d8e663a02dd845d42e09d330ec
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1470364821)
ACK fabb95e7bf02f3d8e663a02dd845d42e09d330ec
💬 willcl-ark commented on issue "RFC: Replacing Boost Process":
(https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1470373138)
@theStack Just tested on MacOS 13.1 (without BDB and QT) and all functional and unit tests passed ✅
(https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1470373138)
@theStack Just tested on MacOS 13.1 (without BDB and QT) and all functional and unit tests passed ✅
👍 vasild approved a pull request: "p2p: Improve diversification of new connections"
(https://github.com/bitcoin/bitcoin/pull/27264)
ACK 72e8ffd7f8dbf908e65da6d012ede914596737ab
IMO the last commit from #19860 can also be brought up: `doc: peer selection rules do not apply to MANUAL and ADDR_FETCH`
(https://github.com/bitcoin/bitcoin/pull/27264)
ACK 72e8ffd7f8dbf908e65da6d012ede914596737ab
IMO the last commit from #19860 can also be brought up: `doc: peer selection rules do not apply to MANUAL and ADDR_FETCH`
💬 TheCharlatan commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#issuecomment-1470396179)
Code review ACK fadf8b818226dc60adf88e927160f9c9680473a4
(https://github.com/bitcoin/bitcoin/pull/25781#issuecomment-1470396179)
Code review ACK fadf8b818226dc60adf88e927160f9c9680473a4
💬 achow101 commented on pull request "wallet: Turn `destdata` entries into `CAddressBookData` fields":
(https://github.com/bitcoin/bitcoin/pull/27215#issuecomment-1470407704)
Closing in favor of #27224
(https://github.com/bitcoin/bitcoin/pull/27215#issuecomment-1470407704)
Closing in favor of #27224
✅ achow101 closed a pull request: "wallet: Turn `destdata` entries into `CAddressBookData` fields"
(https://github.com/bitcoin/bitcoin/pull/27215)
(https://github.com/bitcoin/bitcoin/pull/27215)
💬 achow101 commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137453458)
I strongly dislike using the empty string to indicate deletion. I think it would be better to have a separate `RemoveReceiveRequest`.
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137453458)
I strongly dislike using the empty string to indicate deletion. I think it would be better to have a separate `RemoveReceiveRequest`.
💬 achow101 commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137458690)
I'm not convinced that this needs to be a map. Receive requests are only created by the GUI, and the GUI only creates them for a newly retrieved address, so it shouldn't be possible to have multiple receive requests.
Additionally, receive requests are identified by an int which is being (un)stringified. This could just be an int to avoid that constant conversion in the GUI.
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137458690)
I'm not convinced that this needs to be a map. Receive requests are only created by the GUI, and the GUI only creates them for a newly retrieved address, so it shouldn't be possible to have multiple receive requests.
Additionally, receive requests are identified by an int which is being (un)stringified. This could just be an int to avoid that constant conversion in the GUI.
💬 dergoegge commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1137459181)
nit: I think it would be nice if access to this was wrapped in a getter/setter, e.g. `Importing(), {Start,Stop}Importing()`.
My thinking is that we might (in the future) want to mock the `BlockManager` to avoid expensive i/o in the fuzz tests for example. Having a clear interface defined would make that easier. A simple member like this is not a huge blocker for that so feel free to ignore.
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1137459181)
nit: I think it would be nice if access to this was wrapped in a getter/setter, e.g. `Importing(), {Start,Stop}Importing()`.
My thinking is that we might (in the future) want to mock the `BlockManager` to avoid expensive i/o in the fuzz tests for example. Having a clear interface defined would make that easier. A simple member like this is not a huge blocker for that so feel free to ignore.
💬 willcl-ark commented on issue "Allow groups of accounts to access the RPC cookie file":
(https://github.com/bitcoin/bitcoin/issues/25270#issuecomment-1470420202)
Am I wrong in thinking that there is probably still demand for this, even though #26088 was closed (because #17127 was merged, which does not address this issue), and we have an number of currently-working alternatives:
1. The raspiblitz startupnotify script: `-startupnotify="chmod g+r /home/bitcoin/.bitcoin/.cookie"`
2. With user+password by using `stunnel` to situate the RPC port behind ssl: `stunnel -d 8332 -r 127.0.0.1:8332 -p /path/to/your/certificate.pem -k /path/to/your/key.pem -o /v
...
(https://github.com/bitcoin/bitcoin/issues/25270#issuecomment-1470420202)
Am I wrong in thinking that there is probably still demand for this, even though #26088 was closed (because #17127 was merged, which does not address this issue), and we have an number of currently-working alternatives:
1. The raspiblitz startupnotify script: `-startupnotify="chmod g+r /home/bitcoin/.bitcoin/.cookie"`
2. With user+password by using `stunnel` to situate the RPC port behind ssl: `stunnel -d 8332 -r 127.0.0.1:8332 -p /path/to/your/certificate.pem -k /path/to/your/key.pem -o /v
...
💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#issuecomment-1470427455)
> I made some changes to this in a branch that could be adopted https://github.com/ryanofsky/bitcoin/commits/review.27217.1-edit
I've adopted these changes.
(https://github.com/bitcoin/bitcoin/pull/27217#issuecomment-1470427455)
> I made some changes to this in a branch that could be adopted https://github.com/ryanofsky/bitcoin/commits/review.27217.1-edit
I've adopted these changes.
💬 ryanofsky commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470430978)
@desirepl thanks for clarifying your use case, that makes a lot of sense. I see the appeal of placing a stub bitcoin.conf in the default location that points to other blocks, index, and log directories, as well as to another config file. This lets you specify disk layout separately from network configuration, and makes applications and command line utilities work "magically" as you say without requiring manual arguments.
So I can see how behavior (3) from https://github.com/bitcoin/bitcoin/is
...
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470430978)
@desirepl thanks for clarifying your use case, that makes a lot of sense. I see the appeal of placing a stub bitcoin.conf in the default location that points to other blocks, index, and log directories, as well as to another config file. This lets you specify disk layout separately from network configuration, and makes applications and command line utilities work "magically" as you say without requiring manual arguments.
So I can see how behavior (3) from https://github.com/bitcoin/bitcoin/is
...