💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1160736280)
In commit "wallet: Replace use of purpose strings with an enum" (ccddce7582a51fd2857ce4d142076167631e9cfe)
re: https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1149457810
I think I agree with the suggestion to use a switch statement here. Also:
- It looks like behavior of this code has been changed unintentionally to no longer return hidden in the "refund" case.
- The "if purpose not set, guess" logic is no longer here needed because it is implemented at the lower layer
So
...
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1160736280)
In commit "wallet: Replace use of purpose strings with an enum" (ccddce7582a51fd2857ce4d142076167631e9cfe)
re: https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1149457810
I think I agree with the suggestion to use a switch statement here. Also:
- It looks like behavior of this code has been changed unintentionally to no longer return hidden in the "refund" case.
- The "if purpose not set, guess" logic is no longer here needed because it is implemented at the lower layer
So
...
📝 fanquake opened a pull request: "test: LLVM/Clang 16 for MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27436)
Similar to other CI infra changes we've made recently. Move to LLVM/Clang 16 for the MSAN jobs (which is currently using LLVM 12).
(https://github.com/bitcoin/bitcoin/pull/27436)
Similar to other CI infra changes we've made recently. Move to LLVM/Clang 16 for the MSAN jobs (which is currently using LLVM 12).
💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1500389847)
@Sjors the filter works almost like a grep, so it depends on the release. Starting with v23 the apple binaries were renamed.
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1500389847)
@Sjors the filter works almost like a grep, so it depends on the release. Starting with v23 the apple binaries were renamed.
⚠️ Bandarfaqih opened an issue: "Enhancing Privacy in the Bitcoin Protocol."
(https://github.com/bitcoin/bitcoin/issues/27437)
This BIP proposes the direct implementation of several privacy and anonymity-enhancing features in the Bitcoin protocol, including CoinJoin, zk-SNARKs, ring signatures, and stealth addresses directly into the Bitcoin protocol. These features were envisioned by Satoshi Nakamoto and other contributors as essential tools for preserving user privacy on the network.
Motivation
Bitcoin's current level of privacy and anonymity is limited due to its public ledger, which allows anyone to view all trans
...
(https://github.com/bitcoin/bitcoin/issues/27437)
This BIP proposes the direct implementation of several privacy and anonymity-enhancing features in the Bitcoin protocol, including CoinJoin, zk-SNARKs, ring signatures, and stealth addresses directly into the Bitcoin protocol. These features were envisioned by Satoshi Nakamoto and other contributors as essential tools for preserving user privacy on the network.
Motivation
Bitcoin's current level of privacy and anonymity is limited due to its public ledger, which allows anyone to view all trans
...
💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160783338)
Could also bump this to 16.0.1 now https://github.com/llvm/llvm-project/compare/llvmorg-16.0.0...llvmorg-16.0.1.
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160783338)
Could also bump this to 16.0.1 now https://github.com/llvm/llvm-project/compare/llvmorg-16.0.0...llvmorg-16.0.1.
💬 fanquake commented on issue "Enhancing Privacy in the Bitcoin Protocol BIP Proposal":
(https://github.com/bitcoin/bitcoin/issues/27437#issuecomment-1500396999)
Thanks. However BIPs (and related discussion) should be sent to the bitcoin-dev mailing list. Opening an issue here is not the right place. See https://github.com/bitcoin/bips#readme.
(https://github.com/bitcoin/bitcoin/issues/27437#issuecomment-1500396999)
Thanks. However BIPs (and related discussion) should be sent to the bitcoin-dev mailing list. Opening an issue here is not the right place. See https://github.com/bitcoin/bips#readme.
✅ fanquake closed an issue: "Enhancing Privacy in the Bitcoin Protocol BIP Proposal"
(https://github.com/bitcoin/bitcoin/issues/27437)
(https://github.com/bitcoin/bitcoin/issues/27437)
💬 vasild commented on pull request "doc: FreeBSD DataDirectoryGroupReadable Setting":
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500398188)
I usually resort to `git rebase -i`. With the current history (a877011f64):
```sh
git rebase -i HEAD~3
```
an editor will open that contains this:
```
pick 0a6f9b4440 doc: Updating tor.md doc to include mention of FreeBSD requiring the DataDirectoryGroupReadable setting
pick 3c531ed814 doc: update DataDirectoryGroupReadable 1 in tor.md
pick a877011f64 doc: update DataDirectoryGroupReadable 1 in tor.md
```
change the last two lines to begin with `f` and `f -C`, like this:
```
pick 0
...
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500398188)
I usually resort to `git rebase -i`. With the current history (a877011f64):
```sh
git rebase -i HEAD~3
```
an editor will open that contains this:
```
pick 0a6f9b4440 doc: Updating tor.md doc to include mention of FreeBSD requiring the DataDirectoryGroupReadable setting
pick 3c531ed814 doc: update DataDirectoryGroupReadable 1 in tor.md
pick a877011f64 doc: update DataDirectoryGroupReadable 1 in tor.md
```
change the last two lines to begin with `f` and `f -C`, like this:
```
pick 0
...
👍 MarcoFalke approved a pull request: "test: LLVM/Clang 16 for MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27436#pullrequestreview-1376328538)
lgtm
(https://github.com/bitcoin/bitcoin/pull/27436#pullrequestreview-1376328538)
lgtm
💬 MarcoFalke commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160787798)
any reason to change this here and elsewhere? Either you use `update-alternatives`, or hardcode the version, but doing both seems not needed?
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160787798)
any reason to change this here and elsewhere? Either you use `update-alternatives`, or hardcode the version, but doing both seems not needed?
💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790846)
Right, no need to change these with update-alternatives. Dropped.
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790846)
Right, no need to change these with update-alternatives. Dropped.
💬 fanquake commented on pull request "test: LLVM/Clang 16 for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790999)
Moved to 16.0.1.
(https://github.com/bitcoin/bitcoin/pull/27436#discussion_r1160790999)
Moved to 16.0.1.
🤔 Sjors reviewed a pull request: "[Draft / POC] Silent Payments"
(https://github.com/bitcoin/bitcoin/pull/24897#pullrequestreview-1376326898)
Don't forget to rebase. The bech32 hrp stuff moved to `kernel/chainparams.{h,cpp}`.
> The silentpayment index has been removed from the codebase. … A new PR, with the index, will be added soon.
That's nice. It's more important to prove functionality, can improve performance later.
The `rawtr()` functionality has been merged, so the dependency can be removed from the PR description.
Enabling the ECDH module in `libsecp256k1` might be a reason to put silent payments behind a configure
...
(https://github.com/bitcoin/bitcoin/pull/24897#pullrequestreview-1376326898)
Don't forget to rebase. The bech32 hrp stuff moved to `kernel/chainparams.{h,cpp}`.
> The silentpayment index has been removed from the codebase. … A new PR, with the index, will be added soon.
That's nice. It's more important to prove functionality, can improve performance later.
The `rawtr()` functionality has been merged, so the dependency can be removed from the PR description.
Enabling the ECDH module in `libsecp256k1` might be a reason to put silent payments behind a configure
...
💬 Sjors commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160786689)
3742cc75571a60bda52a41923c8244535af9ff23: maybe move that above `UNKNOWN`?
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160786689)
3742cc75571a60bda52a41923c8244535af9ff23: maybe move that above `UNKNOWN`?
💬 Sjors commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160804856)
9143c7e1f1710f123072cc272b66ab2cad0eb57a: this commit won't compile, probably needs something from a later commit
(https://github.com/bitcoin/bitcoin/pull/24897#discussion_r1160804856)
9143c7e1f1710f123072cc272b66ab2cad0eb57a: this commit won't compile, probably needs something from a later commit
💬 vasild commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1500433830)
`e42ce20c65...ebfe47e594`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1500433830)
`e42ce20c65...ebfe47e594`: rebase due to conflicts
💬 jessebarton commented on pull request "doc: FreeBSD DataDirectoryGroupReadable Setting":
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500477935)
@vasild Thanks that helped a ton!
Looks like I have it down to one now.
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500477935)
@vasild Thanks that helped a ton!
Looks like I have it down to one now.
💬 jessebarton commented on pull request "doc: FreeBSD DataDirectoryGroupReadable Setting":
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500484059)
@achow101 Did you mean to push to this branch?
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1500484059)
@achow101 Did you mean to push to this branch?
💬 pinheadmz commented on issue "RPC wont bind without an IP address on a non-localhost interface":
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-1500485576)
I know this is an old one, but can you offer steps to reproduce? I down'ed all interfaces except `lo` on macOS and rpc bound fine:
```sh
--> ifconfig | grep UP -A 5
lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> mtu 16384
options=1203<RXCSUM,TXCSUM,TXSTATUS,SW_TIMESTAMP>
inet 127.0.0.1 netmask 0xff000000
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x1
nd6 options=201<PERFORMNUD,DAD>
```
```
2023-04-07T17:19:42Z Binding RPC on address ::1 port 18443
2023
...
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-1500485576)
I know this is an old one, but can you offer steps to reproduce? I down'ed all interfaces except `lo` on macOS and rpc bound fine:
```sh
--> ifconfig | grep UP -A 5
lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> mtu 16384
options=1203<RXCSUM,TXCSUM,TXSTATUS,SW_TIMESTAMP>
inet 127.0.0.1 netmask 0xff000000
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0x1
nd6 options=201<PERFORMNUD,DAD>
```
```
2023-04-07T17:19:42Z Binding RPC on address ::1 port 18443
2023
...
👍 vasild approved a pull request: "httpserver, rest: fix segmentation fault on evhttp_uri_get_query"
(https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1376409023)
ACK 4f68c30b2d52f39c7c7465d49ba41c2e70f486ba
Would be nice to split the actual fix from the other changes. I guess it would be easier for other reviewers too.
I would have liked to remove the `replySent` argument, but that's secondary to this PR which fixes a serious bug. Let's get the bug fixed and if I feel strong enough about it, then maybe I will open a followup to remove that parameter.
(https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1376409023)
ACK 4f68c30b2d52f39c7c7465d49ba41c2e70f486ba
Would be nice to split the actual fix from the other changes. I guess it would be easier for other reviewers too.
I would have liked to remove the `replySent` argument, but that's secondary to this PR which fixes a serious bug. Let's get the bug fixed and if I feel strong enough about it, then maybe I will open a followup to remove that parameter.