💬 ryanofsky commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1450530407)
Concept ACK on refreshing state of conflicted transactions in the wallet when a block is disconnected. I think there is a potential problem with the implementation though because it isn't recursively marking conflicted transactions as unconfirmed, only marking the top level transactions without recursing.
I think a good approach could be to steal from the previous implementation of this idea in commits b8b4592d985fa36dd0dddfd0e2bc6daf8df6e79c and 3c8bd6814ca88715d3d56994b10dc2c5f3e38e2d from
...
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1450530407)
Concept ACK on refreshing state of conflicted transactions in the wallet when a block is disconnected. I think there is a potential problem with the implementation though because it isn't recursively marking conflicted transactions as unconfirmed, only marking the top level transactions without recursing.
I think a good approach could be to steal from the previous implementation of this idea in commits b8b4592d985fa36dd0dddfd0e2bc6daf8df6e79c and 3c8bd6814ca88715d3d56994b10dc2c5f3e38e2d from
...
💬 TheCharlatan commented on pull request "guix: pass `--enable-initfini-array` to release GCC":
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450562166)
I can confirm that this adds relevant `.fini_array`sections and gets rid of the `.ctors` and `.dtors` sections in the bitcoin x86 _64 linux binaries by comparing the binaries of this PR's `HEAD` and its `HEAD~1`.
`objdump -h bitcoind` on fc733e9 (HEAD) :
```
bitcoind: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .interp 0000001c 0000000000000350 0000000000000350 00000350 2**0
CONTENT
...
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450562166)
I can confirm that this adds relevant `.fini_array`sections and gets rid of the `.ctors` and `.dtors` sections in the bitcoin x86 _64 linux binaries by comparing the binaries of this PR's `HEAD` and its `HEAD~1`.
`objdump -h bitcoind` on fc733e9 (HEAD) :
```
bitcoind: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .interp 0000001c 0000000000000350 0000000000000350 00000350 2**0
CONTENT
...
💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450575944)
ACK 802cc1ef536e11944608fe9ab782d3e962037703
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450575944)
ACK 802cc1ef536e11944608fe9ab782d3e962037703
💬 jonatack commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1450599651)
@pinheadmz Thanks for working on unit test coverage -- will circle back soon.
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1450599651)
@pinheadmz Thanks for working on unit test coverage -- will circle back soon.
💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122161205)
I ' ll actually remove the variable `executableCommand` since it's only used once, I prefer avoiding memory overhead and better speed execution (althoug minimal). Okey with the other 2 variables
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122161205)
I ' ll actually remove the variable `executableCommand` since it's only used once, I prefer avoiding memory overhead and better speed execution (althoug minimal). Okey with the other 2 variables
💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122164062)
Indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122164062)
Indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.
💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122165135)
Removed extra parenthesis, and I agree with extra long lines, but as I said before, indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122165135)
Removed extra parenthesis, and I agree with extra long lines, but as I said before, indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.
💬 furszy commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122170162)
If the view is not accessible anymore, this changes aren't needed.
(same for the transaction table model ones)
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122170162)
If the view is not accessible anymore, this changes aren't needed.
(same for the transaction table model ones)
💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1122178962)
> > upgraded the legacy wallet into a descriptors wallet
>
> I think it's best to have only one way to go from a legacy wallet to a descriptor wallet (the migration RPC), so in the long run it's easier to keep testing this stuff.
yeah, agree.
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1122178962)
> > upgraded the legacy wallet into a descriptors wallet
>
> I think it's best to have only one way to go from a legacy wallet to a descriptor wallet (the migration RPC), so in the long run it's easier to keep testing this stuff.
yeah, agree.
💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1450711073)
@willcl-ark thanks, I added comments and release notes.
I also wrote a tiny testing package using [libjson-rpc-cpp](https://github.com/cinemast/libjson-rpc-cpp) to check against an "outside" JSON-RPC 2.0 implementation. The package is https://github.com/pinheadmz/jsonrpc-bitcoin and [seems to like the 2.0 implementation so far.](https://github.com/pinheadmz/jsonrpc-bitcoin#examples)
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1450711073)
@willcl-ark thanks, I added comments and release notes.
I also wrote a tiny testing package using [libjson-rpc-cpp](https://github.com/cinemast/libjson-rpc-cpp) to check against an "outside" JSON-RPC 2.0 implementation. The package is https://github.com/pinheadmz/jsonrpc-bitcoin and [seems to like the 2.0 implementation so far.](https://github.com/pinheadmz/jsonrpc-bitcoin#examples)
💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122210010)
yeah, I'll do.
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122210010)
yeah, I'll do.
💬 mzumsande commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122020763)
if this is only necessary for windows, would it be possible to hide it behind `#ifdef WIN32` so that the unit tests don't run 3*2=6 seconds longer for everyone else (which is quite a lot)?
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122020763)
if this is only necessary for windows, would it be possible to hide it behind `#ifdef WIN32` so that the unit tests don't run 3*2=6 seconds longer for everyone else (which is quite a lot)?
💬 fanquake commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450764766)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450764766)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3
💬 fanquake commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#issuecomment-1450765037)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3.
(https://github.com/bitcoin/bitcoin/pull/27165#issuecomment-1450765037)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3.
💬 sipa commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450771004)
@fanquake Wrong issue, actually; the original was about `miniscript_stable`: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56270
The one you link to was about `miniscript_smart`, but that is in fact also addressed by #27165.
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450771004)
@fanquake Wrong issue, actually; the original was about `miniscript_stable`: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56270
The one you link to was about `miniscript_smart`, but that is in fact also addressed by #27165.
💬 fanquake commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450773295)
> Wrong issue, actually;
Whoops. Yes, both ended up being addressed here.
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450773295)
> Wrong issue, actually;
Whoops. Yes, both ended up being addressed here.
💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1450801900)
Rebased past #27172.
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1450801900)
Rebased past #27172.
💬 fanquake commented on pull request "build: LLVM 15 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1450803042)
Rebased past #27172.
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1450803042)
Rebased past #27172.
💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122285875)
Thanks, I guarded the sleep for WIN32, cleaned up the lint and mentioned the undo data in OP.
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122285875)
Thanks, I guarded the sleep for WIN32, cleaned up the lint and mentioned the undo data in OP.
💬 vostrnad commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1450843657)
Is there a reason the `v2transport` option should be off by default? Does it even need to exist at all? I was accidentally running this for a few days with v2 turned off and was confused why nodes advertising `P2P_V2` weren't actually using the v2 protocol. Everything works fine now.
Also, it would be nice to have the `transport_protocol_type` and `v2_session_id` fields displayed in the GUI, although that might possibly belong to a separate PR in the GUI repo (not sure what the development pr
...
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1450843657)
Is there a reason the `v2transport` option should be off by default? Does it even need to exist at all? I was accidentally running this for a few days with v2 turned off and was confused why nodes advertising `P2P_V2` weren't actually using the v2 protocol. Everything works fine now.
Also, it would be nice to have the `transport_protocol_type` and `v2_session_id` fields displayed in the GUI, although that might possibly belong to a separate PR in the GUI repo (not sure what the development pr
...