π¬ sipa commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070233390)
Coding style nit: `} else {`.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070233390)
Coding style nit: `} else {`.
π¬ sipa commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070235644)
Coding style nit: space after `if`.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070235644)
Coding style nit: space after `if`.
π¬ instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2844817333)
@Christewart that's a different rule, if you add a test PR and it's not already covered I'll ack it!
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2844817333)
@Christewart that's a different rule, if you add a test PR and it's not already covered I'll ack it!
π€ Bonobirho99 reviewed a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-2809778055)
- [ ] @smola 
# the solution SIDRA cap market on Mainn list auditor internal server it is maljefayri ceo
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-2809778055)
- [ ] @smola 
# the solution SIDRA cap market on Mainn list auditor internal server it is maljefayri ceo
π¬ Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2844850180)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d
π² πͺ
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2844850180)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d
π² πͺ
π Sjors approved a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2809792896)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d π² πͺ
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2809792896)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d π² πͺ
π¬ sonnyxsm commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844905503)
this is not your wastebucket.
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844905503)
this is not your wastebucket.
π¬ sipa commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2844916056)
utACK e976bd3045010ee217aa0f2dca4c962aabb789d5
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2844916056)
utACK e976bd3045010ee217aa0f2dca4c962aabb789d5
π¬ oxqnd commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2844922900)
Could I try this issue?
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2844922900)
Could I try this issue?
π¬ hebasto commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-2844930021)
The next one is https://github.com/bitcoin/bitcoin/pull/32394.
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-2844930021)
The next one is https://github.com/bitcoin/bitcoin/pull/32394.
π¬ hebasto commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-2844931617)
> Partially resolves: #19303
Concept ACK on that.
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-2844931617)
> Partially resolves: #19303
Concept ACK on that.
π¬ sipa commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#discussion_r2070328953)
Super nitty, I was just curious about what the possible ways of doing this are, and maybe this is preferable:
Making `GetAllImpl` a static member, so it doesn't need to be passed both a `self` and a `this` pointer:
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 8fb8a876b3f..58bcd9e84e6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5646,14 +5646,14 @@ std::optional<uint256> ChainstateManager::SnapshotBlockhash() const
}
template <typename T>
-au
...
(https://github.com/bitcoin/bitcoin/pull/32364#discussion_r2070328953)
Super nitty, I was just curious about what the possible ways of doing this are, and maybe this is preferable:
Making `GetAllImpl` a static member, so it doesn't need to be passed both a `self` and a `this` pointer:
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 8fb8a876b3f..58bcd9e84e6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5646,14 +5646,14 @@ std::optional<uint256> ChainstateManager::SnapshotBlockhash() const
}
template <typename T>
-au
...
π¬ mzumsande commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2844940123)
> @mzumsande does that diagram look right to you?
Yes - very nice diagram!
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2844940123)
> @mzumsande does that diagram look right to you?
Yes - very nice diagram!
π¬ stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2070349543)
Thank you, I think I understand now. The commit message and the placement of the new `CheckBlockIndex()` call in 79bef1bc52afab860782d6d9a6016b93f94c4503 being before (rather than after) the block index mutations led me to believe that we were worried about _other_ threads leaving the block index in an inconsistent state, and made this particular addition feel rather arbitrary.
Instead, it seems you want to ensure that `InvalidateBlock()` does not leave the block index in an inconsistent state
...
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2070349543)
Thank you, I think I understand now. The commit message and the placement of the new `CheckBlockIndex()` call in 79bef1bc52afab860782d6d9a6016b93f94c4503 being before (rather than after) the block index mutations led me to believe that we were worried about _other_ threads leaving the block index in an inconsistent state, and made this particular addition feel rather arbitrary.
Instead, it seems you want to ensure that `InvalidateBlock()` does not leave the block index in an inconsistent state
...
π instagibbs approved a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2809960189)
reACK 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7
`git range-diff master a0becaaa9c7390bc80d5864f7fffb32e21502a50 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7`
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2809960189)
reACK 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7
`git range-diff master a0becaaa9c7390bc80d5864f7fffb32e21502a50 7208ec2f1f0b9d72e1b4248dae9adbcc5ab625f7`
π¬ Crypt-iQ commented on issue "p2p: lingering entries in `mapBlockSource`":
(https://github.com/bitcoin/bitcoin/issues/29410#issuecomment-2845013677)
I think we should either: 1) document all of `mapBlockSource` quirks with comments and merge tests that improve the code coverage in this area, or 2) stop usage of `mapBlockSource` in the `BlockChecked` callback and instead only have punishment / reward happen when blocks are received in-order. I have done a bit of option 1) and will summarize current behavior below:
**Punishment:**
- i) This happens in or out of IBD. Notice the lack of an IBD check [here](https://github.com/bitcoin/bitcoin/blo
...
(https://github.com/bitcoin/bitcoin/issues/29410#issuecomment-2845013677)
I think we should either: 1) document all of `mapBlockSource` quirks with comments and merge tests that improve the code coverage in this area, or 2) stop usage of `mapBlockSource` in the `BlockChecked` callback and instead only have punishment / reward happen when blocks are received in-order. I have done a bit of option 1) and will summarize current behavior below:
**Punishment:**
- i) This happens in or out of IBD. Notice the lack of an IBD check [here](https://github.com/bitcoin/bitcoin/blo
...
π ismaelsadeeq opened a pull request: "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity"
(https://github.com/bitcoin/bitcoin/pull/32395)
This PR fixes #32105 and related issues: #32178, #31116, and #31032.
The main motivation is outlined in [this comment](https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2830820374)).
Bitcoin Core users have reported several issues where nodes are unable to provide fee rate estimates, despite having run long enough to do so for a given target. This typically occurs when there is low activity on the network.
As a result, the issue primarily affects nodes running on test network
...
(https://github.com/bitcoin/bitcoin/pull/32395)
This PR fixes #32105 and related issues: #32178, #31116, and #31032.
The main motivation is outlined in [this comment](https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2830820374)).
Bitcoin Core users have reported several issues where nodes are unable to provide fee rate estimates, despite having run long enough to do so for a given target. This typically occurs when there is low activity on the network.
As a result, the issue primarily affects nodes running on test network
...
π€ furszy reviewed a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2810007391)
In 425c39c6f520a1b0d18c6e782141f711509b2947:
Could probably also remove the proxy functions we have in the test framework for those RPC commands. Look [at this line](https://github.com/bitcoin/bitcoin/blob/fc6346dbc8dc3db40aad4079210332b5f8b332ed/test/functional/test_framework/test_node.py#L979) for example (same for importpubkey, importprivkey and addmultisigaddress).
Also, on an orthogonal topic β I was wondering if, instead of completely removing `importaddress()`, we might want to quic
...
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2810007391)
In 425c39c6f520a1b0d18c6e782141f711509b2947:
Could probably also remove the proxy functions we have in the test framework for those RPC commands. Look [at this line](https://github.com/bitcoin/bitcoin/blob/fc6346dbc8dc3db40aad4079210332b5f8b332ed/test/functional/test_framework/test_node.py#L979) for example (same for importpubkey, importprivkey and addmultisigaddress).
Also, on an orthogonal topic β I was wondering if, instead of completely removing `importaddress()`, we might want to quic
...
π hebasto opened a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396)
Windows [application manifests ](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) provide several benefitsβsuch as enhanced security settings, and the ability to set a process-wide code page (required for https://github.com/bitcoin/bitcoin/pull/32380), as well as granular control over supported Windows versions. Most of these benefits lie beyond the scope of this PR and will be evaluated separately.
On the current master branch @ fc6346dbc8dc3db40aad4079210332b5f8b
...
(https://github.com/bitcoin/bitcoin/pull/32396)
Windows [application manifests ](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) provide several benefitsβsuch as enhanced security settings, and the ability to set a process-wide code page (required for https://github.com/bitcoin/bitcoin/pull/32380), as well as granular control over supported Windows versions. Most of these benefits lie beyond the scope of this PR and will be evaluated separately.
On the current master branch @ fc6346dbc8dc3db40aad4079210332b5f8b
...
π¬ hebasto commented on pull request "[PoC] Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2845072660)
Please review https://github.com/bitcoin/bitcoin/pull/32396 first.
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2845072660)
Please review https://github.com/bitcoin/bitcoin/pull/32396 first.