Bitcoin Core Github
44 subscribers
122K links
Download Telegram
📝 fanquake opened a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084)
Now that changes have been made in GCC, to fix the build failures.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1637129953)
@vasild OK I got this branch passing all CI. I ran tests locally on Ubuntu and MacOS and also tested the feature in production on both platforms with `tor --SocksPort unix:/...` Everything is working! yay.

The real fix was here actually (diff below) Embarrassingly simple. This was a bug on Linux but NOT on macOS.

I did not need the `SendComplete` changes or `Socks5` changes which is great because that keeps review simple and the branch focused. I will be happy to review those changes if
...
👍 TheCharlatan approved a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084#pullrequestreview-1531767457)
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb
💬 jonatack commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1637131364)
Verified that the issue doesn't occur with the i2pd router, only with the Java I2P router.
👍 hebasto approved a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084#pullrequestreview-1531781254)
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb, I've verified that the fix commit is present in all branches starting from `gcc-11`.
hebasto closed an issue: "Sign PSBT: Can't verify change output"
(https://github.com/bitcoin-core/gui/issues/732)
🚀 hebasto merged a pull request: "Show own outputs on PSBT signing window"
(https://github.com/bitcoin-core/gui/pull/740)
💬 GBKS commented on issue "Creating a Wallet Feature Guidelines Document":
(https://github.com/bitcoin/bitcoin/issues/28062#issuecomment-1637177521)
I think both lists are important. Maybe you can kick off that effort?

I can't speak to the core wallet, but with the related GUI project I am helping with, we are trying to create solid design documentation from the start ([see here](https://bitcoincore.app)), which may also end up including a good amount of feature specs. We're still early in this, but I do think it's worth the effort to document things well. It gets easier to make decisions, newcomers can more easily understand the goals an
...
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1637239099)
I am a complete libevent noob, but I noticed some odd behavior that makes me think this is a race condition. I used the following diff that logged both sides' port numbers and the above python script:

<details>
<summary>diff</summary>
<br>

```
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 849e9b482..8bb9d407f 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -223,13 +223,21 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
// comp
...
💬 doperiddle commented on issue "BIP324 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/27634#issuecomment-1637262311)
> This issue will be updated to reflect the current state of [BIP324](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki) integration.
>
> PRs ready for review:
>
> * [Make poly1305 support incremental computation + modernize #27993](https://github.com/bitcoin/bitcoin/pull/27993)
>
> Overall plan:
>
> * [x] ElligatorSwift integration in Bitcoin Core: [BIP324: ElligatorSwift integrations #27479](https://github.com/bitcoin/bitcoin/pull/27479)
>
> * [x] Dependency:
...
📝 theStack opened a pull request: "refactor: use Span for SipHash::Write"
(https://github.com/bitcoin/bitcoin/pull/28085)
This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers).
💬 russeree commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637417549)
Concept ACK

I do have a few questions about the implementation of the RPCHelpMan parameter text for 'longpollid' when used for a template instead of a propsal. My primary issue is the block proposal definition for ```longpollid``` at https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L613
varies from the current PRs RPCHelpMan when used as an optional parameter for a block template. But the use of the passed parameter for ```longpollid``` at
...
💬 MarcoFalke commented on issue "valgrind: Syscall param ppoll(ufds.events) points to uninitialised byte(s)":
(https://github.com/bitcoin/bitcoin/issues/28072#issuecomment-1637472437)
Ok, let's continue discussion there.
MarcoFalke closed an issue: "valgrind: Syscall param ppoll(ufds.events) points to uninitialised byte(s)"
(https://github.com/bitcoin/bitcoin/issues/28072)
💬 MarcoFalke commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1637473943)
Closing for now, but please do let us know if this happens again on any commit, including the one you reported already above.
MarcoFalke closed an issue: "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837"
(https://github.com/bitcoin/bitcoin/issues/27963)
💬 MarcoFalke commented on issue "libsecp CI failure [no wallet, libbitcoinkernel] [focal]":
(https://github.com/bitcoin/bitcoin/issues/28079#issuecomment-1637477709)
May be good to check if this reproduces with the random seed? cc @jonasnick
💬 S3RK commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1264962807)
in 9dc5bbe042f4d1febde8753a8c56da35eedd9d3e

I know this preserves current behaviour, but don't we want to use `long_term_fee_rate` to determine cost of chance?

@murchandamus
💬 S3RK commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1264968527)
in ea57562b4ab4071fd1a716f09e13d19ff68e99f1

I see how it's useful for the next commit, but the contract of the method is not really clear without reading the code.
💬 S3RK commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#issuecomment-1637521709)
Approach ACK

I think setting change related params to 0 in case where we reuse existing output is the correct way to go.