💬 MarcoFalke commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1639528790)
Concept ACK 20b49460b35268db59f7efcb02736b0e31191a74

Probably an edge case, but it seems useful to have this in case connections are opened at the same time for some reason.

💬 MarcoFalke commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1639585127)
Looks like this triggers an OOM for some reason, when it shouldn't?

💬 S3RK commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1266301780)
Let's document which part of `CoinSelectionParams` is modified and which is not.

💬 MarcoFalke commented on pull request "refactor: use Span for SipHash::Write":
(https://github.com/bitcoin/bitcoin/pull/28085#discussion_r1266314950)
> the error message "could not match 'Span' against 'vector'" still appears. (Or, did I misinterpret your second sentence and with "drop it" you meant to drop the template idea in general?).

Nah, clearly I don't understand C++. The overload *should* work, though, similar to the HexStr overloads:

```cpp
/**
* Convert a span of bytes to a lower-case hexadecimal string.
*/
std::string HexStr(const Span<const uint8_t> s);
inline std::string HexStr(const Span<const char> s) { return HexS
...

💬 MarcoFalke commented on pull request "refactor: Make more transaction size variables `int32_t`":
(https://github.com/bitcoin/bitcoin/pull/28059#issuecomment-1639698787)
Are you still working on this?

💬 MarcoFalke commented on pull request "refactor: use Span for SipHash::Write":
(https://github.com/bitcoin/bitcoin/pull/28085#issuecomment-1639745003)
lgtm ACK 7d92b1430a6fd42c4438810640576830d0ff8d13

💬 MarcoFalke commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1639753496)
https://bugs.kde.org/show_bug.cgi?id=472329

💬 darosior commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1266463081)
I agree with Marco. My first reaction was hey we can't have our cake and eat it too, but in the case of the Miniscript targets we can: `miniscript_script` and `miniscript_string` could be left more generic by not discarding any coverage while `miniscript_smart` and `miniscript_stable` would.

🤔 darosior reviewed a pull request: "Add support for "partial" fuzzers that indicate usefulness"
(https://github.com/bitcoin/bitcoin/pull/27552#pullrequestreview-1534556964)
Concept ACK

💬 fanquake 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-1639826695)
> The I2P router may be in such a state

> Accept() fails because the I2P router sends something that is not Base64 on the socket:

It's not clear to me if this an issue in our code, or an issue with the router (or both), especially if everything "works fine" with one router, but not the other. Have these issues been reported upstream?

⚠️ fanquake opened an issue: "failure in wallet_resendwallettransactions.py --legacy-wallet"
(https://github.com/bitcoin/bitcoin/issues/28094)
https://cirrus-ci.com/task/5656433702731776?logs=ci#L7912 from #26288:
```bash
................................................................

194/268 - wallet_resendwallettransactions.py --legacy-wallet failed, Duration: 111 s
stdout:
2023-07-17T21:30:03.351000Z TestFramework (INFO): PRNG seed is: 4625691275301666838
2023-07-17T21:30:03.352000Z TestFramework (INFO): Initializing test directory /tmp/cirrus-ci-build/ci/scrat
...

💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1266482537)
Same nit here: `BOOST_CHECK_EQUAL(hextag, HexStr(tagres))`

🚀 fanquake merged a pull request: "validation: use noexcept instead of deprecated throw()"
(https://github.com/bitcoin/bitcoin/pull/28090)

💬 fanquake commented on pull request "subtree: update libsecp256k1 to latest master":
(https://github.com/bitcoin/bitcoin/pull/28093#issuecomment-1639891093)
Added a commit to fix the Windows builds.

⚠️ fanquake opened an issue: "guix: re-enable exported symbol checking for RISC-V"
(https://github.com/bitcoin/bitcoin/issues/28095)
This is currently disabled, when it can likely be fixed and re-enabled:
https://github.com/bitcoin/bitcoin/blob/673acab223c0f896767b1ae784659df9f95452ae/contrib/devtools/symbol-check.py#L210

A Guix build with the exception dropped shows there are symbols being exported in the RISC-V bins:
```bash
/opt/homebrew/opt/binutils/bin/objdump -T bitcoind

0000000000080500 w DF .text 00000000000000a6 Base _ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEC1IS3_EEPKcRKS3_
0000000000
...

⚠️ fanquake opened an issue: "32-bit Linux: build flags lost with depends & overriden CC(X)"
(https://github.com/bitcoin/bitcoin/issues/28096)
Building depends for 32-bit Linux (i686-pc-linux-gnu) but setting `CC`/`CXX`, causes us to loose build flags, and ultimately build a 64-bit executable:
```bash
# make -C depends/ HOST=i686-pc-linux-gnu -j9 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_USDT=1 NO_NATPMP=1 CC=clang CXX=clang++
...
copying packages: boost libevent
to: /home/ubuntu/bitcoin/depends/i686-pc-linux-gnu

./autogen.sh
CONFIG_SITE=/home/ubuntu/bitcoin/depends/i686-pc-linux-gnu/share/config.site ./configure
make -9
...
...

📝 fanquake opened a pull request: "depends: xcb-proto 1.15.2"
(https://github.com/bitcoin/bitcoin/pull/28097)
Resolves build failures with Python 3.12, i.e building on rawhide:
```bash
make -C depends -j9
...
make[3]: Nothing to be done for 'install-exec-am'.
/usr/bin/mkdir -p '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_proto/1.14.1-4a91ac9dc41/bitcoin/depends/aarch64-unknown-linux-gnu/lib/python3.12/site-packages/xcbgen'
/usr/bin/install -c -m 644 __init__.py error.py expr.py align.py matcher.py state.py xtypes.py '/bitcoin/depends/work/staging/aarch64-unknown-linux-gnu/xcb_pro
...

💬 vasild 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-1639987591)
@fanquake, There is an issue with the I2P router - there is a discrepancy between the router behavior and the SAM specification. Then, this unexpected behavior causes Bitcoin Core to log way too many messages. It becomes a problem with Bitcoin Core too. I should report this upstream as per https://geti2p.net/en/faq#bug.

⚠️ MarcoFalke opened an issue: "ci: Future of macOS and Windows MSVC CI tasks"
(https://github.com/bitcoin/bitcoin/issues/28098)
Cirrus CI will cap the community cluster, see https://cirrus-ci.org/blog/2023/07/17/limiting-free-usage-of-cirrus-ci/ .

Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks. Not sure if this is worth it, given that appveyor CI used to cost 50 USD/mo. Also, I am not sure if a sponsor can be found. Personally, I don't use macOS nor Windows, and I can't recommend it to anyone, so I don't mind if the tasks are removed. Though, I am opening this issue to gather fee
...

💬 fanquake commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1639995430)
> Looks like this triggers an OOM for some reason, when it shouldn't?

Looks like this is the same bug with overriding cxxflags dropping other flags. Will fix that.