💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867824658)
@brunoerg most likely yes, but I can't find a full list anywhere.
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867824658)
@brunoerg most likely yes, but I can't find a full list anywhere.
📝 maflcko opened a pull request: " refactor: Allow std::span construction from CKey "
(https://github.com/bitcoin/bitcoin/pull/29133)
Is is possible to construct a `Span` from a reference to a `CKey`. However, the same is not possible with `std::span`.
Fix that, and add a commit to verify the fix.
(https://github.com/bitcoin/bitcoin/pull/29133)
Is is possible to construct a `Span` from a reference to a `CKey`. However, the same is not possible with `std::span`.
Fix that, and add a commit to verify the fix.
💬 brunoerg commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867855528)
> @brunoerg most likely yes, but I can't find a full list anywhere.
Me either. I've some of them but I manually mapped.
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867855528)
> @brunoerg most likely yes, but I can't find a full list anywhere.
Me either. I've some of them but I manually mapped.
💬 fanquake commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867858105)
> Adding one more bit to complicate
Thanks, I'll pile on some more. I've alluded to this in other threads (#24994), but still haven't seen a summary of:
* What currently happens with `lld` + `libstdc++`?
* What currently happens with `lld` + `libc++`?
* Maybe even, what happens with [`mold`](https://github.com/rui314/mold) and either standard library?
* What happens under LTO, in any combination of the above?
There is much more to consider here than "What does `ld` happen to do", and t
...
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867858105)
> Adding one more bit to complicate
Thanks, I'll pile on some more. I've alluded to this in other threads (#24994), but still haven't seen a summary of:
* What currently happens with `lld` + `libstdc++`?
* What currently happens with `lld` + `libc++`?
* Maybe even, what happens with [`mold`](https://github.com/rui314/mold) and either standard library?
* What happens under LTO, in any combination of the above?
There is much more to consider here than "What does `ld` happen to do", and t
...
💬 mzumsande commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-1867862091)
I added a commit with a benchmark, and another one that allow to specify the frequency for `-checkblockindex` (analogous to the way `-checkaddrman` and `-checkmempool` work).
> Another optimization that might be worth it, it is to replace the std::multimap<CBlockIndex*,CBlockIndex*> forward with an std::unordered_map<CBlockIndex*, std::vector<CBlockIndex*>> forward.
I tried that in `https://github.com/mzumsande/bitcoin/commit/51aae54cfafc9700581e69dc129866cff76925ae` (directly on master wi
...
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-1867862091)
I added a commit with a benchmark, and another one that allow to specify the frequency for `-checkblockindex` (analogous to the way `-checkaddrman` and `-checkmempool` work).
> Another optimization that might be worth it, it is to replace the std::multimap<CBlockIndex*,CBlockIndex*> forward with an std::unordered_map<CBlockIndex*, std::vector<CBlockIndex*>> forward.
I tried that in `https://github.com/mzumsande/bitcoin/commit/51aae54cfafc9700581e69dc129866cff76925ae` (directly on master wi
...
💬 sipa commented on pull request "lib: add taproot support to libconsensus":
(https://github.com/bitcoin/bitcoin/pull/28539#issuecomment-1867869031)
@panicfarm libconsensus had certainly contained all the logic for taproot validation ever since Bitcoin Core had it, but it was not exposed through the library interface, for two reasons:
* The API requires passing in flags to select which softfork rules to apply to validation. No such flag was exposed for taproot.
* Taproot validation requires access to more information that wasn't present in the libconsensus API (specifically, the UTXO data of all spent UTXOs by a transaction). New functions
...
(https://github.com/bitcoin/bitcoin/pull/28539#issuecomment-1867869031)
@panicfarm libconsensus had certainly contained all the logic for taproot validation ever since Bitcoin Core had it, but it was not exposed through the library interface, for two reasons:
* The API requires passing in flags to select which softfork rules to apply to validation. No such flag was exposed for taproot.
* Taproot validation requires access to more information that wasn't present in the libconsensus API (specifically, the UTXO data of all spent UTXOs by a transaction). New functions
...
💬 hebasto commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867869356)
Our current release process uses a very particular toolchain. While LTO might be considered as its update in the near future, it is not clear how other combinations of tools are related to this problem? Unless we would prefer to alter the release toolchain instead of moving some code into a separated translation unit to fix the shared library symbol visibility.
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867869356)
Our current release process uses a very particular toolchain. While LTO might be considered as its update in the near future, it is not clear how other combinations of tools are related to this problem? Unless we would prefer to alter the release toolchain instead of moving some code into a separated translation unit to fix the shared library symbol visibility.
💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867873246)
Thanks, I was also trying to think of names for these other than void descriptors. For example maybe they could be called data descriptors and written like data(xpub...) to convey the idea that these descriptors are just used to hold data and don't produce scriptpubkeys.
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867873246)
Thanks, I was also trying to think of names for these other than void descriptors. For example maybe they could be called data descriptors and written like data(xpub...) to convey the idea that these descriptors are just used to hold data and don't produce scriptpubkeys.
💬 fanquake commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867877948)
> Our current release process
This isn't specific to the release process. Anyone can, and should expect to be able to build "working" libs/bins outside of Guix.
> it is not clear how other combinations of tools are related to this problem?
If you "fix' anything by doing something that isn't supported by those tools, i.e use a special link/compile flag, or rely on ld or libc++ specific behaviors, you're going to break things/be incompatible with anyone using any other tools. Also, any ch
...
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867877948)
> Our current release process
This isn't specific to the release process. Anyone can, and should expect to be able to build "working" libs/bins outside of Guix.
> it is not clear how other combinations of tools are related to this problem?
If you "fix' anything by doing something that isn't supported by those tools, i.e use a special link/compile flag, or rely on ld or libc++ specific behaviors, you're going to break things/be incompatible with anyone using any other tools. Also, any ch
...
💬 mzumsande commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-1867883982)
Benchmark results:
<details>
<summary>master</summary>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 331,165.67 | 3,019.64 | 0.3% | 0.01 | `CheckBlockIndex`
| 320,603.00 | 3,119.12 | 1.7% | 0.01 | `CheckBlockIndex`
| 315,683.67 | 3,167.73 | 0.4% | 0.01 | `CheckBlockIndex`
| 329,077.3
...
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-1867883982)
Benchmark results:
<details>
<summary>master</summary>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 331,165.67 | 3,019.64 | 0.3% | 0.01 | `CheckBlockIndex`
| 320,603.00 | 3,119.12 | 1.7% | 0.01 | `CheckBlockIndex`
| 315,683.67 | 3,167.73 | 0.4% | 0.01 | `CheckBlockIndex`
| 329,077.3
...
👋 mzumsande's pull request is ready for review: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339)
(https://github.com/bitcoin/bitcoin/pull/28339)
💬 hebasto commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867886871)
> If you "fix' anything by doing something that isn't supported by those tools, i.e use a special link/compile flag, or rely on ld or libc++ specific behaviors, you're going to break things/be incompatible with anyone using any other tools.
Right. Requiring that a fix does not introduce any regressions is obvious.
By the way, there were no comments regarding any introduced break in https://github.com/bitcoin/bitcoin/pull/24994.
I mean, what is the point to analyze the current behavior wit
...
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867886871)
> If you "fix' anything by doing something that isn't supported by those tools, i.e use a special link/compile flag, or rely on ld or libc++ specific behaviors, you're going to break things/be incompatible with anyone using any other tools.
Right. Requiring that a fix does not introduce any regressions is obvious.
By the way, there were no comments regarding any introduced break in https://github.com/bitcoin/bitcoin/pull/24994.
I mean, what is the point to analyze the current behavior wit
...
💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867889571)
> This is not actually true. Thinking about this more, I wonder if a more natural way of representing standalone keys in the wallet is just to treat them as a new [descriptor type](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md#reference), like `void(KEY)`, where the void descriptor would hold the key, but instead of [expanding](https://github.com/bitcoin/bitcoin/blob/4b1196a9855dcd188a24f393aa2fa21e2d61f061/src/script/descriptor.h#L98) to a scriptPubKey or set of scriptPubKey
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867889571)
> This is not actually true. Thinking about this more, I wonder if a more natural way of representing standalone keys in the wallet is just to treat them as a new [descriptor type](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md#reference), like `void(KEY)`, where the void descriptor would hold the key, but instead of [expanding](https://github.com/bitcoin/bitcoin/blob/4b1196a9855dcd188a24f393aa2fa21e2d61f061/src/script/descriptor.h#L98) to a scriptPubKey or set of scriptPubKey
...
🤔 murchandamus reviewed a pull request: "wallet: Add CoinGrinder coin selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/27877#pullrequestreview-1794686545)
Rebased and addressed comments by @kashifs. Thanks for the thorough read, @kashifs.
(https://github.com/bitcoin/bitcoin/pull/27877#pullrequestreview-1794686545)
Rebased and addressed comments by @kashifs. Thanks for the thorough read, @kashifs.
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435191286)
Thanks, I’ll fix that
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435191286)
Thanks, I’ll fix that
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435192987)
Thanks, I’ve amended the documentation
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435192987)
Thanks, I’ve amended the documentation
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435191774)
Fixed, thanks
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1435191774)
Fixed, thanks
📝 TomBriar opened a pull request: "Compressed Bitcoin Transactions"
(https://github.com/bitcoin/bitcoin/pull/29134)
This is a draft PR and reference implementation for a Compressed Bitcoin Transaction, As stated in the doc the main application for this is steganography, satellite/radio broadcast, and other low bandwidth channels with a high CPU availability on decompression.
doc: Added Compressed Transaction Schema Documentation
util: Added a variable length bitstream encoder
script: Added the rest of the IsPayTo Script functions, And Split ExtractDestination into two functions for ease of use
node: Add
...
(https://github.com/bitcoin/bitcoin/pull/29134)
This is a draft PR and reference implementation for a Compressed Bitcoin Transaction, As stated in the doc the main application for this is steganography, satellite/radio broadcast, and other low bandwidth channels with a high CPU availability on decompression.
doc: Added Compressed Transaction Schema Documentation
util: Added a variable length bitstream encoder
script: Added the rest of the IsPayTo Script functions, And Split ExtractDestination into two functions for ease of use
node: Add
...
💬 murchandamus commented on issue "Flaky `wallet_transactiontime_rescan.py --legacy-wallet` functional test":
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1867929889)
Well, if it’s just me that is affected that might be resolved by getting a faster computer.
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1867929889)
Well, if it’s just me that is affected that might be resolved by getting a faster computer.
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1435262125)
I don't quite follow: The fixed seeds will eventually be added to addrman as a fallback unconditionally, both in the case where we got sufficient addresses from addrfetch peers and in the case where that didn't work for some reason.
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1435262125)
I don't quite follow: The fixed seeds will eventually be added to addrman as a fallback unconditionally, both in the case where we got sufficient addresses from addrfetch peers and in the case where that didn't work for some reason.