π¬ willcl-ark commented on issue "Creating a Wallet Feature Guidelines Document":
(https://github.com/bitcoin/bitcoin/issues/28062#issuecomment-3411201458)
Closing this as it hasnβt seen any activity for a while. If youβd like to continue the discussion or feel the topic is still important, please leave a comment and we can reopen it.
(https://github.com/bitcoin/bitcoin/issues/28062#issuecomment-3411201458)
Closing this as it hasnβt seen any activity for a while. If youβd like to continue the discussion or feel the topic is still important, please leave a comment and we can reopen it.
π¬ diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411215057)
Going with the revert approach as per the previous discussion.
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411215057)
Going with the revert approach as per the previous discussion.
β
willcl-ark closed an issue: "Include torrent in binary download verification "
(https://github.com/bitcoin/bitcoin/issues/27702)
(https://github.com/bitcoin/bitcoin/issues/27702)
π¬ willcl-ark commented on issue "Include torrent in binary download verification ":
(https://github.com/bitcoin/bitcoin/issues/27702#issuecomment-3411219605)
There doesnβt seem to have been much activity around implementing this feature.
If itβs something youβre still interested in, please feel free to open a pull request or if you think this issue should be re-opened leave a comment in here.
(https://github.com/bitcoin/bitcoin/issues/27702#issuecomment-3411219605)
There doesnβt seem to have been much activity around implementing this feature.
If itβs something youβre still interested in, please feel free to open a pull request or if you think this issue should be re-opened leave a comment in here.
π¬ hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3411277879)
> What was your intention with this patch, @fanquake ?
@fanquake
Can you please follow up here?
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3411277879)
> What was your intention with this patch, @fanquake ?
@fanquake
Can you please follow up here?
π¬ fanquake commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3411347583)
> What was your intention with this patch, @fanquake ?
At this point I can't remember.
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3411347583)
> What was your intention with this patch, @fanquake ?
At this point I can't remember.
π fanquake merged a pull request: "[28.x] Backport & finalise 28.3"
(https://github.com/bitcoin/bitcoin/pull/33613)
(https://github.com/bitcoin/bitcoin/pull/33613)
π¬ hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411368744)
> Reading the description, I'm still not really clear on the goals here. It seems like this is just adding even more ways to compile for Windows, which we have to add more CI / and more work arounds for, but doesn't actually improve the Windows binaries we are shipping to end users in any way.
Building Bitcoin Core from source has always been considered one of the best practices. Not supporting an effective compiler, such as clang-cl on Windows, prevents users from achieving optimal performan
...
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411368744)
> Reading the description, I'm still not really clear on the goals here. It seems like this is just adding even more ways to compile for Windows, which we have to add more CI / and more work arounds for, but doesn't actually improve the Windows binaries we are shipping to end users in any way.
Building Bitcoin Core from source has always been considered one of the best practices. Not supporting an effective compiler, such as clang-cl on Windows, prevents users from achieving optimal performan
...
π¬ hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411395080)
Rebased to refresh the CI.
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411395080)
Rebased to refresh the CI.
π€ ismaelsadeeq reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3339641710)
Review the main implementation e6315c24326016cfaee5bd046e8b2e4e1088ac6b
I've not noticed any major issue, dropped a few minor comments and nits.
If any comment become stale after the update feel free to ignore it
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3339641710)
Review the main implementation e6315c24326016cfaee5bd046e8b2e4e1088ac6b
I've not noticed any major issue, dropped a few minor comments and nits.
If any comment become stale after the update feel free to ignore it
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432258434)
In "Add new (unused) limits for cluster size/count" 2801e80528a3a1c2949a8fda6338882613a673e5
nit: be specific it is in virtual size
```suggestion
argsman.AddArg("-limitclustersize=<n>", strprintf("Do not accept transactions whose size with all in-mempool connected transactions exceeds <n> virtual kilobytes (default: %u)", DEFAULT_CLUSTER_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
```
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432258434)
In "Add new (unused) limits for cluster size/count" 2801e80528a3a1c2949a8fda6338882613a673e5
nit: be specific it is in virtual size
```suggestion
argsman.AddArg("-limitclustersize=<n>", strprintf("Do not accept transactions whose size with all in-mempool connected transactions exceeds <n> virtual kilobytes (default: %u)", DEFAULT_CLUSTER_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
```
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432032703)
In "Add sigops adjusted weight calculator" f2eff17c6c4fc945f6fd761564212802107a1d7d
nit: use snake case.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432032703)
In "Add sigops adjusted weight calculator" f2eff17c6c4fc945f6fd761564212802107a1d7d
nit: use snake case.
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432321152)
In "test: update feature_rbf.py replacement test" 429bdbecfde93c54374fb3098e357e18556e7e21
nit: This should live in `functional/test_framework/mempool_util.py`
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432321152)
In "test: update feature_rbf.py replacement test" 429bdbecfde93c54374fb3098e357e18556e7e21
nit: This should live in `functional/test_framework/mempool_util.py`
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432516613)
In "Do not allow mempool clusters to exceed configured limits" 5a388c0d595b2318fea4b1dce977e2d5ff1abc48
I try to not use the magic number by trimming using
`pool.DynamicMemoryUsage() - usage_of_5 - usage_of_7` but test fail because tx 6 is also evicted.
However when I subtract only the usage of 5 the test succeed.
I grab the usages by computing the delta in memory usage after insertion of the tx.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432516613)
In "Do not allow mempool clusters to exceed configured limits" 5a388c0d595b2318fea4b1dce977e2d5ff1abc48
I try to not use the magic number by trimming using
`pool.DynamicMemoryUsage() - usage_of_5 - usage_of_7` but test fail because tx 6 is also evicted.
However when I subtract only the usage of 5 the test succeed.
I grab the usages by computing the delta in memory usage after insertion of the tx.
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432430377)
In "Do not allow mempool clusters to exceed configured limits" 5a388c0d595b2318fea4b1dce977e2d5ff1abc48
If you define the max cluster count in mempool util it can be reused here.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432430377)
In "Do not allow mempool clusters to exceed configured limits" 5a388c0d595b2318fea4b1dce977e2d5ff1abc48
If you define the max cluster count in mempool util it can be reused here.
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432661442)
In "Check cluster limits when using -walletrejectlongchains" 1102ac7f74ac2f48760b46be58c7deb70fa727cf
nit: also state that it should be safe to create a new change set here because we lock mempool cs.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432661442)
In "Check cluster limits when using -walletrejectlongchains" 1102ac7f74ac2f48760b46be58c7deb70fa727cf
nit: also state that it should be safe to create a new change set here because we lock mempool cs.
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435949023)
In "Select transactions for blocks based on chunk feerate" a8be743aeb42ec8ab613f822989a11a2f2ce70ac
nit: It will be better for us replace package with chunk here and other places in the miner?
```diff
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 1bfe7714440..e80d55c1f31 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -192,12 +192,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
return std::move(pblocktemplate);
}
-bool Bloc
...
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435949023)
In "Select transactions for blocks based on chunk feerate" a8be743aeb42ec8ab613f822989a11a2f2ce70ac
nit: It will be better for us replace package with chunk here and other places in the miner?
```diff
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 1bfe7714440..e80d55c1f31 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -192,12 +192,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
return std::move(pblocktemplate);
}
-bool Bloc
...
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436048271)
In "test: rewrite PopulateMempool to not violate mempool policy (cluster size) limits" afb9003bc602a77ed15bf4e79277d960937a5c28
nitty-nit: instead of undo, spent outputs might be better?
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436048271)
In "test: rewrite PopulateMempool to not violate mempool policy (cluster size) limits" afb9003bc602a77ed15bf4e79277d960937a5c28
nitty-nit: instead of undo, spent outputs might be better?
π¬ ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436033865)
In "Select transactions for blocks based on chunk feerate" a8be743aeb42ec8ab613f822989a11a2f2ce70ac
nit: maybe reserve again after clear here?
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2436033865)
In "Select transactions for blocks based on chunk feerate" a8be743aeb42ec8ab613f822989a11a2f2ce70ac
nit: maybe reserve again after clear here?
π¬ fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411433463)
> prevents users from achieving optimal performance,
Clang on Windows produces better performing binaries than MSVC?
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-3411433463)
> prevents users from achieving optimal performance,
Clang on Windows produces better performing binaries than MSVC?