💬 ryanofsky commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867721948)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865
> The new keys would need to be stored in a new database field
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://g
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867721948)
re: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865
> The new keys would need to be stored in a new database field
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://g
...
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435094066)
Done
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435094066)
Done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435094504)
Done as suggested
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435094504)
Done as suggested
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435096415)
Sure, thanks!. Done as suggested.
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435096415)
Sure, thanks!. Done as suggested.
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435097429)
Dropped per comment https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1434937507. Thanks
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435097429)
Dropped per comment https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1434937507. Thanks
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435097994)
Sure. Have reordered the commits.
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435097994)
Sure. Have reordered the commits.
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867747340)
Mmm, on the bright side, both the master and this PR have a really unclear message when the device is not connected :-)
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867747340)
Mmm, on the bright side, both the master and this PR have a really unclear message when the device is not connected :-)
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867748967)
@achow101 any thoughts on having HWI catch this cryptic `0x6982` status code and throw a more clear error (that we can then just pass on to the user)?
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867748967)
@achow101 any thoughts on having HWI catch this cryptic `0x6982` status code and throw a more clear error (that we can then just pass on to the user)?
💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867754041)
@ryanofsky interesting idea, I especially like the backup implication, because descriptors can be used for paper backups.
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867754041)
@ryanofsky interesting idea, I especially like the backup implication, because descriptors can be used for paper backups.
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435131324)
This is the only change I did not implement. I think we should discuss further where to place this constants before moving them to the header. It seems that with every new unit test, we will be forced to move some/most constants from `net_processing.cpp` to the header.
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1435131324)
This is the only change I did not implement. I think we should discuss further where to place this constants before moving them to the header. It seems that with every new unit test, we will be forced to move some/most constants from `net_processing.cpp` to the header.
💬 brunoerg commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867779809)
> @achow101 any thoughts on having HWI catch this cryptic 0x6982 status code and throw a more clear error (that we can then just pass on to the user)?
Not sure, but this status code is specific from Ledger?
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1867779809)
> @achow101 any thoughts on having HWI catch this cryptic 0x6982 status code and throw a more clear error (that we can then just pass on to the user)?
Not sure, but this status code is specific from Ledger?
💬 theuni commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867794427)
Adding one more bit to complicate and maybe explain why I described the boundary as a firewall... We build `libbitcoinconsensus.so` with guix using `-static-libstdc++` (side-note, we should perhaps move this into the build-system rather than guix).
So the idea here is that the entire c++ implementation should be compiled into the shared lib with no unresolved c++ stdlib symbols, and exporting only C symbols. That way the end-user can use any c++ impl (if any) they want without fear of collisi
...
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1867794427)
Adding one more bit to complicate and maybe explain why I described the boundary as a firewall... We build `libbitcoinconsensus.so` with guix using `-static-libstdc++` (side-note, we should perhaps move this into the build-system rather than guix).
So the idea here is that the entire c++ implementation should be compiled into the shared lib with no unresolved c++ stdlib symbols, and exporting only C symbols. That way the end-user can use any c++ impl (if any) they want without fear of collisi
...
💬 hebasto commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1867811444)
> I'd like to help with this, but I just don't understand most of the discussion so far.
My apologies about that.
> I see that this fixes some kind of a bug on windows and is supposed to have other benefits mentioned in the PR description, but I don't see a clear description of the bug anywhere.
https://github.com/bitcoin/bitcoin/pull/24994#issue-1216293178:
> This PR prunes all of [unnecessary](https://github.com/bitcoin/bitcoin/pull/25020#issuecomment-1142151675) symbols being expor
...
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1867811444)
> I'd like to help with this, but I just don't understand most of the discussion so far.
My apologies about that.
> I see that this fixes some kind of a bug on windows and is supposed to have other benefits mentioned in the PR description, but I don't see a clear description of the bug anywhere.
https://github.com/bitcoin/bitcoin/pull/24994#issue-1216293178:
> This PR prunes all of [unnecessary](https://github.com/bitcoin/bitcoin/pull/25020#issuecomment-1142151675) symbols being expor
...
💬 sipa commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1867811476)
utACK fab41697a5448ef2861f65795bd63a4ccdda6a40
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1867811476)
utACK fab41697a5448ef2861f65795bd63a4ccdda6a40
💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1435164703)
Looks like I'm not the only one who took a minute to get this 😅
I wouldn't mind expanding the comment if there is am overall agreement that it is incomplete
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1435164703)
Looks like I'm not the only one who took a minute to get this 😅
I wouldn't mind expanding the comment if there is am overall agreement that it is incomplete
💬 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
...