Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 willcl-ark commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#issuecomment-1794436797)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33

Looks good to me!
💬 dergoegge commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1794443638)
Here's a coverage report: https://dergoegge.github.io/bitcoin-coverage/pr28578/fuzz.coverage/index.html (~1000h CPU time).

Is there a reason not to cover more of `DescriptorScriptPubKeyMan` in this PR? e.g. `DescriptorScriptPubKeyMan::SignTransaction` or `DescriptorScriptPubKeyMan::FillPSBT`.
🚀 fanquake merged a pull request: "build: Drop no longer needed MSVC warning suppressions"
(https://github.com/bitcoin/bitcoin/pull/28798)
💬 fanquake commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794459605)
```bash
test/functional/feature_assumeutxo.py:78: refering ==> referring
test/functional/feature_assumeutxo.py:115: refering ==> referring
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
👍 dergoegge approved a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1714698328)
ACK fa7ba926300f44b350d04262e569cc607e4991d8
💬 maflcko commented on pull request "Fix typos":
(https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1794494686)
> I was still waiting for feedback from maintainers

DrahtBot has a summary comment, which has links to all feedback, including that of maintainers, see https://github.com/bitcoin/bitcoin/pull/28605#issuecomment-1750139214
💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794513714)
Updated a20c1d2b81a702f97267d4f098f13ed80e9320cf -> 836a0a7566e166556f80303a9b3dd9560587159c ([simplifyMemPoolInteractions_7](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_7) -> [simplifyMemPoolInteractions_8](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_7..simplifyMemPoolInteractions_8))

* Addressed @Sjors's [comment](https://github.com/bitcoin/bit
...
🤔 ismaelsadeeq reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1714693345)
Strong ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

The whole mempool transactions can also be used to create `manual_entries` and `descendant_caches` and use the new constructor to build a block template to get a fee rate histogram of the block template, with just a modification to `BuildMockTemplate` to enable getting the block template fee rate histogram.
it will be more desirable to modify this than `BlockAssembler`.

Due to the use of `GatherClusters`, which has a 500 transaction D
...
💬 ismaelsadeeq commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1383041819)
I think just stating that this is a constructor where `MiniMinerMempoolEntry` are constructed manually is enough as this can be used with mempool transactions, but with the intent of building a block template to get a mempool fee rate histogram
```suggestion
* It is assumed that all
```
💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794571887)
Updated 836a0a7566e166556f80303a9b3dd9560587159c -> 11b8735852bd7439c4a07c88d07d7dfb13a3c7b7 ([simplifyMemPoolInteractions_8](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_8) -> [simplifyMemPoolInteractions_9](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_8..simplifyMemPoolInteractions_9))

* Fixed msvc build error by using `auto` instead of `CTxMemPo
...
⚠️ dergoegge opened an issue: "fuzz: Re-introduce i2p target"
(https://github.com/bitcoin/bitcoin/issues/28803)
We deleted the i2p target (#28692) because it wasn't effective at covering the i2p code and as a result never found any bugs. Someone should re-work the target and then re-introduce it to the repo.

Some of the short comings of the previous harness were identified:
* The temporary file created for the private key should be deleted at the end of each iteration (https://github.com/bitcoin/bitcoin/blob/106ab20f121f14d021725c8a657999079dbabfc1/src/test/fuzz/i2p.cpp#L34), or better yet don't use a
...
💬 willcl-ark commented on issue "High CPU load when network traffic page left open":
(https://github.com/bitcoin-core/gui/issues/768#issuecomment-1794692567)
Tested again with 26.0rc2 binary from bitcoincore.org, and the issue is still present for me. So long as the graph content is relatively "varied", then one CPU constantly thrashes between ~60-100%.

![image](https://github.com/bitcoin-core/gui/assets/6606587/e925a2b0-1177-45e4-a6b1-596aae4a1cda)

Seems to happen even more readily if I set the scale to 5m where even with fewer data points shown (I think) it pretty much just pins a single core to 100%

![image](https://github.com/bitcoin-cor
...
💬 maflcko commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383242173)
```suggestion
CTxMemPoolEntry* GetEntry(const uint256& hash) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs);
```

Instead of optional ref, could just use a pointer?
💬 maflcko commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1794721136)
To preserve CI resources, only one task is re-run to detect silent merge conflicts.
💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1794726325)
Rebased. This should be possible to review, and should work on all platforms, I guess, except for macOS?

Once and if this is merged, I'll follow-up with a `fs.h` cleanup.
💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383268650)
Yeah, I thought this would better convey its semantics, but with all the force pushes over the past two hours, it seems like it is not intuitive to work with, so I'll revert to using a pointer.
💬 fanquake commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1794834244)
> I can't see anything obvious that would do that.

Sent a patch upstream, https://lists.gnu.org/archive/html/guix-patches/2023-11/msg00362.html, to see if we can get some unversioned pointers to stable/longterm added.
👍 BrandonOdiwuor approved a pull request: "wallet: cache descriptor ID to avoid repeated descriptor string creation"
(https://github.com/bitcoin/bitcoin/pull/28799#pullrequestreview-1715206734)
ACK 5e6bc6d830664a5afeb5d5bd7e7b3818a01376b7

looks good to me
🤔 glozow reviewed a pull request: "test: bugfix CheckPackageMempoolAcceptResult return all error strings"
(https://github.com/bitcoin/bitcoin/pull/28788#pullrequestreview-1715221980)
utACK 5380f055136ea99f76cd3df2c2add081852d35d0
💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1383369213)
Added comment