💬 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?
(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
(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
(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.
(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
(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?
(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
...
(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))`
(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)
(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.
(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
...
(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
...
...
(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
...
(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.
(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
...
(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.
(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.
💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1639995493)
The tests from https://github.com/bitcoin/bitcoin/pull/27509 failed because of this which prompted me to open this PR.
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1639995493)
The tests from https://github.com/bitcoin/bitcoin/pull/27509 failed because of this which prompted me to open this PR.
💬 MarcoFalke commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1640000922)
Ok, an alternative would be to embed the `-Wno-return-type` into the compiler `CC` and `CXX` env vars, like I did for the valgrind task.
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1640000922)
Ok, an alternative would be to embed the `-Wno-return-type` into the compiler `CC` and `CXX` env vars, like I did for the valgrind task.
💬 MarcoFalke commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640003448)
Isn't `-m32` set by oss-fuzz already? Also, if you compile locally, I'd always assume you'd have to embed `-m32` into the `CC` and `CXX` env vars.
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640003448)
Isn't `-m32` set by oss-fuzz already? Also, if you compile locally, I'd always assume you'd have to embed `-m32` into the `CC` and `CXX` env vars.
💬 fanquake commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640008851)
> Isn't -m32 set by oss-fuzz already?
Yes. I think in the oss-fuzz case it's an additional interaction with `DEBUG=1`, which causes sqlite to loose the flags.
> Also, if you compile locally, I'd always assume you'd have to embed -m32 into the CC and CXX env vars.
Builds work as expected when not overriding CC & CXX, I don't think there's a reason to expect that not to work, without additional flags, when using a different compiler.
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1640008851)
> Isn't -m32 set by oss-fuzz already?
Yes. I think in the oss-fuzz case it's an additional interaction with `DEBUG=1`, which causes sqlite to loose the flags.
> Also, if you compile locally, I'd always assume you'd have to embed -m32 into the CC and CXX env vars.
Builds work as expected when not overriding CC & CXX, I don't think there's a reason to expect that not to work, without additional flags, when using a different compiler.