💬 brunoerg commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3601527993)
Can be tested with the following diff:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 5dac413319..ec417b97ac 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3535,7 +3535,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
LOCK(pfrom.m_subver_mutex);
pfrom.cleanSubVer = cleanSubVer;
}
- peer->m_starting_height = starting_height;
+ peer->m_starting_height
...
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3601527993)
Can be tested with the following diff:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 5dac413319..ec417b97ac 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3535,7 +3535,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
LOCK(pfrom.m_subver_mutex);
pfrom.cleanSubVer = cleanSubVer;
}
- peer->m_starting_height = starting_height;
+ peer->m_starting_height
...
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3601633659)
> @alfonsoromanz can you rebase/ followup with the test failures here?
Yes, I will take care of it
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3601633659)
> @alfonsoromanz can you rebase/ followup with the test failures here?
Yes, I will take care of it
👍 laanwj approved a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3529672889)
Re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db
```
$ git range-diff 3bb30658e631ed45b6c8609292facc7ae3dd0f61..fa2fd0ba1fbd4085df24a2c5472636957db28521 b30262dcaa28c40a0a5072847b7194b3db203160..fad61185861a6a9ed806c387aa63d2b31262b1db
```
⋮
* `4: 4444edeecc528ccd88b7dd78ffddbccd69762132 ! 4: fae612424b3e70acd6011a4459518174463b3424 contrib: Remove confusing and redundant encoding from IO` - the only differing commit, checked that it is rebase
⋮
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3529672889)
Re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db
```
$ git range-diff 3bb30658e631ed45b6c8609292facc7ae3dd0f61..fa2fd0ba1fbd4085df24a2c5472636957db28521 b30262dcaa28c40a0a5072847b7194b3db203160..fad61185861a6a9ed806c387aa63d2b31262b1db
```
⋮
* `4: 4444edeecc528ccd88b7dd78ffddbccd69762132 ! 4: fae612424b3e70acd6011a4459518174463b3424 contrib: Remove confusing and redundant encoding from IO` - the only differing commit, checked that it is rebase
⋮
💬 brunoerg commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#issuecomment-3601640495)
Rebased
(https://github.com/bitcoin/bitcoin/pull/33993#issuecomment-3601640495)
Rebased
👍 stickies-v approved a pull request: "init: point out -stopatheight may be imprecise"
(https://github.com/bitcoin/bitcoin/pull/33993#pullrequestreview-3529707358)
ACK e4703863655135856d0d6ad54a7021d3326bf071
(https://github.com/bitcoin/bitcoin/pull/33993#pullrequestreview-3529707358)
ACK e4703863655135856d0d6ad54a7021d3326bf071
💬 stickies-v commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#discussion_r2580857428)
"Blocks after target height may be processed during shutdown." is a bit more concise and has my preference.
(https://github.com/bitcoin/bitcoin/pull/33993#discussion_r2580857428)
"Blocks after target height may be processed during shutdown." is a bit more concise and has my preference.
⚠️ plebhash opened an issue: "Mining IPC `createNewBlock` should not return before IBD is over"
(https://github.com/bitcoin/bitcoin/issues/33994)
### Please describe the feature you'd like to see added.
Mining IPC `createNewBlock` should not return before IBD is over
### Is your feature related to a problem, if so please describe it.
on SRI [`bitcoin_core_sv2`](https://github.com/stratum-mining/sv2-apps/tree/main/bitcoin-core-sv2) crate, we want to wait for IBD is over as part of the bootstrapping processes
discussing with @ismaelsadeeq and @Shourya742 during BTrust dev day, we came to the conclusion that `createNewBlock` shouldn't ev
...
(https://github.com/bitcoin/bitcoin/issues/33994)
### Please describe the feature you'd like to see added.
Mining IPC `createNewBlock` should not return before IBD is over
### Is your feature related to a problem, if so please describe it.
on SRI [`bitcoin_core_sv2`](https://github.com/stratum-mining/sv2-apps/tree/main/bitcoin-core-sv2) crate, we want to wait for IBD is over as part of the bootstrapping processes
discussing with @ismaelsadeeq and @Shourya742 during BTrust dev day, we came to the conclusion that `createNewBlock` shouldn't ev
...
💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2580917867)
Thanks! Reworked.
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2580917867)
Thanks! Reworked.
🤔 alexanderwiederin reviewed a pull request: "kernel: Expose reusable `PrecomputedTransactionData` in script validation"
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3529821349)
ACK https://github.com/bitcoin/bitcoin/pull/33891/commits/17d3575cfcb41adfd364e7a1912a4701786f7455
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3529821349)
ACK https://github.com/bitcoin/bitcoin/pull/33891/commits/17d3575cfcb41adfd364e7a1912a4701786f7455
💬 alexanderwiederin commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2580952377)
nit: wonder if "efficient" here could be misleading.
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2580952377)
nit: wonder if "efficient" here could be misleading.
💬 fanquake commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3601824876)
Note that 0972f5504021b482b27523fd3bcb8036cf6b439c has broken our `gen-manpages.py` script, by appending additional text (for some bins) to the line containing the version info.
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3601824876)
Note that 0972f5504021b482b27523fd3bcb8036cf6b439c has broken our `gen-manpages.py` script, by appending additional text (for some bins) to the line containing the version info.
💬 stickies-v commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2581001217)
> I think there will always be a possibility to argue one way or the other,
I agree, so even though I have a different view, it's not that practically important so this shouldn't be a blocker. This PR is an improvement with either approach.
> it seems like it should propagate the error up as a fatal one.
I don't think it needs to be propagated as fatal, the failing subsystem / callsite can just log its issue as a warning (when it has no concept or knowledge on fatality), and then higher
...
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2581001217)
> I think there will always be a possibility to argue one way or the other,
I agree, so even though I have a different view, it's not that practically important so this shouldn't be a blocker. This PR is an improvement with either approach.
> it seems like it should propagate the error up as a fatal one.
I don't think it needs to be propagated as fatal, the failing subsystem / callsite can just log its issue as a warning (when it has no concept or knowledge on fatality), and then higher
...
💬 stickies-v commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3601835371)
ACK fa45a1503eee603059166071857215ea9bd7242a
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3601835371)
ACK fa45a1503eee603059166071857215ea9bd7242a
👍 rkrux approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3530005825)
lgtm re-ACK 76c092ff805833a9adf84f669f0455bc2e0bba8b
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3530005825)
lgtm re-ACK 76c092ff805833a9adf84f669f0455bc2e0bba8b
💬 brunoerg commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2581145624)
Just ran a mutation testing for this PR and this is the only unkilled mutant - feel free to ignore if it doesn't make sense to address:
```diff
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f734296b24..13ddbb672f 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -880,7 +880,7 @@ public:
for (const CTransactionRef& tx : m_block_template->block.vtx | std::views::drop(1)) {
auto ref_count{m_node.template_tx_refs.find(tx)};
...
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2581145624)
Just ran a mutation testing for this PR and this is the only unkilled mutant - feel free to ignore if it doesn't make sense to address:
```diff
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f734296b24..13ddbb672f 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -880,7 +880,7 @@ public:
for (const CTransactionRef& tx : m_block_template->block.vtx | std::views::drop(1)) {
auto ref_count{m_node.template_tx_refs.find(tx)};
...
💬 m3dwards commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3602078949)
ACK ec8eb013a9bfceb324b309f13b8946b05292a993
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3602078949)
ACK ec8eb013a9bfceb324b309f13b8946b05292a993
🚀 fanquake merged a pull request: "log: Use more severe log level (warn/err) where appropriate"
(https://github.com/bitcoin/bitcoin/pull/33960)
(https://github.com/bitcoin/bitcoin/pull/33960)
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3602119940)
rebased and force-pushed to resolve the CI test failures.
Also addressed feedback from theStack
- moved the `validate_snapshot_import` and `complete_background_validation` methods into an own refactoring commit
- added a comma after `["-fastprune", "-prune=1"]` for smaller future diff if another node is added
- deduplicated the expected error message within `test_backup_during_background_sync_pruned_node` so it doesn't have to be adapted twice if it ever changes
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3602119940)
rebased and force-pushed to resolve the CI test failures.
Also addressed feedback from theStack
- moved the `validate_snapshot_import` and `complete_background_validation` methods into an own refactoring commit
- added a comma after `["-fastprune", "-prune=1"]` for smaller future diff if another node is added
- deduplicated the expected error message within `test_backup_during_background_sync_pruned_node` so it doesn't have to be adapted twice if it ever changes
💬 ismaelsadeeq commented on issue "Mining IPC `createNewBlock` should not return before IBD is over":
(https://github.com/bitcoin/bitcoin/issues/33994#issuecomment-3602165226)
@plebhash It will be nice to get to the root of the issue and see whether it's bitcoin core or from the template provider.
Can you please provide the logs and setups accessible in a text file and not a discord link I think some people might not have discord.
cc @Sjors I attempt to reproduce the failure but was not successful yet.
blocking `createnewblock` from creating template during ibd is potential solution and would also delegate the responsibility of polling isinitialblockdownload to the
...
(https://github.com/bitcoin/bitcoin/issues/33994#issuecomment-3602165226)
@plebhash It will be nice to get to the root of the issue and see whether it's bitcoin core or from the template provider.
Can you please provide the logs and setups accessible in a text file and not a discord link I think some people might not have discord.
cc @Sjors I attempt to reproduce the failure but was not successful yet.
blocking `createnewblock` from creating template during ibd is potential solution and would also delegate the responsibility of polling isinitialblockdownload to the
...
💬 sdaftuar commented on pull request "fuzz: gate mempool entry based on weight":
(https://github.com/bitcoin/bitcoin/pull/33985#issuecomment-3602179098)
ACK 804329400a73df00dfd7a5209c659d4a22b9ce47
(https://github.com/bitcoin/bitcoin/pull/33985#issuecomment-3602179098)
ACK 804329400a73df00dfd7a5209c659d4a22b9ce47