π¬ 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.
π¬ hebasto commented on pull request "[PoC] Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2845084753)
Rebased on https://github.com/bitcoin/bitcoin/pull/32396.
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2845084753)
Rebased on https://github.com/bitcoin/bitcoin/pull/32396.
π€ mzumsande reviewed a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2810060421)
Given that #30214 wants to removes `GetAll()` anyway in 64fc6603525f515c5303f557b038753904c6541(FYI @ryanofsky), the simplest solution might be to just not use `GetAll` and just iterate over `m_ibd_chainstate.get()` and `m_snapshot_chainstate.get()` by hand - this would have the added benefit that we could replace the (`IsUsable(cs)` by just a not-nullptr check for cs, because I don't see a conceptual reason why a disabled chainstate (whether it's invalid, or fully validated) shouldn't be subjec
...
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2810060421)
Given that #30214 wants to removes `GetAll()` anyway in 64fc6603525f515c5303f557b038753904c6541(FYI @ryanofsky), the simplest solution might be to just not use `GetAll` and just iterate over `m_ibd_chainstate.get()` and `m_snapshot_chainstate.get()` by hand - this would have the added benefit that we could replace the (`IsUsable(cs)` by just a not-nullptr check for cs, because I don't see a conceptual reason why a disabled chainstate (whether it's invalid, or fully validated) shouldn't be subjec
...
π¬ darosior commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2845092974)
> Would it be possible to summarize the pros and cons raised across the internets in the PR description - if for no other reason, to avoid repeated comments on the same topic?
I gave the rationale for this [on the mailing list](https://gnusha.org/pi/bitcoindev/rhfyCHr4RfaEalbfGejVdolYCVWIyf84PT2062DQbs5-eU8BPYty5sGyvI3hKeRZQtVC7rn_ugjUWFnWCymz9e9Chbn7FjWJePllFhZRKYk=@protonmail.com/). I also gave more background in [a blog post](https://antoinep.com/posts/relay_policy_drama/) i just published
...
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2845092974)
> Would it be possible to summarize the pros and cons raised across the internets in the PR description - if for no other reason, to avoid repeated comments on the same topic?
I gave the rationale for this [on the mailing list](https://gnusha.org/pi/bitcoindev/rhfyCHr4RfaEalbfGejVdolYCVWIyf84PT2062DQbs5-eU8BPYty5sGyvI3hKeRZQtVC7rn_ugjUWFnWCymz9e9Chbn7FjWJePllFhZRKYk=@protonmail.com/). I also gave more background in [a blog post](https://antoinep.com/posts/relay_policy_drama/) i just published
...
π¬ TheCharlatan commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2845097684)
> If IBD speedup is not the main motivation, what else is the motivation, given that the title mentions [IBD]?
I think block serialization being fast could be useful for future kernel users. Reading and de-serializing blocks is one of the common operations I'd expect outside of just pure block validation.
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2845097684)
> If IBD speedup is not the main motivation, what else is the motivation, given that the title mentions [IBD]?
I think block serialization being fast could be useful for future kernel users. Reading and de-serializing blocks is one of the common operations I'd expect outside of just pure block validation.
π¬ achow101 commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2845199058)
One thing to mention is that several contributors are not comfortable with transferring the ownership of the bitcoin github org to anyone else, so regardless of any moves, the current bitcoin org owners will remain unchanged and no new owners added.
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2845199058)
One thing to mention is that several contributors are not comfortable with transferring the ownership of the bitcoin github org to anyone else, so regardless of any moves, the current bitcoin org owners will remain unchanged and no new owners added.
π¬ adamandrews1 commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2845219326)
> Could I try this issue?
Would you mind reviewing https://github.com/bitcoin/bitcoin/pull/31298 instead?
This solution is still up to date, it just hasn't gotten the right reviewers.
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2845219326)
> Could I try this issue?
Would you mind reviewing https://github.com/bitcoin/bitcoin/pull/31298 instead?
This solution is still up to date, it just hasn't gotten the right reviewers.
π€ adamandrews1 reviewed a pull request: "rpc: Support v3 raw transactions creation"
(https://github.com/bitcoin/bitcoin/pull/31936#pullrequestreview-2810199344)
utACK 63d9b9e3b8e225eccd55209e7a4213604ec88425
Suggested some improvements, but not blocking ones IMO
(https://github.com/bitcoin/bitcoin/pull/31936#pullrequestreview-2810199344)
utACK 63d9b9e3b8e225eccd55209e7a4213604ec88425
Suggested some improvements, but not blocking ones IMO
π¬ adamandrews1 commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2070507164)
The final parameter (tx version) for this test should be set to `TX_MAX_STANDARD_VERSION + 1` or at least to a number with headroom like `999`.
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2070507164)
The final parameter (tx version) for this test should be set to `TX_MAX_STANDARD_VERSION + 1` or at least to a number with headroom like `999`.