🚀 achow101 merged a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056)
(https://github.com/bitcoin/bitcoin/pull/29056)
💬 theuni commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866692642)
@hebasto #24994 counted on linker side-effects to fix things IIRC. As do your other suggestions as far as I can see. If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866692642)
@hebasto #24994 counted on linker side-effects to fix things IIRC. As do your other suggestions as far as I can see. If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.
🤔 stickies-v reviewed a pull request: "Nuke adjusted time (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1793314902)
Concept ACK, and code LGTM 87c577e9742d7154826c755a7fe320f34fd54c81 (but needs rebase).
It is unfortunate that this is not a strict security improvement (by increasing the dependency on the local system clock, which can also be tampered with), but not having consensus rely on a majority of arbitrary peers (phrased very well [here](https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1264632475)) is a worthwhile improvement and in my view probably safer. A wrong system clock should be pr
...
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1793314902)
Concept ACK, and code LGTM 87c577e9742d7154826c755a7fe320f34fd54c81 (but needs rebase).
It is unfortunate that this is not a strict security improvement (by increasing the dependency on the local system clock, which can also be tampered with), but not having consensus rely on a majority of arbitrary peers (phrased very well [here](https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1264632475)) is a worthwhile improvement and in my view probably safer. A wrong system clock should be pr
...
💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434312496)
```suggestion
if (std::abs(median) > m_opts.time_offset_warn_threshold.count()) {
```
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434312496)
```suggestion
if (std::abs(median) > m_opts.time_offset_warn_threshold.count()) {
```
💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434286266)
nit: Especially since there's no git log for this new code, this comment is pretty difficult to understand for future readers. Would suggest adding a link to e.g. https://github.com/bitcoin/bitcoin/commit/93659379bdedc29a87fe351ba2950a2c976aebd9 for context
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1434286266)
nit: Especially since there's no git log for this new code, this comment is pretty difficult to understand for future readers. Would suggest adding a link to e.g. https://github.com/bitcoin/bitcoin/commit/93659379bdedc29a87fe351ba2950a2c976aebd9 for context
💬 hebasto commented on pull request "build: Remove HAVE_CONSENSUS_LIB":
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866699802)
@theuni
> If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.
https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1862877305:
> The underlying problem is described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022, and it was not considered as a bug.
(https://github.com/bitcoin/bitcoin/pull/29123#issuecomment-1866699802)
@theuni
> If there's an issue with a compiler/stdlib symbol visibility, let's track it down and get the issues reported upstream as opposed to working around them with brittle workarounds.
https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1862877305:
> The underlying problem is described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022, and it was not considered as a bug.
👍 hebasto approved a pull request: "build: Remove HAVE_CONSENSUS_LIB"
(https://github.com/bitcoin/bitcoin/pull/29123#pullrequestreview-1793422827)
ACK b37a171dbbcd9a7a8934099be1b508a0ea4f05a8.
(https://github.com/bitcoin/bitcoin/pull/29123#pullrequestreview-1793422827)
ACK b37a171dbbcd9a7a8934099be1b508a0ea4f05a8.
👋 brunoerg's pull request is ready for review: "wallet, rpc: add BIP44 `account` in `createwallet`"
(https://github.com/bitcoin/bitcoin/pull/29129)
(https://github.com/bitcoin/bitcoin/pull/29129)
💬 theuni commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1866725154)
> Seems to suggest that it is a good thing for these symbols to be exported so the library and its callers use the same symbols, particularly type_info symbols
We purposefully export only a C api to avoid exactly these type issues :)
The user's/lib's c++ impls should be completely firewalled from each-other.
So I agree that this is annoying and maybe worth a hack to fix. But I'd like to exhaust all other options first.
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1866725154)
> Seems to suggest that it is a good thing for these symbols to be exported so the library and its callers use the same symbols, particularly type_info symbols
We purposefully export only a C api to avoid exactly these type issues :)
The user's/lib's c++ impls should be completely firewalled from each-other.
So I agree that this is annoying and maybe worth a hack to fix. But I'd like to exhaust all other options first.
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/29125)
The creation and this repository deserve more comprehensive expansions and a wider audience. Therefore, it requires universal license updates for further survival. This pull request proposes minor license updates that include:
* Constraints of harmful use
* Broader definition of who and how can this software
* Universality
* Inclusion of derivatives
(https://github.com/bitcoin/bitcoin/pull/29125)
The creation and this repository deserve more comprehensive expansions and a wider audience. Therefore, it requires universal license updates for further survival. This pull request proposes minor license updates that include:
* Constraints of harmful use
* Broader definition of who and how can this software
* Universality
* Inclusion of derivatives
💬 alpeshvas commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866760674)
> Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
>
> The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
>
> If this PR will fix the fee rates back to normal I am on board.
Fee rates will anyway
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866760674)
> Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
>
> The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
>
> If this PR will fix the fee rates back to normal I am on board.
Fee rates will anyway
...
💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866775284)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866636473
> If you have a blank wallet, why would you want creating a multisig desriptor and creating the hd key to be two separate steps, and require a separate `sethdseed` call before whatever call is used to add the multisig descriptor? Wouldn't the flow be simpler if you just made one call to `createwalletdescriptor` or `importdescriptor` and it did both things? (**EDIT**: Actually rereading this, I can see why this wouldn't
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866775284)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866636473
> If you have a blank wallet, why would you want creating a multisig desriptor and creating the hd key to be two separate steps, and require a separate `sethdseed` call before whatever call is used to add the multisig descriptor? Wouldn't the flow be simpler if you just made one call to `createwalletdescriptor` or `importdescriptor` and it did both things? (**EDIT**: Actually rereading this, I can see why this wouldn't
...
👍 TheCharlatan approved a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880#pullrequestreview-1793505067)
ACK b335710782c2545e6eeed67b5e1763c07eab26b0
Guix builds (aarch64 & x86_64)
```
65981453fcb83338ed76435dc8fed06e9903441731b461e4f2c8a04ad102043c guix-build-b335710782c2/output/aarch64-linux-gnu/SHA256SUMS.part
3127d18bc55a7a207eb7e1252feaf24f767d5983e5d064323671139659c4f3d7 guix-build-b335710782c2/output/aarch64-linux-gnu/bitcoin-b335710782c2-aarch64-linux-gnu-debug.tar.gz
0a67737c010e742249f07dd3ca3ae289df02edf2dedd8d6a5a07b291921bab47 guix-build-b335710782c2/output/aarch64-linux-gnu/
...
(https://github.com/bitcoin/bitcoin/pull/28880#pullrequestreview-1793505067)
ACK b335710782c2545e6eeed67b5e1763c07eab26b0
Guix builds (aarch64 & x86_64)
```
65981453fcb83338ed76435dc8fed06e9903441731b461e4f2c8a04ad102043c guix-build-b335710782c2/output/aarch64-linux-gnu/SHA256SUMS.part
3127d18bc55a7a207eb7e1252feaf24f767d5983e5d064323671139659c4f3d7 guix-build-b335710782c2/output/aarch64-linux-gnu/bitcoin-b335710782c2-aarch64-linux-gnu-debug.tar.gz
0a67737c010e742249f07dd3ca3ae289df02edf2dedd8d6a5a07b291921bab47 guix-build-b335710782c2/output/aarch64-linux-gnu/
...
💬 TheCharlatan commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#discussion_r1434399894)
Nit: I guess this should mention `std::sort` too? Or maybe even put that into a separate patch?
(https://github.com/bitcoin/bitcoin/pull/28880#discussion_r1434399894)
Nit: I guess this should mention `std::sort` too? Or maybe even put that into a separate patch?
💬 petertodd commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866795214)
On Thu, Dec 21, 2023 at 07:47:05AM -0800, farahats9 wrote:
> Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
>
> The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
>
> If this PR will fix the fee rates
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866795214)
On Thu, Dec 21, 2023 at 07:47:05AM -0800, farahats9 wrote:
> Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
>
> The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
>
> If this PR will fix the fee rates
...
💬 PerpetualWar commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866837996)
> > Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
> > The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
> > If this PR will fix the fee rates back to normal I am on board.
>
> Fee rates will anywa
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1866837996)
> > Just wanted to add my 2 cents here as a layman using and holding bitcoin. If the transaction fee rates remain this high for someone using bitcoin to transfer money from wallet A to wallet B then I don't see a future for bitcoin.
> > The original goal of bitcoin is to make it a currency and a valued asset, if you sacrifice doing this in order to have secondary features then you will fail at both.
> > If this PR will fix the fee rates back to normal I am on board.
>
> Fee rates will anywa
...
💬 GregTonoski commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1866890269)
Seems to be fixed in 26.0. Thanks.
Example:
```
./bitcoin-cli.exe -testnet signrawtransactionwithkey 020000000165ef750aac862b0177cadb77961bf1a936e2bec0376b286f5b4e1b6255cf3a960000000000fdffffff012c1a0000000000002251203b82b2b2a9185315da6f80da5f06d0440d8a5e1457fa93387c2d919c86ec878600000000 '["cV628xvqToz45dwdPmTcJ9RgEVnWMwP8dpZBGzb9LfTk3sBHFNwc"]' '[{"txid":"963acf55621b4e5b6f286b37c0bee236a9f11b9677dbca77012b86ac0a75ef65","vout":0,"scriptPubKey":"512055355ca83c973f1d97ce0e3843c85d78905af16b
...
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1866890269)
Seems to be fixed in 26.0. Thanks.
Example:
```
./bitcoin-cli.exe -testnet signrawtransactionwithkey 020000000165ef750aac862b0177cadb77961bf1a936e2bec0376b286f5b4e1b6255cf3a960000000000fdffffff012c1a0000000000002251203b82b2b2a9185315da6f80da5f06d0440d8a5e1457fa93387c2d919c86ec878600000000 '["cV628xvqToz45dwdPmTcJ9RgEVnWMwP8dpZBGzb9LfTk3sBHFNwc"]' '[{"txid":"963acf55621b4e5b6f286b37c0bee236a9f11b9677dbca77012b86ac0a75ef65","vout":0,"scriptPubKey":"512055355ca83c973f1d97ce0e3843c85d78905af16b
...
💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1866934995)
Force pushed twice to rebase and fix the CI problem, ready for review, thanks, @furszy!
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1866934995)
Force pushed twice to rebase and fix the CI problem, ready for review, thanks, @furszy!
💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865)
achow101, S3RK, Sjors, and me talked about this on a call. I think the plan for now is that achow will make 2 independent PRs targeted at different use-cases:
- First PR would add a `gethdkey` method like the one in this PR and a `createwalletdescriptor` method like the one in [#25907](https://github.com/bitcoin/bitcoin/pull/25907) that should both make reusing existing hd keys easier. Specifically these methods should be useful for adding new types of descriptors to existing wallets and upgr
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865)
achow101, S3RK, Sjors, and me talked about this on a call. I think the plan for now is that achow will make 2 independent PRs targeted at different use-cases:
- First PR would add a `gethdkey` method like the one in this PR and a `createwalletdescriptor` method like the one in [#25907](https://github.com/bitcoin/bitcoin/pull/25907) that should both make reusing existing hd keys easier. Specifically these methods should be useful for adding new types of descriptors to existing wallets and upgr
...
👋 luke-jr's pull request is ready for review: "Prune locks"
(https://github.com/bitcoin/bitcoin/pull/19463)
(https://github.com/bitcoin/bitcoin/pull/19463)