π¬ maflcko commented on pull request "[DO NOT MERGE] testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2072953283)
> maybe just a startup flag to override the minimum difficulty?
I don't think consensus rules of remote nodes can be affected by a local startup flag (or re-compilation).
If someone wanted to create a block locally only, they could use regtest.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2072953283)
> maybe just a startup flag to override the minimum difficulty?
I don't think consensus rules of remote nodes can be affected by a local startup flag (or re-compilation).
If someone wanted to create a block locally only, they could use regtest.
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2072977453)
`9297437af2...cc760207b8`: rebase and address suggestions:
* Give a startup warning if `-privatebroadcast=1`, `-proxyrandomize=0` and the Tor network is reachable (i.e. we will use Tor for private broadcast).
* Enforce `-walletbroadcast=0` if `-privatebroadcast=1` because it would be confusing to have the wallet do the traditional broadcast while the `sendrawtransaction` RPC does a private broadcast. Furthermore if a wallet transaction is sent via `sendrawtransaction` and ends up in the memp
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2072977453)
`9297437af2...cc760207b8`: rebase and address suggestions:
* Give a startup warning if `-privatebroadcast=1`, `-proxyrandomize=0` and the Tor network is reachable (i.e. we will use Tor for private broadcast).
* Enforce `-walletbroadcast=0` if `-privatebroadcast=1` because it would be confusing to have the wallet do the traditional broadcast while the `sendrawtransaction` RPC does a private broadcast. Furthermore if a wallet transaction is sent via `sendrawtransaction` and ends up in the memp
...
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1576618058)
Right, removed the conflicting address and reduced the logging to log only if adding to the addrman fails.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1576618058)
Right, removed the conflicting address and reduced the logging to log only if adding to the addrman fails.
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1576619537)
I added broadcast to IPv4 and IPv6 peers through the Tor proxy. Thanks!
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1576619537)
I added broadcast to IPv4 and IPv6 peers through the Tor proxy. Thanks!
π hernanmarino's pull request is ready for review: "test: add missing tests for Assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/29428)
(https://github.com/bitcoin/bitcoin/pull/29428)
π hebasto's pull request is ready for review: "refactor: Rename `subprocess.hpp` to follow our header name conventions"
(https://github.com/bitcoin/bitcoin/pull/29910)
(https://github.com/bitcoin/bitcoin/pull/29910)
π¬ hernanmarino commented on pull request "test: add missing tests for Assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2072984921)
> Are you still working on this?
I havenΒ΄t been able to think of / work on other tests to add, but the current commit test is ready for review, so i am taking the PR out of Draft status.
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2072984921)
> Are you still working on this?
I havenΒ΄t been able to think of / work on other tests to add, but the current commit test is ready for review, so i am taking the PR out of Draft status.
π¬ hebasto commented on pull request "refactor: Rename `subprocess.hpp` to follow our header name conventions":
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2072985412)
Rebase and undrafted.
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2072985412)
Rebase and undrafted.
π¬ hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072986554)
https://github.com/bitcoin/bitcoin/pull/29910 is the next one :)
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072986554)
https://github.com/bitcoin/bitcoin/pull/29910 is the next one :)
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2072989163)
> Avoiding already connected peers would work around this, but perhaps it's sufficient just to warn on startup if privatebroadcast=1 and proxyrandomize=0
In `master` we already avoid connecting to an already connected address, regardless of the connection type:
https://github.com/bitcoin/bitcoin/blob/256e1703197fdddd78bc6d659431cd0fc3b63cde/src/net.cpp#L2862
I added a startup warning anyway.
> > > I assume you are restricting the feature to sendraw so the wallet doesn't get invol
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2072989163)
> Avoiding already connected peers would work around this, but perhaps it's sufficient just to warn on startup if privatebroadcast=1 and proxyrandomize=0
In `master` we already avoid connecting to an already connected address, regardless of the connection type:
https://github.com/bitcoin/bitcoin/blob/256e1703197fdddd78bc6d659431cd0fc3b63cde/src/net.cpp#L2862
I added a startup warning anyway.
> > > I assume you are restricting the feature to sendraw so the wallet doesn't get invol
...
π¬ maflcko commented on pull request "test: add missing tests for Assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2072998404)
I was asking, because you said you'd update the branch in February: https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-1965145449
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2072998404)
I was asking, because you said you'd update the branch in February: https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-1965145449
π¬ theStack commented on pull request "refactor: Rename `subprocess.hpp` to follow our header name conventions":
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2072999520)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2072999520)
Concept ACK
π¬ hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2073002343)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2073002343)
Rebased.
π€ mzumsande reviewed a pull request: "net: update comment for service bit support info for seed.bitcoin.sipa.be"
(https://github.com/bitcoin/bitcoin/pull/29809#pullrequestreview-2017881075)
I also think it would be good to get this out of chainparams, bitcoind only ever uses "x9" anyway as far as I can see, so I'm not sure why the status of other combinations needs to be tracked here.
Maybe adding the info in the first place was meant as a service to other software that would actually use those other combinations?
(https://github.com/bitcoin/bitcoin/pull/29809#pullrequestreview-2017881075)
I also think it would be good to get this out of chainparams, bitcoind only ever uses "x9" anyway as far as I can see, so I'm not sure why the status of other combinations needs to be tracked here.
Maybe adding the info in the first place was meant as a service to other software that would actually use those other combinations?
π¬ theStack commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2073028810)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2073028810)
Concept ACK
π stickies-v approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2017977113)
ACK 477c03b42825084ac344050d97ea206a82ba0eb6
Apologies for my UB string_view code suggestion, I didn't realize `operator[pos]` behaves differently for string_view vs string when `pos==size()`. What you pushed looks like an elegant fix while keeping the copying minimal, nice.
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2017977113)
ACK 477c03b42825084ac344050d97ea206a82ba0eb6
Apologies for my UB string_view code suggestion, I didn't realize `operator[pos]` behaves differently for string_view vs string when `pos==size()`. What you pushed looks like an elegant fix while keeping the copying minimal, nice.
π¬ kosuodhmwa commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073172477)
Now i've set it up without that "txindex" parameter = enabled... still the same issue...

(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073172477)
Now i've set it up without that "txindex" parameter = enabled... still the same issue...

π¬ kosuodhmwa commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073175791)


-> Screenshot #2 is the VirtualBox Host System location for the virtual drive that shows the space problem in VM
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073175791)


-> Screenshot #2 is the VirtualBox Host System location for the virtual drive that shows the space problem in VM
π¬ kosuodhmwa commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073179924)
So a ~1TB disk is not enough for the .bitcoin data directory wtf!?
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2073179924)
So a ~1TB disk is not enough for the .bitcoin data directory wtf!?
π¬ Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2073191753)
I spun up a VM with macOS 11 Big Sur and installed the command line tools (I had to download them from Apple, because `xcode-select --install` got stuck, but that might be a VirtualBox issue).
I then installed Homebrew and did `brew install llvm`, which should install llvm 18. The openssl@3 install failed due to one broken test.
I'll continue to test, but the VM is excruciatingly slow...
Will update to drop the `@14` if that works.
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2073191753)
I spun up a VM with macOS 11 Big Sur and installed the command line tools (I had to download them from Apple, because `xcode-select --install` got stuck, but that might be a VirtualBox issue).
I then installed Homebrew and did `brew install llvm`, which should install llvm 18. The openssl@3 install failed due to one broken test.
I'll continue to test, but the VM is excruciatingly slow...
Will update to drop the `@14` if that works.