💬 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

📝 MarcoFalke opened a pull request: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087)
Currently the CI containers always run on the host architecture, and only wrap `bitcoind` into `qemu-user` when needed. This has many issues:

* The `i386` tasks can not be run on non-x86 hosts.
* `config.guess` isn't present when building the CI image, which is fine. But it prints a warning, see https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1446580353
* The python tests are run on the host architecture, making it harder to find architecture specific bugs. See for example h
...

💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265149164)
fec00d787b5530bbe892bef575fbb3cdc9d586ca

nit: you may drop the `args.` part and it will work because `bypass_limits` var is initialized above.

👍 dergoegge approved a pull request: "fuzz: Flatten all FUZZ_TARGET macros into one"
(https://github.com/bitcoin/bitcoin/pull/28065#pullrequestreview-1532456010)
ACK fa6dfaaf45bde465969fa7d8fa6b6574a497a6ca

💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265162383)
34eab37b08bb9f23459aad179685a505a46759d7
Seems to be still talking about `current_time`?

💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265171521)
This is done in the following commit [c88deec](https://github.com/bitcoin/bitcoin/pull/27675/commits/c88deec99d53f55ff76fedaa30c318be6b72d085)

💬 stickies-v commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637867309)
> But the use of the passed parameter for `longpollid` at
> https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L715-L721
>
> is the same for both a template and a proposal.

`longpollid` is neither used as a parameter nor returned as a result for `mode==proposal`. The line you reference is never reached in that case, as we [return or throw within this code block](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e7
...

💬 russeree commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637880277)
> >
> > is the same for both a template and a proposal.
>
> `longpollid` is neither used as a parameter nor returned as a result for `mode==proposal`. The line you reference is never reached in that case, as we [return or throw within this code block](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L660-L687). See also [the corresponding results](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/minin
...

💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1637899668)
ACK 0753120b6cb270914753d30a2626272f896a8545 modulo [this (probably lost?) comment](https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265162383)

My favorite achievement of this PR is dropping the complication of associated logic :)

💬 dergoegge commented on pull request "[PoC] Structure aware fuzzing with libprotobuf-mutator":
(https://github.com/bitcoin/bitcoin/pull/26975#issuecomment-1637905257)
> Did you find that this was useless in a benchmark, or did you just close for fun?

Just wasn't my top priority but I plan on revisiting this. I've also been working on a library that does the same as libprotobuf-mutator but for Cap'n Proto, which might be more appropriate if we go the multi-process route (could also be used to fuzz the multi-process interfaces).

💬 MarcoFalke commented on issue "libsecp CI failure [no wallet, libbitcoinkernel] [focal]":
(https://github.com/bitcoin/bitcoin/issues/28079#issuecomment-1637929740)
Depending on how often this happens, we could also bump the subtree here after merge?