Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👍 hebasto approved a pull request: "ci: Add missing amd64 to win64-cross task"
(https://github.com/bitcoin/bitcoin/pull/28295#pullrequestreview-1584534121)
ACK fa56d17a4b868f42fa45bea0d1f0c78de75e7838
💬 fjahr commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1683891978)
> The onus in on you to show us that your PR will not break existing use cases.

His argument is, of course, that the use cases are already broken and thus the setting may as well be changed to reflect reality and help the network. I find the evidence presented convincing enough.

> If you don't think these use cases exist, go ahead and prove it.

Proving a negative in this context seems nearly impossible. How should he do that? Visit every website on the web and check if they accept 0-co
...
💬 jsarenik commented on issue "CheckBlockIndex stalls for extremely long time":
(https://github.com/bitcoin/bitcoin/issues/25897#issuecomment-1683911282)
@verdy-p Thank you for reporting this issue in such a detail! I experience it also for some time (running 25 or master). I do txindex and coinstatsindex, see be.anyone.eu.org
💬 jsarenik commented on issue "CheckBlockIndex stalls for extremely long time":
(https://github.com/bitcoin/bitcoin/issues/25897#issuecomment-1683919093)
My workaround is to regularly do

```
pkill -9 -f b-shutoff
```
💬 Sjors commented on pull request "getblocktemplate improvements for segwit and sigops":
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-1683925822)
Marked as draft because I'm now building on top of #26201, which touches and adds a relevant test to `getblocktemplate`. I'll unrebase and pick the test if that PR doesn't make it.

I dropped 6ab119d4c1c8cf3d9c0994c22d75d016a30cdd6a: it's still necessary for the (mining software) client to tell us it supports SegWit. This is merely a checkbox now.

In the context of dropping `segwit` as a required rule I wrote:

> We do the same thing with `csv`.

I guess the argument here is that, unlik
...
🚀 fanquake merged a pull request: "ci: Add missing amd64 to win64-cross task"
(https://github.com/bitcoin/bitcoin/pull/28295)
💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#issuecomment-1683968332)
re-ACK 07cb6c4

The rebase on #27981 looks good to me. I didn't look at the fuzzer changes. Otherwise it's just some extra comments.
💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#issuecomment-1683975067)
Updates since a few days ago:
* Moved the introduction of the `have_next_message` argument of `GetBytesToSend`, and testing thereof, to #28196.
* Rebased after merged of #27981 (sorry @Sjors, the earlier rebase was incorrect, it only looked at `vSendMsg.empty()` for the returned `data_left` in `SocketSendData()`).
* Added a few more comments.
* Some performance improvements to the fuzz test.
💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1683978028)
rebased, should be trivial to re-ACK
💬 fjahr commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1298507798)
If `BASE_ROOT_DIR` isn't used differently yes, I wasn't 100% sure about that. Done.
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#issuecomment-1684037736)
Addressed all (style) feedback so far, I think :sweat_smile:
💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#issuecomment-1684058189)
> Looks like CI is red, but it is not shown here?
>
> [bitcoin/bitcoin/actions/runs/5899744715](https://github.com/bitcoin/bitcoin/actions/runs/5899744715)

@achow101 @fanquake

I'm kindly asking you to update Actions permissions according to this PR description.
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-1684075929)
[30b0f57 ](https://github.com/bitcoin/bitcoin/commit/30b0f57b19eddd47fa137d0c20ae39174b54dbad)to [17028c1](https://github.com/bitcoin/bitcoin/commit/17028c1fec3f109b139e5e23f535c86f5a1ac2a4):
Rebased, expanded log entry with `this might take up to 2 minutes`
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1298579606)
added the log.
⚠️ Sjors opened an issue: "macOS poll() tracking issue"
(https://github.com/bitcoin/bitcoin/issues/28297)
Since #14336 we use `poll()` on systems which support it properly and fall back to `select()` otherwise.

@jamesob did some research on the implementation used on macOS and [found it lacking](https://github.com/bitcoin/bitcoin/pull/14336#issuecomment-437384408). Although in principle our current use of it shouldn't cause a problem.

I checked to see if perhaps Apple has improved things since 2018. They moved the repo, so the `knote_fdfind` implementation is now here: https://github.com/apple
...
💬 murchandamus commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1298590709)
Added
💬 fanquake commented on issue "macOS poll() tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28297#issuecomment-1684090201)
> In other words, we should probably leave things as they are for now,

So no need for an issue then? This is already tracked in the code.
👋 achow101's pull request is ready for review: "wallet: Use CTxDestination in CRecipient instead of just scriptPubKey"
(https://github.com/bitcoin/bitcoin/pull/28246)
💬 darosior commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1684100400)
> > Seems like this could result in banning pre-Segwit and pre-Taproot nodes?
>
> It would only impact nodes that produce/relay invalid segwit/taproot spends, ie ones that don't implement standardness checks. It also only triggers discouragement not banning, so affected peers can still reconnect. If we don't want to do that, we should probably drop `MaybePunishNodeForTx` entirely.

An invalid p2sh-wrapped Segwit spend [would be considered standard](https://github.com/bitcoin/bitcoin/blob/97
...
💬 Sjors commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1684100848)
Post merge concept ACK. From my (very) limited understanding of sockets, this makes sense. Thanks @ajtowns for the description https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1644977188.
💬 Sjors commented on issue "macOS poll() tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28297#issuecomment-1684102520)
The code currently refers to a locked pull request where nobody can comment on progress. If you unlock it, then closing this one should be fine.