Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 andrewtoth commented on pull request "RFC: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2051775760)
The entry has an empty coin here, it was just created. So this is just subtracting zero.
💬 fjahr commented on pull request "test: Fix feature_pruning test after nTime typo fix":
(https://github.com/bitcoin/bitcoin/pull/32312#issuecomment-2817298481)
Code change looks good to me. The commit message doesn't seem to say anything about the typo fix though. It should be mentioned there and that the rest of the change is fixing a secondary issue caused by this. You could also split this in two separate commits.
💬 l0rinc commented on pull request "RFC: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2051806640)
If you think that's better, I don't mind - done
💬 l0rinc commented on pull request "RFC: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2051806668)
> In practice it's not possible to get !inserted unless the assume utxo payload was generated incorrectly.

As far as I can see that's not a guarantee, but I've also noted it in the commit message that currently this is the case.

> kept the original behavior of moving the coin here as well

Sure, if you think that's better - changed!
💬 l0rinc commented on pull request "RFC: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2051806682)
Thanks, I'll add an assert in that case to explain why this branch wasn't symmetric with the other one.
👋 l0rinc's pull request is ready for review: "coins: Fix `cachedCoinsUsage` usages in `CCoinsViewCache`"
(https://github.com/bitcoin/bitcoin/pull/32313)
⚠️ Dustin4444 opened an issue: "new"
(https://github.com/bitcoin/bitcoin/issues/32314)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [x] I still think this issue should be opened here

### Report

> [!IMPORTANT]
>
pinheadmz closed an issue: "new"
(https://github.com/bitcoin/bitcoin/issues/32314)
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2051989121)
nit: `weekday` and `month` can be a `string_view`.
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2051985554)
nit: this comment seems to be incorrect, since `TCP_NODELAY` is only set once by `SockMan`.
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2052027205)
Can we return a `string_view` to the internal buffer?
(preventing allocation & copy)
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2052025637)
Can we return a `string_view` to the internal buffer?
(preventing allocation & copy)
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2052024440)
Maybe it would be better (performance-wise) to create the string after the loop is over?
```c++
std::optional<std::string> LineReader::ReadLine()
{
if (it == end) {
return std::nullopt;
}

auto line_start = it;
size_t count = 0;
while (it != end) {
char c = static_cast<char>(*it);
++it;
++count;
if (c == '\n') break;
if (count >= max_read) throw std::runtime_error("max_read exceeded by LineReader");
}
co
...
💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2051998964)
nit: maybe using `std::span` will be simpler?
For example:
```c++
auto span = std::as_bytes(std::span(str));
return {span.begin(), span.end()};
```
👍 rkrux approved a pull request: "rpc: Allow fullrbf fee bump in (psbt)bumpfee"
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2780807756)
reACK fa86190e6ed2aeda7bcceaf96f52403816bcd751

```
git range-diff fa16051...fa86190
```

New changes: Updating the commit message, the release docs, functional test name & logs, and unit test comment.
💬 rkrux commented on pull request "rpc: Allow fullrbf fee bump in (psbt)bumpfee":
(https://github.com/bitcoin/bitcoin/pull/31953#discussion_r2052122275)
Nit: I missed this in my previous review, but now that the error is not being asserted, this particular test doesn't have any assertions, and understanding it relies on few internals such as the `replaceable` argument not being passed in `bumpfee` RPC and the transaction created on the `peer_node` has `walletrbf=0` set, which ultimately leads to BIP 125 replaceability signalling not being set.
It'd be nice to assert for the transaction inputs sequence by using a more generalised version of the
...
👍 rkrux approved a pull request: "wallet: Automatically repair corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2780893803)
reACK 670d02bc380c22fa4c985b72e8209f475ed0e7d0

```
git range-diff bdcd435...670d02b
```

Using `find` instead of `at` now, few other functional test naming & comments changes done as suggested earlier.
💬 Sjors commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2818160927)
@instagibbs while writing my [reply on a mailinglist post](https://groups.google.com/g/bitcoindev/c/d6ZO7gXGYbQ/m/J0eUMnVdDAAJ) I found myself wondering if the `MAX_SCRIPT_SIZE` limit applied to the text after `OP_RETURN`, either in standardness (if the 83 byte limit is dropped) or in consensus.

An `OP_RETURN` with a bit over 10,000 bytes (but less than 100K) would answer that, without reading the code.
💬 maflcko commented on pull request "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)":
(https://github.com/bitcoin/bitcoin/pull/32305#issuecomment-2818284555)
(fresh CI)
maflcko closed a pull request: "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)"
(https://github.com/bitcoin/bitcoin/pull/32305)