💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574979427)
That's more an implementation detail that can be found by reading the header file, no?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574979427)
That's more an implementation detail that can be found by reading the header file, no?
💬 sedited commented on pull request "refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-3593597789)
Looking at this patch, I am asking myself why we're keeping the single threaded / no checkqueue case in the first place. This seems to introduce a vector of jobs for both cases now, so maybe we could just create a queue without any workers?
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-3593597789)
Looking at this patch, I am asking myself why we're keeping the single threaded / no checkqueue case in the first place. This seems to introduce a vector of jobs for both cases now, so maybe we could just create a queue without any workers?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574980697)
I would prefer to split out the implementation, the tests, the fuzz harness... But you prefer those all be in the same commit?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574980697)
I would prefer to split out the implementation, the tests, the fuzz harness... But you prefer those all be in the same commit?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574982084)
> (i.e. to stop propagation to disk)
The parent could be called, but this is faster since we skip calling `ReallocateCache`.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574982084)
> (i.e. to stop propagation to disk)
The parent could be called, but this is faster since we skip calling `ReallocateCache`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574988562)
I'm not sure if we need all these deletes here? What are we gaining from this?
We need to have move assignment for it to compile. It can't automatically move the `atomic_flag`. So, it doesn't really matter if it gets called or not, we still need to define it. And, since it only happens before we start doing work, might as well make it simple and not bother moving the other fields. I don't think we need to bother reserving since we keep the capacity over many blocks. The looping is just extra
...
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574988562)
I'm not sure if we need all these deletes here? What are we gaining from this?
We need to have move assignment for it to compile. It can't automatically move the `atomic_flag`. So, it doesn't really matter if it gets called or not, we still need to define it. And, since it only happens before we start doing work, might as well make it simple and not bother moving the other fields. I don't think we need to bother reserving since we keep the capacity over many blocks. The looping is just extra
...
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574995185)
It doesn't simulate going to disk. It simulates not setting the coin in `ProcessInputInBackground` even though the base has it. Then the `if (ret->second.coin.IsSpent()) [[unlikely]] {` branch is executed in `FetchCoin` and the coin is fetched from base via `GetCoinWithoutMutating`.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574995185)
It doesn't simulate going to disk. It simulates not setting the coin in `ProcessInputInBackground` even though the base has it. Then the `if (ret->second.coin.IsSpent()) [[unlikely]] {` branch is executed in `FetchCoin` and the coin is fetched from base via `GetCoinWithoutMutating`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575017536)
> would basically pass (but hang) if we forget to Reset
A hanging test is treated as failure in the CI. I don't think it's necessary to do anything else here.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575017536)
> would basically pass (but hang) if we forget to Reset
A hanging test is treated as failure in the CI. I don't think it's necessary to do anything else here.
📝 hebasto opened a pull request: "depends, doc: Add `tcl` as build dependency for `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33975)
Since https://github.com/bitcoin/bitcoin/pull/32655, the `sqlite` package has relied on a native C compiler being incidentally available on the system under the default names. However, this is not always the case. For example, on Ubuntu 24.04:
```
$ gmake -C depends sqlite CC=gcc-14 CXX=g++-14
gmake: Entering directory '/bitcoin/depends'
Configuring sqlite...
No installed jimsh or tclsh, building local bootstrap jimsh0
No working C compiler found. Tried cc and gcc.
gmake: *** [funcs.mk:34
...
(https://github.com/bitcoin/bitcoin/pull/33975)
Since https://github.com/bitcoin/bitcoin/pull/32655, the `sqlite` package has relied on a native C compiler being incidentally available on the system under the default names. However, this is not always the case. For example, on Ubuntu 24.04:
```
$ gmake -C depends sqlite CC=gcc-14 CXX=g++-14
gmake: Entering directory '/bitcoin/depends'
Configuring sqlite...
No installed jimsh or tclsh, building local bootstrap jimsh0
No working C compiler found. Tried cc and gcc.
gmake: *** [funcs.mk:34
...
💬 hebasto commented on pull request "depends, doc: Add `tcl` as build dependency for `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33975#discussion_r2575046363)
FWIW, [`jimsh`](https://packages.ubuntu.com/noble/jimsh) works as well.
`tcl` is chosen for consistency with the Guix script.
(https://github.com/bitcoin/bitcoin/pull/33975#discussion_r2575046363)
FWIW, [`jimsh`](https://packages.ubuntu.com/noble/jimsh) works as well.
`tcl` is chosen for consistency with the Guix script.
📝 SmartDever02 opened a pull request: "net: Complete net/net_processing split refactoring"
(https://github.com/bitcoin/bitcoin/pull/33976)
```
Completes the net split refactoring (#33958) by cleanly separating `net` and `net_processing` layers through four incremental phases. This refactoring addresses the tight coupling between network connection management (`CConnman`/`CNode`) and peer message processing (`PeerManager`), enabling better code organization and maintainability.
## Phase 1: Move connection state from CNode to Peer
Moved connection-specific state from `CNode` to the `Peer` structure to reduce direct dependencie
...
(https://github.com/bitcoin/bitcoin/pull/33976)
```
Completes the net split refactoring (#33958) by cleanly separating `net` and `net_processing` layers through four incremental phases. This refactoring addresses the tight coupling between network connection management (`CConnman`/`CNode`) and peer message processing (`PeerManager`), enabling better code organization and maintainability.
## Phase 1: Move connection state from CNode to Peer
Moved connection-specific state from `CNode` to the `Peer` structure to reduce direct dependencie
...
💬 pinheadmz commented on pull request "net: Complete net/net_processing split refactoring":
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594200550)
You closed that entire issue? All four of these "phases"... in two commits? You are ranked #2 on Gittensor, what is that
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594200550)
You closed that entire issue? All four of these "phases"... in two commits? You are ranked #2 on Gittensor, what is that
💬 pinheadmz commented on pull request "net: Complete net/net_processing split refactoring":
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594207361)
Did you get all these compile errors when you built this code locally for testing as well? Or is it just me
```
In file included from /home/pinheadmz/Desktop/work/bitcoin/src/netmessagemaker.h:9,
from /home/pinheadmz/Desktop/work/bitcoin/src/test/util/net.h:9,
from /home/pinheadmz/Desktop/work/bitcoin/src/test/util/net.cpp:5:
/home/pinheadmz/Desktop/work/bitcoin/src/net.h:676:1: error: expected class-name before ‘{’ token
676 | {
| ^
/home/pinh
...
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594207361)
Did you get all these compile errors when you built this code locally for testing as well? Or is it just me
```
In file included from /home/pinheadmz/Desktop/work/bitcoin/src/netmessagemaker.h:9,
from /home/pinheadmz/Desktop/work/bitcoin/src/test/util/net.h:9,
from /home/pinheadmz/Desktop/work/bitcoin/src/test/util/net.cpp:5:
/home/pinheadmz/Desktop/work/bitcoin/src/net.h:676:1: error: expected class-name before ‘{’ token
676 | {
| ^
/home/pinh
...
📝 vostrnad opened a pull request: "script: Remove dead code from OP_CHECKMULTISIG"
(https://github.com/bitcoin/bitcoin/pull/33977)
The stack size check before the dummy element check in OP_CHECKMULTISIG is redundant. The stack is previously validated to have least *i* elements (where *i* is number of pubkeys + number of signatures + 3), and then *i* - 1 elements are popped off the stack, leaving at least one element still in.
The redundant check was added in #3843 (commit 6380180821917c22ecfd89128ee60aae6f4cac33). There it's easy to see why it's redundant – it's inserted into a code path where another `popstack` would pr
...
(https://github.com/bitcoin/bitcoin/pull/33977)
The stack size check before the dummy element check in OP_CHECKMULTISIG is redundant. The stack is previously validated to have least *i* elements (where *i* is number of pubkeys + number of signatures + 3), and then *i* - 1 elements are popped off the stack, leaving at least one element still in.
The redundant check was added in #3843 (commit 6380180821917c22ecfd89128ee60aae6f4cac33). There it's easy to see why it's redundant – it's inserted into a code path where another `popstack` would pr
...
💬 SmartDever02 commented on pull request "net: Complete net/net_processing split refactoring":
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594274227)
@pinheadmz Thank you for your feedback, when I built this on my locally, it worked as well. Let me check again...
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594274227)
@pinheadmz Thank you for your feedback, when I built this on my locally, it worked as well. Let me check again...
✅ achow101 closed a pull request: "net: Complete net/net_processing split refactoring"
(https://github.com/bitcoin/bitcoin/pull/33976)
(https://github.com/bitcoin/bitcoin/pull/33976)
💬 achow101 commented on pull request "net: Complete net/net_processing split refactoring":
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594425989)
ai slop
(https://github.com/bitcoin/bitcoin/pull/33976#issuecomment-3594425989)
ai slop
📝 codomposer opened a pull request: "test: add test loading a wallet backup on a pruned node"
(https://github.com/bitcoin/bitcoin/pull/33978)
# Add test for loading wallet backup on pruned node
## Summary
This PR adds a test case for loading a wallet backup on a pruned node to the `wallet_assumeutxo.py` functional test suite, addressing the TODO item that was previously documented in the test file.
## Motivation
Testing wallet backup restoration on pruned nodes is important because:
- Pruned nodes are commonly used to save disk space
- Users need confidence that wallet backups can be restored even on pruned nodes
- This
...
(https://github.com/bitcoin/bitcoin/pull/33978)
# Add test for loading wallet backup on pruned node
## Summary
This PR adds a test case for loading a wallet backup on a pruned node to the `wallet_assumeutxo.py` functional test suite, addressing the TODO item that was previously documented in the test file.
## Motivation
Testing wallet backup restoration on pruned nodes is important because:
- Pruned nodes are commonly used to save disk space
- Users need confidence that wallet backups can be restored even on pruned nodes
- This
...
📝 codomposer opened a pull request: "test: add test loading a wallet backup on a pruned node"
(https://github.com/bitcoin/bitcoin/pull/33979)
# Add test for loading wallet backup on pruned node
## Summary
This PR adds a test case for loading a wallet backup on a pruned node to the `wallet_assumeutxo.py` functional test suite, addressing the TODO item that was previously documented in the test file.
## Motivation
Testing wallet backup restoration on pruned nodes is important because:
- Pruned nodes are commonly used to save disk space
- Users need confidence that wallet backups can be restored even on pruned nodes
- This
...
(https://github.com/bitcoin/bitcoin/pull/33979)
# Add test for loading wallet backup on pruned node
## Summary
This PR adds a test case for loading a wallet backup on a pruned node to the `wallet_assumeutxo.py` functional test suite, addressing the TODO item that was previously documented in the test file.
## Motivation
Testing wallet backup restoration on pruned nodes is important because:
- Pruned nodes are commonly used to save disk space
- Users need confidence that wallet backups can be restored even on pruned nodes
- This
...
✅ maflcko closed a pull request: "test: add test loading a wallet backup on a pruned node"
(https://github.com/bitcoin/bitcoin/pull/33979)
(https://github.com/bitcoin/bitcoin/pull/33979)
💬 maflcko commented on pull request "test: add test loading a wallet backup on a pruned node":
(https://github.com/bitcoin/bitcoin/pull/33979#issuecomment-3595042502)
thx, but there already is a pull request up
(https://github.com/bitcoin/bitcoin/pull/33979#issuecomment-3595042502)
thx, but there already is a pull request up