💬 ismaelsadeeq commented on issue "Why does `submitpackage` require at least two transactions":
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2412384410)
It won't hurt to broadcast a single transaction if it passes consensus and policy checks via `submitpackage` RPC.
From the [package relay BIP](https://github.com/bitcoin/bips/blob/632f143fadaed8c91bba12d096e62feb2a308054/bip-0331.mediawiki#definitions) and the [policy documentation](https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md), I see that a package is defined as a child transaction with its unconfirmed ancestors, topologically sorted.
I think the intent of the che
...
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2412384410)
It won't hurt to broadcast a single transaction if it passes consensus and policy checks via `submitpackage` RPC.
From the [package relay BIP](https://github.com/bitcoin/bips/blob/632f143fadaed8c91bba12d096e62feb2a308054/bip-0331.mediawiki#definitions) and the [policy documentation](https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md), I see that a package is defined as a child transaction with its unconfirmed ancestors, topologically sorted.
I think the intent of the che
...
💬 1440000bytes commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2412386930)
@denavila any update on 3 and 4 from https://github.com/bitcoin-core/gui/pull/733#issuecomment-2083190086?
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2412386930)
@denavila any update on 3 and 4 from https://github.com/bitcoin-core/gui/pull/733#issuecomment-2083190086?
💬 TheBlueMatt commented on issue "Why does `submitpackage` require at least two transactions":
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2412484265)
From a Bitcoin Core perspective, sure. From the users' perspective I have a bundle of transactions and I want to get them relayed. They may be a package, they may not, I want Bitcoin Core to figure that out, not me.
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2412484265)
From a Bitcoin Core perspective, sure. From the users' perspective I have a bundle of transactions and I want to get them relayed. They may be a package, they may not, I want Bitcoin Core to figure that out, not me.
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2412512371)
Finished the same benchmarks on a HDD with very similar results:
<details>
<summary>benchmark</summary>
```
# hyperfine \
--runs 1 \
--export-json /mnt/my_storage/ibd_full-write-chainstate-every-hour.json \
--parameter-list COMMIT 5e90501abe13731579c61b13092b31b9fc4f8c4d,ffc9df87a0da35b451720556bdb3f0f65f315c8f,f5e7b1508a06c0c7a9c06d1ac270840fd72e9e6f \
--prepare 'rm -rf /mnt/my_storage/BitcoinData/* && git checkout {COMMIT} && git clean -fxd && git reset \
--hard && cmake -B build
...
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2412512371)
Finished the same benchmarks on a HDD with very similar results:
<details>
<summary>benchmark</summary>
```
# hyperfine \
--runs 1 \
--export-json /mnt/my_storage/ibd_full-write-chainstate-every-hour.json \
--parameter-list COMMIT 5e90501abe13731579c61b13092b31b9fc4f8c4d,ffc9df87a0da35b451720556bdb3f0f65f315c8f,f5e7b1508a06c0c7a9c06d1ac270840fd72e9e6f \
--prepare 'rm -rf /mnt/my_storage/BitcoinData/* && git checkout {COMMIT} && git clean -fxd && git reset \
--hard && cmake -B build
...
💬 brunoerg commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2413148520)
Worth adding a release note?
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2413148520)
Worth adding a release note?
📝 l0rinc opened a pull request: "CI: Add label to scripted-diffs"
(https://github.com/bitcoin/bitcoin/pull/31089)
testing
(https://github.com/bitcoin/bitcoin/pull/31089)
testing
✅ hebasto closed an issue: "CMake-based build system tracking issue"
(https://github.com/bitcoin/bitcoin/issues/28607)
(https://github.com/bitcoin/bitcoin/issues/28607)
💬 hebasto commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2413162020)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2413162020)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
📝 l0rinc converted_to_draft a pull request: "CI: Add label to scripted-diffs"
(https://github.com/bitcoin/bitcoin/pull/31089)
Add a `scripted-diff` label for pull requests that contain `scripted-diff:` commits to make sure that CI is running them properly.
(https://github.com/bitcoin/bitcoin/pull/31089)
Add a `scripted-diff` label for pull requests that contain `scripted-diff:` commits to make sure that CI is running them properly.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1800679447)
This was introduced in b335710782c2545e6eeed67b5e1763c07eab26b0.
Related to https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1867610348.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1800679447)
This was introduced in b335710782c2545e6eeed67b5e1763c07eab26b0.
Related to https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1867610348.
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800702587)
Can you suggest another name? It's a map to get peer info, hence `m_peer_info`.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800702587)
Can you suggest another name? It's a map to get peer info, hence `m_peer_info`.
💬 hebasto commented on issue "Run coverage functional tests in parallel":
(https://github.com/bitcoin/bitcoin/issues/26736#issuecomment-2413249267)
> This was probably resolved by `cmake`.
From the "Compiling for test coverage" section of [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage):
> To enable test parallelism:
>
> cmake -DJOBS=$(nproc) -P build/Coverage.cmake
(https://github.com/bitcoin/bitcoin/issues/26736#issuecomment-2413249267)
> This was probably resolved by `cmake`.
From the "Compiling for test coverage" section of [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage):
> To enable test parallelism:
>
> cmake -DJOBS=$(nproc) -P build/Coverage.cmake
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800708031)
Grep will show `AddTxAnnouncement` called with `p2p_inv=false` in the new orphan handling code. We don't exit because we might already have a low feerate parent in reconsiderable rejects. You can also comment this out to see what fails.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800708031)
Grep will show `AddTxAnnouncement` called with `p2p_inv=false` in the new orphan handling code. We don't exit because we might already have a low feerate parent in reconsiderable rejects. You can also comment this out to see what fails.
💬 laanwj commented on pull request "Don't zero-after-free `DataStream`: Faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2413262312)
Concept ACK. Using zero-after-free allocators for all CDataStream usage is overkill. i don't think there are any, but if there are places where it is used (wallet) where security against key leaks is important, we could parametrize `CDataStream<AllocatorType>` and pass a `secure_allocator`. Agree that zeroing adding extra security against buffer overflows is a red herring.
i expect this can make a significant differences on systems with relatively slow CPU or memory, like ARM systems (will r
...
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2413262312)
Concept ACK. Using zero-after-free allocators for all CDataStream usage is overkill. i don't think there are any, but if there are places where it is used (wallet) where security against key leaks is important, we could parametrize `CDataStream<AllocatorType>` and pass a `secure_allocator`. Agree that zeroing adding extra security against buffer overflows is a red herring.
i expect this can make a significant differences on systems with relatively slow CPU or memory, like ARM systems (will r
...
💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2413278619)
Unfortunately limiting CPU and memory usage in Windows itself and in Project Lasso didn't work. Core still hangs while indexing after a few hours & the blocks are corrupted afterwards. It must be a hardware issue, as no one else experiences this issue.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2413278619)
Unfortunately limiting CPU and memory usage in Windows itself and in Project Lasso didn't work. Core still hangs while indexing after a few hours & the blocks are corrupted afterwards. It must be a hardware issue, as no one else experiences this issue.
💬 hebasto commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413296884)
> > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
>
> If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
You mean "13.0" and "12.0", right?
For the reference, macOS 12 was last updated on 2024-07-29. The three most recent Apple updates
...
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413296884)
> > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
>
> If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
You mean "13.0" and "12.0", right?
For the reference, macOS 12 was last updated on 2024-07-29. The three most recent Apple updates
...
💬 hebasto commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413322066)
If we raise the minimum supported macOS version to 13.0, it would mean dropping support for hardware like MacBook Pro (15-inch, 2016) and older, which Apple classifies as ["obsolete worldwide"](https://support.apple.com/en-us/102772).
I'll update this PR shortly.
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413322066)
If we raise the minimum supported macOS version to 13.0, it would mean dropping support for hardware like MacBook Pro (15-inch, 2016) and older, which Apple classifies as ["obsolete worldwide"](https://support.apple.com/en-us/102772).
I'll update this PR shortly.
💬 l0rinc commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1800757127)
I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept) and adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
Could we configure them via env variables instead and call the benchmark with e.g. `TEST_DATA_DIR=a/b/c bench` instead?
As an example, if a few of our tests require a for example a timestamp for whatever reason, we wouldn't add it to the testing
...
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1800757127)
I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept) and adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
Could we configure them via env variables instead and call the benchmark with e.g. `TEST_DATA_DIR=a/b/c bench` instead?
As an example, if a few of our tests require a for example a timestamp for whatever reason, we wouldn't add it to the testing
...
💬 maflcko commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413340369)
Not sure what the goal here would be? Why add a label, when there is already a commit message prefix, which is identical in the meaning?
It would be better to just fix the bugs like https://github.com/bitcoin/bitcoin/issues/29692#issuecomment-2411656801 to ensure the CI is red when there is an issue.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413340369)
Not sure what the goal here would be? Why add a label, when there is already a commit message prefix, which is identical in the meaning?
It would be better to just fix the bugs like https://github.com/bitcoin/bitcoin/issues/29692#issuecomment-2411656801 to ensure the CI is red when there is an issue.
💬 maflcko commented on pull request "miniscript: convert non-critical asserts to Assumes":
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-2413349588)
@sipa @darosior It would be good to mark this as up-for-grabs, if you dropped working on it.
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-2413349588)
@sipa @darosior It would be good to mark this as up-for-grabs, if you dropped working on it.