👍 prusnak approved a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282#pullrequestreview-2123899637)
utACK
(https://github.com/bitcoin/bitcoin/pull/30282#pullrequestreview-2123899637)
utACK
💬 achow101 commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174428338)
ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174428338)
ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
🚀 achow101 merged a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058)
(https://github.com/bitcoin/bitcoin/pull/30058)
💬 achow101 commented on pull request "Release `LockData::dd_mutex` before calling `*_detected` functions":
(https://github.com/bitcoin/bitcoin/pull/26781#issuecomment-2174448016)
@hebasto Are you planning on addressing review feedback here?
***
> I think it would be better if it had another commit getting rid of StdMutex/StdLockGuard. Otherwise nothing is really taking advantage of the change.
I agree. The PR description sounded like this was going to be the ultimate change, but it does not appear to have actually been done.
(https://github.com/bitcoin/bitcoin/pull/26781#issuecomment-2174448016)
@hebasto Are you planning on addressing review feedback here?
***
> I think it would be better if it had another commit getting rid of StdMutex/StdLockGuard. Otherwise nothing is really taking advantage of the change.
I agree. The PR description sounded like this was going to be the ultimate change, but it does not appear to have actually been done.
🚀 achow101 merged a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)
(https://github.com/bitcoin/bitcoin/pull/28984)
💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-2174450450)
> > > ```diff
> > > --- a/test/functional/wallet_import_rescan.py
> > > +++ b/test/functional/wallet_import_rescan.py
> > > @@ -293,6 +293,7 @@ class ImportRescanTest(BitcoinTestFramework):
> > > add_to_wallet=False,
> > > inputs=[unspent_txid_map[variant.initial_txid]],
> > > outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}],
> > > + locktime=0,
> > > subtract_fee_from_outputs=[0]
> > >
...
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-2174450450)
> > > ```diff
> > > --- a/test/functional/wallet_import_rescan.py
> > > +++ b/test/functional/wallet_import_rescan.py
> > > @@ -293,6 +293,7 @@ class ImportRescanTest(BitcoinTestFramework):
> > > add_to_wallet=False,
> > > inputs=[unspent_txid_map[variant.initial_txid]],
> > > outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}],
> > > + locktime=0,
> > > subtract_fee_from_outputs=[0]
> > >
...
💬 cbergqvist commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1643503903)
(Came across this block again while working on something else. It struck me that the exception handling looked redundant as we catch the same exact exception types inside `JSONRPCExec(jreq, /*catch_errors=*/true`). *But* the `jreq.parse()` call will throw on missing methods etc.
One could move out `response = JSONRPCExec(jreq, /*catch_errors=*/true);` to after the block, but then one would need to guard against using a `jreq` that failed to parse. So the current version of the block is probab
...
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1643503903)
(Came across this block again while working on something else. It struck me that the exception handling looked redundant as we catch the same exact exception types inside `JSONRPCExec(jreq, /*catch_errors=*/true`). *But* the `jreq.parse()` call will throw on missing methods etc.
One could move out `response = JSONRPCExec(jreq, /*catch_errors=*/true);` to after the block, but then one would need to guard against using a `jreq` that failed to parse. So the current version of the block is probab
...
💬 mzumsande commented on issue "RFC: Assumeutxo and large forks and reorgs":
(https://github.com/bitcoin/bitcoin/issues/30288#issuecomment-2174570099)
The problem I see with allowing the background sync to target the most-work chain is that it doesn't seem to fit well into the design philosophy that has been merged so far. The background sync is treated as a low-priority task that is mostly done as an extra precaution after having synced the snapshot to the tip. At some point in the future (I know this would be contentious) we might even want to give users the opt-in option to skip it completely to save bandwidth.
But as it is, the background
...
(https://github.com/bitcoin/bitcoin/issues/30288#issuecomment-2174570099)
The problem I see with allowing the background sync to target the most-work chain is that it doesn't seem to fit well into the design philosophy that has been merged so far. The background sync is treated as a low-priority task that is mostly done as an extra precaution after having synced the snapshot to the tip. At some point in the future (I know this would be contentious) we might even want to give users the opt-in option to skip it completely to save bandwidth.
But as it is, the background
...
📝 CharlesCNorton opened a pull request: "fix: typo in benchmark documentation"
(https://github.com/bitcoin/bitcoin/pull/30296)
Corrected "coins selection" to "coin selection" for consistency with the other items listed.
Justification:
Consistency: Aligns with other singular nouns in the list.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation f
...
(https://github.com/bitcoin/bitcoin/pull/30296)
Corrected "coins selection" to "coin selection" for consistency with the other items listed.
Justification:
Consistency: Aligns with other singular nouns in the list.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation f
...
💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2174686278)
Rebased on master (resolving a trivial merging conflict in the test runner list after the merge of #28984).
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2174686278)
Rebased on master (resolving a trivial merging conflict in the test runner list after the merge of #28984).
💬 theStack commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643600316)
> Is the sole reason for creating 10 tagged wallets to be thorough? Or is there any other reason as well?
Yes it's thoroughness, no other reason. Since the fixed bugs only occur with a certain probability, creating several random tagged wallets in a row increase the likelihood of catching the bugs in a single test run (nothing special about the number 10 though).
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643600316)
> Is the sole reason for creating 10 tagged wallets to be thorough? Or is there any other reason as well?
Yes it's thoroughness, no other reason. Since the fixed bugs only occur with a certain probability, creating several random tagged wallets in a row increase the likelihood of catching the bugs in a single test run (nothing special about the number 10 though).
⚠️ techy2 opened an issue: "unrecognized command line option '-std=c++20' "
(https://github.com/bitcoin/bitcoin/issues/30297)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
building depends qt
Project ERROR: Cannot run target compiler 'g++ -m64'. Output:
===================
Using built-in specs.
COLLECT_GCC=g++
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.4.0-1u
...
(https://github.com/bitcoin/bitcoin/issues/30297)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
building depends qt
Project ERROR: Cannot run target compiler 'g++ -m64'. Output:
===================
Using built-in specs.
COLLECT_GCC=g++
OFFLOAD_TARGET_NAMES=nvptx-none:hsa
OFFLOAD_TARGET_DEFAULT=1
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.4.0-1u
...
💬 kevkevinpal commented on pull request "upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2174738368)
looks like this is the diff between verison 2.2.7 and 2.2.8 in case anyone is looking
https://github.com/miniupnp/miniupnp/compare/miniupnpc_2_2_7...miniupnpc_2_2_8
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2174738368)
looks like this is the diff between verison 2.2.7 and 2.2.8 in case anyone is looking
https://github.com/miniupnp/miniupnp/compare/miniupnpc_2_2_7...miniupnpc_2_2_8
💬 sipa commented on issue "unrecognized command line option '-std=c++20' ":
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174741292)
You need a C++20 compiler to build Bitcoin Core 27.x. In particular, GCC 10.1 or Clang 14 are supported (see https://github.com/bitcoin/bitcoin/blob/v27.1/doc/dependencies.md).
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174741292)
You need a C++20 compiler to build Bitcoin Core 27.x. In particular, GCC 10.1 or Clang 14 are supported (see https://github.com/bitcoin/bitcoin/blob/v27.1/doc/dependencies.md).
💬 kevkevinpal commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2174757007)
Concept ACK [fa58c75](https://github.com/bitcoin/bitcoin/pull/30263/commits/fa58c75880334cc58ccc8e5d491df8f0e587bf42)
I'm on fedora and was able to build properly
```
uname -a
Linux fedora 6.2.15-300.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Thu May 11 17:37:39 UTC 2023 x86_64 GNU/Linux
clang --version
clang version 16.0.6 (Fedora 16.0.6-4.fc38)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang++ --version
clang version 16.0.6 (Fedora 16.0.6-4.fc38)
Targ
...
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2174757007)
Concept ACK [fa58c75](https://github.com/bitcoin/bitcoin/pull/30263/commits/fa58c75880334cc58ccc8e5d491df8f0e587bf42)
I'm on fedora and was able to build properly
```
uname -a
Linux fedora 6.2.15-300.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Thu May 11 17:37:39 UTC 2023 x86_64 GNU/Linux
clang --version
clang version 16.0.6 (Fedora 16.0.6-4.fc38)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang++ --version
clang version 16.0.6 (Fedora 16.0.6-4.fc38)
Targ
...
✅ techy2 closed an issue: "unrecognized command line option '-std=c++20' "
(https://github.com/bitcoin/bitcoin/issues/30297)
(https://github.com/bitcoin/bitcoin/issues/30297)
💬 techy2 commented on issue "unrecognized command line option '-std=c++20' ":
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174814665)
Ok
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174814665)
Ok
💬 techy2 commented on issue "unrecognized command line option '-std=c++20' ":
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174879320)
upgraded to gcc 10.3, all works now
(https://github.com/bitcoin/bitcoin/issues/30297#issuecomment-2174879320)
upgraded to gcc 10.3, all works now
💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#issuecomment-2175051513)
Got it, nice.
On Tue, Jun 18, 2024, 06:43 Sebastian Falbesoner ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In test/functional/feature_framework_miniwallet.py
> <https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643600316>:
>
> > @@ -31,6 +34,20 @@ def test_tx_padding(self):
> assert_greater_than_or_equal(tx.get_weight(), target_weight)
> assert_greater_than_or_equal(target_weight
...
(https://github.com/bitcoin/bitcoin/pull/30076#issuecomment-2175051513)
Got it, nice.
On Tue, Jun 18, 2024, 06:43 Sebastian Falbesoner ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In test/functional/feature_framework_miniwallet.py
> <https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643600316>:
>
> > @@ -31,6 +34,20 @@ def test_tx_padding(self):
> assert_greater_than_or_equal(tx.get_weight(), target_weight)
> assert_greater_than_or_equal(target_weight
...
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2175075429)
> Looks like the Apple Clang shipping with Xcode 15, is based on Clang 15. Xcode 16 (still in beta) will ship with a Apple Clang based on Clang 16.
Well, for some reason it can compile ranges fine, without further changes, so at least to some extend it seems to be based on llvm 16 (maybe an intermediate random commit from the `main` dev branch)?
In any case, I am not changing the Xcode/macOS/Apple requirements here. Someone else can do it, if there is need for it and it is acceptable to do
...
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2175075429)
> Looks like the Apple Clang shipping with Xcode 15, is based on Clang 15. Xcode 16 (still in beta) will ship with a Apple Clang based on Clang 16.
Well, for some reason it can compile ranges fine, without further changes, so at least to some extend it seems to be based on llvm 16 (maybe an intermediate random commit from the `main` dev branch)?
In any case, I am not changing the Xcode/macOS/Apple requirements here. Someone else can do it, if there is need for it and it is acceptable to do
...