Bitcoin Core Github
44 subscribers
122K links
Download Telegram
👍 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.
📝 MarcoFalke opened a pull request: "fuzz: Bump FuzzedDataProvider.h"
(https://github.com/bitcoin/bitcoin/pull/28086)
Also, remove suppression.
💬 MarcoFalke commented on pull request "refactor: use Span for SipHash::Write":
(https://github.com/bitcoin/bitcoin/pull/28085#discussion_r1264993873)
```suggestion
CSipHasher& Write(Span<const B> data);
```

I wonder if this should be made a template on `B` and instantiated for `std::byte` and `unsigned char` in the cpp file? Otherwise it will need to be touched again in the future if someone wants to hash a `std::byte` span.
💬 MarcoFalke commented on pull request "refactor: use Span for SipHash::Write":
(https://github.com/bitcoin/bitcoin/pull/28085#discussion_r1264994458)
```suggestion
t |= uint64_t{data.front()} << (8 * (c % 8));
```

style-nit, if it compiles?
💬 jonasnick commented on issue "libsecp CI failure [no wallet, libbitcoinkernel] [focal]":
(https://github.com/bitcoin/bitcoin/issues/28079#issuecomment-1637705247)
Thanks for reporting. This is an actual bug in the secp256k1 test harness. https://github.com/bitcoin-core/secp256k1/pull/1378