π¬ frankomosh commented on pull request "fuzz: add mempool_dag fuzzer for transaction dependency testing":
(https://github.com/bitcoin/bitcoin/pull/33038#discussion_r2223623449)
i'll generate report and only proceed if there is new coverage
(https://github.com/bitcoin/bitcoin/pull/33038#discussion_r2223623449)
i'll generate report and only proceed if there is new coverage
π frankomosh converted_to_draft a pull request: "fuzz: add mempool_dag fuzzer for transaction dependency testing"
(https://github.com/bitcoin/bitcoin/pull/33038)
adds a new target, `mempool_dag`, to test mempool DAG invariants by constructing transaction chains with controlled parent/child dependencies. main focus areas include:
a. ancestor / descendant count and size limits (`-limitancestorcount,` `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`)
b. TRUC-policy interaction with non-TRUC transactions
c. fee-rate consistency after partial package eviction
Invariants are asserted after every successful `AcceptToMemoryPool`
...
(https://github.com/bitcoin/bitcoin/pull/33038)
adds a new target, `mempool_dag`, to test mempool DAG invariants by constructing transaction chains with controlled parent/child dependencies. main focus areas include:
a. ancestor / descendant count and size limits (`-limitancestorcount,` `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`)
b. TRUC-policy interaction with non-TRUC transactions
c. fee-rate consistency after partial package eviction
Invariants are asserted after every successful `AcceptToMemoryPool`
...
β
Zeegaths closed a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699)
(https://github.com/bitcoin/bitcoin/pull/32699)
π¬ darosior commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3104527522)
@RobinLinus do you plan on addressing review here? Reminder that feature freeze is [less than a month](https://github.com/bitcoin/bitcoin/issues/32275) from now.
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3104527522)
@RobinLinus do you plan on addressing review here? Reminder that feature freeze is [less than a month](https://github.com/bitcoin/bitcoin/issues/32275) from now.
π¬ PeterWrighten commented on pull request "wallet, test: Replace MockableDatabase with in-memory SQLiteDatabase":
(https://github.com/bitcoin/bitcoin/pull/33032#issuecomment-3104700592)
ACK 78897
(https://github.com/bitcoin/bitcoin/pull/33032#issuecomment-3104700592)
ACK 78897
π Zeegaths reopened a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699)
Added correct links to the docs in place of the missing docs' paths.
Fixes #32565
(https://github.com/bitcoin/bitcoin/pull/32699)
Added correct links to the docs in place of the missing docs' paths.
Fixes #32565
π¬ b-l-u-e commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2223787842)
There's an unnecessary semicolon on its own line that should be removed.
(https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2223787842)
There's an unnecessary semicolon on its own line that should be removed.
π¬ glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223796103)
Yeah, I think it's a good idea to add an error saying "this option doesn't do anything anymore, remove it from your bitcoin.conf" and continue.
Do you think it's worthwhile to still support `-maxorphantxs=0` e.g. for memory-conscious users?
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223796103)
Yeah, I think it's a good idea to add an error saying "this option doesn't do anything anymore, remove it from your bitcoin.conf" and continue.
Do you think it's worthwhile to still support `-maxorphantxs=0` e.g. for memory-conscious users?
π¬ glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223800920)
Hm, I didn't think so either, but wonder what could have caused https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2217735632 π€
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223800920)
Hm, I didn't think so either, but wonder what could have caused https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2217735632 π€
π¬ sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223809881)
Hmm, my best guess is a compiler/sanitizer bug.
Try something like
```c++
auto rng_seed = rng.rand256();
FastRandomContext rand_ctx(rng_seed);
```
?
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223809881)
Hmm, my best guess is a compiler/sanitizer bug.
Try something like
```c++
auto rng_seed = rng.rand256();
FastRandomContext rand_ctx(rng_seed);
```
?
π¬ furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223829051)
> Hmm I can imagine that this can be blocking when you want to stop instantly; but I guess there is a reason why you did not return here and then empty the work queue in Stop.
Yes. We need to fulfill all promises so there are no dangling futures waiting for the worker to finish executing the task.
In the future, we could avoid this by tracking all the promises and triggering a "shutdown" exception during stop.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223829051)
> Hmm I can imagine that this can be blocking when you want to stop instantly; but I guess there is a reason why you did not return here and then empty the work queue in Stop.
Yes. We need to fulfill all promises so there are no dangling futures waiting for the worker to finish executing the task.
In the future, we could avoid this by tracking all the promises and triggering a "shutdown" exception during stop.
π¬ furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223838649)
> Why 0?
Parallel sync is disabled by default. Weβre currently not tracking the number of threads spawned by Core, so I chose not to make assumptions here (don't want indexes threads competing with net/validation ones). Itβs safer to let users specify the appropriate number for their setup. In the future, we could improve this by adding thread tracking object/mechanism and picking up the best number for their machine.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223838649)
> Why 0?
Parallel sync is disabled by default. Weβre currently not tracking the number of threads spawned by Core, so I chose not to make assumptions here (don't want indexes threads competing with net/validation ones). Itβs safer to let users specify the appropriate number for their setup. In the future, we could improve this by adding thread tracking object/mechanism and picking up the best number for their machine.
π pablomartin4btc approved a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3044920696)
tACK faa3e684118bffa7a98cf76eeeb59243219df900
Managed to reproduce the issue with the patch provided in the PR description, this branch fixes it. Nice detail handling `KeyboardInterrupt` for debugging purpose. Code-wise refactoring: better reading removing redundant catches.
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3044920696)
tACK faa3e684118bffa7a98cf76eeeb59243219df900
Managed to reproduce the issue with the patch provided in the PR description, this branch fixes it. Nice detail handling `KeyboardInterrupt` for debugging purpose. Code-wise refactoring: better reading removing redundant catches.
π stickies-v approved a pull request: "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs"
(https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3044915254)
re-ACK af0ad72a0ed778c7e6f83b1a290705cefe23d78a
(https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3044915254)
re-ACK af0ad72a0ed778c7e6f83b1a290705cefe23d78a
π¬ stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223852456)
nit: what's the purpose of wrapping this in `namespace wallet`?
Relatedly, should probably make `TestWalletName` `static`.
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223852456)
nit: what's the purpose of wrapping this in `namespace wallet`?
Relatedly, should probably make `TestWalletName` `static`.
π¬ stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223857169)
nit: can keep comma to avoid touching this line
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223857169)
nit: can keep comma to avoid touching this line
π¬ stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223866061)
```suggestion
- `unloadwallet` - Return RPC_INVALID_PARAMETER when both the RPC wallet endpoint
and wallet_name parameters are unspecified. Previously the RPC failed with a JSON
parsing error.
```
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223866061)
```suggestion
- `unloadwallet` - Return RPC_INVALID_PARAMETER when both the RPC wallet endpoint
and wallet_name parameters are unspecified. Previously the RPC failed with a JSON
parsing error.
```
π¬ ismaelsadeeq commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223903289)
Makes sense, I think we can make the bounds not just an arbitrary number but tied it to the cores of the machine.
That will prevent footgoon whereby user will spawn more threads than the machine can handle leading to degraded performance due to lots of context switching.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223903289)
Makes sense, I think we can make the bounds not just an arbitrary number but tied it to the cores of the machine.
That will prevent footgoon whereby user will spawn more threads than the machine can handle leading to degraded performance due to lots of context switching.
β
achow101 closed a pull request: "test: functional test for incomplete PSBT with additional signatures required"
(https://github.com/bitcoin/bitcoin/pull/33035)
(https://github.com/bitcoin/bitcoin/pull/33035)
π¬ achow101 commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3104989559)
What is the purpose of this test? There are already multiple tests that do exactly what you have added, there is nothing useful being added here.
Please do not use a LLM to write your code.
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3104989559)
What is the purpose of this test? There are already multiple tests that do exactly what you have added, there is nothing useful being added here.
Please do not use a LLM to write your code.