💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376702343)
Is this copying `nOrderPosNext` from source wallet to the `watchonly` one ?
```suggestion
watchonly_batch.WriteOrderPosNext(this->nOrderPosNext);
```
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376702343)
Is this copying `nOrderPosNext` from source wallet to the `watchonly` one ?
```suggestion
watchonly_batch.WriteOrderPosNext(this->nOrderPosNext);
```
🤔 mzumsande reviewed a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3264142883)
Looks good - just some minor comments.
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3264142883)
Looks good - just some minor comments.
💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376698894)
do we need to keep BLOCK_FAILED_MASK in `chain.h` after it's no longer used? After all, it's only used as a mask (unlike BLOCK_FAILED_CHILD).
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376698894)
do we need to keep BLOCK_FAILED_MASK in `chain.h` after it's no longer used? After all, it's only used as a mask (unlike BLOCK_FAILED_CHILD).
💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376682303)
commit message ca2421d52792b32754f1801492822b999839ec3a:
"since the previous commit removes BLOCK_FAILED_CHILD" - only the following commit removes it. Or do you mean "any special logic for BLOCK_FAILED_CHILD"?
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376682303)
commit message ca2421d52792b32754f1801492822b999839ec3a:
"since the previous commit removes BLOCK_FAILED_CHILD" - only the following commit removes it. Or do you mean "any special logic for BLOCK_FAILED_CHILD"?
💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376701606)
comment still mentions a "mask"
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376701606)
comment still mentions a "mask"
💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376728821)
Wouldn't it be safe to delete only the watchonly wallet file ?
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376728821)
Wouldn't it be safe to delete only the watchonly wallet file ?
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376734029)
Oops fixed
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376734029)
Oops fixed
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376736505)
> Wouldn't it be safe to delete only the watchonly wallet file ?
I don't think we should be leaving behind the wallet directory, and possibly the `-journal` file either.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376736505)
> Wouldn't it be safe to delete only the watchonly wallet file ?
I don't think we should be leaving behind the wallet directory, and possibly the `-journal` file either.
💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376758113)
Can `desc.cache` be empty here ?
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376758113)
Can `desc.cache` be empty here ?
💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2376784754)
Is there a configure flag I need? I'm just running gen-manpages.py after building.
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2376784754)
Is there a configure flag I need? I'm just running gen-manpages.py after building.
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376795377)
pendantic: it's redundant *when an RBF is encountered*, but not otherwise and the result is cached so pretty cheap to throw around
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376795377)
pendantic: it's redundant *when an RBF is encountered*, but not otherwise and the result is cached so pretty cheap to throw around
📝 glozow opened a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476)
Built on top of #33415
Includes backports of
- #33106
- #30125
- #30948
- #30784
(https://github.com/bitcoin/bitcoin/pull/33476)
Built on top of #33415
Includes backports of
- #33106
- #30125
- #30948
- #30784
💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3330324239)
Opened #33476 to follow
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3330324239)
Opened #33476 to follow
💬 fanquake commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#discussion_r2376849213)
Can this just be `CMAKE_SKIP_RPATH`? (also move to `CONFIGFLAGS`)
(https://github.com/bitcoin/bitcoin/pull/33470#discussion_r2376849213)
Can this just be `CMAKE_SKIP_RPATH`? (also move to `CONFIGFLAGS`)
💬 fanquake commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2376868135)
Leftover from rebase?
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2376868135)
Leftover from rebase?
💬 maflcko commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2376962309)
seems a bit odd to have the number of nodes in a test influence whether or not the test has to be edited to remove or add `-par=1` everywhere. Would it not be easier to just globally set `-par=2` for all funtional tests?
```diff
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index e5a5938f07..42bb213dd3 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -459,6 +459,7 @@ def write_config(conf
...
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2376962309)
seems a bit odd to have the number of nodes in a test influence whether or not the test has to be edited to remove or add `-par=1` everywhere. Would it not be easier to just globally set `-par=2` for all funtional tests?
```diff
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index e5a5938f07..42bb213dd3 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -459,6 +459,7 @@ def write_config(conf
...
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2377071137)
Yes, I wondered if that would be more invasive to other tests though.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2377071137)
Yes, I wondered if that would be more invasive to other tests though.
📝 fjahr opened a pull request: "Rollback for dumptxoutset without invalidating blocks"
(https://github.com/bitcoin/bitcoin/pull/33477)
This is an alternative approach to implement `dumptxoutset` with rollback that was discussed a few times. It does not rely on `invalidateblock` and `reconsiderblock` and instead creates a temporary copy of the coins DB, modifies this copy by rolling back as many blocks as necessary and then creating the dump from this temp copy DB. See also https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-1978480989, https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3012406102 and #29565 dis
...
(https://github.com/bitcoin/bitcoin/pull/33477)
This is an alternative approach to implement `dumptxoutset` with rollback that was discussed a few times. It does not rely on `invalidateblock` and `reconsiderblock` and instead creates a temporary copy of the coins DB, modifies this copy by rolling back as many blocks as necessary and then creating the dump from this temp copy DB. See also https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-1978480989, https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3012406102 and #29565 dis
...
💬 fjahr commented on pull request "rpc: Fix dumptxoutset rollback with competing forks":
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3330754815)
> Regarding the test failure - let me investigate and fix that. Would you prefer I:
I have opened #33477 now, as you can see there I am describing that there are some trade-offs between the approach and master which this PR would then be applied to. Let's see what reviewers think about those trade-offs. You should keep it open and ideally fix the test. If the CI is failing that usually signals to reviewers that the PR isn't ready for consideration since the code will need to change anyway. So
...
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3330754815)
> Regarding the test failure - let me investigate and fix that. Would you prefer I:
I have opened #33477 now, as you can see there I am describing that there are some trade-offs between the approach and master which this PR would then be applied to. Let's see what reviewers think about those trade-offs. You should keep it open and ideally fix the test. If the CI is failing that usually signals to reviewers that the PR isn't ready for consideration since the code will need to change anyway. So
...
💬 fjahr commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#issuecomment-3330760975)
cc @Sjors since you were asking for this approach a few times :)
(https://github.com/bitcoin/bitcoin/pull/33477#issuecomment-3330760975)
cc @Sjors since you were asking for this approach a few times :)