💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056839286)
I'll leave this as-is for now.
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056839286)
I'll leave this as-is for now.
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056840721)
See https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056813556.
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056840721)
See https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056813556.
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056844763)
If `open` fails an exception will be thrown and the block below it will not be executed.
https://docs.python.org/3/reference/compound_stmts.html#with
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056844763)
If `open` fails an exception will be thrown and the block below it will not be executed.
https://docs.python.org/3/reference/compound_stmts.html#with
🚀 achow101 merged a pull request: "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan"
(https://github.com/bitcoin/bitcoin/pull/32023)
(https://github.com/bitcoin/bitcoin/pull/32023)
💬 achow101 commented on pull request "validation: set BLOCK_FAILED_CHILD correctly":
(https://github.com/bitcoin/bitcoin/pull/31835#issuecomment-2825477728)
ACK 3c3548a70eedb8dcf6a4a8d605a4a12e814c7cac
(https://github.com/bitcoin/bitcoin/pull/31835#issuecomment-2825477728)
ACK 3c3548a70eedb8dcf6a4a8d605a4a12e814c7cac
🚀 achow101 merged a pull request: "validation: set BLOCK_FAILED_CHILD correctly"
(https://github.com/bitcoin/bitcoin/pull/31835)
(https://github.com/bitcoin/bitcoin/pull/31835)
💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2825517598)
Looks like `WalletMigration` bench fails on this branch (which is now run in the CI).
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2825517598)
Looks like `WalletMigration` bench fails on this branch (which is now run in the CI).
💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2056922934)
Reworked and upstreamed the fix in https://github.com/arun11299/cpp-subprocess/pull/112.
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2056922934)
Reworked and upstreamed the fix in https://github.com/arun11299/cpp-subprocess/pull/112.
💬 furszy commented on issue "ci: failure in Windows cross-test":
(https://github.com/bitcoin/bitcoin/issues/32291#issuecomment-2825587643)
Can be closed.
(https://github.com/bitcoin/bitcoin/issues/32291#issuecomment-2825587643)
Can be closed.
💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2825599793)
Fixed CI
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2825599793)
Fixed CI
✅ hebasto closed an issue: "ci: failure in Windows cross-test"
(https://github.com/bitcoin/bitcoin/issues/32291)
(https://github.com/bitcoin/bitcoin/issues/32291)
💬 hebasto commented on issue "ci: failure in Windows cross-test":
(https://github.com/bitcoin/bitcoin/issues/32291#issuecomment-2825623597)
Fixed in https://github.com/bitcoin/bitcoin/pull/32309.
(https://github.com/bitcoin/bitcoin/issues/32291#issuecomment-2825623597)
Fixed in https://github.com/bitcoin/bitcoin/pull/32309.
📝 w0xlt opened a pull request: "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys"
(https://github.com/bitcoin/bitcoin/pull/32332)
Currently, `XOnlyPubKey::GetKeyIDs()` has `vector` as return type, but always returns a pair of public key IDs.
This PR changes the code for readability reasons.
There may be negligible efficiency gains, but the goal is to make the code more readable and to make the intent of the function clear.
There is no change in behavior.
(https://github.com/bitcoin/bitcoin/pull/32332)
Currently, `XOnlyPubKey::GetKeyIDs()` has `vector` as return type, but always returns a pair of public key IDs.
This PR changes the code for readability reasons.
There may be negligible efficiency gains, but the goal is to make the code more readable and to make the intent of the function clear.
There is no change in behavior.
🤔 murchandamus reviewed a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2788928413)
ACK bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2788928413)
ACK bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633
💬 ajtowns commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825824998)
> I think the main conceptual question is what should happen in the case of a stalling situation where the addnode peer is actually slow and causing congestion?
IMHO: It could be nice if we could "score" peers during IBD, and scale the number of blocks we request from them at once according to that score. In that case, automatic peers with low scores would get dropped, and manual peers with low scores would just not get any blocks requested.
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825824998)
> I think the main conceptual question is what should happen in the case of a stalling situation where the addnode peer is actually slow and causing congestion?
IMHO: It could be nice if we could "score" peers during IBD, and scale the number of blocks we request from them at once according to that score. In that case, automatic peers with low scores would get dropped, and manual peers with low scores would just not get any blocks requested.
💬 jonatack commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825834909)
Appreciate the thoughtful feedback. I'll look at an update.
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2825834909)
Appreciate the thoughtful feedback. I'll look at an update.
📝 nervana21 opened a pull request: "doc: Add missing top-level description to pruneblockchain RPC"
(https://github.com/bitcoin/bitcoin/pull/32333)
Previously, the `pruneblockchain` RPC help output included only the method signature and arguments, with no top-level description explaining its purpose or constraints.
This PR adds a top-level description, improving documentation consistency and alerting users to the potential impacts of using the command.
(https://github.com/bitcoin/bitcoin/pull/32333)
Previously, the `pruneblockchain` RPC help output included only the method signature and arguments, with no top-level description explaining its purpose or constraints.
This PR adds a top-level description, improving documentation consistency and alerting users to the potential impacts of using the command.
🤔 hodlinator reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2788618744)
Got some mixed messaging here since you told me repeatedly in private not to investigate failures I reported (https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2821410082) on my previous branch (https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2783071854) but then chided me in public (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056174501). Anyways, the previous branch was not the sliced bread I believed it to be, looking back.
---
New branch: https://
...
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2788618744)
Got some mixed messaging here since you told me repeatedly in private not to investigate failures I reported (https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2821410082) on my previous branch (https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2783071854) but then chided me in public (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056174501). Anyways, the previous branch was not the sliced bread I believed it to be, looking back.
---
New branch: https://
...
💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056830517)
In 2a50ec0d17b12f2adb1bf8872592b898810e9ac4:
`key` is no longer used.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056830517)
In 2a50ec0d17b12f2adb1bf8872592b898810e9ac4:
`key` is no longer used.
💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057656754)
Inside a ctor which initializes `m_obfuscation{0}` through the header, you have:
```C++
...
{
std::vector<unsigned char> obfuscate_key_vector(Obfuscation::SIZE_BYTES, '\000');
...
if (...) {
... (modify obfuscate_key_vector)
}
m_obfuscation = obfuscate_key_vector;
}
}
```
Do you feel a need to `assert(m_obfuscation == 0)` at the beginning of the block because the ctor has ~30 preceeding lines? Can't think of another rea
...
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057656754)
Inside a ctor which initializes `m_obfuscation{0}` through the header, you have:
```C++
...
{
std::vector<unsigned char> obfuscate_key_vector(Obfuscation::SIZE_BYTES, '\000');
...
if (...) {
... (modify obfuscate_key_vector)
}
m_obfuscation = obfuscate_key_vector;
}
}
```
Do you feel a need to `assert(m_obfuscation == 0)` at the beginning of the block because the ctor has ~30 preceeding lines? Can't think of another rea
...