💬 dergoegge commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1786428201)
> i wasn't sure which of the current usages should be mockable and didn't want to decide that here
fair enough!
`MockableSteadyClock` sgtm!
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1786428201)
> i wasn't sure which of the current usages should be mockable and didn't want to decide that here
fair enough!
`MockableSteadyClock` sgtm!
💬 dergoegge commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1786431421)
> in my current tests i in fact reset it in every test case.
Right, this wouldn't work with my approach as this is a global.
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1786431421)
> in my current tests i in fact reset it in every test case.
Right, this wouldn't work with my approach as this is a global.
💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1786435183)
I'd say it might be a good idea moving the removal until after validation - that should address both scenarios.
We are going to validate it anyway, so I don't really see any downsides to that.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1786435183)
I'd say it might be a good idea moving the removal until after validation - that should address both scenarios.
We are going to validate it anyway, so I don't really see any downsides to that.
💬 ismaelsadeeq commented on pull request "doc: update IBD requirements in doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/30992#discussion_r1786435782)
> whereas "hundreds" might be construed as or implies "not thousands."
I just wanted to be specific on the what is the requirement currently.
"several hundred or more" is not specific.
> it conveys the floor to expect a bit more, and when the requirement breaks past a terabyte, "or more" allows more time before updating again.
Fair enough, it is likely to break hundreds, so this comment can be ignored; it's a non-blocking nit.
(https://github.com/bitcoin/bitcoin/pull/30992#discussion_r1786435782)
> whereas "hundreds" might be construed as or implies "not thousands."
I just wanted to be specific on the what is the requirement currently.
"several hundred or more" is not specific.
> it conveys the floor to expect a bit more, and when the requirement breaks past a terabyte, "or more" allows more time before updating again.
Fair enough, it is likely to break hundreds, so this comment can be ignored; it's a non-blocking nit.
💬 Christewart commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786441085)
Done in 27709f5
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786441085)
Done in 27709f5
💬 andrewtoth commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786455459)
The path won't be `bitcoin-core`, but `bitcoin-28.0`.
```suggestion
cd /path/to/bitcoin-28.0/bin
```
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786455459)
The path won't be `bitcoin-core`, but `bitcoin-28.0`.
```suggestion
cd /path/to/bitcoin-28.0/bin
```
💬 hebasto commented on pull request "qt6: Fix linking when configured with `-DENABLE_WALLET=OFF`":
(https://github.com/bitcoin-core/gui/pull/837#discussion_r1786455836)
Reworked.
(https://github.com/bitcoin-core/gui/pull/837#discussion_r1786455836)
Reworked.
💬 andrewtoth commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786457075)
Oh I see this is the template.
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786457075)
Oh I see this is the template.
✅ fanquake closed a pull request: "mempool_accept.py changes"
(https://github.com/bitcoin/bitcoin/pull/31025)
(https://github.com/bitcoin/bitcoin/pull/31025)
💬 andrewtoth commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391746594)
reACK 27709f51ee02ed4f8c9c7e1c25c6f16faa0ef08b
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391746594)
reACK 27709f51ee02ed4f8c9c7e1c25c6f16faa0ef08b
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2391751712)
@jarolrod @Sjors @maflcko @jonatack @promag @laanwj
Please consider reviewing the following PRs split from this one for upfront merging:
- https://github.com/bitcoin/bitcoin/pull/31010
- https://github.com/bitcoin-core/gui/pull/837
- https://github.com/bitcoin-core/gui/pull/838
- https://github.com/bitcoin-core/gui/pull/839
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2391751712)
@jarolrod @Sjors @maflcko @jonatack @promag @laanwj
Please consider reviewing the following PRs split from this one for upfront merging:
- https://github.com/bitcoin/bitcoin/pull/31010
- https://github.com/bitcoin-core/gui/pull/837
- https://github.com/bitcoin-core/gui/pull/838
- https://github.com/bitcoin-core/gui/pull/839
💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053)
So, `txdownloadman_impl` still crashes on
```
Assert(!txdownload_impl.RecentRejectsFilter().contains(package.back()->GetWitnessHash().ToUint256()));
```
Here's the input: `FVVaWm9vHQpOVVr/dQA9GnItbAAAPv///////3UA83ByLGwAAD7//////xZvb28=`
Did some investigating.
1. Non-segwit child gets added to the orphanage.
2. Same child but segwit gets rejected (`TX_INPUTS_NOT_STANDARD`).
3. Parent gets added to rejects reconsiderable. Forms a package with the first child.
4. Txid of that child is
...
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053)
So, `txdownloadman_impl` still crashes on
```
Assert(!txdownload_impl.RecentRejectsFilter().contains(package.back()->GetWitnessHash().ToUint256()));
```
Here's the input: `FVVaWm9vHQpOVVr/dQA9GnItbAAAPv///////3UA83ByLGwAAD7//////xZvb28=`
Did some investigating.
1. Non-segwit child gets added to the orphanage.
2. Same child but segwit gets rejected (`TX_INPUTS_NOT_STANDARD`).
3. Parent gets added to rejects reconsiderable. Forms a package with the first child.
4. Txid of that child is
...
💬 Sjors commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391770881)
> but `-pausebackgroundsync` seems like the wrong way to go to me
What other approach do you have in mind?
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391770881)
> but `-pausebackgroundsync` seems like the wrong way to go to me
What other approach do you have in mind?
💬 Christewart commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786497041)
I modified the 28.0 release notes to use this path: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft
(https://github.com/bitcoin/bitcoin/pull/30982#discussion_r1786497041)
I modified the 28.0 release notes to use this path: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft
⚠️ JayBitron opened an issue: "Unable to cross compile on linux for macos (28.x branch)"
(https://github.com/bitcoin/bitcoin/issues/31027)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Running the command `make HOST=x86_64-apple-darwin -C depends` on Bitcoin Core 28.x no longer compiles the dependencies as expected. It fails.
### Expected behaviour
The command `make HOST=x86_64-apple-darwin -C depends` should successfully compile all required dependencies without errors, as it did in previous versions of Bitcoin Core.
### Steps to reproduce
make HOST=x86_64-apple-dar
...
(https://github.com/bitcoin/bitcoin/issues/31027)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Running the command `make HOST=x86_64-apple-darwin -C depends` on Bitcoin Core 28.x no longer compiles the dependencies as expected. It fails.
### Expected behaviour
The command `make HOST=x86_64-apple-darwin -C depends` should successfully compile all required dependencies without errors, as it did in previous versions of Bitcoin Core.
### Steps to reproduce
make HOST=x86_64-apple-dar
...
💬 stickies-v commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391838987)
> What other approach do you have in mind?
Perhaps enumerating the problem(s) you're trying to solve would be a better starting point?
As I've outlined before already:
- I think benchmarking can be solved in other ways, e.g. from pre-created datadirs, or by running a fork with ~this diff applied
- user wants to avoid draining his battery / internet: ctrl+c until he's able to resume, like he would with a regular IBD
- user doesn't want to do background validation at all: like I said ear
...
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391838987)
> What other approach do you have in mind?
Perhaps enumerating the problem(s) you're trying to solve would be a better starting point?
As I've outlined before already:
- I think benchmarking can be solved in other ways, e.g. from pre-created datadirs, or by running a fork with ~this diff applied
- user wants to avoid draining his battery / internet: ctrl+c until he's able to resume, like he would with a regular IBD
- user doesn't want to do background validation at all: like I said ear
...
💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391859626)
From https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2307124171)
> `Find1P1CPackage` doesn't actually check the normal reject filter at all with respect to the child, only reconsiderable, so that assertion gets hit. I suspect that's something we don't want it to do, even if it's not possible to hit inside net_processing.
Yes, this was my initial thought too.
> I think it might be more that `MempoolRejectedTx` doesn't remember if a tx failed for a different reason before c
...
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391859626)
From https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2307124171)
> `Find1P1CPackage` doesn't actually check the normal reject filter at all with respect to the child, only reconsiderable, so that assertion gets hit. I suspect that's something we don't want it to do, even if it's not possible to hit inside net_processing.
Yes, this was my initial thought too.
> I think it might be more that `MempoolRejectedTx` doesn't remember if a tx failed for a different reason before c
...
💬 Sjors commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391870256)
The main use case I have in mind is some who doesn't want to do background validation. The best reason for that is to save bandwidth, since storage can be saved by pruning just as well. They may later decide to perform this validation.
This PR enables that now in a fairly trivial way: it just doesn't download the blocks. Using process separation and the kernel is not something we can do in the near future. It also seems _more_ complicated than the approach here.
> user wants to avoid drain
...
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2391870256)
The main use case I have in mind is some who doesn't want to do background validation. The best reason for that is to save bandwidth, since storage can be saved by pruning just as well. They may later decide to perform this validation.
This PR enables that now in a fairly trivial way: it just doesn't download the blocks. Using process separation and the kernel is not something we can do in the near future. It also seems _more_ complicated than the approach here.
> user wants to avoid drain
...
💬 Christewart commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391884194)
This seems to be a spurious CI failure as this PR is a docs only change, can someone restart ? https://github.com/bitcoin/bitcoin/actions/runs/11164991068/job/31035622913?pr=30982
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391884194)
This seems to be a spurious CI failure as this PR is a docs only change, can someone restart ? https://github.com/bitcoin/bitcoin/actions/runs/11164991068/job/31035622913?pr=30982
💬 achow101 commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391888806)
ACK 27709f51ee02ed4f8c9c7e1c25c6f16faa0ef08b
Tested on MacOS 15.0 on both x86_64 and arm64. On x86_64, `codesign` was not needed, but it was needed on arm64. `xattr` needed for both.
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2391888806)
ACK 27709f51ee02ed4f8c9c7e1c25c6f16faa0ef08b
Tested on MacOS 15.0 on both x86_64 and arm64. On x86_64, `codesign` was not needed, but it was needed on arm64. `xattr` needed for both.