💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#issuecomment-1492224218)
Tested that this fixes the coin selection + use available balance GUI issue for me. Haven't reviewed the code yet.
(https://github.com/bitcoin/bitcoin/pull/26699#issuecomment-1492224218)
Tested that this fixes the coin selection + use available balance GUI issue for me. Haven't reviewed the code yet.
🤔 vostrnad requested changes to a pull request: "script: add description for the functionality of each opcode"
(https://github.com/bitcoin/bitcoin/pull/27109)
(https://github.com/bitcoin/bitcoin/pull/27109)
💬 vostrnad commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1131953241)
typo in multi
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1131953241)
typo in multi
💬 vostrnad commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1131953155)
misspelled occurring (more instances further below)
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1131953155)
misspelled occurring (more instances further below)
💬 vostrnad commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1154694800)
Additionally, at least according to Bitcoin Optech, "multisignature" should not be confused with "multisig": https://bitcoinops.org/en/topics/multisignature/
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1154694800)
Additionally, at least according to Bitcoin Optech, "multisignature" should not be confused with "multisig": https://bitcoinops.org/en/topics/multisignature/
💬 Sjors commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1154696924)
I guess we can say "for multiple signatures" (but nobody reading this deep inside the codebase will be confused)
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1154696924)
I guess we can say "for multiple signatures" (but nobody reading this deep inside the codebase will be confused)
💬 MarcoFalke commented on pull request "test: Remove python3.5 workaround":
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492272522)
This patch here disables the server timeout, so there shouldn't be any reason for the socket to be closed while `send`ing on macos. However, if that workaround is still needed, it should be guarded by a macos check. So my suggestion would be to assume the workaround is no longer needed, unless someone can prove otherwise?
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492272522)
This patch here disables the server timeout, so there shouldn't be any reason for the socket to be closed while `send`ing on macos. However, if that workaround is still needed, it should be guarded by a macos check. So my suggestion would be to assume the workaround is no longer needed, unless someone can prove otherwise?
💬 furszy commented on issue "Selecting two coins will abort with "The amount exceeds your balance."":
(https://github.com/bitcoin-core/gui/issues/688#issuecomment-1492388878)
> Just to clarify: IIUC this was "caused" by https://github.com/bitcoin/bitcoin/pull/25685 which is only in master. We assuming we fix this before v25.0 there's nothing to backport.
Yes, that is the goal. #26699 is on the v25.0 milestone.
As it's a bug fix PR, it can get merged post feature freeze. Still, would be nice to start reviewing it asap.
(https://github.com/bitcoin-core/gui/issues/688#issuecomment-1492388878)
> Just to clarify: IIUC this was "caused" by https://github.com/bitcoin/bitcoin/pull/25685 which is only in master. We assuming we fix this before v25.0 there's nothing to backport.
Yes, that is the goal. #26699 is on the v25.0 milestone.
As it's a bug fix PR, it can get merged post feature freeze. Still, would be nice to start reviewing it asap.
💬 ProofOfKeags commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1492411342)
> I still don't see why anyone would want this feature except for re-scanning an old wallet on a pruned node
We ran into this issue as a result of having different dependent services that wanted access to different amounts of block history. If you prune to height N, and you install a different service that wants height N-M, the only solution is to do a full resync and download all the way back to N-N (aka 0). When M is small, this is needlessly expensive.
It is probably a rare use case, th
...
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1492411342)
> I still don't see why anyone would want this feature except for re-scanning an old wallet on a pruned node
We ran into this issue as a result of having different dependent services that wanted access to different amounts of block history. If you prune to height N, and you install a different service that wants height N-M, the only solution is to do a full resync and download all the way back to N-N (aka 0). When M is small, this is needlessly expensive.
It is probably a rare use case, th
...
💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1492411947)
Rebased on latest version of #27021, then rebased on master to resolve merge conflicts.
Needed to reintroduce access to the Chain interface in `ChooseSelectionResult()`.
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1492411947)
Rebased on latest version of #27021, then rebased on master to resolve merge conflicts.
Needed to reintroduce access to the Chain interface in `ChooseSelectionResult()`.
💬 hebasto commented on pull request "net processing: #26140 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27379#issuecomment-1492418189)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27379#issuecomment-1492418189)
Concept ACK.
👍 jarolrod approved a pull request: "Update translation source file for v25.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/724)
ACK e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7
Went through the translation process again, confirming that plurals requires manual intervention in the translation source file.
I have a zero-diff with this branch after following the process steps.
(https://github.com/bitcoin-core/gui/pull/724)
ACK e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7
Went through the translation process again, confirming that plurals requires manual intervention in the translation source file.
I have a zero-diff with this branch after following the process steps.
💬 hebasto commented on pull request "net processing: #26140 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27379#discussion_r1154779794)
Nice!
(https://github.com/bitcoin/bitcoin/pull/27379#discussion_r1154779794)
Nice!
👍 hebasto approved a pull request: "net processing: #26140 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/27379)
ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.
(https://github.com/bitcoin/bitcoin/pull/27379)
ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.
💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1492439997)
Could someone remove the `Needs rebase` label and add a v25 milestone? Thanks.
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1492439997)
Could someone remove the `Needs rebase` label and add a v25 milestone? Thanks.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154783411)
Yeah this one may be tricky to work around. Some of the CI temp dir paths are crazy long like that. I'm going to add an init check & test to throw on socket paths longer than 92 characters. For the sake of passing the tests, I'll try using a temp path that is in the parent of `test_runner_...` and we'll see if that is short enough to pass on the CI VMs.
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154783411)
Yeah this one may be tricky to work around. Some of the CI temp dir paths are crazy long like that. I'm going to add an init check & test to throw on socket paths longer than 92 characters. For the sake of passing the tests, I'll try using a temp path that is in the parent of `test_runner_...` and we'll see if that is short enough to pass on the CI VMs.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154738130)
To literally answer your question: I was not clear on what `-onion` actually does! Now I do, adding this in next update.
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154738130)
To literally answer your question: I was not clear on what `-onion` actually does! Now I do, adding this in next update.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739870)
agreed, updated.
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739870)
agreed, updated.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739487)
I only added this for my own debugging, we could even just take it out. There is also a "proxy failed to initialize" error message in `Socks5()` which will get thrown by `ConnectThroughProxy()` below.
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1154739487)
I only added this for my own debugging, we could even just take it out. There is also a "proxy failed to initialize" error message in `Socks5()` which will get thrown by `ConnectThroughProxy()` below.
💬 LarryRuane commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#discussion_r1154827107)
Are you sure these are protected by `cs_main`, is it? Running `gdb test_bitcoin` on the PR branch (leaving some irrelevant stuff out):
```
(gdb) b MakeBlockInfo
(gdb) run
Thread 29 "b-basic block f" hit Breakpoint 1, kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#0 kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#1 0x000055555623fa81 in BaseIndex::ThreadSync (this=0x7fffffffad00) at index/base.cpp:201
(gdb) p cs_main
$1 = {
...
(https://github.com/bitcoin/bitcoin/pull/27006#discussion_r1154827107)
Are you sure these are protected by `cs_main`, is it? Running `gdb test_bitcoin` on the PR branch (leaving some irrelevant stuff out):
```
(gdb) b MakeBlockInfo
(gdb) run
Thread 29 "b-basic block f" hit Breakpoint 1, kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#0 kernel::MakeBlockInfo (index=0x55555749b018, data=0x0) at kernel/chain.cpp:14
#1 0x000055555623fa81 in BaseIndex::ThreadSync (this=0x7fffffffad00) at index/base.cpp:201
(gdb) p cs_main
$1 = {
...